Closed Bug 1215679 Opened 5 years ago Closed 5 years ago
Sanitizer ODR checking for cppunittests
http://firstname.lastname@example.org/try-linux64-asan/try_ubuntu64-asan_vm_test-cppunit-bm68-tests1-linux64-build404.txt.gz 10:46:25 INFO - TEST-START | TestSyncRunnable 10:46:25 INFO - PROCESS | 2292 | 10:46:25 INFO - ================================================================= 10:46:25 INFO - ==2292==ERROR: AddressSanitizer: odr-violation (0x00000148ac40): 10:46:25 INFO -  size=142 'SSL_ImplementedCiphers' sslenum.c:49:16 10:46:25 INFO -  size=142 'SSL_ImplementedCiphers' sslenum.c:49:16 10:46:25 INFO - These globals were registered at these points: 10:46:25 INFO - : 10:46:25 INFO - #0 0x434c83 in __asan_register_globals /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_globals.cc:217:3 10:46:25 INFO - #1 0x994deb in asan.module_ctor (/builds/slave/test/build/tests/cppunittest/TestSyncRunnable+0x994deb) 10:46:25 INFO - : 10:46:25 INFO - #0 0x434c83 in __asan_register_globals /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_globals.cc:217:3 10:46:25 INFO - #1 0x7f46fea64a6b in asan.module_ctor (/builds/slave/test/build/application/firefox/libssl3.so+0x62a6b) 10:46:25 INFO - ==2292==HINT: if you don't care about these warnings you may set ASAN_OPTIONS=detect_odr_violation=0 10:46:25 INFO - SUMMARY: AddressSanitizer: odr-violation: global 'SSL_ImplementedCiphers' at sslenum.c:49:16 10:46:25 INFO - ==2292==ABORTING 10:46:25 WARNING - TEST-UNEXPECTED-FAIL | TestSyncRunnable | test failed with return code 1 And many other tests.
The easiest fix here would be to set ASAN_OPTIONS=detect_odr_violation=0 as it suggests, at least for these test functions.
Both declarations, for SSL_ImplementedCiphers, are the same. From the part about where the globals were registered, I'm guessing that this file is getting linked into both libssl3.so and the object file for the specific tests. I'm also seeing this in CPP unit tests for WebRTC stuff, so the issue may not be in NSS per se.
Glandium, do you think there's something easy that can be done here? It doesn't really seem like a big deal, so I could just disable this check for CPP unit tests if needed.
The question is whether we really want this: https://dxr.mozilla.org/mozilla-central/rev/1a157155a4fe0074b3d03b54fe9e466472c2cd56/media/mtransport/test/moz.build#117-121
These failures are all under media/mtransport. It looks like ekr added this back in bug 1022812. ekr do you have an opinion on having this vs. doing ODR ("One definition rule") violation checking in ASan C++ unit tests?  https://en.wikipedia.org/wiki/One_Definition_Rule
Assignee: nobody → nobody
Component: Libraries → WebRTC: Networking
Product: NSS → Core
Version: trunk → Trunk
I recall adding the static linkage. Would it be a terrible thing to add another guard on that static linkage? That way developers (i.e., ekr and myself probably) can statically link NSS if they need to debug NSS, but most builds would just rely on dynamic linkage.
I'm just going to disable ODR violation checking for now. If somebody wants to figure out what to do with the mtransport tests and enable it later, that's fine with me. The disable leaks thing is needed because we are overriding the default settings. try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a92b2576959
Oops, I meant to ask a testing person for a review here. Anyways, it is unfortunate to disable some testing, but we are clearly not doing this at all right now, so we're still getting more coverage, over all.
Attachment #8676997 - Flags: review?(jmaher)
Comment on attachment 8676997 [details] [diff] [review] Ignore ODR violations in ASan CPP unit tests. Review of attachment 8676997 [details] [diff] [review]: ----------------------------------------------------------------- lgtm
Attachment #8676997 - Flags: review?(jmaher) → review+
Summary: AddressSanitizer: odr-violation: global 'SSL_ImplementedCiphers' at sslenum.c:49:16 with clang 3.7 on Linux → Disable AddressSanitizer ODR checking for cppunittests
You need to log in before you can comment on or make changes to this bug.