ASan doesn't instrument the +load methods

[ramosian-glider]

Originally reported on Google Code with ID 33

To reproduce, build asan_tests32, run
$ gobjdump -D bin_darwin/asan_test32
...

0013b460 <+[LoadSomething load]>:
 13b460:       55                      push   %ebp
 13b461:       89 e5                   mov    %esp,%ebp
 13b463:       53                      push   %ebx
 13b464:       57                      push   %edi
 13b465:       56                      push   %esi
 13b466:       83 ec 0c                sub    $0xc,%esp
 13b469:       e8 00 00 00 00          call   13b46e <+[LoadSomething
load]+0xe>
 13b46e:       5b                      pop    %ebx
 13b46f:       8d b3 72 75 1b 00       lea    0x1b7572(%ebx),%esi
 13b475:       89 34 24                mov    %esi,(%esp)
 13b478:       e8 4f 77 17 00          call   2b2bcc <_strlen$stub>
 13b47d:       85 c0                   test   %eax,%eax
 13b47f:       74 22                   je     13b4a3 <+[LoadSomething
load]+0x43>
 13b481:       31 ff                   xor    %edi,%edi
 13b483:       8d 9b 72 75 1b 00       lea    0x1b7572(%ebx),%ebx
 13b489:       0f 1f 80 00 00 00 00    nopl   0x0(%eax)
 13b490:       8a 06                   mov    (%esi),%al
 13b492:       88 45 f3                mov    %al,-0xd(%ebp)
 13b495:       89 1c 24                mov    %ebx,(%esp)
 13b498:       46                      inc    %esi
 13b499:       47                      inc    %edi
 13b49a:       e8 2d 77 17 00          call   2b2bcc <_strlen$stub>
 13b49f:       39 c7                   cmp    %eax,%edi
 13b4a1:       72 ed                   jb     13b490 <+[LoadSomething
load]+0x30>
 13b4a3:       83 c4 0c                add    $0xc,%esp
 13b4a6:       5e                      pop    %esi
 13b4a7:       5f                      pop    %edi
 13b4a8:       5b                      pop    %ebx
 13b4a9:       5d                      pop    %ebp
 13b4aa:       c3                      ret
 13b4ab:       0f 1f 44 00 00          nopl   0x0(%eax,%eax,1)
...

, and note the missing __asan_init calls.

This regression was probably introduced by r148842, which introduced __attribute__((no_address_safety_analysis))

Reported by ramosian.glider on 2012-01-30 08:19:21

[ramosian-glider]

GetOrCreateLLVMFunction appears not to be called for every function in the module.
Applying the attached patch to tools/clang/lib/CodeGen/CodeGenModule.cpp produces the
following output for a simple module:

=======================================================
$ cat t.c 
extern void bar();
extern void baz();
void foo() {
  bar();
  baz();
}
void woo() {
  bar();
  baz();
}
void goo() {
  bar();
  baz();
}
=======================================================
clang++ -c t.c
clang: warning: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
error: GetOrCreateLLVMFunction: _Z3foov
error: GetOrCreateLLVMFunction: HERE
error: GetOrCreateLLVMFunction: _Z3barv
error: GetOrCreateLLVMFunction: HERE
error: GetOrCreateLLVMFunction: _Z3bazv
error: GetOrCreateLLVMFunction: HERE
=======================================================
clang++  -faddress-sanitizer -c t.c
clang: warning: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
error: GetOrCreateLLVMFunction: _Z3foov
error: GetOrCreateLLVMFunction: HERE
error: GetOrCreateLLVMFunction: THERE

error: GetOrCreateLLVMFunction: _Z3barv
error: GetOrCreateLLVMFunction: HERE
error: GetOrCreateLLVMFunction: THERE

error: GetOrCreateLLVMFunction: _Z3bazv
error: GetOrCreateLLVMFunction: HERE
error: GetOrCreateLLVMFunction: THERE
=======================================================

Reported by ramosian.glider on 2012-01-30 11:33:37

---

- _Attachment: [codegen.patch](https://storage.googleapis.com/google-code-attachments/address-sanitizer/issue-33/comment-1/codegen.patch)_

[ramosian-glider]

Please disregard this. Using error messages for debugging is not a good thing to do.
I've added fprintf() calls instead of them and looks like GetOrCreateLLVMFunction is
called for every function but those having the "internal" attribute. This includes
e.g. static functions:

==========================================================

$ svn diff tools/clang/lib/CodeGen/CodeGenModule.cpp 
Index: tools/clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- tools/clang/lib/CodeGen/CodeGenModule.cpp   (revision 149231)
+++ tools/clang/lib/CodeGen/CodeGenModule.cpp   (working copy)
@@ -979,6 +979,7 @@
                                        llvm::Type *Ty,
                                        GlobalDecl D, bool ForVTable,
                                        llvm::Attributes ExtraAttrs) {
+   fprintf(stderr, "GetOrCreateLLVMFunction: %s\n", MangledName.str().c_str());
   // Lookup the entry, lazily creating it if necessary.
   llvm::GlobalValue *Entry = GetGlobalValue(MangledName);
   if (Entry) {
@@ -1025,6 +1026,7 @@
     const FunctionDecl *FD = cast_or_null<FunctionDecl>(D.getDecl());
     if (!FD || !FD->hasAttr<NoAddressSafetyAnalysisAttr>())
       F->addFnAttr(llvm::Attribute::AddressSafety);
+   fprintf(stderr, "AddressSafety: %s\n", MangledName.str().c_str());
   }

   // This is the first use or definition of a mangled name.  If there is a
================================================
$ cat t.c
extern void bar();
extern void baz();
void foo() {
  bar();
  baz();
}
void woo() {
  bar();
  baz();
}
static void goo() {
  bar();
  baz();
}
================================================
$ clang++   -faddress-sanitizer t.c -c
clang: warning: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
GetOrCreateLLVMFunction: _Z3foov
AddressSafety: _Z3foov
GetOrCreateLLVMFunction: _Z3barv
AddressSafety: _Z3barv
GetOrCreateLLVMFunction: _Z3bazv
AddressSafety: _Z3bazv
GetOrCreateLLVMFunction: _Z3woov
AddressSafety: _Z3woov
GetOrCreateLLVMFunction: _Z3barv
GetOrCreateLLVMFunction: _Z3bazv
==================================================

Reported by ramosian.glider on 2012-01-30 14:43:00

[ramosian-glider]

>> This regression was probably introduced by r148842
It is very unlikely

Could you please explain the problem once again? 
Why we can't have a test for this on Linux? 
Why is the right solution to do something in the instrumentation module and not in
the run-time? 

Reported by konstantin.s.serebryany on 2012-01-30 19:13:07

[ramosian-glider]

Our problem is that for some reason the +load methods of ObjC classes do not get the
AddressSafety attribute and thus are not instrumented.
It is required to insert a call to __asan_init at the beginning of each such method,
because they are executed by the ObjC runtime before the static constructors.
As I can tell, GetOrCreateLLVMFunction() (which sets up the AddressSafety attribute)
is never called for such methods -- maybe because they're marked as "internal" in the
LLVM IR. Thus it's questionable whether GetOrCreateLLVMFunction is the right place
to set this attribute.

Here's the minimal test:
===================

$ cat goo.mm 
#include <stdio.h>
#import <CoreFoundation/CFBase.h>
#import <Foundation/NSObject.h>

char kStartupStr[] = "Hello world!\n";

@interface LoadSomething : NSObject { }
@end

@implementation LoadSomething
+(void) load {
  printf(kStartupStr);
}
@end

int main() {
  return 0;
}
==================
$ clang goo.mm -o goo  -framework Foundation -faddress-sanitizer
$ gobjdump -d goo
...
0000000100001c30 <+[LoadSomething load]>:
   100001c30:   55                      push   %rbp
   100001c31:   48 89 e5                mov    %rsp,%rbp
   100001c34:   48 83 ec 20             sub    $0x20,%rsp
   100001c38:   48 8d 05 21 48 01 00    lea    0x14821(%rip),%rax        # 100016460
<_kStartupStr>
   100001c3f:   48 89 7d f8             mov    %rdi,-0x8(%rbp)
   100001c43:   48 89 75 f0             mov    %rsi,-0x10(%rbp)
   100001c47:   48 89 c7                mov    %rax,%rdi
   100001c4a:   b0 00                   mov    $0x0,%al
   100001c4c:   e8 b9 c0 00 00          callq  10000dd0a <_printf$stub>
   100001c51:   89 45 ec                mov    %eax,-0x14(%rbp)
   100001c54:   48 83 c4 20             add    $0x20,%rsp
   100001c58:   5d                      pop    %rbp
   100001c59:   c3                      retq   
   100001c5a:   66 0f 1f 44 00 00       nopw   0x0(%rax,%rax,1)
...

Reported by ramosian.glider on 2012-01-30 19:58:57

[ramosian-glider]

% clang -emit-llvm goo.mm -S -o -  -framework Foundation -faddress-sanitizer
...
define internal void @"\01+[LoadSomething load]"(i8* %self, i8* %_cmd) uwtable ssp
{
...

r148846 is to blame -- it now requires the function to have address_safety attribute.

And the frontend for some reason does not add this attribute to this function. 

Still, I'd prefer to have a solution that does not require to instrument these functions.

Reported by konstantin.s.serebryany on 2012-01-30 21:54:09

[ramosian-glider]

For the record: the test in tests/asan_mac_test.mm which is supposed to catch this problem
did not work, because the code was optimized away. 

Fixed test would SEGV with this stack trace: 
#0  0x0013a80e in access_memory (a=0x2f0ae0 "If your test didn't crash, AddressSanitizer
is instrumenting the +load methods correctly.") at tests/asan_mac_test.mm:37
#1  0x0013a87f in +[LoadSomething load] (self=0x131a044, _cmd=0x92e7a5f8) at tests/asan_mac_test.mm:59
#2  0x90e08bb4 in call_load_methods ()
#3  0x90e0892e in load_images ()
#4  0x8fe036c8 in __dyld__ZN4dyldL12notifySingleE17dyld_image_statesPK11ImageLoader
()
#5  0x8fe0d30a in __dyld__ZN11ImageLoader23recursiveInitializationERKNS_11LinkContextEj
()
#6  0x8fe0d3d1 in __dyld__ZN11ImageLoader15runInitializersERKNS_11LinkContextE ()
#7  0x8fe024a9 in __dyld__ZN4dyld24initializeMainExecutableEv ()
#8  0x8fe07950 in __dyld__ZN4dyld5_mainEPK12macho_headermiPPKcS5_S5_ ()
#9  0x8fe018b1 in __dyld__ZN13dyldbootstrap5startEPK12macho_headeriPPKcl ()
#10 0x8fe01057 in __dyld__dyld_start ()


Ideally, we would call __asan_init somewhere before call_load_methods, but I have no
idea how to make it. 
So, for now let's make sure " load]" functions are instrumented. 
Patch coming soon. 

Reported by konstantin.s.serebryany on 2012-01-30 23:49:16

[ramosian-glider]

r149300 should have fixed the problem. 

Reported by konstantin.s.serebryany on 2012-01-30 23:59:02

[ramosian-glider]

r149302 enables the test for this bug. 

Reported by konstantin.s.serebryany on 2012-01-31 00:00:30

• Status changed: Fixed

[ramosian-glider]

You've made the tests pass, but this doesn't necessarily acknowledge that the tool works
:)

====================================================
$ svn diff tests/asan_mac_test.mm
Index: tests/asan_mac_test.mm
===================================================================
--- tests/asan_mac_test.mm  (revision 149369)
+++ tests/asan_mac_test.mm  (working copy)
@@ -8,6 +8,8 @@
 #import <CoreFoundation/CFBase.h>
 #import <Foundation/NSObject.h>

+extern int volatile glob = 0;
+
 void CFAllocatorDefaultDoubleFree() {
   void *mem =  CFAllocatorAllocate(kCFAllocatorDefault, 5, 0);
   CFAllocatorDeallocate(kCFAllocatorDefault, mem);
@@ -57,6 +59,7 @@
   for (int i = 0; i < strlen(kStartupStr); i++) {
     access_memory(&kStartupStr[i]);  // make sure no optimizations occur.
   }
+  glob = 0xf00ba7;
   // Don't print anything here not to interfere with the death tests.
 }
====================================================
$ make -f Makefile.old  b32
...
$ gobjdump -d bin_darwin/asan_test32
...

0013a7c0 <+[LoadSomething load]>:
  13a7c0:       55                      push   %ebp
  13a7c1:       89 e5                   mov    %esp,%ebp
  13a7c3:       53                      push   %ebx
  13a7c4:       57                      push   %edi
  13a7c5:       56                      push   %esi
  13a7c6:       83 ec 0c                sub    $0xc,%esp
  13a7c9:       e8 00 00 00 00          call   13a7ce <+[LoadSomething load]+0xe>
  13a7ce:       5e                      pop    %esi
  13a7cf:       e8 ec e8 f0 ff          call   490c0 <___asan_init>
  13a7d4:       8d be f2 62 1b 00       lea    0x1b62f2(%esi),%edi
  13a7da:       89 3c 24                mov    %edi,(%esp)
  13a7dd:       e8 38 63 17 00          call   2b0b1a <_strlen$stub>
  13a7e2:       85 c0                   test   %eax,%eax
  13a7e4:       74 26                   je     13a80c <+[LoadSomething load]+0x4c>
  13a7e6:       31 db                   xor    %ebx,%ebx
  13a7e8:       0f 1f 84 00 00 00 00    nopl   0x0(%eax,%eax,1)
  13a7ef:       00 
  13a7f0:       8d 84 1e f2 62 1b 00    lea    0x1b62f2(%esi,%ebx,1),%eax
  13a7f7:       89 04 24                mov    %eax,(%esp)
  13a7fa:       e8 81 ff ff ff          call   13a780 <_access_memory>
  13a7ff:       89 3c 24                mov    %edi,(%esp)
  13a802:       43                      inc    %ebx
  13a803:       e8 12 63 17 00          call   2b0b1a <_strlen$stub>
  13a808:       39 c3                   cmp    %eax,%ebx
  13a80a:       72 e4                   jb     13a7f0 <+[LoadSomething load]+0x30>
  13a80c:       c7 86 52 e9 1d 01 a7    movl   $0xf00ba7,0x11de952(%esi)
  13a813:       0b f0 00 
  13a816:       83 c4 0c                add    $0xc,%esp
  13a819:       5e                      pop    %esi
  13a81a:       5f                      pop    %edi
  13a81b:       5b                      pop    %ebx
  13a81c:       5d                      pop    %ebp
  13a81d:       c3                      ret    
  13a81e:       66 90                   xchg   %ax,%ax
====================================================

So the ObjC functions are still not getting the AddressSafety attribute.

Reported by ramosian.glider on 2012-01-31 08:53:45

• Status changed: Accepted

[ramosian-glider]

The following patch appears to fix the problem with AddressSafety:

===================================================================
Index: tools/clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- tools/clang/lib/CodeGen/CodeGenModule.cpp   (revision 149369)
+++ tools/clang/lib/CodeGen/CodeGenModule.cpp   (working copy)
@@ -520,6 +520,13 @@
   else if (Features.getStackProtector() == LangOptions::SSPReq)
     F->addFnAttr(llvm::Attribute::StackProtectReq);

+  if (Features.AddressSanitizer) {
+    // When AddressSanitizer is enabled, set AddressSafety attribute
+    // unless __attribute__((no_address_safety_analysis)) is used.
+    if (!D->hasAttr<NoAddressSafetyAnalysisAttr>())
+      F->addFnAttr(llvm::Attribute::AddressSafety);
+  }
+
   unsigned alignment = D->getMaxAlignment() / Context.getCharWidth();
   if (alignment)
     F->setAlignment(alignment);
===================================================================

If this works for our tests, we'll also need to remove the corresponding code from
CodeGenModule::GetOrCreateLLVMFunction.

Reported by ramosian.glider on 2012-01-31 09:20:48

[ramosian-glider]

LLVM's `make check` passes for me (modulo the seven failing tests broken by Chris today),
so does ASan's `make t32`
I'm going to test this on Linux and send the patch for review.

Reported by ramosian.glider on 2012-01-31 09:37:11

[ramosian-glider]

Fixed in Clang r149605

Reported by ramosian.glider on 2012-02-02 12:14:01

• Status changed: Fixed

[ramosian-glider]

Adding Project:AddressSanitizer as part of GitHub migration.

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

• Labels added: ProjectAddressSanitizer