diff mbox

[asan] Handle noreturn calls with __asan_handle_no_return ()

Message ID 20121205150651.GW2315@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Dec. 5, 2012, 3:06 p.m. UTC 
On Wed, Dec 05, 2012 at 03:49:52PM +0100, Jakub Jelinek wrote:
> +FAIL: g++.dg/asan/asan_test.C  -O2  AddressSanitizer_PthreadExitTest execution test
> 
> but that looks like a library problem to me:
> 
> AddressSanitizer CHECK failed: ../../../../../libsanitizer/asan/asan_rtl.cc:271 "((curr_thread)) != (0)" (0x0, 0x0)

Actually, the problem was that libasan was linked in after libpthread. 
Perhaps we need some driver hacks to make sure -lasan comes before -lpthread
when added automatically for -fsanitize=address (and similarly for -ltsan).
For now just tweaking dg-options.

Ok for trunk? (of course, depends on all the earlier patches from today and
Wei's patch).

With this the only failures I get are:
FAIL: g++.dg/asan/asan_test.C  -O2  AddressSanitizer_BitFieldPositiveTest x->bf1 = 0 execution test
FAIL: g++.dg/asan/asan_test.C  -O2  AddressSanitizer_BitFieldPositiveTest x->bf2 = 0 execution test
FAIL: g++.dg/asan/asan_test.C  -O2  AddressSanitizer_BitFieldPositiveTest x->bf3 = 0 execution test
FAIL: g++.dg/asan/asan_test.C  -O2  AddressSanitizer_BitFieldPositiveTest x->bf4 = 0 execution test
FAIL: g++.dg/asan/asan_test.C  -O2  AddressSanitizer_GlobalStringConstTest Ident(p[15]) execution test
FAIL: g++.dg/asan/asan_test.C  -O2  AddressSanitizer_FileNameInGlobalReportTest Ident(p[15]) output pattern test, should match zoo.*asan_test.cc
which is 4x missing bitfield instrumentation and twice unanalyzed thing.

2012-12-05  Jakub Jelinek  <jakub@redhat.com>

	* g++.dg/asan/asan_test.C: Link -lasan before -lpthread.
	* g++.dg/asan/deep-thread-stack-1.C: Likewise.  Remove dg-skip-if.



	Jakub

Comments

Jakub Jelinek Dec. 5, 2012, 3:43 p.m. UTC | #1 
Hi!

On Wed, Dec 05, 2012 at 04:06:51PM +0100, Jakub Jelinek wrote:
> FAIL: g++.dg/asan/asan_test.C  -O2  AddressSanitizer_GlobalStringConstTest Ident(p[15]) execution test

So, this one fails because we don't instrument string literals, in GCC
they aren't anything close to global variables (like they are in LLVM?)
Perhaps we could, handle STRING_CSTs in categorize_decl_for_section
like flag_mudflap handles them (essentially disable -fmerge-constants),
add code to output_constant_def_contents to emit the padding around it
if protected and in asan_finish_file traverse constant_pool_htab (),
looking for STRING_CSTs that were TREE_ASM_WRITTEN.

> FAIL: g++.dg/asan/asan_test.C  -O2  AddressSanitizer_FileNameInGlobalReportTest Ident(p[15]) output pattern test, should match zoo.*asan_test.cc

This one fails to match because the filename in that case isn't
asan_test.cc, but asan_test.C.  Can it use say
ASAN_TEST_NAME macro,
#ifndef ASAN_TEST_NAME
# define ASAN_TEST_NAME "asan_test.cc"
#endif

and replace "zoo.*asan_test.cc" with "zoo.*" ASAN_TEST_NAME (and similarly
in one of the disabled tests)?  Then asan_test.C could just
#define ASAN_TEST_NAME "asan_test.C"
before #include "asan_test.cc"

	Jakub
Richard Biener Dec. 5, 2012, 4:13 p.m. UTC | #2 
On Wed, Dec 5, 2012 at 4:43 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> On Wed, Dec 05, 2012 at 04:06:51PM +0100, Jakub Jelinek wrote:
>> FAIL: g++.dg/asan/asan_test.C  -O2  AddressSanitizer_GlobalStringConstTest Ident(p[15]) execution test
>
> So, this one fails because we don't instrument string literals, in GCC
> they aren't anything close to global variables (like they are in LLVM?)
> Perhaps we could, handle STRING_CSTs in categorize_decl_for_section
> like flag_mudflap handles them (essentially disable -fmerge-constants),
> add code to output_constant_def_contents to emit the padding around it
> if protected and in asan_finish_file traverse constant_pool_htab (),
> looking for STRING_CSTs that were TREE_ASM_WRITTEN.

For LTO we need to remove the STRING_CST special handling in the IL
anyway (and use CONST_DECLs for them).  See PR50199.

The fallout is of course not very high on my list to fix ;)  But I have some
proof-of-concept patch somewhere.

Richard.

>> FAIL: g++.dg/asan/asan_test.C  -O2  AddressSanitizer_FileNameInGlobalReportTest Ident(p[15]) output pattern test, should match zoo.*asan_test.cc
>
> This one fails to match because the filename in that case isn't
> asan_test.cc, but asan_test.C.  Can it use say
> ASAN_TEST_NAME macro,
> #ifndef ASAN_TEST_NAME
> # define ASAN_TEST_NAME "asan_test.cc"
> #endif
>
> and replace "zoo.*asan_test.cc" with "zoo.*" ASAN_TEST_NAME (and similarly
> in one of the disabled tests)?  Then asan_test.C could just
> #define ASAN_TEST_NAME "asan_test.C"
> before #include "asan_test.cc"
>
>         Jakub
Konstantin Serebryany Dec. 5, 2012, 5:42 p.m. UTC | #3 
On Wed, Dec 5, 2012 at 7:43 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> On Wed, Dec 05, 2012 at 04:06:51PM +0100, Jakub Jelinek wrote:
>> FAIL: g++.dg/asan/asan_test.C  -O2  AddressSanitizer_GlobalStringConstTest Ident(p[15]) execution test
>
> So, this one fails because we don't instrument string literals, in GCC
> they aren't anything close to global variables (like they are in LLVM?)

In LLVM, string literals are just regular constant global arrays of chars.
And global overflows in such literals are remarkably frequent, so it's
a good idea to implement this.

> Perhaps we could, handle STRING_CSTs in categorize_decl_for_section
> like flag_mudflap handles them (essentially disable -fmerge-constants),
> add code to output_constant_def_contents to emit the padding around it
> if protected and in asan_finish_file traverse constant_pool_htab (),
> looking for STRING_CSTs that were TREE_ASM_WRITTEN.
>
>> FAIL: g++.dg/asan/asan_test.C  -O2  AddressSanitizer_FileNameInGlobalReportTest Ident(p[15]) output pattern test, should match zoo.*asan_test.cc
>
> This one fails to match because the filename in that case isn't
> asan_test.cc, but asan_test.C.  Can it use say
> ASAN_TEST_NAME macro,
> #ifndef ASAN_TEST_NAME
> # define ASAN_TEST_NAME "asan_test.cc"
> #endif
>
> and replace "zoo.*asan_test.cc" with "zoo.*" ASAN_TEST_NAME (and similarly
> in one of the disabled tests)?  Then asan_test.C could just
> #define ASAN_TEST_NAME "asan_test.C"
> before #include "asan_test.cc"

I'd rather replace it with a regexp asan_test.{cc,C} or even simpler,
drop the extension. Ok?

--kcc

>
>         Jakub
Jakub Jelinek Dec. 5, 2012, 5:46 p.m. UTC | #4 
On Wed, Dec 05, 2012 at 09:42:08PM +0400, Konstantin Serebryany wrote:
> On Wed, Dec 5, 2012 at 7:43 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> > So, this one fails because we don't instrument string literals, in GCC
> > they aren't anything close to global variables (like they are in LLVM?)
> 
> In LLVM, string literals are just regular constant global arrays of chars.
> And global overflows in such literals are remarkably frequent, so it's
> a good idea to implement this.

Yeah, I'm almost done with it.

> > This one fails to match because the filename in that case isn't
> > asan_test.cc, but asan_test.C.  Can it use say
> > ASAN_TEST_NAME macro,
> > #ifndef ASAN_TEST_NAME
> > # define ASAN_TEST_NAME "asan_test.cc"
> > #endif
> >
> > and replace "zoo.*asan_test.cc" with "zoo.*" ASAN_TEST_NAME (and similarly
> > in one of the disabled tests)?  Then asan_test.C could just
> > #define ASAN_TEST_NAME "asan_test.C"
> > before #include "asan_test.cc"
> 
> I'd rather replace it with a regexp asan_test.{cc,C} or even simpler,
> drop the extension. Ok?

That would be asan_test.(cc|C) (or does gtest have some weirdo regexp
extensions (I'm just aware that it is a subset).
But yeah, dropping the extension altogether is fine with me.

	Jakub
Konstantin Serebryany Dec. 5, 2012, 5:59 p.m. UTC | #5 
On Wed, Dec 5, 2012 at 9:46 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Dec 05, 2012 at 09:42:08PM +0400, Konstantin Serebryany wrote:
>> On Wed, Dec 5, 2012 at 7:43 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > So, this one fails because we don't instrument string literals, in GCC
>> > they aren't anything close to global variables (like they are in LLVM?)
>>
>> In LLVM, string literals are just regular constant global arrays of chars.
>> And global overflows in such literals are remarkably frequent, so it's
>> a good idea to implement this.
>
> Yeah, I'm almost done with it.
>
>> > This one fails to match because the filename in that case isn't
>> > asan_test.cc, but asan_test.C.  Can it use say
>> > ASAN_TEST_NAME macro,
>> > #ifndef ASAN_TEST_NAME
>> > # define ASAN_TEST_NAME "asan_test.cc"
>> > #endif
>> >
>> > and replace "zoo.*asan_test.cc" with "zoo.*" ASAN_TEST_NAME (and similarly
>> > in one of the disabled tests)?  Then asan_test.C could just
>> > #define ASAN_TEST_NAME "asan_test.C"
>> > before #include "asan_test.cc"
>>
>> I'd rather replace it with a regexp asan_test.{cc,C} or even simpler,
>> drop the extension. Ok?
>
> That would be asan_test.(cc|C) (or does gtest have some weirdo regexp
> extensions (I'm just aware that it is a subset).
> But yeah, dropping the extension altogether is fine with me.

Just dropped the extension in upstream r169392.
At some point we need to update the libsanitizer/merge.sh to
automatically merge the tests,
but perhaps only after all the tests start passing.


--kcc

>
>         Jakub
Dodji Seketeli Dec. 11, 2012, 4 p.m. UTC | #6 
Jakub Jelinek <jakub@redhat.com> writes:

> Actually, the problem was that libasan was linked in after libpthread. 
> Perhaps we need some driver hacks to make sure -lasan comes before -lpthread
> when added automatically for -fsanitize=address (and similarly for -ltsan).
> For now just tweaking dg-options.


> Ok for trunk? (of course, depends on all the earlier patches from today and
> Wei's patch).

This is OK, thanks.
diff mbox

Patch

--- gcc/testsuite/g++.dg/asan/asan_test.C.jj	2012-12-03 12:43:20.000000000 +0100
+++ gcc/testsuite/g++.dg/asan/asan_test.C	2012-12-05 15:50:36.686157455 +0100
@@ -2,7 +2,7 @@ 
 // { dg-skip-if "" { *-*-* } { "*" } { "-O2" } }
 // { dg-skip-if "" { *-*-* } { "-flto" } { "" } }
 // { dg-additional-sources "asan_globals_test.cc" }
-// { dg-options "-fsanitize=address -fno-builtin -Wall -Wno-format -Werror -g -DASAN_UAR=0 -DASAN_HAS_EXCEPTIONS=1 -DASAN_HAS_BLACKLIST=0 -DASAN_USE_DEJAGNU_GTEST=1 -lpthread -ldl" }
+// { dg-options "-fsanitize=address -fno-builtin -Wall -Wno-format -Werror -g -DASAN_UAR=0 -DASAN_HAS_EXCEPTIONS=1 -DASAN_HAS_BLACKLIST=0 -DASAN_USE_DEJAGNU_GTEST=1 -lasan -lpthread -ldl" }
 // { dg-additional-options "-DASAN_NEEDS_SEGV=1" { target { ! arm*-*-* } } }
 // { dg-additional-options "-DASAN_LOW_MEMORY=1 -DASAN_NEEDS_SEGV=0" { target arm*-*-* } }
 // { dg-additional-options "-DASAN_AVOID_EXPENSIVE_TESTS=1" { target { ! run_expensive_tests } } }
--- gcc/testsuite/g++.dg/asan/deep-thread-stack-1.C.jj	2012-12-05 11:45:31.000000000 +0100
+++ gcc/testsuite/g++.dg/asan/deep-thread-stack-1.C	2012-12-05 15:51:20.687897439 +0100
@@ -1,6 +1,5 @@ 
 // { dg-do run { target pthread } }
-// { dg-skip-if "" { *-*-* } { "*" } { "" } }
-// { dg-options "-lpthread" }
+// { dg-options "-lasan -lpthread" }
 // { dg-shouldfail "asan" }
 
 #include <pthread.h>