Bug 183366 - REGRESSION(r229309): s_exceptionInstructions allocation change causing crashes in LLInt on WPE
Summary: REGRESSION(r229309): s_exceptionInstructions allocation change causing crashe...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-03-06 06:12 PST by Zan Dobersek
Modified: 2018-03-06 09:39 PST (History)
8 users (show)

See Also:


Attachments
Mini-revert patch (2.31 KB, patch)
2018-03-06 06:18 PST, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch (2.98 KB, patch)
2018-03-06 06:48 PST, Zan Dobersek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2018-03-06 06:12:01 PST
In r229309, changes in the LLInt::Data::s_exceptionInstructions allocation process are causing crashes on WPE, making the testing bot quit early.

Not yet sure what exactly is the cause for this, but the s_exceptionInstructions symbol appears in both libWPEWebKit.so and libTestRunnerInjectedBundle.so, and it might be that it clashes because of that duplication.
Comment 1 Yusuke Suzuki 2018-03-06 06:16:44 PST
(In reply to Zan Dobersek from comment #0)
> In r229309, changes in the LLInt::Data::s_exceptionInstructions allocation
> process are causing crashes on WPE, making the testing bot quit early.
> 
> Not yet sure what exactly is the cause for this, but the
> s_exceptionInstructions symbol appears in both libWPEWebKit.so and
> libTestRunnerInjectedBundle.so, and it might be that it clashes because of
> that duplication.

OK, let's annotate this with JS_EXPORTDATA.
Comment 2 Zan Dobersek 2018-03-06 06:18:42 PST
Created attachment 335089 [details]
Mini-revert patch
Comment 3 Michael Catanzaro 2018-03-06 06:22:06 PST
(In reply to Yusuke Suzuki from comment #1)
> OK, let's annotate this with JS_EXPORTDATA.

WPE doesn't use export macros. (Same for GTK.)
Comment 4 Zan Dobersek 2018-03-06 06:48:41 PST
Created attachment 335093 [details]
Patch
Comment 5 Zan Dobersek 2018-03-06 06:50:42 PST
(In reply to Zan Dobersek from comment #4)
> Created attachment 335093 [details]
> Patch

This avoids the problem on the WPE port by using a narrowed-down list of libraries that the injected bundle .so should be linked against. As such, it avoids the JavaScriptCore archive being linked into the final .so object, avoiding duplicate symbols.

Will also test how the GTK+ port is affected by this change, if at all. But in general, on that port the JSC symbols are all bundled into the libjavascriptcoregtk.so object.
Comment 6 Zan Dobersek 2018-03-06 07:08:04 PST
(In reply to Zan Dobersek from comment #5)
> Will also test how the GTK+ port is affected by this change, if at all. But
> in general, on that port the JSC symbols are all bundled into the
> libjavascriptcoregtk.so object.

No problem with GTK+ with the patch.
Comment 7 Yusuke Suzuki 2018-03-06 09:06:55 PST
(In reply to Michael Catanzaro from comment #3)
> (In reply to Yusuke Suzuki from comment #1)
> > OK, let's annotate this with JS_EXPORTDATA.
> 
> WPE doesn't use export macros. (Same for GTK.)

Oops, thanks!
Comment 8 Zan Dobersek 2018-03-06 09:38:09 PST
Comment on attachment 335093 [details]
Patch

Clearing flags on attachment: 335093

Committed r229325: <https://trac.webkit.org/changeset/229325>
Comment 9 Zan Dobersek 2018-03-06 09:38:14 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2018-03-06 09:39:32 PST
<rdar://problem/38182758>