Closed Bug 912959 Opened 6 years ago Closed 6 years ago

libxul.so link fails - ToNumberSlow() is built as a hidden symbol

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27
Tracking Status
firefox26 --- fixed
firefox27 --- fixed

People

(Reporter: stransky, Assigned: stransky)

Details

(Whiteboard: [qa-])

Attachments

(1 file, 2 obsolete files)

Attached patch js-header.patch (obsolete) — Splinter Review
jsnum.h declares ToNumberSlow() by two ways, as extern bool and JS_PUBLIC_API(bool) which causes link failure on gcc-4.8.1-1.fc19.x86_64/Fedora 19. An attached patch sync those declarations.

Downstream bug: https://bugzilla.redhat.com/show_bug.cgi?id=1004342
Attachment #800109 - Flags: review?(luke)
Jan: I don't see any use of ToNumber() in shell/js.cpp and jsnum.h isn't exported for general mozilla consumption so could the fix instead be to remove the JS_PUBLIC_API?
(In reply to Luke Wagner [:luke] from comment #1)
> Jan: I don't see any use of ToNumber() in shell/js.cpp and jsnum.h isn't
> exported for general mozilla consumption so could the fix instead be to
> remove the JS_PUBLIC_API?

To be clear, I didn't write the patch, just requested review on it.

Martin, can you do this, post a new patch and request review from :luke? Thanks!
Flags: needinfo?(stransky)
Well, it's used in dom/bindings/PrimitiveConversions.h. Shall it be transferred to JS_ValueToNumber()?
Flags: needinfo?(stransky) → needinfo?(luke)
Attached patch don't export ToNumber() (obsolete) — Splinter Review
Okay, something like this one?
Attachment #800109 - Attachment is obsolete: true
Attachment #800109 - Flags: review?(luke)
Attachment #800808 - Flags: review?(luke)
Comment on attachment 800109 [details] [diff] [review]
js-header.patch

Oh wait, I don't know how I missed this but there of course is JS::ToNumber in jsapi.h, so the first patch was exactly the right fix.  Sorry for the confusion!
Attachment #800109 - Flags: review+
Attachment #800109 - Attachment is obsolete: false
Attachment #800808 - Attachment is obsolete: true
Attachment #800808 - Flags: review?(luke)
Flags: needinfo?(luke)
Keywords: checkin-needed
The patch fixes the failure for me when building Thunderbird on Linux, 32bit, gcc 4.8.1.
Comment on attachment 807167 [details] [diff] [review]
js_header v2

This one handles the Windows failures, try seems to be ok.
Attachment #807167 - Flags: review?(luke)
Attachment #800109 - Attachment is obsolete: true
Attachment #807167 - Flags: review?(luke) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/60c4c441b5a2
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Assignee: general → stransky
Comment on attachment 807167 [details] [diff] [review]
js_header v2

[Approval Request Comment]
User impact if declined: Build failure in some setups.
Testing completed (on m-c, etc.): This has been on m-c for a while, now, and I've been patching aurora locally successfully.
Risk to taking this patch (and alternatives if risky): None. This only makes a declaration in a header match the actual declaration of the function in the source file.
String or IDL/UUID changes made by this patch: None
Attachment #807167 - Flags: approval-mozilla-aurora?
Attachment #807167 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.