609248 - Trunk build fails with gcc 4.3.2 on debain 5.0.5 in StubCalls.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Asaf Romano (gone)]

With a simple --enable-debug mozconfig on debain 5.0.5 (and ggc 4.3), my trunk build fails in StubCalls.cpp.

/home/asaf/dev/mozilla-trunk/mozilla-central/js/src/methodjit/StubCalls.cpp: In function ‘bool StubEqualityOp(js::VMFrame&) [with int EQ = 1, bool IFNAN = false]’:
/home/asaf/dev/mozilla-trunk/mozilla-central/js/src/methodjit/StubCalls.cpp:1086:   instantiated from here
/home/asaf/dev/mozilla-trunk/mozilla-central/js/src/methodjit/StubCalls.cpp:1015: error: no matching function for call to ‘SameType(js::Value&, js::Value&)’
/home/asaf/dev/mozilla-trunk/mozilla-central/js/src/methodjit/StubCalls.cpp: In function ‘bool StubEqualityOp(js::VMFrame&) [with int EQ = 0, bool IFNAN = true]’:
/home/asaf/dev/mozilla-trunk/mozilla-central/js/src/methodjit/StubCalls.cpp:1094:   instantiated from here
/home/asaf/dev/mozilla-trunk/mozilla-central/js/src/methodjit/StubCalls.cpp:1015: error: no matching function for call to ‘SameType(js::Value&, js::Value&)’
/home/asaf/dev/mozilla-trunk/mozilla-central/js/src/nanojit/NativeX64.h: At global scope:
/home/asaf/dev/mozilla-trunk/mozilla-central/js/src/nanojit/NativeX64.h:345: warning: ‘nanojit::SavedRegs’ defined but not used
/home/asaf/dev/mozilla-trunk/mozilla-central/js/src/nanojit/NativeX64.h:353: warning: ‘nanojit::SingleByteStoreRegs’ defined but not use

Comment 1

[Mike Hommey [:glandium]]

Note, you don't even need --enable-debug to trigger this build error.

Comment 2

[Mike Hommey [:glandium]]

Attachment: Move SameType outside js::Value

It looks like some versions of gcc don't like inline friend methods. Moving SameType function definition outside js::Value makes it build.

Comment 3

[Asaf Romano (gone)]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38850 seems related.

Comment 4

[Asaf Romano (gone)]

(In reply to comment #2)
> Created attachment 487867 [details] [diff] [review]
> Move SameType outside js::Value
> 
> It looks like some versions of gcc don't like inline friend methods. Moving
> SameType function definition outside js::Value makes it build.

WFM too.

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 487867 [details] [diff] [review]
Move SameType outside js::Value

This works around a bug in some versions of gcc (PR/38850)

Comment 6

[Luke Wagner [:luke]]

Comment on attachment 487867 [details] [diff] [review]
Move SameType outside js::Value

Thanks!

>+JS_ALWAYS_INLINE
>+bool SameType(const Value &lhs, const Value &rhs) {
>+    return JSVAL_SAME_TYPE_IMPL(lhs.data, rhs.data);
>+}

Nit:

bool JS_ALWAYS_INLINE
SameType(...) {
  ...
}

Comment 7

[Mike Hommey [:glandium]]

> bool JS_ALWAYS_INLINE

I guess you mean JS_ALWAYS_INLINE bool, which is the form apparently used elsewhere.

Comment 8

[Mike Hommey [:glandium]]

I guess Debian stable is not the only place where one could find a buggy gcc, and it would be sad that beta7 can't be built where beta 6 could (beta6 and earlier didn't have this problem, since the bug was only triggered because of the methodjit code).

Comment 9

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Can you reference the gcc bug from the place where it used to be inlined in the class definition?  That will help us avoid regressing it due to an obviously correct refactoring.

Wouldn't block on it, but if it passes try then I'll approve it.

Comment 10

[Brendan Eich [:brendan]]

(In reply to comment #6)
> Comment on attachment 487867 [details] [diff] [review]
> Move SameType outside js::Value
> 
> Thanks!
> 
> >+JS_ALWAYS_INLINE
> >+bool SameType(const Value &lhs, const Value &rhs) {

Nit on top of nit: the { goes in column 1 on the next line.

/be

Comment 11

[Mike Hommey [:glandium]]

> Wouldn't block on it, but if it passes try then I'll approve it.

http://stage.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-8e7339d4f74d/

The patch I pushed addresses the various comments:
http://hg.mozilla.org/try/rev/8e7339d4f74d

Comment 12

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 487867 [details] [diff] [review]
Move SameType outside js::Value

thanks, speculative a+

Comment 13

[Mike Hommey [:glandium]]

Can I land this before beta7 ?

Comment 14

[Mike Hommey [:glandium]]

http://hg.mozilla.org/mozilla-central/rev/129428f0d81e