Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[loongarch*-linux] -Crelocation-model=static should not access external data directly #118053

Closed
heiher opened this issue Nov 19, 2023 · 8 comments · Fixed by #119162
Closed

[loongarch*-linux] -Crelocation-model=static should not access external data directly #118053

heiher opened this issue Nov 19, 2023 · 8 comments · Fixed by #119162
Labels
C-bug Category: This is a bug. O-loongarch Target: LoongArch (LA32R, LA32S, LA64) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@heiher
Copy link
Contributor

heiher commented Nov 19, 2023

I tried this code:

use std::ffi::CStr;
use std::os::raw::c_char;

extern "C" {
    static environ: *const *const c_char;
}

fn main() {
    unsafe {
        let mut env_ptr = environ;

        while !(*env_ptr).is_null() {
            let c_str_ptr = *env_ptr;
            let rust_str = CStr::from_ptr(c_str_ptr);

            println!("{:?}", rust_str.to_string_lossy());
            env_ptr = env_ptr.offset(1);
        }
    }
}

I expected to see the environments variables printed.

Instead, this happened: Aborted (core dumped)

Meta

rustc --version --verbose:

rustc 1.74.0 (79e9716c9 2023-11-13)
binary: rustc
commit-hash: 79e9716c980570bfd1f666e3b16ac583f0168962
commit-date: 2023-11-13
host: loongarch64-unknown-linux-gnu
release: 1.74.0
LLVM version: 17.0.4
Backtrace

thread 'main' panicked at t.rs:12:16:
misaligned pointer dereference: address must be a multiple of 0x8 but is 0x28ee81ef1c000aef
stack backtrace:
   0: rust_begin_unwind
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/std/src/panicking.rs:597:5
   1: core::panicking::panic_nounwind_fmt
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/panicking.rs:106:14
   2: core::panicking::panic_misaligned_pointer_dereference
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/panicking.rs:193:5
   3: t::main
             at ./t.rs:12:16
   4: core::ops::function::FnOnce::call_once
             at /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
thread caused non-unwinding panic. aborting.
Aborted (core dumped)

@heiher heiher added the C-bug Category: This is a bug. label Nov 19, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 19, 2023
@heiher
Copy link
Contributor Author

heiher commented Nov 19, 2023

@rustbot label O-loongarch

@rustbot rustbot added the O-loongarch Target: LoongArch (LA32R, LA32S, LA64) label Nov 19, 2023
@heiher
Copy link
Contributor Author

heiher commented Nov 19, 2023

This is the same issue as llvm/llvm-project#71645. For the linux kernel, we need to access external data directly when the relocation model is static to avoid GOT. It would better to add the -Cdirect-access-external-data argument to tune this behavior like clang.

@xry111
Copy link
Contributor

xry111 commented Nov 20, 2023

From https://doc.rust-lang.org/rustc/codegen-options/index.html:

static - non-relocatable code, machine instructions may use absolute addressing modes.

So in the kernel it just should not be used for arch/loongarch: at least when CONFIG_RELOCATABLE=y we expect vmlinux to be a PIE, so non-relocatable code should not be allowed.

arch/um already has:

KBUILD_RUSTFLAGS += -Crelocation-model=pie

Can we have the same?

@xry111
Copy link
Contributor

xry111 commented Nov 20, 2023

From https://doc.rust-lang.org/rustc/codegen-options/index.html:

static - non-relocatable code, machine instructions may use absolute addressing modes.

So in the kernel it just should not be used for arch/loongarch: at least when CONFIG_RELOCATABLE=y we expect vmlinux to be a PIE, so non-relocatable code should not be allowed.

arch/um already has:

KBUILD_RUSTFLAGS += -Crelocation-model=pie

Can we have the same?

Oops, it will create GOT in vmlinux because we don't have -Cdirect-access-external-data. I think we should add -Cdirect-access-external-data, or make a new relocation model static-pie which is suitable for static PIEs like vmlinux.

@xry111
Copy link
Contributor

xry111 commented Nov 20, 2023

arch/um already has:

KBUILD_RUSTFLAGS += -Crelocation-model=pie

Can we have the same?

Oops, it will create GOT in vmlinux because we don't have -Cdirect-access-external-data. I think we should add -Cdirect-access-external-data, or make a new relocation model static-pie which is suitable for static PIEs like vmlinux.

Another issue is we don't have a differentiate between KBUILD_RUSTFLAGS_MODULE and KBUILD_RUSTFLAGS_KERNEL yet. With either -Cdirect-access-external-data or -Crelocation-model=static-pie we still need such a differentiate.

@heiher
Copy link
Contributor Author

heiher commented Nov 20, 2023

arch/um already has:

KBUILD_RUSTFLAGS += -Crelocation-model=pie

Can we have the same?

Oops, it will create GOT in vmlinux because we don't have -Cdirect-access-external-data. I think we should add -Cdirect-access-external-data, or make a new relocation model static-pie which is suitable for static PIEs like vmlinux.

Another issue is we don't have a differentiate between KBUILD_RUSTFLAGS_MODULE and KBUILD_RUSTFLAGS_KERNEL yet. With either -Cdirect-access-external-data or -Crelocation-model=static-pie we still need such a differentiate.

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Makefile?h=next-20231120#n568
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Makefile?h=next-20231120#n571

@heiher
Copy link
Contributor Author

heiher commented Nov 20, 2023

From https://doc.rust-lang.org/rustc/codegen-options/index.html:

static - non-relocatable code, machine instructions may use absolute addressing modes.

So in the kernel it just should not be used for arch/loongarch: at least when CONFIG_RELOCATABLE=y we expect vmlinux to be a PIE, so non-relocatable code should not be allowed.

arch/um already has:

KBUILD_RUSTFLAGS += -Crelocation-model=pie

Can we have the same?

Updated: heiher/linux@a590c2f#diff-0e1f2cb4db0de17a95f13ee71d503167892fb028b00507977fcf9568066b33ecR88

@heiher
Copy link
Contributor Author

heiher commented Nov 20, 2023

From https://doc.rust-lang.org/rustc/codegen-options/index.html:

static - non-relocatable code, machine instructions may use absolute addressing modes.

So in the kernel it just should not be used for arch/loongarch: at least when CONFIG_RELOCATABLE=y we expect vmlinux to be a PIE, so non-relocatable code should not be allowed.
arch/um already has:

KBUILD_RUSTFLAGS += -Crelocation-model=pie

Can we have the same?

Oops, it will create GOT in vmlinux because we don't have -Cdirect-access-external-data. I think we should add -Cdirect-access-external-data, or make a new relocation model static-pie which is suitable for static PIEs like vmlinux.

In clang, -fdirect-access-external-data only works with no-pic. Therefore, even when using -fdirect-access-external-data with -fPIE, there are still GOT entries. I just noticed that is different from GCC.

Update: Function doesn't support direct access in PIE mode.

llvm/llvm-project@e2e82c9#diff-e724febedab9c1a2832bf2056d208ff02ddcb2e6f90b5a653afc9b19ac78a5d7L991-L995

@saethlin saethlin added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Nov 28, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 7, 2024
…, r=petrochenkov

Add unstable `-Z direct-access-external-data` cmdline flag for `rustc`

The new flag has been described in the Major Change Proposal at rust-lang/compiler-team#707

Fixes rust-lang#118053
Nadrieril added a commit to Nadrieril/rust that referenced this issue Feb 7, 2024
…, r=petrochenkov

Add unstable `-Z direct-access-external-data` cmdline flag for `rustc`

The new flag has been described in the Major Change Proposal at rust-lang/compiler-team#707

Fixes rust-lang#118053
@bors bors closed this as completed in 7954c28 Feb 7, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 7, 2024
Rollup merge of rust-lang#119162 - heiher:direct-access-external-data, r=petrochenkov

Add unstable `-Z direct-access-external-data` cmdline flag for `rustc`

The new flag has been described in the Major Change Proposal at rust-lang/compiler-team#707

Fixes rust-lang#118053
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-loongarch Target: LoongArch (LA32R, LA32S, LA64) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants