Closed Bug 1059255 Opened 10 years ago Closed 10 years ago

Green jobs contain "TEST-UNEXPECTED-FAIL | check_stdcxx | We do not want these libstdc++ symbols to be used" spam

Categories

(Core :: General, defect)

Product:
Core
Component:
General
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla35
Tracking Flags:
Tracking Status
firefox33 --- unaffected
firefox34 --- fixed
firefox35 --- fixed
firefox-esr31 --- unaffected

People

(Reporter: cbook, Assigned: glandium)

References

(Blocks 1 open bug,
URL
)

Details

(Keywords: intermittent-failure)

Attachments

(2 files, 2 obsolete files)

Make stdc++compat errors fatal
1.59 KB, patch
mshal
: review+
Details | Diff | Splinter Review
Stop building stdc++compat as a real library
3.19 KB, patch
mshal
: review+
Details | Diff | Splinter Review
Linux x86-64 mozilla-inbound asan build on 2014-08-27 01:09:38 PDT for push 66e350a413a6 slave: b-linux64-hp-0034 https://tbpl.mozilla.org/php/getParsedLog.php?id=46840896&tree=Mozilla-Inbound TEST-UNEXPECTED-FAIL | check_stdcxx | We do not want these libstdc++ symbols to be used:
These appear in green jobs & more than just ASAN. Comes from: http://hg.mozilla.org/mozilla-central/annotate/47c9418fbc28/config/config.mk#l693 697 ifeq (,$(filter $(OS_TARGET),WINNT Darwin)) 698 CHECK_TEXTREL = @$(TOOLCHAIN_PREFIX)readelf -d $(1) | grep TEXTREL > /dev/null && echo 'TEST-UNEXPECTED-FAIL | check_textrel | We do not want text relocations in libraries and programs' || true 699 endif -> Bug 956398 Mike, the TEST-UNEXPECTED-FAILs aren't making the job fail (there are lots of green jobs that all contain these failures). Please can we switch to $(error foo) instead?
Blocks: 956398, log-SnR
Flags: needinfo?(mh+mozilla)
Summary: Intermittent ASAN | check_stdcxx | We do not want these libstdc++ symbols to be used: → Green jobs contain "TEST-UNEXPECTED-FAIL | check_stdcxx | We do not want these libstdc++ symbols to be used" spam
Why are those not turning red or orange? IIRC that's what happens on "normal" builds.
Flags: needinfo?(mh+mozilla)
(In reply to Mike Hommey [:glandium] (out from Sep 6 to Sep 22) from comment #6) > Why are those not turning red or orange? IIRC that's what happens on > "normal" builds. The run isn't aborted, so is returning with a zero return code. This leave mozharness/buildbot to detect the TEST-UNEXPECTED-FAIL, which is not reliable (and something that we should/are going to move away from). I think the mozharness cases that catch TEST-UNEXPECTED-FAIL only apply to test jobs, not builds. Either way, if this is a fatal failure, please can you make it either abort the run or else return non-zero at the end of compile? :-)
Note that "fixing" those to deliberately turn the build red would turn ASAN builds perma-red. Someone needs to make those messages go away first, and afaict, the necessary symbols *are* in stdc++compat, so it might be a linkage order problem. Who's the owner of asan builds these days?
Agreed the existing failures need fixing before this is fatal, but in the meantime can we switch this to a warning that doesn't trigger a false positive with the log parser? (not being fatal but matching the fatal error regex is the worst of both worlds from a confusion to devs/sheriffs pov, since people miss real errors in runs and mis-star)
The thing is those are *not* false positives in other builds.
Until check_stdcxx & check_textrel failures make the run fail, use a failure string that won't cause log parser false positives. (Appears in error summary means the failure was fatal. Whilst being fatal is the desired end-state, that's not the current situation.)
Attachment #8481116 - Flags: review?(mh+mozilla)
Assignee: nobody → emorley
Status: NEW → ASSIGNED
(In reply to Mike Hommey [:glandium] (out from Sep 6 to Sep 22) from comment #14) > The thing is those are *not* false positives in other builds. Yeah but if a real failure did occur in the other builds, it still wouldn't turn the run orange. Let's swap the failure message for now & you can take a look at making this fatal in another bug :-)
Comment on attachment 8481116 [details] [diff] [review] Remove TEST-UNEXPECTED-FAIL from non-fatal error messages Review of attachment 8481116 [details] [diff] [review]: ----------------------------------------------------------------- Builds *do* turn orange with the current code: https://tbpl.mozilla.org/?tree=Try&rev=c29e3f25a3d4 Your patch would completely break that.
Attachment #8481116 - Flags: review?(mh+mozilla) → review-
Oh weird. The ASAN jobs shouldn't be that different. Either way, I think a $(error foo) approach here would be best, once the problem is fixed for ASAN jobs - since relying on a string making the run fail (vs a non-zero return code) is something that is we want to move away from.
Assignee: emorley → nobody
Attachment #8481116 - Attachment is obsolete: true
Status: ASSIGNED → NEW
Flags: needinfo?(choller)
The PKIX ASan failures with these "SUMMARY: AddressSanitizer: use-after-poison" should be marked as bug 1039166.
This is a very recent regression, I was able to bisect it locally: The first bad revision is: changeset: 201640:2e3703f9be76 user: Jacek Caban date: Tue Aug 26 13:50:27 2014 +0200 summary: Bug 1055627 - Pass MOZ_GLUE_PROGRAM_LDFLAGS after STATIC_LIBS to linker. r=glandium Mike, can you please check how the patch broke this and recommend a fix? Thanks.
Blocks: 1055627
Flags: needinfo?(choller) → needinfo?(mh+mozilla)
Try removing NO_EXPAND_LIBS = 1 in build/unix/stdc++compat/Makefile.in and see if that removes those messages.
Flags: needinfo?(mh+mozilla)
I commented that line out, but now it doesn't build anymore. I get this: 16:40.24 /usr/bin/ld.gold.real: error: /srv/repos/browser/mozilla-central/objdir-ff-asan64opt/toolkit/library/../../build/unix/stdc++compat/stdc++compat.o: multiple definition of 'std::ctype<char>::_M_widen_init() const' 16:40.24 /usr/bin/ld.gold.real: /srv/repos/browser/mozilla-central/objdir-ff-asan64opt/toolkit/library/../../build/unix/stdc++compat/stdc++compat.o: previous definition here 16:40.24 /usr/bin/ld.gold.real: error: /srv/repos/browser/mozilla-central/objdir-ff-asan64opt/toolkit/library/../../build/unix/stdc++compat/stdc++compat.o: multiple definition of 'std::__detail::_List_node_base::_M_reverse()' 16:40.24 /usr/bin/ld.gold.real: /srv/repos/browser/mozilla-central/objdir-ff-asan64opt/toolkit/library/../../build/unix/stdc++compat/stdc++compat.o: previous definition here 16:40.24 /usr/bin/ld.gold.real: error: /srv/repos/browser/mozilla-central/objdir-ff-asan64opt/toolkit/library/../../build/unix/stdc++compat/stdc++compat.o: multiple definition of 'std::__detail::_List_node_base::_M_transfer(std::__detail::_List_node_base*, std::__detail::_List_node_base*)' plus a few more of the same kind, when linking libxul.so.
Flags: needinfo?(mh+mozilla)
Incidentally, this makes the build fail because stdc++compat.o then appears twice on the libxul linkage command line. This, in turn, is because widget/xremoteclient creates both a program and an intermediate library for libxul. The Program() template adds stdc++compat to the directory data, which ends up being added to libxul as well. While eventually we'll be able to have Programs and Libraries in the same directory without such side effects, it's not the case yet, so separate them out for now.
Attachment #8485378 - Flags: review?(mshal)
Assignee: nobody → mh+mozilla
Status: NEW → ASSIGNED
Flags: needinfo?(mh+mozilla)
Attachment #8485384 - Flags: review?(mshal)
Comment on attachment 8485378 [details] [diff] [review] Stop building stdc++compat as a real library Gah, that fails linking gtest-libxul for a similar reason.
Attachment #8485378 - Flags: review?(mshal)
Incidentally, this makes the build fail because stdc++compat.o then appears twice on the libxul linkage command line. This, in turn, is because widget/xremoteclient creates both a program and an intermediate library for libxul. The Program() template adds stdc++compat to the directory data, which ends up being added to libxul as well. The same kind of issue arises when linking the gtest-enabled libxul. While eventually we'll be able to avoid those problems, it's not the case yet, so work around the issue by making expand-libs skip .desc files that appear multiple times. https://tbpl.mozilla.org/?tree=Try&rev=f6c08d75e636
Attachment #8485393 - Flags: review?(mshal)
Attachment #8485378 - Attachment is obsolete: true
Comment on attachment 8485384 [details] [diff] [review] Make stdc++compat errors fatal It might be simpler if you use -v to invert the return code of the first grep, since you wouldn't need the 'true' at the end, so 'false' could work as expected.
Attachment #8485384 - Flags: review?(mshal) → review+
Comment on attachment 8485393 [details] [diff] [review] Stop building stdc++compat as a real library > if os.path.exists(arg + conf.LIBS_DESC_SUFFIX): >- with open(arg + conf.LIBS_DESC_SUFFIX, 'r') as f: >+ desc = os.path.abspath(arg + conf.LIBS_DESC_SUFFIX) >+ if desc in self._descs: >+ return [] >+ self._descs.add(desc) >+ with open(desc, 'r') as f: Any reason not to move desc above the first if? Eg: desc = os.path.abspath(arg + conf.LIBS_DESC_SUFFIX) if os.path.exists(desc): if desc in self._descs: ...
Attachment #8485393 - Flags: review?(mshal) → review+
https://hg.mozilla.org/mozilla-central/rev/00ddd6873d65 https://hg.mozilla.org/mozilla-central/rev/1a7fb15552ab
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: