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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Martin Stránský [:stransky] (ni? me)]

Attachment: js-header.patch

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

Comment 1

[Luke Wagner [: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?

Comment 2

[Jan de Mooij [:jandem]]

(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!

Comment 3

[Martin Stránský [:stransky] (ni? me)]

Well, it's used in dom/bindings/PrimitiveConversions.h. Shall it be transferred to JS_ValueToNumber()?

Comment 4

[Martin Stránský [:stransky] (ni? me)]

Attachment: don't export ToNumber()

Okay, something like this one?

Comment 5

[Luke Wagner [: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!

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c3b01d22caa9

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Backed out for Windows bustage.
https://hg.mozilla.org/integration/mozilla-inbound/rev/486b61c42838

https://tbpl.mozilla.org/php/getParsedLog.php?id=27577249&tree=Mozilla-Inbound

Comment 8

[:aceman]

The patch fixes the failure for me when building Thunderbird on Linux, 32bit, gcc 4.8.1.

Comment 9

[Martin Stránský [:stransky] (ni? me)]

Attachment: js_header v2

Try: https://tbpl.mozilla.org/?tree=Try&rev=f02e5bd0304c

Comment 10

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 807167 [details] [diff] [review]
js_header v2

This one handles the Windows failures, try seems to be ok.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/60c4c441b5a2

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/60c4c441b5a2

Comment 13

[Mike Hommey [:glandium]]

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

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7c710add681e