| Issue |
208819
|
| Summary |
Potential codegen issue causing bad register clobbering in rust binary
|
| Labels |
llvm:codegen,
rust
|
| Assignees |
|
| Reporter |
PiJoules
|
Filing this bug in case someone else runs into this later or has ideas on why it might be happening. Unfortunately getting a minimal reproducer of this outside of Fuchsia has been extremely difficult, so apologies in advance if no one else is able to make sense of this. I don't expect this to be addressed anytime soon but wanted to leave this documented.
Full context in https://g-issues.fuchsia.dev/issues/531813306.
--------------
The rust data-encoding crate contains this function:
```
fn decode_block<B: Static<usize>, M: Static<bool>>(
bit: B, msb: M, values: &[u8; 256], input: &[u8], output: &mut [u8],
) -> Result<(), usize> {
debug_assert!(output.len() <= enc(bit.val()));
debug_assert_eq!(input.len(), encode_len(bit, output.len()));
let bit = bit.val();
let msb = msb.val();
let mut x = 0u64;
for j in 0 .. input.len() {
let y = values[input[j] as usize];
check!(j, y < 1 << bit);
x |= u64::from(y) << (bit * order(msb, dec(bit), j));
}
for (j, output) in output.iter_mut().enumerate() {
*output = ((x >> (8 * order(msb, enc(bit), j))) & 0xff) as u8;
}
Ok(())
}
```
This is a helper function used by `HEXLOWER.decode` which takes a string of hex characters (`input`) and converts it to an array of integers corresponding to the hex (`output`). For a given hex string `"eb8ac26b5c9ef0279e3be3e82262a93bce16fe58ee422500d38caf461c65a3b6"`, we would expect `output` to be something like
```
+ let expected_bytes: [u8; 32] = [
+ 0xeb, 0x8a, 0xc2, 0x6b, 0x5c, 0x9e, 0xf0, 0x27,
+ 0x9e, 0x3b, 0xe3, 0xe8, 0x22, 0x62, 0xa9, 0x3b,
+ 0xce, 0x16, 0xfe, 0x58, 0xee, 0x42, 0x25, 0x00,
+ 0xd3, 0x8c, 0xaf, 0x46, 0x1c, 0x65, 0xa3, 0xb6,
+ ];
```
but instead it returns `[0x40, 0x40, 0x40, 0x40, ...]`. I managed to make a small test for this
```
+ #[test]
+ fn test_hexlower_decode_bug() {
+ let input_hex = "eb8ac26b5c9ef0279e3be3e82262a93bce16fe58ee422500d38caf461c65a3b6";
+ let expected_bytes: [u8; 32] = [
+ 0xeb, 0x8a, 0xc2, 0x6b, 0x5c, 0x9e, 0xf0, 0x27,
+ 0x9e, 0x3b, 0xe3, 0xe8, 0x22, 0x62, 0xa9, 0x3b,
+ 0xce, 0x16, 0xfe, 0x58, 0xee, 0x42, 0x25, 0x00,
+ 0xd3, 0x8c, 0xaf, 0x46, 0x1c, 0x65, 0xa3, 0xb6,
+ ];
+ let decoded = HEXLOWER.decode(input_hex.as_bytes()).expect("HEXLOWER.decode failed");
+ assert_eq!(decoded.as_slice(), &expected_bytes);
+ }
```
We saw this issue arise when rolling our clang and rust toolchains. Specifically, what we are doing is moving our rust revision from `64a965e9013a9d14e83c4d370af26f6be6bf96fb` to `c55fad5a9048ad0f7cff2a4867a297647af9392b`, and we're moving both our clang toolchain *and* the llvm revision rust is built on top of from `deb6854eec93529b2bd30178d400ad2ee7665cd4` to `dde579becf31add55c5a473f28fd010df36d2490`. We're confident this isn't because of any rust issue since we can keep the llvm rev pinned for both toolchains and move the rust rev forward and this doesn't lead to the issue we see. Keeping the rust rev pinned to what we're rolling rust to, we did a bisect on llvm (keeping the rev on both the clang toolchain and llvm used by rust the same) and found https://github.com/llvm/llvm-project/pull/140623 led to this issue, but it's not entirely clear why that commit leads to this issue.
Below is the asm for `decode_block`
on `0672a177f71eb7e556c6c434425916e9b430fdac`:
https://gist.github.com/PiJoules/5e61e4721085fb754919939ca91ab971
and `ae35674951e4d20d45ed6202e641976c9c055f22` (one commit before the bisected commit): https://gist.github.com/PiJoules/e8722c72fcf9f5f8273478fb23240539
Not going to pretend I understand all the vector instructions in either of these, but the first thing I do notice which looks like incorrect codegen is t1 which should correspond to the `let mut x = 0u64`. In the working asm:
```
88600: 4301 li t1, 0x0 // x is initialized to zero
...
75800: 00666333 or t1, a2, t1 // x is or-equaled
...
75812: 5e034457 vmv.v.x v8, t1 // x is moved into v8 for some vectorization for the second loop
```
In the broken asm:
```
88600: 4301 li t1, 0x0 // x = 0
...
8862a: 00666333 or t1, a2, t1 // x |= ...
...
// x is overwritten with the vector register length and used for whatever this is
88646: c2202373 csrr t1, vlenb
8864a: 030a slli t1, t1, 0x2
8864c: 20634333 sh2add t1, t1, t1
88650: 6385 lui t2, 0x1
88652: 40710133 sub sp, sp, t2
88656: e002 sd zero, 0x0(sp)
88658: 40730333 sub t1, t1, t2
8865c: fe735be3 bge t1, t2, 0x88652 <data_encoding::decode_block::<data_encoding::N4, data_encoding::Bt>+0x54>
...
8866a: 5e034457 vmv.v.x v8, t1 // x is moved again for the vectorization in the second loop, but it was overwritten by the above arithmetic
```
so that commit somehow leads to `t1` getting clobbered. The compile invocation used to build that code is
```
RUST_BACKTRACE=1 ../../../../rust-checkouts/install/fuchsia-rust/bin/rustc --color=always --crate-name=data_encoding ../../third_party/rust_crates/vendor/data-encoding-2.11.0/src/lib.rs --crate-type=rlib --emit=dep-
info=obj/third_party/rust_crates/data-encoding-v2_11_0.actual/libdata_encoding-669750a516a1aadc.rlib.d,link,metadata=obj/third_party/rust_crates/data-encoding-v2_11_0.actual/libdata_encoding-669750a516a1aadc.rmeta -Zdep-info-omit-d-target
-Cmetadata=obj/third_party/rust_crates/data-encoding-v2_11_0 @rust_api_level_cfg_flags.txt --cap-lints=allow -Cmetadata=669750a516a1aadc -Cextra-filename=-669750a516a1aadc --cfg=feature="alloc" --cfg=feature="default" --cfg=feature="std" -
Clink-arg=--color-diagnostics --cfg=__rust_toolchain="c55fad5a9048ad0f7cff2a4867a297647af9392b" -Zemit-stack-sizes -Clink-arg=--mllvm=--stack-size-section -Zbinary-dep-depinfo -Clinker=../../../../llvm-repos/llvm-project-1-build/bin/lld -
Clink-arg=-L../../../../llvm-repos/llvm-project-1-build/lib/riscv64-unknown-fuchsia/noexcept -Clink-arg=-L../../../../llvm-repos/llvm-project-1-build/lib/clang/23/lib/riscv64-unknown-fuchsia -Clink-arg=--pack-dyn-relocs=relr -Clink-arg=-
dynamic-linker=ld.so.1 -Clink-arg=--icf=all -Clink-arg=-zrel -L gen/zircon/public/sysroot/lib -Clink-arg=--sysroot=gen/zircon/public/sysroot -Zsanitizer=shadow-call-stack -Cllvm-args=--target-abi=lp64d -Ctarget-feature=+m -Ctarget-
feature=+a -Ctarget-feature=+zicsr -Ctarget-feature=+c -Ctarget-feature=+zihintpause -Ctarget-feature=+zba -Ctarget-feature=+zbb -Ctarget-feature=+zbs -Ctarget-feature=+zicbom -Ctarget-feature=+zicbop -Ctarget-feature=+zicboz -Ctarget-
feature=+zkt -Ccodegen-units=1 -Zremap-cwd-prefix=. -Cforce-frame-pointers -Zfunction-sections -Clink-arg=--gc-sections -Copt-level=s -Clink-arg=-O2 -Clink-arg=--bp-compression-sort=function -Clink-arg=--mllvm=-relocation-model=pic -Clink-
arg=--mllvm=--enable-tlsdesc -Clink-arg=--mllvm=-wholeprogramdevirt-branch-funnel-threshold=0 -Clink-arg=--mllvm=-mattr=+a -Clink-arg=--mllvm=-mattr=+c -Clink-arg=--mllvm=-mattr=+f -Clink-arg=--mllvm=-mattr=+d -Clink-arg=--mllvm=-mattr=+
relax -Clto=fat -Zdylib-lto -Clinker-plugin-lto -Zsplit-lto-unit -Clink-arg=--mllvm=-relocation-model=pic -Clink-arg=--mllvm=--enable-tlsdesc -Clink-arg=--mllvm=-wholeprogramdevirt-branch-funnel-threshold=0 -Clink-arg=--mllvm=-mattr=+a -
Clink-arg=--mllvm=-mattr=+c -Clink-arg=--mllvm=-mattr=+f -Clink-arg=--mllvm=-mattr=+d -Clink-arg=--mllvm=-mattr=+relax -Cdebuginfo=2 -Dwarnings -Cdebug-assertions=no --error-format=human -Zallow-features= --target riscv64gc-unknown-fuchsia
--cap-lints=deny -Aunknown_lints -Wmissing_unsafe_on_extern -Wunsafe_attr_outside_unsafe -Wunused_crate_dependencies -Dderef_nullptr -Dinvalid_value -Dunused_must_use -Arenamed_and_removed_lints -Anon_local_definitions -
Amismatched_lifetime_syntaxes -Cpanic=abort -Cforce-unwind-tables=yes -Zpanic_abort_tests -Csymbol-mangling-version=v0 -Clink-arg=--icf=all -Cprefer-dynamic -Clink-args=-zstack-size=0x200000 --edition=2018 -o
obj/third_party/rust_crates/data-encoding-v2_11_0.actual/libdata_encoding-669750a516a1aadc.rlib -Zshell-argfiles @shell:obj/third_party/rust_crates/data-encoding-v2_11_0.actual/libdata_encoding-669750a516a1aadc.rlib.rsp
```
and the link invocation is
```
RUST_BACKTRACE=1 ../../../../rust-checkouts/install/fuchsia-rust/bin/rustc --color=always --crate-name=tuf_lib_test ../../third_party/rust_crates/vendor/tuf-0.3.0-beta14/src/lib.rs --crate-type=bin --emit=dep-info=./tuf_lib_test.d,link,
metadata=./exe.unstripped/tuf_lib_test.rmeta -Zdep-info-omit-d-target -Clink-args=--Map="./exe.unstripped/tuf_lib_test.map" -Cmetadata=obj/src/sys/pkg/lib/tuf/tuf_lib_test @rust_api_level_cfg_flags.txt -Aunused_imports -Adead_code -
Adeprecated --cap-lints=allow --test -Clink-arg=--color-diagnostics --cfg=__rust_toolchain="c55fad5a9048ad0f7cff2a4867a297647af9392b" -Zemit-stack-sizes -Clink-arg=--mllvm=--stack-size-section -Zbinary-dep-depinfo -Clinker=../../../..
/llvm-repos/llvm-project-1-build/bin/lld -Clink-arg=-L../../../../llvm-repos/llvm-project-1-build/lib/riscv64-unknown-fuchsia/noexcept -Clink-arg=-L../../../../llvm-repos/llvm-project-1-build/lib/clang/23/lib/riscv64-unknown-fuchsia -
Clink-arg=--pack-dyn-relocs=relr -Clink-arg=-dynamic-linker=ld.so.1 -Clink-arg=--icf=all -Clink-arg=-zrel -L gen/zircon/public/sysroot/lib -Clink-arg=--sysroot=gen/zircon/public/sysroot -Zsanitizer=shadow-call-stack -Cllvm-args=--target-
abi=lp64d -Ctarget-feature=+m -Ctarget-feature=+a -Ctarget-feature=+zicsr -Ctarget-feature=+c -Ctarget-feature=+zihintpause -Ctarget-feature=+zba -Ctarget-feature=+zbb -Ctarget-feature=+zbs -Ctarget-feature=+zicbom -Ctarget-feature=+zicbop
-Ctarget-feature=+zicboz -Ctarget-feature=+zkt -Ccodegen-units=1 -Zremap-cwd-prefix=. -Cforce-frame-pointers -Zfunction-sections -Clink-arg=--gc-sections -Copt-level=s -Clink-arg=-O2 -Clink-arg=--bp-compression-sort=function -Clink-arg=--
mllvm=-relocation-model=pic -Clink-arg=--mllvm=--enable-tlsdesc -Clink-arg=--mllvm=-wholeprogramdevirt-branch-funnel-threshold=0 -Clink-arg=--mllvm=-mattr=+a -Clink-arg=--mllvm=-mattr=+c -Clink-arg=--mllvm=-mattr=+f -Clink-arg=--mllvm=-
mattr=+d -Clink-arg=--mllvm=-mattr=+relax -Clto=fat -Zdylib-lto -Clinker-plugin-lto -Zsplit-lto-unit -Clink-arg=--mllvm=-relocation-model=pic -Clink-arg=--mllvm=--enable-tlsdesc -Clink-arg=--mllvm=-wholeprogramdevirt-branch-funnel-
threshold=0 -Clink-arg=--mllvm=-mattr=+a -Clink-arg=--mllvm=-mattr=+c -Clink-arg=--mllvm=-mattr=+f -Clink-arg=--mllvm=-mattr=+d -Clink-arg=--mllvm=-mattr=+relax -Cdebuginfo=2 -Dwarnings -Cdebug-assertions=no --error-format=human -Zallow-
features= --target riscv64gc-unknown-fuchsia --cap-lints=deny -Aunknown_lints -Wmissing_unsafe_on_extern -Wunsafe_attr_outside_unsafe -Wunused_crate_dependencies -Dderef_nullptr -Dinvalid_value -Dunused_must_use -Arenamed_and_removed_lints
-Anon_local_definitions -Amismatched_lifetime_syntaxes -Cpanic=abort -Cforce-unwind-tables=yes -Zpanic_abort_tests -Csymbol-mangling-version=v0 -Clink-arg=--icf=all -Cprefer-dynamic -Clink-args=-zstack-size=0x200000 --edition=2021 -
ldylib=fdio -Clink-arg=riscv64-shared/link_stub/libfdio.so -o "./exe.unstripped/tuf_lib_test" -Zshell-argfiles @shell:./tuf_lib_test.rsp
```
Unfortunately coming up with a standalone minimal repro has been very difficult. Since this is LTO'd rust, it could be that LTO is taking advantage of some latent UB or there's some existing codegen bug tickled by that bisected commit. I haven't been able to repro this as something standalone or without lto. Not expecting much out of this, but just wanted to document this. There's probably some combination of switches we could do to magically make this dissappear.
![]()
_______________________________________________
llvm-bugs mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs
