Repository navigation
cranelift-fuzzgen fuzzbug: "Segv on unknown address in cranelift_filetests::function_runner::CompiledFunction::call::ha6b9162cd2e" #4761
Description
https://oss-fuzz.com/testcase-detail/4896860279537664
ERROR: AddressSanitizer: SEGV on unknown address (pc 0x6250000014d5 bp 0x7fff30133db0 sp 0x7fff30133ad0 T0)
--
| ==743==The signal is caused by a READ memory access.
| ==743==Hint: this fault was caused by a dereference of a high value address (see register values below). Disassemble the provided pc to learn which register was used.
| SCARINESS: 20 (wild-addr-read)
| #0 0x6250000014d5 (<unknown module>)
| #1 0x62500000175d (<unknown module>)
| #0 0x5579a6d0c6c4 in cranelift_filetests::function_runner::CompiledFunction::call::ha6b9162cd2e21784 wasmtime/cranelift/filetests/src/function_runner.rs:183:9
input:
IFWoAAAAAE0AAABSSUl9LCB7eAx6ZcYNR9T0RvCdMjI0MjaWMgAAAAA7ACypAH19f319fX0AAAA7
JAABqQB9fX19fX19fX0IAAAAAAAAAQAAwyvDw8PDAADJ/zw8PEHDw8MAAGUBb/PGANpGAAAAAAAj
OwAsqW9vb29vb28AfX19fX19fQAAADskAAGpAH19fX19fX19fQ7/////////CAAAAAAAAAEAAMMr
w8PDwwAAyf88PDxBw8PDAABlAW/zxgDaRgAAAAAAAAAAAAAAAACTgoL/////////MAAAAAAAAAUA
AAAACIKCgoKCgkEAgoKCgoJNAAAAgoKCgoKCgoKCgoKCwoKCgoKCgoKSioIEAAcAgoKCgoKCgoKC
goKCgoKCGIKC//8wAAAAAAAABQAAAAAIgoKCgoKCQQCCgoKCgk0AAACCgoKCgoKCgoKCgoLCeoKC
eYKCgpKKggQAAACCgoKCgoKCgoKCgoKCgoIYgoKCgoKCgoKCgoKCFACCgjOCgoKCgoL/////goKC
goKCgoKCgoKCgoJBAUFB
cc @afonso360
Activity
afonso360 commentedon Aug 23, 2022
Formatted
afonso360 commentedon Aug 24, 2022
Minimized this to:
And this stops failing if we remove the
stack_store v5, ss0, so I think its related to that.bjorn3 commentedon Aug 26, 2022
Yes, vector instructions that do a load or store as part of their operand need it to be 16 byte aligned. Only the unaligned variants of vector loads and stores (like movupd) allow arbitrary alignment.
afonso360 commentedon Aug 26, 2022
I've tried to minimize this further, and here's what I got
Minimized CLIF
Removing any of those instructions causes this to stop crashing.
Disassembly
In the debugger this crashes in
andnpd xmm1, xmmword ptr [rip + 0x108](offset:1a8in the disassembly) which is emitted as part of thefcopysigninstruction (the other uses ofandnpdare SIMD instructions which we don't emit).I think this is loading a constant from the constant pool after the function at offset
2b8. The address is 8 aligned (does it need to be 16?). And also, the value at that address is not the same that is emitted as part offcopysign, so I'm not sure what's going on.wasmtime/cranelift/codegen/src/isa/x64/lower.isle
Lines 3334 to 3338 in 05ffdc2
(posting this again since I deleted it by accident)
afonso360 commentedon Aug 26, 2022
Yep, that does seem to fix it, thanks!
jameysharp commentedon Aug 26, 2022
@cfallin @abrown @elliottt: Is there a more targeted way to fix this than reserving 16 bytes for every entry in the constant pool, or should we just merge #4789?
This is cool: cranelift-fuzzgen found a codegen bug that involves interaction between distant parts of the backend, and requires an unlucky stack layout. That's exactly the kind of thing I'd hope for from it. And good work on minimizing the failing input, @afonso360!
elliottt commentedon Aug 26, 2022
I've managed to minimize the test down a bit, this will segfault for any
explicit_slotvalues greater than 0, but fail the test assertion otherwise.jameysharp commentedon Aug 26, 2022
After digging into this, we've determined that the problem here is not the constant alignment, exactly. The root cause is that we're trying to load values from memory which are not 16 bytes long, using instructions that load 16 bytes.
This specific bug was introduced this week in cee4b20, while migrating
fcopysignto ISLE.The old lowering always did an 8-byte (or 4-byte, for F32) load to put the operands into general-purpose registers first. Then it moved from the GPRs to XMM registers, and never used the memory-operand variants of these SIMD instructions.
The natural translation of that pattern to ISLE has revealed a footgun around implicit conversions to xmm-or-mem operands. It's only safe to use the memory operand variant of these instructions for types which really are 16 bytes in size.
@elliottt is working on fixing that implicit conversion, which should resolve this issue and avoid all future problems of this particular type.
x64: Ensure that constants are always 16 bytes for XmmMem (#4790)