1215679 - Disable AddressSanitizer ODR checking for cppunittests

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect)

Description

[(no longer active)]

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-0cfedd968806/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 -    [1] size=142 'SSL_ImplementedCiphers' sslenum.c:49:16
10:46:25     INFO -    [2] size=142 'SSL_ImplementedCiphers' sslenum.c:49:16
10:46:25     INFO -  These globals were registered at these points:
10:46:25     INFO -    [1]:
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 -    [2]:
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.

Comment 1

[Andrew McCreight [:mccr8]]

The easiest fix here would be to set ASAN_OPTIONS=detect_odr_violation=0 as it suggests, at least for these test functions.

Comment 2

[Andrew McCreight [:mccr8]]

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.

Comment 3

[Andrew McCreight [:mccr8]]

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.

Comment 4

[Mike Hommey [:glandium]]

The question is whether we really want this:
https://dxr.mozilla.org/mozilla-central/rev/1a157155a4fe0074b3d03b54fe9e466472c2cd56/media/mtransport/test/moz.build#117-121

Comment 5

[Andrew McCreight [:mccr8]]

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"[1]) violation checking in ASan C++ unit tests?

[1] https://en.wikipedia.org/wiki/One_Definition_Rule

Comment 6

[Martin Thomson [:mt:]]

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.

Comment 7

[Andrew McCreight [:mccr8]]

Attachment: Ignore ODR violations in ASan CPP unit tests.

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

Comment 8

[Andrew McCreight [:mccr8]]

Attachment: Ignore ODR violations in ASan CPP unit tests.

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.

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8676997 [details] [diff] [review]
Ignore ODR violations in ASan CPP unit tests.

Review of attachment 8676997 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/08a76850aef1

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/08a76850aef1