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] False positive ODR violation error for global data (e.g. vtable) #101671

Open
glebov-andrey opened this issue Aug 2, 2024 · 3 comments
Open
Labels
compiler-rt:asan Address sanitizer false-positive Warning fires when it should not

Comments

@glebov-andrey
Copy link

Hi!

This issue initially manifested itself as an ODR violation for vtable for boost::filesystem::filesystem_error when Boost.Filesystem is a static library linked into two shared libraries (built with Clang).
After some minimization it appears to be an issue common to all global non-weak data (symbol types D and B in nm's output). On Clang this affects vtables while on GCC it does not because its vtables are weak symbols (or maybe they just aren't checked since they don't have associated __odr_asan. symbols?).

Here's the setup which reproduces the issue on both Clang and GCC:

  • Static library fs includes an extern int global_var.
  • Shared library lib uses global_var from fs somehow to avoid it being discarded.
  • Executable exe uses global_var from fs somehow to avoid it being discarded, and also links to lib.

When running the executable we get the error:

=================================================================
==16997==ERROR: AddressSanitizer: odr-violation (0x7fb16f5bafc0):
  [1] size=4 'fs::global_var' fs_error.cpp in out/exe
  [2] size=4 'fs::global_var' fs_error.cpp in out/lib.so
These globals were registered at these points:
  [1]:
    #0 0x7fb16eadc752  (out/exe+0x41752)
    #1 0x7fb16eadd8f9  (out/exe+0x428f9)
    #2 0x7fb16e489eba  (/lib/x86_64-linux-gnu/libc.so.6+0x29eba) (BuildId: 490fef8403240c91833978d494d39e537409b92e)

  [2]:
    #0 0x7fb16eadc752  (out/exe+0x41752)
    #1 0x7fb16eadd8f9  (out/exe+0x428f9)
    #2 0x7fb16ea5647d  (/lib64/ld-linux-x86-64.so.2+0x647d) (BuildId: 4186944c50f8a32b47d74931e3f512b811813b64)

==16997==HINT: if you don't care about these errors you may set ASAN_OPTIONS=detect_odr_violation=0
SUMMARY: AddressSanitizer: odr-violation: global 'fs::global_var' at fs_error.cpp in out/exe
==16997==ABORTING

I've minimized the flags required and really the only one is -fsanitize=address:

cpp_flags=(-fPIC -O0 -fsanitize=address)
link_flags=("-Wl,-rpath,\$ORIGIN")

-fvisibility=hidden doesn't affect anything as long as the symbols of interest are marked [[gnu::visibility("default")]].

I'm attaching a zip file with a full example: asan-odr-repro.zip

Tested on Linux x86_64.
Clang 16.0.6, 18.1.8 and main all exhibit the same behavior including for vtables.
GCC 11.4 and 12.3 only exhibit the behavior for global variables.

@EugeneZelenko EugeneZelenko added compiler-rt:asan Address sanitizer false-positive Warning fires when it should not and removed new issue labels Aug 2, 2024
@glebov-andrey
Copy link
Author

So I've been thinking more about this error and I understand why on a "physical" level it's correct - after all there are two copies of the symbol definition.

On the other hand, from the point of view of the developer, that's not really the case. There is one source file, one compiler invocation, one object file/library, one dependency graph node with the definition. The fact that it gets duplicated at runtime is just an implementation detail.

As far as I understand, the POSIX runtime linking model (assuming correct visibility handling) specifically makes it so that shared libraries act a lot like static libraries, in the sense that there is only one "active" copy of each symbol at runtime, and those symbols' addresses compare equal from different libraries. This means that after runtime symbol resolution there is actually only one definition (from the abstract machine's point of view), as the other definition is not observable. This is exactly why Boost.Filesystem marks its filesystem_error class with visibility("default") - so its vtable is the same across shared library boundaries.

Now if the symbols came from different compiler invocations then this would clearly be an ODR violation.
I don't know if it's feasible to compare duplicate symbols' "provenance" for equality, and I understand if it's decided that the current behavior is correct.

That being said, it seems odd that GCC and Clang have different behaviors in the case of vtables. This is a case which must come up a lot in real codebases, especially for exception types.

@Dinistro
Copy link
Contributor

I would assume that your linkage of a .a file into both the .so and the executable is the problem here. The .a file contains the symbol definition, which is duplicated.

@glebov-andrey
Copy link
Author

I would assume that your linkage of a .a file into both the .so and the executable is the problem here. The .a file contains the symbol definition, which is duplicated.

This is definitely the case, and it's probably (strictly speaking) the correct behavior from the point of view of the standard.

On the other hand, the standard doesn't really acknowledge the existence of shared libraries. And so from this point of view the more "useful" behavior might be to check provenance of the symbols (some kind of TU compilation UUID maybe?).
As I mentioned in my second comment, from the point of view of the developer, there is only one TU containing the definition. This would definitely be useful for cases like vtable for boost::filesystem::filesystem_error.

A separate issue is that specifically for vtables, GCC and Clang disagree on their weakness/strength, and as a result on sanitizer instrumentation.

All in all, I totally understand if this is deemed to be not-a-bug.
In my specific case we switched to the shared ASAN runtime and --exclude-libs ALL to closer match non-sanitized builds.
Another reason I could see not to change the behavior is for "compatibility" with Windows, where identical symbols from different DLLs don't actually get merged at runtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler-rt:asan Address sanitizer false-positive Warning fires when it should not
Projects
None yet
Development

No branches or pull requests

3 participants