WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
144680
[JSC] Erratum (843419) in Cortex-A53
https://bugs.webkit.org/show_bug.cgi?id=144680
Summary
[JSC] Erratum (843419) in Cortex-A53
Gabor Loki
Reported
2015-05-06 03:05:13 PDT
If there is a load or store instruction at a page boundary which uses the result of an ADRP instruction as a base it might access an incorrect address. There is a second variant of this if the base register is written by an instruction immediately after an ADRP to the same register, might also cause incorrect address access.
Attachments
Patch
(1.94 KB, patch)
2015-05-06 03:12 PDT
,
Gabor Loki
no flags
Details
Formatted Diff
Diff
Patch
(1.86 KB, patch)
2015-05-11 06:22 PDT
,
Gabor Loki
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Gabor Loki
Comment 1
2015-05-06 03:06:39 PDT
Although the adrp instruction is never used from ARM64Assembler.h, it is possible to add a simple workaround for this. I will upload a patch soon.
Gabor Loki
Comment 2
2015-05-06 03:12:59 PDT
Created
attachment 252464
[details]
Patch
Michael Saboff
Comment 3
2015-05-07 15:24:00 PDT
Comment on
attachment 252464
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=252464&action=review
> Source/JavaScriptCore/assembler/ARM64Assembler.h:3668 > + nop();
Why this third nop? I thought that the bug was an issue only for a dependent instruction at the page boundary. The check looks for the page boundary and inserts two nop's that should take care of the problem, making the last nop unnecessary.
> Source/JavaScriptCore/assembler/ARM64Assembler.h:3670 > + ALWAYS_INLINE void nopCortexA53Fix843419() > + { > +#if CPU(ARM64_CORTEXA53) > + if (UNLIKELY((m_buffer.codeSize() & 0xFF8) == 0xFF8)) { > + nop(); > + nop(); > + } > + nop(); > +#endif > + }
This nop padding needs to happen at link time. I don't think we have any guarantees that the page offset at assemble time will be the same after we link. Even if we did, there is some code compaction that can happen at link time, primarily due to branch type specialization.
Gabor Loki
Comment 4
2015-05-08 01:49:15 PDT
There are two cases: 1) If the ADRP is near to the page boundary (last two instruction) you will need more than two instructions between the ADRP and the load or store. 2) If the ADRP is followed by an instruction which writes the result register you will need an addition register. So, there is no unnecessary nops.
> This nop padding needs to happen at link time. I don't think we have any guarantees that the page offset at assemble time will be the same after we link. > Even if we did, there is some code compaction that can happen at link time, primarily due to branch type specialization.
Sad to hear that. It was a long time when I last touched the JSC. I am sure that the appropriate fix will be to check all the conditions which cause the issue, but it surely slow down the code generation - because the load and store instructions are very frequent. If we do these checks as a later pass after code generation (link time), the effect to the performance will be the same. I think the most easiest way to fix this - which might not influence the performance much - is to generate three nops after ADRP. How does it sound?
Gabor Loki
Comment 5
2015-05-11 06:22:55 PDT
Created
attachment 252858
[details]
Patch
Michael Saboff
Comment 6
2015-05-11 07:08:40 PDT
(In reply to
comment #4
)
> There are two cases: > 1) If the ADRP is near to the page boundary (last two instruction) you will > need more than two instructions between the ADRP and the load or store. > 2) If the ADRP is followed by an instruction which writes the result > register you will need an addition register. > > So, there is no unnecessary nops. > > > This nop padding needs to happen at link time. I don't think we have any guarantees that the page offset at assemble time will be the same after we link. > > Even if we did, there is some code compaction that can happen at link time, primarily due to branch type specialization. > > Sad to hear that. It was a long time when I last touched the JSC. > > I am sure that the appropriate fix will be to check all the conditions which > cause the issue, but it surely slow down the code generation - because the > load and store instructions are very frequent. If we do these checks as a > later pass after code generation (link time), the effect to the performance > will be the same. > > I think the most easiest way to fix this - which might not influence the > performance much - is to generate three nops after ADRP. > > How does it sound?
Sounds fine. As a heads up, there has been some discussion to change the current movz/movk/movk absolute address materialization to adrp/add.
Michael Saboff
Comment 7
2015-05-11 07:09:02 PDT
Comment on
attachment 252858
[details]
Patch r=me
WebKit Commit Bot
Comment 8
2015-05-12 01:16:54 PDT
Comment on
attachment 252858
[details]
Patch Clearing flags on attachment: 252858 Committed
r184170
: <
http://trac.webkit.org/changeset/184170
>
WebKit Commit Bot
Comment 9
2015-05-12 01:17:01 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug