On Tue, Mar 05, 2024 at 01:14:19PM +0100, Miguel Ojeda wrote: > On Tue, Mar 5, 2024 at 12:58 PM Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote: > > > > Add flags to support the shadow call stack sanitizer, both in the > > dynamic and non-dynamic modes. > > > > Right now, the compiler will emit the warning "unknown feature specified > > for `-Ctarget-feature`: `reserve-x18`". However, the compiler still > > passes it to the codegen backend, so the flag will work just fine. Once > > rustc starts recognizing the flag (or provides another way to enable the > > feature), it will stop emitting this warning. See [1] for the relevant > > issue. > > > > Currently, the compiler thinks that the aarch64-unknown-none target > > doesn't support -Zsanitizer=shadow-call-stack, so the build will fail if > > you enable shadow call stack in non-dynamic mode. However, I still think > > it is reasonable to add the flag now, as it will at least fail the build > > when using an invalid configuration, until the Rust compiler is fixed to > > list -Zsanitizer=shadow-call-stack as supported for the target. See [2] > > for the feature request to add this. > > > > I have tested this change with Rust Binder on an Android device using > > CONFIG_DYNAMIC_SCS. Without the -Ctarget-feature=+reserve-x18 flag, the > > phone crashes immediately on boot, and with the flag, the phone appears > > to work normally. > > > > This contains a TODO to add the -Zuse-sync-unwind=n flag. The flag > > defaults to n, so it isn't a problem today, but the flag is unstable, so > > the default could change in a future compiler release. > > > > Link: https://github.com/rust-lang/rust/issues/121970 [1] > > Link: https://github.com/rust-lang/rust/issues/121972 [2] > > Signed-off-by: Alice Ryhl <aliceryhl@xxxxxxxxxx> > > --- > > This patch raises the question of whether we should change the Rust > > aarch64 support to use a custom target.json specification. If we do > > that, then we can fix both the warning for dynamic SCS and the > > build-failure for non-dynamic SCS without waiting for a new version of > > rustc with the mentioned issues fixed. > > If the arm64 maintainers are OK with the warning being triggered in that case: Sorry, I meant to reply on this at the time... > Acked-by: Miguel Ojeda <ojeda@xxxxxxxxxx> > > Otherwise partially reverting to the `target.json` approach sounds good too. > > I added the `-Zuse-sync-unwind=n` to the list at > https://github.com/Rust-for-Linux/linux/issues/2. Given the default is > what we want, I have put it in the "Good to have" section. I think we have time to do this properly, like we did for the clang enablement a few years ago. In hindsight, avoiding hacks for the early toolchains back then was a really good idea because it meant we could rely on a solid baseline set of compiler features from the start. So, please can we fix this in rustc and just have SCS dependent on that? Cheers, Will
