New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ASan instrumentation should work with -O0 #11

Closed
ramosian-glider opened this Issue Aug 31, 2015 · 7 comments

Comments

Projects
None yet
2 participants
@ramosian-glider
Member

ramosian-glider commented Aug 31, 2015

Originally reported on Google Code with ID 11

What steps will reproduce the problem?

$ cat try.c
#include <stdio.h>
int main() { printf("Ok\n"); return(0); }

$ clang -o try -g3 -D__APPLE__ -faddress-sanitizer try.c
$ ./try
Ok==33560== 
CHECK failed: summary at asan_thread_registry.cc:99, pthread_self=0x7fff70989cc0
Segmentation fault

$ gdb ./try
(gdb) r
(gdb) bt
#0  0x0000000000000000 in ?? ()
#1  0x000000010000b3af in __asan::AsanThread::ClearShadowForThreadStack (this=0x10242a9a0)
at asan_thread.cc:51
#2  0x000000010000b6b1 in __asan::AsanThread::~AsanThread (this=0x10242a9a0) at asan_thread.cc:45
#3  0x000000010000c3d0 in __asan::AsanThreadRegistry::~AsanThreadRegistry (this=0x10042a9a0)
at asan_thread_registry.h:29
#4  0x000000010000fe15 in __tcf_0 () at asan_thread_registry.cc:24
#5  0x00007fff8640a374 in __cxa_finalize ()
#6  0x00007fff8640a28c in exit ()
#7  0x0000000100000f53 in start () at asan_thread_registry.h:28

(gdb) up
(gdb) p *this
$1 = {
  static kInvalidTid = -1, 
  summary_ = 0x0, 
  start_routine_ = 0, 
  arg_ = 0x0, 
  stack_top_ = 0, 
  stack_bottom_ = 0, 
  fake_stack_ = {
    static kMinStackFrameSizeLog = 9, 
    static kMaxStackFrameSizeLog = 16, 
    static kMaxStackMallocSize = 65536, 
    static kNumberOfSizeClasses = 8, 
    stack_size_ = 0, 
    alive_ = false, 
    allocated_size_classes_ = {0, 0, 0, 0, 0, 0, 0, 0}, 
...

$ clang --version
clang version 3.1 (trunk 144800)
Target: x86_64-apple-darwin10.8.0
Thread model: posix

The attached patch fixed it for me

Reported by reini.urban on 2011-11-22 02:21:14


- _Attachment: [protect-empty-prog.patch](https://storage.googleapis.com/google-code-attachments/address-sanitizer/issue-11/comment-0/protect-empty-prog.patch)_
@ramosian-glider

This comment has been minimized.

Show comment
Hide comment
@ramosian-glider

ramosian-glider Aug 31, 2015

Member

Reported by konstantin.s.serebryany on 2011-11-22 02:36:20

  • Status changed: Accepted
Member

ramosian-glider commented Aug 31, 2015

Reported by konstantin.s.serebryany on 2011-11-22 02:36:20

  • Status changed: Accepted
@ramosian-glider

This comment has been minimized.

Show comment
Hide comment
@ramosian-glider

ramosian-glider Aug 31, 2015

Member
This happens because ASan does not currently work with -O0. In fact the module remains
uninstrumented, thus __asan_init() is not called and the runtime library is in inconsistent
state. This problem cannot be fixed without calling __asan_init(), so the proposed
patch is invalid.

The short-term workaround is to pass an additional -O1 option to the compiler:
  $ clang try.c -faddress-sanitizer -o try -O1
  $ ./try
  Ok

A proper fix on the ASan side is to allow performing the instrumentation with -O0.
ASan instrumentation is conceptually different from code optimization, so this is correct.

Reported by ramosian.glider on 2011-11-22 08:18:41

  • Labels added: Priority-High, OpSys-All
  • Labels removed: Priority-Medium
Member

ramosian-glider commented Aug 31, 2015

This happens because ASan does not currently work with -O0. In fact the module remains
uninstrumented, thus __asan_init() is not called and the runtime library is in inconsistent
state. This problem cannot be fixed without calling __asan_init(), so the proposed
patch is invalid.

The short-term workaround is to pass an additional -O1 option to the compiler:
  $ clang try.c -faddress-sanitizer -o try -O1
  $ ./try
  Ok

A proper fix on the ASan side is to allow performing the instrumentation with -O0.
ASan instrumentation is conceptually different from code optimization, so this is correct.

Reported by ramosian.glider on 2011-11-22 08:18:41

  • Labels added: Priority-High, OpSys-All
  • Labels removed: Priority-Medium
@ramosian-glider

This comment has been minimized.

Show comment
Hide comment
@ramosian-glider

ramosian-glider Aug 31, 2015

Member
Thanks. This explains why I found no single error in the Perl testsuite and a fair number
of CPAN modules so far. Too good to be true. No single SIGSEGV. With -O1 it works fine

Reported by reini.urban on 2011-11-22 14:18:44

Member

ramosian-glider commented Aug 31, 2015

Thanks. This explains why I found no single error in the Perl testsuite and a fair number
of CPAN modules so far. Too good to be true. No single SIGSEGV. With -O1 it works fine

Reported by reini.urban on 2011-11-22 14:18:44

@ramosian-glider

This comment has been minimized.

Show comment
Hide comment
@ramosian-glider

ramosian-glider Aug 31, 2015

Member
Attached is a patch for Clang that fixes the problem with -O0 (this is not a replacement
for llvm/clang.patch, it should be applied separately)

Once someone on the LLVM-dev approves it, we'll commit it into Clang.

Reported by ramosian.glider on 2011-11-23 08:11:19


- _Attachment: [clang.patch](https://storage.googleapis.com/google-code-attachments/address-sanitizer/issue-11/comment-4/clang.patch)_
Member

ramosian-glider commented Aug 31, 2015

Attached is a patch for Clang that fixes the problem with -O0 (this is not a replacement
for llvm/clang.patch, it should be applied separately)

Once someone on the LLVM-dev approves it, we'll commit it into Clang.

Reported by ramosian.glider on 2011-11-23 08:11:19


- _Attachment: [clang.patch](https://storage.googleapis.com/google-code-attachments/address-sanitizer/issue-11/comment-4/clang.patch)_
@ramosian-glider

This comment has been minimized.

Show comment
Hide comment
@ramosian-glider

ramosian-glider Aug 31, 2015

Member
Works for me, thanks. At least for the perl Configure probes.

Got now into other problems unrelated to this patch (fails with and without) to be
handled in a separate ticket.

Reported by reini.urban on 2011-11-23 16:41:18

Member

ramosian-glider commented Aug 31, 2015

Works for me, thanks. At least for the perl Configure probes.

Got now into other problems unrelated to this patch (fails with and without) to be
handled in a separate ticket.

Reported by reini.urban on 2011-11-23 16:41:18

@ramosian-glider

This comment has been minimized.

Show comment
Hide comment
@ramosian-glider

ramosian-glider Aug 31, 2015

Member
The O0 patch has landed. 
The build instructions updated. (note, they are unstable, nothing is guaranteed to
work until we are done integrating with llvm). 

Reported by konstantin.s.serebryany on 2011-12-01 05:56:58

  • Status changed: Fixed
Member

ramosian-glider commented Aug 31, 2015

The O0 patch has landed. 
The build instructions updated. (note, they are unstable, nothing is guaranteed to
work until we are done integrating with llvm). 

Reported by konstantin.s.serebryany on 2011-12-01 05:56:58

  • Status changed: Fixed
@ramosian-glider

This comment has been minimized.

Show comment
Hide comment
@ramosian-glider

ramosian-glider Aug 31, 2015

Member
Adding Project:AddressSanitizer as part of GitHub migration.

Reported by ramosian.glider on 2015-07-30 09:12:58

  • Labels added: ProjectAddressSanitizer
Member

ramosian-glider commented Aug 31, 2015

Adding Project:AddressSanitizer as part of GitHub migration.

Reported by ramosian.glider on 2015-07-30 09:12:58

  • Labels added: ProjectAddressSanitizer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment