Remove nopFill_ debug functionality in arm/arm64 assembler
Categories
(Core :: JavaScript Engine: JIT, enhancement)
Tracking
()
People
(Reporter: lth, Assigned: lth)
References
Details
The IonAssemblerBufferWithConstantPools has a debug function called nopFill_. This is a smallish unsigned that is set when the buffer is constructed (it's a constructor argument); the buffer will insert this number of NOP instructions between any other two instructions (except in exceptional situations) and thereby perturb the code size; the debugging effect of this is to force the spilling of constant pools in different places, essentially a form of fuzzing.
This functionality is no longer used by anyone for anything, and in addition, it conflicts with assumptions about code generation elsewhere in the compiler. Specifically, various pieces of code assume that if they flush the assembler buffer and then do an AutoForbidPoolsAndNops, they will know what the offset they obtained before the AutoForbidPoolsAndNops is the address of the instructions within the window they're trying to emit. (Wasm prologue code is like this.) But AutoForbidPoolsAndNops may emit nops before those instructions. Obviously that (and similar wrinkles) could be fixed, but if nobody's using this functionality any more, why bother?
|
Assignee | |
Comment 1•5 years ago
|
||
In the process of removing this I have discovered that we do use it, triggering it with shell options. It's in fuzz-flags, and in some wasm/asm test cases (searchfox for arm-asm-nop-fill). I'm going to have to investigate those test cases before stripping the code.
|
|
Assignee | |
Comment 2•5 years ago
|
||
OK, so while AutoForbidPoolsAndNops looks like it can emit nops (because it calls enterNoPool which calls insertNops()), it doesn't emit nops because AutoForbidPoolsAndNops is derived from AutoForbidNops, and so nop emission has been disabled at that point, and so we should be good.
Fun(?) fact, not everyone uses AutoForbidPoolsAndNops; some code calls enterNoPool directly, notably pushFakeReturnAddress. Based on comments in the ARM code this is probably wrong if nopfill is enabled, but benign.
As I have other fish to fry, I'll close this as WONTFIX.
Description
•