Skip to content

Statics don't support alignments larger than the page size #70022

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

Open
Amanieu opened this issue Mar 15, 2020 · 20 comments
Open

Statics don't support alignments larger than the page size #70022

Amanieu opened this issue Mar 15, 2020 · 20 comments
Assignees
Labels
A-align Area: alignment control (`repr(align(N))` and so on) A-target-specs Area: Compile-target specifications C-bug Category: This is a bug. E-needs-investigation Call for partcipation: This issues needs some investigation to determine current status I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness O-linux Operating system: Linux O-windows-gnu Toolchain: GNU, Operating system: Windows O-windows-msvc Toolchain: MSVC, Operating system: Windows P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Amanieu
Copy link
Member

Amanieu commented Mar 15, 2020

Example code:

#[repr(align(0x100000))]
struct Aligned(u8);

static X: Aligned = Aligned(0);

fn main() {
    let x = Aligned(0);
    println!("{:#x}", &x as *const _ as usize);
    println!("{:#x}", &X as *const _ as usize);
    println!("{:#x}", Box::into_raw(Box::new(Aligned(0))) as usize);
}

Output:

0x7ffec8000000
0x55c9b2596000
0x7f2879f00000

I am not sure about other platforms, but in the case of Linux this happens because the ELF loader in the kernel ignores the p_align field of the ELF program headers and assumes page-alignment instead.

@Amanieu Amanieu added C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 15, 2020
@jonas-schievink jonas-schievink added the O-linux Operating system: Linux label Mar 15, 2020
@mati865
Copy link
Contributor

mati865 commented Mar 16, 2020

Slight offtopic but windows-gnu rustc 1.43.0-nightly (c20d7eecb 2020-03-11) hangs trying to compile this code:

Stack
rustc_driver-3846b4b120a304cc.dll!ZN57_$LT$log..ParseLevelError$u20$as$u20$core..fmt..Debug$GT$3fmt17hbfdab30e317c4c20E+0x8b36bb
rustc_driver-3846b4b120a304cc.dll!ZN57_$LT$log..ParseLevelError$u20$as$u20$core..fmt..Debug$GT$3fmt17hbfdab30e317c4c20E+0xbc7444
rustc_driver-3846b4b120a304cc.dll!ZN57_$LT$log..ParseLevelError$u20$as$u20$core..fmt..Debug$GT$3fmt17hbfdab30e317c4c20E+0xbc72f7
rustc_driver-3846b4b120a304cc.dll!ZN57_$LT$log..ParseLevelError$u20$as$u20$core..fmt..Debug$GT$3fmt17hbfdab30e317c4c20E+0xb30d26
rustc_driver-3846b4b120a304cc.dll!ZN57_$LT$log..ParseLevelError$u20$as$u20$core..fmt..Debug$GT$3fmt17hbfdab30e317c4c20E+0xafe3d4
rustc_driver-3846b4b120a304cc.dll!ZN57_$LT$log..ParseLevelError$u20$as$u20$core..fmt..Debug$GT$3fmt17hbfdab30e317c4c20E+0xe1b07a
rustc_driver-3846b4b120a304cc.dll!ZN57_$LT$log..ParseLevelError$u20$as$u20$core..fmt..Debug$GT$3fmt17hbfdab30e317c4c20E+0x17d8948
rustc_driver-3846b4b120a304cc.dll!ZN57_$LT$log..ParseLevelError$u20$as$u20$core..fmt..Debug$GT$3fmt17hbfdab30e317c4c20E+0x47d40
rustc_driver-3846b4b120a304cc.dll!ZN18rustc_codegen_llvm5type_120_$LT$impl$u20$rustc_codegen_ssa..traits..type_..LayoutTypeMethods$u20$for$u20$rustc_codegen_llvm..context..CodegenCx$GT$16reg_backend_type17h2e5962d8cdec0839E+0x1c57a
rustc_driver-3846b4b120a304cc.dll!ZN93_$LT$rustc_codegen_llvm..back..write..DiagnosticHandlers$u20$as$u20$core..ops..drop..Drop$GT$4drop17h9feab9942b78b38dE+0x232a
rustc_driver-3846b4b120a304cc.dll!ZN89_$LT$rustc_codegen_llvm..llv$u6d$_..OperandBundleDef$u20$as$u20$core..ops..drop..Drop$GT$4drop17h1b289036a4b6c754E+0x7885
rustc_driver-3846b4b120a304cc.dll!ZN81_$LT$rustc_codegen_llvm..llv$u6d$_..ffi..PassKind$u20$as$u20$core..fmt..Debug$GT$3fmt17h33c3c6083599899aE+0x8120
rustc_driver-3846b4b120a304cc.dll!ZN116_$LT$rustc_target..abi..TyLayout$LT$$RF$rustc..ty..TyS$GT$$u20$as$u20$rustc_codegen_llvm..type_of..LayoutLlvmExt$GT$15pointee_info_at17ha9a0722bbe04da49E+0x50a6
std-bb1869756d5e28c4.dll!_rust_maybe_catch_panic+0x19
rustc_driver-3846b4b120a304cc.dll!ZN107_$LT$rustc_codegen_llvm..builder..Builder$u20$as$u20$rustc_codegen_ssa..traits..builder..BuilderMethods$GT$14unchecked_umul17hb224b4a0601dcf96E+0x43222
std-bb1869756d5e28c4.dll+0x88f6
std-bb1869756d5e28c4.dll!ZN3std3sys7windows6thread6Thread3new17h64299152593e9dd6E+0x14c
KERNEL32.DLL!BaseThreadInitThunk+0x14
ntdll.dll!RtlUserThreadStart+0x21

@RalfJung
Copy link
Member

Isn't this technically speaking unsound, because we fail to respect the type's alignment requirement?

Maybe align should just reject anything bigger than the page size, as a stop-gap fix at least.

@RalfJung RalfJung added the I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness label Mar 18, 2020
@jonas-schievink
Copy link
Contributor

Hmm, is it unsound if rustc is doing the right thing, but the OS isn't?

@RalfJung
Copy link
Member

Well it's still unsound, it's just not clear if it is a rustc bug or a linker bug.
But even if it is a linker bug, we should either work around it or forward it upstream.

@Amanieu
Copy link
Member Author

Amanieu commented Mar 18, 2020

The bug is arguably in the kernel. But even if we get this fixed upstream, we still need to support a large range of existing kernel versions.

Also, I just tested this on x86_64-pc-windows-gnu and the same issue also occurs. In this particular case it might be a linker issue though since unlike ELF, the PE section alignment is set to 4KB (which is wrong).

@Amanieu Amanieu added the O-windows-gnu Toolchain: GNU, Operating system: Windows label Mar 18, 2020
@retep998
Copy link
Member

retep998 commented Mar 19, 2020

For pc-windows-msvc it fails to link with fatal error LNK1164: section 0x6 alignment (8192) greater than /ALIGN value. If I add -Clink-arg="/ALIGN:1048576" then it links. If I test it, it overflows the stack. If I lower the alignment to just 0x10000 and do /ALIGN:65536 then it crashes and a debugger tells me Unhandled exception at 0x00007FF6EC990064 in aligntest.exe: 0xC0000005: Access violation writing location 0x000000AD09A9FFF8. Apparently calculations about the stack are broken and it is touching memory way past the stack guard page causing the crash, despite a call __chkstk definitely existing at the start of the function. From what I can tell it is because it aligns the stack after probing the stack.

#[repr(align(0x10000))]
struct Aligned(u8);

static X: Aligned = Aligned(0);

fn main() {
00007FF6EC990000  push        rbp  
00007FF6EC990001  mov         eax,4FFF0h  
00007FF6EC990006  call        __chkstk (07FF6EC9A9D20h)  ; this checks the stack
00007FF6EC99000B  sub         rsp,rax  
00007FF6EC99000E  lea         rbp,[rsp+80h]  
; this rounds down the stack by well over the size of a page
; causing the stack to extend past the guard page
00007FF6EC990016  and         rsp,0FFFFFFFFFFFF0000h 
00007FF6EC99001D  lea         rax,[rsp+10000h]  
    let x = Aligned(0);
00007FF6EC990025  mov         byte ptr [rsp+10000h],0  
    println!("{:#x}", &x as *const _ as usize);
00007FF6EC99002D  mov         qword ptr [rsp+2FF48h],rax  
00007FF6EC990035  lea         rax,[rsp+2FF48h]  
00007FF6EC99003D  mov         qword ptr [rsp+2FF40h],rax  
00007FF6EC990045  mov         rax,qword ptr [rsp+2FF40h]  
00007FF6EC99004D  mov         qword ptr [rsp+2FF50h],rax  
00007FF6EC990055  mov         rcx,qword ptr [rsp+2FF50h]  
00007FF6EC99005D  lea         rdx,[core::fmt::num::{{impl}}::fmt (07FF6EC9A93A0h)]  
00007FF6EC990064  call        core::fmt::ArgumentV1::new<usize> (07FF6EC990450h)  
00007FF6EC990069  mov         qword ptr [rsp+0FFF8h],rax  
00007FF6EC990071  mov         qword ptr [rsp+0FFF0h],rdx  
00007FF6EC990079  lea         rax,[X+10008h (07FF6EC9C2008h)]  
00007FF6EC990080  lea         rcx,[X+10028h (07FF6EC9C2028h)]  
00007FF6EC990087  mov         rdx,qword ptr [rsp+0FFF8h]  
00007FF6EC99008F  mov         qword ptr [rsp+2FF30h],rdx  
00007FF6EC990097  mov         r8,qword ptr [rsp+0FFF0h]  
00007FF6EC99009F  mov         qword ptr [rsp+2FF38h],r8  
00007FF6EC9900A7  lea         r9,[rsp+2FF30h]  
00007FF6EC9900AF  lea         r10,[rsp+2FF00h]  
00007FF6EC9900B7  mov         qword ptr [rsp+0FFE8h],rcx  
00007FF6EC9900BF  mov         rcx,r10  
00007FF6EC9900C2  mov         rdx,rax  
00007FF6EC9900C5  mov         r8d,2  
00007FF6EC9900CB  mov         qword ptr [rsp+20h],1  
00007FF6EC9900D4  mov         rax,qword ptr [rsp+0FFE8h]  
00007FF6EC9900DC  mov         qword ptr [rsp+28h],rax  
00007FF6EC9900E1  mov         qword ptr [rsp+30h],1  
00007FF6EC9900EA  call        core::fmt::Arguments::new_v1_formatted (07FF6EC9904B0h)  
00007FF6EC9900EF  lea         rcx,[rsp+2FF00h]  
00007FF6EC9900F7  call        std::io::stdio::_print (07FF6EC995440h)  
    println!("{:#x}", &X as *const _ as usize);
; notice here that the static is NOT aligned correctly
00007FF6EC9900FC  lea         rax,[X (07FF6EC9B2000h)]  
00007FF6EC990103  mov         qword ptr [rsp+2FFA0h],rax  
00007FF6EC99010B  lea         rax,[rsp+2FFA0h]  
00007FF6EC990113  mov         qword ptr [rsp+2FF98h],rax  
00007FF6EC99011B  mov         rax,qword ptr [rsp+2FF98h]  
00007FF6EC990123  mov         qword ptr [rsp+2FFA8h],rax  
00007FF6EC99012B  mov         rcx,qword ptr [rsp+2FFA8h]  
00007FF6EC990133  lea         rdx,[core::fmt::num::{{impl}}::fmt (07FF6EC9A93A0h)]  
00007FF6EC99013A  call        core::fmt::ArgumentV1::new<usize> (07FF6EC990450h)  
00007FF6EC99013F  mov         qword ptr [rsp+0FFE0h],rax  
00007FF6EC990147  mov         qword ptr [rsp+0FFD8h],rdx  
00007FF6EC99014F  lea         rax,[X+10008h (07FF6EC9C2008h)]  
00007FF6EC990156  lea         rcx,[X+10028h (07FF6EC9C2028h)]  
00007FF6EC99015D  mov         rdx,qword ptr [rsp+0FFE0h]  
00007FF6EC990165  mov         qword ptr [rsp+2FF88h],rdx  
00007FF6EC99016D  mov         r8,qword ptr [rsp+0FFD8h]  
00007FF6EC990175  mov         qword ptr [rsp+2FF90h],r8  
00007FF6EC99017D  lea         r9,[rsp+2FF88h]  
00007FF6EC990185  lea         r10,[rsp+2FF58h]  
00007FF6EC99018D  mov         qword ptr [rsp+0FFD0h],rcx  
00007FF6EC990195  mov         rcx,r10  
00007FF6EC990198  mov         rdx,rax  
00007FF6EC99019B  mov         r8d,2  
00007FF6EC9901A1  mov         qword ptr [rsp+20h],1  
00007FF6EC9901AA  mov         rax,qword ptr [rsp+0FFD0h]  
00007FF6EC9901B2  mov         qword ptr [rsp+28h],rax  
00007FF6EC9901B7  mov         qword ptr [rsp+30h],1  
00007FF6EC9901C0  call        core::fmt::Arguments::new_v1_formatted (07FF6EC9904B0h)  
00007FF6EC9901C5  lea         rcx,[rsp+2FF58h]  
00007FF6EC9901CD  call        std::io::stdio::_print (07FF6EC995440h)  
    println!("{:#x}", Box::into_raw(Box::new(Aligned(0))) as usize);
00007FF6EC9901D2  mov         byte ptr [rsp+30000h],0  
00007FF6EC9901DA  mov         eax,10000h  
00007FF6EC9901DF  mov         rcx,rax  
00007FF6EC9901E2  mov         rdx,rax  
00007FF6EC9901E5  call        alloc::alloc::exchange_malloc (07FF6EC9905F0h)  
00007FF6EC9901EA  mov         rcx,rax  
00007FF6EC9901ED  lea         rdx,[rsp+30000h]  
00007FF6EC9901F5  mov         qword ptr [rsp+0FFC8h],rcx  
00007FF6EC9901FD  mov         rcx,rax  
00007FF6EC990200  mov         r8d,10000h  
00007FF6EC990206  call        memcpy (07FF6EC9AA8D5h)  
00007FF6EC99020B  mov         rcx,qword ptr [rsp+0FFC8h]  
00007FF6EC990213  call        alloc::boxed::{{impl}}::into_raw<aligntest::Aligned> (07FF6EC990860h)  
    println!("{:#x}", Box::into_raw(Box::new(Aligned(0))) as usize);
00007FF6EC990218  mov         qword ptr [rsp+0FFC0h],rax  
00007FF6EC990220  mov         rax,qword ptr [rsp+0FFC0h]  
00007FF6EC990228  mov         qword ptr [rsp+2FFF8h],rax  
00007FF6EC990230  lea         rcx,[rsp+2FFF8h]  
00007FF6EC990238  mov         qword ptr [rsp+2FFF0h],rcx  
00007FF6EC990240  mov         rcx,qword ptr [rsp+2FFF0h]  
00007FF6EC990248  mov         qword ptr [rsp+4FFE8h],rcx  
00007FF6EC990250  mov         rcx,qword ptr [rsp+4FFE8h]  
00007FF6EC990258  lea         rdx,[core::fmt::num::{{impl}}::fmt (07FF6EC9A93A0h)]  
00007FF6EC99025F  call        core::fmt::ArgumentV1::new<usize> (07FF6EC990450h)  
00007FF6EC990264  mov         qword ptr [rsp+0FFB8h],rax  
00007FF6EC99026C  mov         qword ptr [rsp+0FFB0h],rdx  
00007FF6EC990274  lea         rax,[X+10008h (07FF6EC9C2008h)]  
00007FF6EC99027B  lea         rcx,[X+10028h (07FF6EC9C2028h)]  
00007FF6EC990282  mov         rdx,qword ptr [rsp+0FFB8h]  
00007FF6EC99028A  mov         qword ptr [rsp+2FFE0h],rdx  
00007FF6EC990292  mov         r8,qword ptr [rsp+0FFB0h]  
00007FF6EC99029A  mov         qword ptr [rsp+2FFE8h],r8  
00007FF6EC9902A2  lea         r9,[rsp+2FFE0h]  
00007FF6EC9902AA  lea         r10,[rsp+2FFB0h]  
00007FF6EC9902B2  mov         qword ptr [rsp+0FFA8h],rcx  
00007FF6EC9902BA  mov         rcx,r10  
00007FF6EC9902BD  mov         rdx,rax  
00007FF6EC9902C0  mov         r8d,2  
00007FF6EC9902C6  mov         qword ptr [rsp+20h],1  
00007FF6EC9902CF  mov         rax,qword ptr [rsp+0FFA8h]  
00007FF6EC9902D7  mov         qword ptr [rsp+28h],rax  
00007FF6EC9902DC  mov         qword ptr [rsp+30h],1  
00007FF6EC9902E5  call        core::fmt::Arguments::new_v1_formatted (07FF6EC9904B0h)  
00007FF6EC9902EA  lea         rcx,[rsp+2FFB0h]  
00007FF6EC9902F2  call        std::io::stdio::_print (07FF6EC995440h)  
}
00007FF6EC9902F7  nop  
00007FF6EC9902F8  lea         rsp,[rbp+4FF70h]  
00007FF6EC9902FF  pop         rbp  
00007FF6EC990300  ret  

At the end of the day however, look at the lea rax,[X (07FF6EC9B2000h)] and you'll see that the static is also not aligned correctly on pc-windows-msvc.

@retep998 retep998 added the O-windows-msvc Toolchain: MSVC, Operating system: Windows label Mar 19, 2020
@Amanieu
Copy link
Member Author

Amanieu commented Mar 19, 2020

@retep998 Can you open a separate issue for the stack probing? In the meantime, try running the test program with the local and Box lines commented out to see if just the static is aligned properly with -Clink-arg="/ALIGN:1048576". We should see if the Windows loader actually respects the alignment value in the executable.

@retep998
Copy link
Member

@Amanieu It outputs 0x7ff7cf4e2000 so the static is not aligned for pc-windows-msvc either.

@Amanieu
Copy link
Member Author

Amanieu commented Mar 19, 2020

I can see two options here:

  • Ban statics with alignment greater than the page size.
  • Ban all alignments greater than the page size on #[repr(align)].

@retep998
Copy link
Member

retep998 commented Mar 19, 2020

Interestingly, it appears that what is actually happening is rust is specifying the section alignment as 8192. If I just pass /ALIGN:8192 it is enough to hide the linker error, so it's not even a Windows loader issue, it's literally rust telling the linker it only need 8192 as the alignment, and not the actual specified alignment. So there's definitely a bug in Rust and it's possible that once that bug is fixed it won't actually be an issue on some or all platforms.

Notice how in every example output so far, it is aligned to at least 8192. There is no output that is aligned to only 4096, although that might just be coincidence.

@Amanieu
Copy link
Member Author

Amanieu commented Mar 19, 2020

It seems that the PE format can't actually handle sections with an alignment of more than 8192 (search for IMAGE_SCN_ALIGN_8192BYTES).

@retep998
Copy link
Member

retep998 commented Mar 19, 2020

Oh, well in that case I suppose everything is terrible. Statics aligned to over a page should definitely just be forbidden then. Also any sort of automatic promotion of a local or constant to a static. I don't think we need to ban those large alignments in general though, although there's very rarely a reason to use them. Perhaps feature gate all alignments over a page, especially given there are other bugs with them like #70143 ?

@hanna-kruppe
Copy link
Contributor

If we can get away with it (crater run), I would prefer to ban/feature-gate alignment > 4096 entirely since it's a simpler rule than a list of places where types with huge alignment can't be used. I don't expect there are use cases for them that avoid the specific restrictions you mentioned. If you require larger alignment for a heap allocation, you can allocate it yourself even on stable. That's probably a good idea anyway while "guaranteed placement" is lacking.

PS: I specifically say "alignment > 4096" because page size can theoretically vary by target but in practice most systems have a 4KiB minimum page size.

@cramertj
Copy link
Member

cramertj commented Mar 20, 2020

It seems reasonably likely to me that someone at some point will want to use a value aligned to >4KiB targeting a specific platform where they know the exact page size. Banning larger alignments for now is certainly forward-compatible with that, but it might not be a bad idea to think ahead as to how we want to handle that case.

@Amanieu
Copy link
Member Author

Amanieu commented Mar 20, 2020

We could define the max alignment in the target spec. However we would need to move the check later on in the pipeline since AFAIK the parser doesn't have access to the target spec.

@Elinvynia Elinvynia added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 9, 2020
@spastorino spastorino added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jun 17, 2020
@wesleywiser
Copy link
Member

Assigning to @pnkfelix per the "I-unsound issues" 2022 initiative.

@pnkfelix
Copy link
Member

visiting for P-high review.

The ideas posted here are still good ones. I should maybe make this a topic for a screencast investigation.

@SamB
Copy link

SamB commented Jul 15, 2023

I am not sure about other platforms, but in the case of Linux this happens because the ELF loader in the kernel ignores the p_align field of the ELF program headers and assumes page-alignment instead.

Really? The logic for ET_DYN executables with INTERPRETER (PIE binaries) seems to use it, and ET_EXEC executables (non-PIE) use MAP_FIXED_NOREPLACE with a bias of zero (which is already aligned), so as long as the linker has set vaddr correctly, these should all be loaded with correct alignment. (But I could be missing something.)

The only case that seems to actually violate p_align is ET_DYN without INTERPRETER, i.e. dynamic linkers. (Are you writing one of those?)

@Amanieu
Copy link
Member Author

Amanieu commented Jul 15, 2023

This is actually a relatively recent change. Prior to kernel v5.10 the ELF program header alignment was ignored. The relevant change in the glibc dynamic linker was made even more recently. And p_align is still ignored in the latest version of musl's dynamic linker.

byeongkeunahn added a commit to byeongkeunahn/basm-rs that referenced this issue Aug 2, 2023
Even /ALIGN:4096 will still lead to a crash in Debug mode.
The following issues might be relevant:
rust-lang/rust#70022
rust-lang/rust#70143
@jieyouxu jieyouxu added A-align Area: alignment control (`repr(align(N))` and so on) A-target-specs Area: Compile-target specifications labels Nov 22, 2024
@jieyouxu
Copy link
Member

jieyouxu commented Nov 22, 2024

Visited during T-compiler P-high review: having target-spec specific max alignment and adjusting the check makes sense, will need investigation if the limits are target-specific (e.g. does this affect macOS?).

Also cc #70143.

@jieyouxu jieyouxu added the E-needs-investigation Call for partcipation: This issues needs some investigation to determine current status label Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-align Area: alignment control (`repr(align(N))` and so on) A-target-specs Area: Compile-target specifications C-bug Category: This is a bug. E-needs-investigation Call for partcipation: This issues needs some investigation to determine current status I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness O-linux Operating system: Linux O-windows-gnu Toolchain: GNU, Operating system: Windows O-windows-msvc Toolchain: MSVC, Operating system: Windows P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests