Re: [PATCH] IPA ICF + ASAN: do not merge vars with different alignment

[Martin Liška]

On 2/23/21 3:32 PM, Richard Biener wrote:

On Tue, Feb 23, 2021 at 3:22 PM Martin Liška <mli...@suse.cz> wrote:

On 2/23/21 12:56 PM, Richard Biener wrote:

Can't we fix the asan runtime?  Does the same issue happen when merging
two comdat with different alignment and LTO?

All right, there's a detail explanation what happens.
Let's consider the following example:

struct my_struct
{
    unsigned long volatile x;
} __attribute__((aligned(128)));

static int array[5][6] = {};
static struct my_struct variable128 = {1UL};
static struct my_struct variable32 __attribute__((aligned(64))) = {1UL};

Here we have 2 variables (variable128 and variable32) that are merged. Later on,
we decide not to protect the global variable variable128 due to:
        || DECL_ALIGN_UNIT (decl) > 2 * ASAN_RED_ZONE_SIZE

Without ICF we end up with:

         .align 64
         .type   variable32, @object
         .size   variable32, 128
variable32:
         .zero   128
         .zero   32
         .align 128
         .type   variable128, @object
         .size   variable128, 128
variable128:
         .zero   128

As seen, variable32 has .zero 128+32, where 32 is the red-zone (and alignment 
is increased to 64).

With ICF we end up with:

         .align 128
         .type   variable128, @object
         .size   variable128, 128
variable128:
         .zero   128
         .set    variable32,variable128

So variable32 points to variable128 (which has no prepared red zone + alignment 
is the same).
$ nm -n a.out
...
0000000000400b80 r variable128
0000000000400b80 r variable32
0000000000400c00 r array

0000000000400c00 - 0000000000400b80 == sizeof(variable32).

Then we tell libasan what is the variable size and size of the corresponding 
red zone:
$ ASAN_OPTIONS=report_globals=3 ./a.out
...
==20602==Added Global[0x000000403080]: beg=0x000000400b80 size=128/160 
name=variable32 module=asan.c dyn_init=0 odr_indicator=0x000000000000

Ah, so the issue is that ASAN still sees both variables (and isn't
properly cgraph/varpool aware)?

No, in both cases the variable128 is not handled by ASAN (it has too big 
alignment).

So instead of just
keying on different alignment you'd have to verify in ICF whether the
decls are "registered the same" by ASAN, no?

Yes, ICF is too optimistic about alignment of global variables. I'm not sure
I want to call asan_protect_global from ICF.

Or simply not perform any variable ICF when ASAN is enabled?

I think the suggested patch should tell ICF to be strict about alignment
when ASAN is enabled.

Note the issue is quite hairy :)

Martin

And bad thinks happen. So I really think ICF should not merge the variables.
Please provide a comdat test-case :)

Thanks,
Martin