Bug 216893

Summary: [MIPS] Broken build after r267371
Product: WebKit Reporter: Caio Lima <ticaiolima>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, angelos, aperez, darin, ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
WIP - Patch
none
Patch ews-feeder: commit-queue-

Caio Lima
Reported 2020-09-23 13:22:19 PDT
...
Attachments
WIP - Patch (1.51 KB, patch)
2020-09-23 13:23 PDT, Caio Lima 		no flags
DetailsFormatted DiffDiff
Patch (3.88 KB, patch)
2020-09-24 08:51 PDT, Angelos Oikonomopoulos 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Caio Lima
Comment 1 2020-09-23 13:23:36 PDT
Created attachment 409497 [details] WIP - Patch Send patch to check EWS
Angelos Oikonomopoulos
Comment 2 2020-09-24 07:32:52 PDT
OK this should fix it. While the bug in LabelReference.mapChildren is obvious (it just eats up the offset when returning a new LabelReference, essentially setting it to zero), it wasn't clear to us why this bug didn't affect any of the other architectures. I've only compared with X86_64 but suspect the same explanation applies to archs other than MIPS. What happens is that the AST from the parser only contains LabelReference(label, 0) + AddImmediates and AddImmediates.fold correctly constructs a LabelReference with an offset by calling LabelReference.plusOffset. We do call mapChildren on those on X86_64, however only in the context of a call to isASTErroneous which (somewhat ironically) simply checks the existence of Error nodes and discards the rest of the transformed AST, which would have the erroneous LabelReference nodes. However, on MIPS, mapChildren gets called in the lowering stage too, specifically: getModifiedListMIPS -> assignRegistersToTemporaries -> replaceTemporariesWithRegisters -> mapChildren and there the (erroneous) result is getting used, resulting in all LabelReference nodes having an offset of zero.
Angelos Oikonomopoulos
Comment 3 2020-09-24 07:35:53 PDT
With regard to the second hunk of the patch, there was a minor thinko in the initial fix; we need to look up the label in the GOT, not the computed address. Now that we have a good explanation for why the LabelReference.mapChildren issue is not biting other architectures, I think this is good to go in.
Angelos Oikonomopoulos
Comment 4 2020-09-24 08:51:24 PDT
Created attachment 409580 [details] Patch
EWS
Comment 5 2020-09-24 09:17:04 PDT
Committed r267535: <https://trac.webkit.org/changeset/267535> All reviewed patches have been landed. Closing bug and clearing flags on attachment 409580 [details].
Radar WebKit Bug Importer
Comment 6 2020-09-24 09:18:15 PDT
<rdar://problem/69507244>
Note You need to log in before you can comment on or make changes to this bug.