hwasan: Fix relocation errors by adjusting NewGV alias address calculation
#164876
Conversation
…ulation
We don't want to apply `PointerTagShift` to the alias address of `NewGV`
that replaces the `GV` we are instrumenting, because that makes the
address huge and results in relocation errors such as:
hwaddress.7rcbfp3g-cgu.0:(.text.main+0x7c): \
relocation truncated to fit: R_AARCH64_ADR_PREL_PG_HI21 against `.data.rel.ro..L.hwasan'
unless optimizations happens get rid of the faulty calculation.
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-compiler-rt-sanitizer @llvm/pr-subscribers-llvm-transforms Author: Martin Nordholts (Enselic) ChangesWe don't want to apply unless optimizations happens get rid of the faulty calculation. I am not 100% sure this is the correct fix, but when I verified the fix with a Rust test we have, the test passed. LLVM have many more HWAsan tests however, so it would be useful to see if they pass with this fix. Maybe someone can help me trigger HWAsan CI? CC @pcc who added the logic I change in 0930643. Maybe you can quickly determine if this patch is right or wrong? FWIW, here are some backtraces I collected while debugging this. Rust bugIn rust-lang/rust#83989 we get a linker error due to the addend being huge: $ rustc +nightly -Clinker=aarch64-linux-gnu-gcc --target=aarch64-unknown-linux-gnu tests/ui/sanitizer/hwaddress.rs -Z sanitizer=hwaddress -O -g -C codegen-units=16 -C unsafe-allow-abi-mismatch=sanitizer -Clto=no
error: linking with `aarch64-linux-gnu-gcc` failed: exit status: 1
|
= note: hwaddress.hwaddress.71eeac658309bde7-cgu.2.rcgu.o: in function `std::rt::lang_start':
/home/martin/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:205:(.text._ZN3std2rt10lang_start17hae1e7d092f980101E+0x38): relocation truncated to fit: R_AARCH64_ADR_PREL_PG_HI21 against `.data.rel.ro..L.hwasan'
collect2: error: ld returned 1 exit statusand the fix appears to work based on Rust CI as mentioned above Full diff: https://github.com/llvm/llvm-project/pull/164876.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 832592e7663b2..e12ca0404ee9c 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -1763,9 +1763,7 @@ void HWAddressSanitizer::instrumentGlobal(GlobalVariable *GV, uint8_t Tag) {
}
Constant *Aliasee = ConstantExpr::getIntToPtr(
- ConstantExpr::getAdd(
- ConstantExpr::getPtrToInt(NewGV, Int64Ty),
- ConstantInt::get(Int64Ty, uint64_t(Tag) << PointerTagShift)),
+ ConstantExpr::getPtrToInt(NewGV, Int64Ty),
GV->getType());
auto *Alias = GlobalAlias::create(GV->getValueType(), GV->getAddressSpace(),
GV->getLinkage(), "", Aliasee, &M);
|
|
The tag in the symbol address is deliberate. This change would break accessing an address-taken global because the accesses would not be using the correct tag. The fix is likely to modify the Rust compiler to ensure that the It looks like the Rust compiler documentation mentions |
|
Oh... Thank you for taking the time to look that up and letting me know. That worked so let's close this PR. |
tests/ui/sanitizer/hwaddress.rs: Run on aarch64 and remove cgu hack
To avoid linker errors like
relocation truncated to fit: R_AARCH64_ADR_PREL_PG_HI21 against `.data.rel.ro..L.hwasan'
we need to have `-C target-feature=+tagged-globals`, which is documented [here](https://doc.rust-lang.org/beta/unstable-book/compiler-flags/sanitizer.html#hwaddresssanitizer). I learned that [here](llvm/llvm-project#164876 (comment)).
Closes rust-lang#83989
try-job: aarch64-gnu
3 participants
We don't want to apply
PointerTagShiftto the alias address ofNewGVthat replaces theGVwe are instrumenting, because that makes the address huge and results in relocation errors such as:unless optimizations happens get rid of the faulty calculation.
I am not 100% sure this is the correct fix, but when I verified the fix with a Rust test we have, the test passed. LLVM have many more HWAsan tests however, so it would be useful to see if they pass with this fix. Maybe someone can help me trigger HWAsan CI?
CC @pcc who added the logic I change in 0930643. Maybe you can quickly determine if this patch is right or wrong?
FWIW, here are some backtraces I collected while debugging this.
Rust bug
In rust-lang/rust#83989 we get a linker error due to the addend being huge:
and the fix appears to work based on Rust CI as mentioned above