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
Patch (1.86 KB, patch)
2015-05-11 06:22 PDT, Gabor Loki
no flags
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
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
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.