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: