Closed GoogleCodeExporter closed 9 years ago
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
=======================================================
Original comment by ramosian.glider@gmail.com
on 30 Jan 2012 at 11:33
Attachments:
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
==================================================
Original comment by ramosian.glider@gmail.com
on 30 Jan 2012 at 2:43
>> 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?
Original comment by konstant...@gmail.com
on 30 Jan 2012 at 7:13
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)
...
Original comment by ramosian.glider@gmail.com
on 30 Jan 2012 at 7:58
% 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.
Original comment by konstant...@gmail.com
on 30 Jan 2012 at 9:54
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.
Original comment by konstant...@gmail.com
on 30 Jan 2012 at 11:49
r149300 should have fixed the problem.
Original comment by konstant...@gmail.com
on 30 Jan 2012 at 11:59
r149302 enables the test for this bug.
Original comment by konstant...@gmail.com
on 31 Jan 2012 at 12:00
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.
Original comment by ramosian.glider@gmail.com
on 31 Jan 2012 at 8:53
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.
Original comment by ramosian.glider@gmail.com
on 31 Jan 2012 at 9:20
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.
Original comment by ramosian.glider@gmail.com
on 31 Jan 2012 at 9:37
Fixed in Clang r149605
Original comment by ramosian.glider@gmail.com
on 2 Feb 2012 at 12:14
Original issue reported on code.google.com by
ramosian.glider@gmail.com
on 30 Jan 2012 at 8:19