[PATCH][PR sanitizer/71480] Make ASan align string constants to shadow granularity.

[Maxim Ostapenko]

On 10/06/16 14:41, Maxim Ostapenko wrote:
> On 10/06/16 14:18, Jakub Jelinek wrote:
>> On Fri, Jun 10, 2016 at 02:12:37PM +0300, Maxim Ostapenko wrote:
>>> gcc/ChangeLog:
>>>
>>> 2016-06-10  Maxim Ostapenko  <m.ostapenko@samsung.com>
>>>
>>>     PR sanitizer/71480
>>>     * varasm.c (place_block_symbol): Adjust alignment for asan 
>>> protected
>>>     STRING_CSTs even if TREE_CONSTANT_POOL_ADDRESS_P.
>> This is ok.
>>
>>> diff --git a/gcc/testsuite/c-c++-common/asan/pr71480.c 
>>> b/gcc/testsuite/c-c++-common/asan/pr71480.c
>>> new file mode 100644
>>> index 0000000..cf08ec6
>>> --- /dev/null
>>> +++ b/gcc/testsuite/c-c++-common/asan/pr71480.c
>>> @@ -0,0 +1,41 @@
>>> +/* { dg-do compile { target { arm*-*-* powerpc*-*-linux* } } } */
>>> +/* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
>>> +
>>> +#ifdef __cplusplus
>>> +extern "C" {
>>> +#endif
>>> +extern int
>>> +strcmp(const char *s1, const char *s2);
>>> +#ifdef __cplusplus
>>> +}
>>> +#endif
>> Doesn't just using __builtin_strcmp and leaving out the prototype 
>> work too?
>>
>>> +
>>> +__attribute__ ((noinline, noclone)) int
>>> +foo (char *c)
>>> +{
>>> +  return 1;
>>> +}
>>> +
>>> +__attribute__ ((noinline, noclone)) void
>>> +bar (char *c)
>>> +{
>>> +  return;
>>> +}
>>> +
>>> +int main (void)
>>> +{
>>> +  char tpl[20] = "/tmp/test.XXXXXX";
>>> +  int fd = foo (tpl);
>>> +
>>> +  if (fd == -1)
>>> +    return 1;
>>> +
>>> +  bar (tpl);
>>> +
>>> +  if (strcmp (tpl, "/tmp/test.XXXXXX") == 0)
>>> +    return 1;
>>> +
>>> +   return 0;
>>> +}
>>> +
>> But more importantly, the scan-assembler stuff is IMHO too fragile.
>
> Agree.
>
>> Does the bug reproduce actually at runtime, by asan false positive on
>> something or missing needed diagnostics?
>> If you need to add some aligned variable into the block before and/or 
>> after
>> the STRING_CST, so that it is really misaligned, it shouldn't be that 
>> hard
>> to do so.  And the testcase then should be usable on all architectures.
>>
>>     Jakub
>>
>>
>
> For me it reproduces if compile original testcase from BZ with -O2 
> -fsanitize=address -fno-omit-frame-pointer with following error in 
> runtime:
>
> ==436==AddressSanitizer CHECK failed: 
> /home/max/src/tizen/linaro-gcc/libsanitizer/asan/asan_globals.cc:145 
> "((AddrIsAlignedByGranularity(g->beg))) != (0)" (0x0, 0x0)
>     #0 0xb5e79763 in __asan::AsanCheckFailed(char const*, int, char 
> const*, unsigned long long, unsigned long long) (libasan.so.3+0x163763)
>     #1 0xb5e8436f in __sanitizer::CheckFailed(char const*, int, char 
> const*, unsigned long long, unsigned long long) (libasan.so.3+0x16e36f)
>     #2 0xb5d4415f in __asan::RegisterGlobal(__asan_global const*) 
> (libasan.so.3+0x2e15f)
>     #3 0xb5d4573b in __asan_register_globals (libasan.so.3+0x2f73b)
>     #4 0x10ca7 in __libc_csu_init 
> /home/max/build/v6/sources/glibc/csu/elf-init.c:88
>     #5 0xb5b877fb in __libc_start_main 
> /home/max/build/v6/sources/glibc/csu/libc-start.c:24
>
> I'll simplify the original testcase, reproduce the same CHECK failure 
> and repost the patch.

I rewrote the original testcase as you suggested in BZ and verified that 
the problem reproduces w/o the patch and goes away with it. Is it OK now?

-Maxim
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr71480-2.diff
Type: text/x-diff
Size: 1699 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20160610/6c06b14f/attachment.bin>