Repository navigation
x86-64: a float load's base address no longer clobbers the cached rax - #202
Merged
Merged
Conversation
addr_reg put a slot-held address in rcx by loading it into rax and copying it, behind the back of the rax residency cache, which still named the value rax held before. The next instruction that wanted that value used rax as it was: in fuzz seed 1038932645 (x86-64 at -O2, -O3 and -Os) a local array's address was cached in rax, a double was loaded through a pointer kept in a slot, and the short read from the array after it came from the double's base. The address now goes straight from its slot into rcx; rax is not touched. Every other place that writes rax directly runs in an operation that resets the cache first (calls, memcpy, memzero, va_start, 128-bit and x87 arithmetic) or in the prologue; addr_reg was the one that did so in the middle of an operation that trusts it. tests/exec/x86-addr-reg-cache.c is the fuzz program, reduced under UBSan, ASan and clang's return-type and uninitialised warnings; it fails with the old compiler at -O2 and -Os and passes at every level.
A fuzz program whose checksum mixes doubles: RX's double is binary32, so the checksum is another number and the program returns 1 at every level, -O0 included (CI's run of #202).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A wrong-code bug on main, found by the PR fuzz job on #200 (seed 1038932645: x86-64 at
-O2,-O3and-Os; every other board right). It must land before 0.1.0 is tagged.The bug. The x86-64 backend keeps a cache of which value
raxholds.addr_reg, which puts a stack-held address inrcxfor a float load, did it asmov slot,%rax; mov %rax,%rcxwithout telling the cache. The next instruction that wanted the value the cache still named usedraxas it was:The fix. The address is loaded straight from its slot into
rcx;raxis untouched. The other places that writeraxdirectly all run after a cache reset (calls, memcpy, memzero, va_start, 128-bit and x87 arithmetic) or in the prologue; this was the only one in the middle of an operation that trusts the cache.Test.
tests/exec/x86-addr-reg-cache.cis the fuzz program, reduced. The reduction was checked against UBSan, ASan and clang's return-type and uninitialised-variable warnings: a first reduction had droppedreturnstatements, and the guard rejected it. Against the old compiler it fails at-O2and-Os; with the fix it passes at every level. Hand-written versions did not trigger it: whether it happens depends on the allocator spilling both the pointer and the array's address.Checked.
make check: 442/442.-O2 -O3 -Os: all pass.-O1 -O2 -O3 -Os: all pass.exec-Os,regalloc-O2,optimizer,big-functionsandagrees-with-gcc: pass.