Skip to content
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 global instrumentation broken for interposed global variables #36893

Closed
zygoloid mannequin opened this issue May 21, 2018 · 6 comments
Closed

asan global instrumentation broken for interposed global variables #36893

zygoloid mannequin opened this issue May 21, 2018 · 6 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' compiler-rt:asan Address sanitizer

Comments

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented May 21, 2018

Bugzilla Link 37545
Version unspecified
OS Linux
CC @kcc,@rnk,@vitalybuka

Extended Description

Situation:

main binary contains a weak definition of global symbol foo
foo.so contains a strong definition of global symbol foo

ASan instruments the strong definition but not the weak one:

https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/Instrumentation/AddressSanitizer.cpp#L1661

Now, after dynamic linking, the program has only one definition of foo (the one from the main binary) but still has the global registration code from foo.so, which tries to register that global symbol.

In my test case, the registration fails because the weak definition is underaligned for a sanitized global, and ASan reports a bogus ODR violation error. If the alignment happens to match, ASan would presumably instead mark a bogus region of the address space as being global redzone.

As of r332028, Clang creates the above situation for entirely valid code:

// main.cc
#include
struct A;
auto &x = typeid(A*);
void use_foo();
int main() { use_foo(); }

// foo.cc
struct A {
virtual void f();
};
void A::f() {}
void use_foo() {}

$ clang++ foo.cc -shared -o /tmp/foo.so -fsanitize=address
$ clang++ /tmp/foo.so main.cc -o /tmp/main -fsanitize=address
$ /tmp/main
==211158==The following global variable is not properly aligned.
==211158==This may happen if another global with the same name
==211158==resides in another non-instrumented module.
==211158==Or the global comes from a C file built w/o -fno-common.
==211158==In either case this is likely an ODR violation bug,
==211158==but AddressSanitizer can not provide more details.

==211158==ERROR: AddressSanitizer: odr-violation (0x00000050a1d6):
[1] size=3 'typeinfo name for A' -
[2] size=3 'typeinfo name for A' -
These globals were registered at these points:
[1]:
#​0 0x42dcfe in __asan_register_globals[...]/compiler-rt/lib/asan/asan_globals.cc:358:3
#​1 0x7f32dfe18a70 in asan.module_ctor (/tmp/foo.so+0xa70)

[2]:
#​0 0x42dcfe in __asan_register_globals [...]/compiler-rt/lib/asan/asan_globals.cc:358:3
#​1 0x7f32dfe18a70 in asan.module_ctor (/tmp/foo.so+0xa70)

==211158==HINT: if you don't care about these errors you may set ASAN_OPTIONS=detect_odr_violation=0
SUMMARY: AddressSanitizer: odr-violation: global 'typeinfo name for A' at -
==211158==ABORTING

@zygoloid
Copy link
Mannequin Author

zygoloid mannequin commented May 21, 2018

assigned to @vitalybuka

@zygoloid
Copy link
Mannequin Author

zygoloid mannequin commented May 21, 2018

I've reverted r332028 for now, in r332879. Note that this undoes a change that fixes a conformance bug.

@kcc
Copy link
Contributor

kcc commented May 22, 2018

Not sure what to do here.
We can't instrument weak definitions...

We can just avoid instrumenting typeinfo globals as a workaround...

@zygoloid
Copy link
Mannequin Author

zygoloid mannequin commented May 22, 2018

If the global registration data referred to the copy of the symbol from that DSO rather than the one selected by the dynamic linker, I think that would resolve this problem. (I don't know for sure, but I think it may be possible to get that effect by changing the global to an internal symbol with a new name, and adding an external-linkage alias to it with the original name, then using the non-alias name only for the ASan registration data.)

@rnk
Copy link
Collaborator

rnk commented May 26, 2018

The alias trick would work, I guess. Instrumented IR transform would look like:

@​foo = global i32 42
->
@​foo = global alias i32* (getelementptr {i32, [60 x i8]}* @​__asan_rz_foo, i32 0, 0)
@​__asan_rz_foo = internal global {i32, [60 x i8]} { i32 42, [60 x i8] zeroinitializer }
@​__asan_global_foo = ; ... same, using @​__asan_rz_foo

Or, we could try to leverage dso_local information to avoid instrumenting interposable globals. If the user sets -fPIE or doesn't explicitly set -fPIC, we can assume that we'll be linked into an executable, and if so, everything with a definition is dso_local, and everything should be instrumented.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
@MaskRay MaskRay closed this as completed in 1ada819 Nov 3, 2022
@EugeneZelenko EugeneZelenko added the clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' label Nov 3, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 3, 2022

@llvm/issue-subscribers-clang-driver

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' compiler-rt:asan Address sanitizer
Projects
None yet
Development

No branches or pull requests

4 participants