Bug 183366

Summary: REGRESSION(r229309): s_exceptionInstructions allocation change causing crashes in LLInt on WPE
Product: WebKit Reporter: Zan Dobersek <zan>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, mcatanzaro, msaboff, saam, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Mini-revert patch
none
Patch none

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>