Re: [PATCH] Disable "ODR" checking in libsanitizer

[Kostya Serebryany <kcc at google dot com>]

[text only]
Looks good.

I am not planing to work on the fix any time soon, co I encourage you
or someone else interested to send patches to fix the problem in LLVM.
Since you are also committing a test we should not accidentally remove
this patch with the next merge.

Thanks!
--kcc

On Mon, Feb 23, 2015 at 3:02 PM, Kostya Serebryany <kcc@google.com> wrote:
> Looks good.
>
> I am not planing to work on the fix any time soon, co I encourage you or
> someone else interested to send patches to fix the problem in LLVM.
> Since you are also committing a test we should not accidentally remove this
> patch with the next merge.
>
> Thanks!
> --kcc
>
> On Mon, Feb 23, 2015 at 1:05 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>>
>> Hi!
>>
>> The "ODR" checking lib libsanitizer isn't really ODR checking and relies
>> on
>> the LLVM way of registering the same symbol multiple times, where GCC uses
>> private aliases and the only time "ODR violatilation" is reported is when
>> there are symbol aliases.
>>
>> Until libsanitizer rewrites this to use names instead of addresses and has
>> some bit to turn off ODR checking (e.g. for languages where ODR is
>> non-existent or for internal symbols or private symbols), this stuff
>> doesn't
>> make any sense for GCC and breaks valid code.
>>
>> Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.
>>
>> 2015-02-20  Jakub Jelinek  <jakub@redhat.com>
>>
>>         PR bootstrap/63888
>>         * asan/asan_globals.cc (RegisterGlobal): Disable
>> detect_odr_violation
>>         support until it is rewritten upstream.
>>
>>         * c-c++-common/asan/pr63888.c: New test.
>>
>> --- libsanitizer/asan/asan_globals.cc.jj        2014-11-14
>> 00:10:34.000000000 +0100
>> +++ libsanitizer/asan/asan_globals.cc   2015-02-20 11:43:33.179177767
>> +0100
>> @@ -148,7 +148,9 @@ static void RegisterGlobal(const Global
>>    CHECK(AddrIsInMem(g->beg));
>>    CHECK(AddrIsAlignedByGranularity(g->beg));
>>    CHECK(AddrIsAlignedByGranularity(g->size_with_redzone));
>> -  if (flags()->detect_odr_violation) {
>> +  // This "ODR violation" detection is fundamentally incompatible with
>> +  // how GCC registers globals.  Disable as useless until rewritten
>> upstream.
>> +  if (0 && flags()->detect_odr_violation) {
>>      // Try detecting ODR (One Definition Rule) violation, i.e. the
>> situation
>>      // where two globals with the same name are defined in different
>> modules.
>>      if (__asan_region_is_poisoned(g->beg, g->size_with_redzone)) {
>> --- gcc/testsuite/c-c++-common/asan/pr63888.c.jj        2015-02-23
>> 19:07:02.260413707 +0100
>> +++ gcc/testsuite/c-c++-common/asan/pr63888.c   2015-02-23
>> 19:06:31.000000000 +0100
>> @@ -0,0 +1,34 @@
>> +/* PR bootstrap/63888 */
>> +/* { dg-do run } */
>> +
>> +__attribute__((noinline, noclone)) int
>> +foo (int x)
>> +{
>> +  int v = 0;
>> +  switch (x)
>> +    {
>> +    case 11: v = 67; break;
>> +    case 12: v = 68; break;
>> +    case 13: v = 69; break;
>> +    }
>> +  return v;
>> +}
>> +
>> +__attribute__((noinline, noclone)) int
>> +bar (int x)
>> +{
>> +  int v = 0;
>> +  switch (x)
>> +    {
>> +    case 18: v = 67; break;
>> +    case 19: v = 68; break;
>> +    case 20: v = 69; break;
>> +    }
>> +  return v;
>> +}
>> +
>> +int
>> +main ()
>> +{
>> +  return foo (11) - 67 + bar (19) - 68;
>> +}
>>
>>         Jakub
>
>