Bug 150381

Summary: [MIPS] LLInt: fix calculation of Global Offset Table
Product: WebKit Reporter: Guillaume Emont <guijemont>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, commit-queue, jbriance, keith_miller, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 108664    
Attachments:
Description Flags
Patch
mcatanzaro: review+
Patch
none
Patch none

Guillaume Emont
Reported 2015-10-20 17:53:02 PDT
Offlineasm adds a .cpload when we create a label in MIPS (useful for functions so that the GOT can be used to calculate the address of position-independent code). But the code created by the assembler when encountering a .cpload assumes that we jumped to that address. So we need to add a jump to pcBase in initPCRelative(), or otherwise the GOT-related calculations are wrong.
Attachments
Patch (1.79 KB, patch)
2015-10-20 17:56 PDT, Guillaume Emont 		mcatanzaro: review+
DetailsFormatted DiffDiff
Patch (3.11 KB, patch)
2016-01-17 12:24 PST, Konstantin Tokarev 		no flags
DetailsFormatted DiffDiff
Patch (3.05 KB, patch)
2016-01-18 01:35 PST, Konstantin Tokarev 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Guillaume Emont
Comment 1 2015-10-20 17:56:50 PDT
Created attachment 263644 [details] Patch
Michael Catanzaro
Comment 2 2015-12-30 15:06:11 PST
Comment on attachment 263644 [details] Patch Can't hurt anything for non-MIPS, so sure....
Konstantin Tokarev
Comment 3 2016-01-16 04:39:09 PST
Could anyone land this patch? It is needed to get MIPS finally working.
Konstantin Tokarev
Comment 4 2016-01-16 11:34:47 PST
Looks like jmp is not needed here, we just need to update $t9. I will upload different patch.
Konstantin Tokarev
Comment 5 2016-01-17 12:24:12 PST
Created attachment 269193 [details] Patch
Konstantin Tokarev
Comment 6 2016-01-17 12:26:09 PST
New patch fixes the bug without adding unneeded jr instruction after move $t9, $v1
Julien Brianceau
Comment 7 2016-01-18 01:31:46 PST
Comment on attachment 269193 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=269193&action=review LGTM, please just fix ChangeLog file > Source/JavaScriptCore/ChangeLog:3 > + llint: fix calculation of Global Offset Table This line seems redundant with line 5, please remove it > Source/JavaScriptCore/ChangeLog:14 > + instruction setcallreg which does exactlly that. exactlly -> exactly
Konstantin Tokarev
Comment 8 2016-01-18 01:35:51 PST
Created attachment 269212 [details] Patch
Konstantin Tokarev
Comment 9 2016-01-18 01:41:17 PST
Comment on attachment 269193 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=269193&action=review >> Source/JavaScriptCore/ChangeLog:3 >> + llint: fix calculation of Global Offset Table > > This line seems redundant with line 5, please remove it Done >> Source/JavaScriptCore/ChangeLog:14 >> + instruction setcallreg which does exactlly that. > > exactlly -> exactly Done
Julien Brianceau
Comment 10 2016-01-18 02:13:16 PST
Thanks, LGTM
Michael Saboff
Comment 11 2016-01-18 12:17:58 PST
Comment on attachment 269212 [details] Patch r=me
WebKit Commit Bot
Comment 12 2016-01-18 13:07:20 PST
Comment on attachment 269212 [details] Patch Clearing flags on attachment: 269212 Committed r195236: <http://trac.webkit.org/changeset/195236>
Guillaume Emont
Comment 13 2016-01-18 18:38:41 PST
(In reply to comment #6) > New patch fixes the bug without adding unneeded jr instruction after move > $t9, $v1 Indeed, I was a bit lazy with my patch. Thanks!
Note You need to log in before you can comment on or make changes to this bug.