diff mbox

Disable "ODR" checking in libsanitizer

Message ID 20150223210541.GM1746@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Feb. 23, 2015, 9:05 p.m. UTC 
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.


	Jakub

Comments

Kostya Serebryany Feb. 23, 2015, 11:03 p.m. UTC | #1 
[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
>
>
diff mbox

Patch

--- 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;
+}