Closed Bug 609248 Opened 12 years ago Closed 12 years ago

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

Categories

(Core :: JavaScript Engine, defect)

All
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b8
Tracking Status
blocking2.0 --- -
status2.0 --- wanted

People

(Reporter: asaf, Assigned: glandium)

References

()

Details

Attachments

(1 file)

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
Note, you don't even need --enable-debug to trigger this build error.
Summary: Trunk build fails with gcc4.3 on debain 5.0.5 in StubCalls.cpp → Trunk build fails with gcc 4.3.2 on debain 5.0.5 in StubCalls.cpp
It looks like some versions of gcc don't like inline friend methods. Moving SameType function definition outside js::Value makes it build.
Attachment #487867 - Attachment description: Move Same → Move SameType outside js::Value
Attachment #487867 - Attachment is patch: true
Attachment #487867 - Attachment mime type: application/octet-stream → text/plain
(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.
Hardware: x86_64 → All
Version: unspecified → Trunk
Assignee: general → mh+mozilla
Comment on attachment 487867 [details] [diff] [review]
Move SameType outside js::Value

This works around a bug in some versions of gcc (PR/38850)
Attachment #487867 - Flags: review?(lw)
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(...) {
  ...
}
Attachment #487867 - Flags: review?(lw) → review+
> bool JS_ALWAYS_INLINE

I guess you mean JS_ALWAYS_INLINE bool, which is the form apparently used elsewhere.
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).
blocking2.0: --- → ?
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.
blocking2.0: ? → -
status2.0: --- → wanted
(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
> 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/mh@glandium.org-8e7339d4f74d/

The patch I pushed addresses the various comments:
http://hg.mozilla.org/try/rev/8e7339d4f74d
Comment on attachment 487867 [details] [diff] [review]
Move SameType outside js::Value

thanks, speculative a+
Attachment #487867 - Flags: approval2.0+
Can I land this before beta7 ?
http://hg.mozilla.org/mozilla-central/rev/129428f0d81e
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
You need to log in before you can comment on or make changes to this bug.