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

Upgrade to use snmalloc for rust compiler #16

Open
wants to merge 12 commits into
base: nightly-2024-04-02-snmalloc
Choose a base branch
from

Conversation

Taowyoo
Copy link

@Taowyoo Taowyoo commented Jun 20, 2024

  • Changing to the ABI
  • Remove old dlmalloc code
  • Add and use snmalloc
  • Add a help script for building custom toolchain.

Note:

@Taowyoo Taowyoo changed the title Upgrade to use snmalloc for rustt compiler Upgrade to use snmalloc for rust compiler Jun 20, 2024
library/std/Cargo.toml Outdated Show resolved Hide resolved
library/std/src/sys/pal/sgx/abi/entry.S Show resolved Hide resolved
library/std/src/sys/pal/sgx/abi/entry.S Outdated Show resolved Hide resolved
library/std/src/sys/pal/sgx/abi/mod.rs Outdated Show resolved Hide resolved
library/std/src/sys/pal/sgx/abi/mod.rs Outdated Show resolved Hide resolved
mybuild.sh Outdated Show resolved Hide resolved
library/std/src/sys/pal/sgx/abi/tls/mod.rs Outdated Show resolved Hide resolved
library/std/src/sys/pal/sgx/abi/tls/mod.rs Show resolved Hide resolved
@arai-fortanix arai-fortanix self-requested a review June 21, 2024 14:21
@aditijannu aditijannu force-pushed the yx/update-sgx-alloc branch 2 times, most recently from d7d2a3b to 70f6d47 Compare June 26, 2024 23:12
@arai-fortanix arai-fortanix self-requested a review June 27, 2024 17:38
Copy link

@arai-fortanix arai-fortanix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did we land on using an alternative mechanism for allocating the memory, like having elf2sgxs set aside space in the enclave address space?

library/snmalloc-edp/build.rs Show resolved Hide resolved
library/snmalloc-edp/tests/global_alloc.rs Show resolved Hide resolved
@@ -69,6 +82,9 @@ extern "C" fn entry(p1: u64, p2: u64, p3: u64, secondary: bool, p4: u64, p5: u64
let join_notifier = super::thread::Thread::entry();
drop(tls_guard);
drop(join_notifier);
drop(tls);

alloc_thread_cleanup();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In EDP, is there a way for a thread to exit other than by returning from its entry() function? Do we need this cleanup somewhere else?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@raoulstrackx @jethrogb can you help answer this question

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! There are two other ways:

  • There's the primary thread/TCS that executes main, but when that finishes the entire program ends
  • The current thread panics, thenThread::entry doesn't return but in that case the enclave will mark itself as being aborted and all re-entries will be aborted.

Either cases seem fine to not execute the alloc_thread_cleanup function

library/std/src/sys/pal/sgx/abi/mod.rs Show resolved Hide resolved
library/std/src/sys/pal/sgx/abi/tls/mod.rs Outdated Show resolved Hide resolved
library/std/src/sys/pal/sgx/alloc.rs Show resolved Hide resolved
@aditijannu aditijannu force-pushed the yx/update-sgx-alloc branch from 70f6d47 to e9e1270 Compare July 9, 2024 11:58
@aditijannu
Copy link

Where did we land on using an alternative mechanism for allocating the memory, like having elf2sgxs set aside space in the enclave address space?

@arai-fortanix I haven't had chance to look into that. How important or critical is it? Can we go ahead with what we have now and address the alternative mechanism for allocating memory in the future?

I have also addressed all your comments, could you please take another look at the PR?

@arai-fortanix arai-fortanix self-requested a review July 9, 2024 17:44
@arai-fortanix
Copy link

Changing the allocation scheme for the thread-local allocator data doesn't need to block this change.

raoulstrackx pushed a commit that referenced this pull request Sep 16, 2024
better implementation of signed div_floor/ceil

Tracking issue for signed `div_floor`/`div_ceil`: rust-lang#88581.

This PR improves the implementation of those two functions by adding a better branchless algorithm. Side-by-side comparison of `i32::div_floor` on x86-64:

```asm
div_floor_new:                               div_floor_old:
        push    rax                                  push    rax
        test    esi, esi                             test    esi, esi
        je      .LBB0_3                              je      .LBB1_6
        mov     eax, esi                             mov     eax, esi
        not     eax                                  not     eax
        lea     ecx, [rdi - 2147483648]              lea     ecx, [rdi - 2147483648]
        or      ecx, eax                             or      ecx, eax
        je      .LBB0_2                              je      .LBB1_7
        mov     eax, edi                             mov     eax, edi
        cdq                                          cdq
        idiv    esi                                  idiv    esi
        xor     esi, edi                             test    edx, edx
        sar     esi, 31                              setg    cl
        test    edx, edx                             test    esi, esi
        cmove   esi, edx                             sets    dil
        add     eax, esi                             test    dil, cl
        pop     rcx                                  jne     .LBB1_4
        ret                                          test    edx, edx
.LBB0_3:                                             setns   cl
        lea     rdi, [rip + .L__unnamed_1]           test    esi, esi
        call    qword ptr [rip + panic...]          setle   dl
.LBB0_2:                                             or      dl, cl
        lea     rdi, [rip + .L__unnamed_1]           jne     .LBB1_5
        call    qword ptr [rip + panic...]   .LBB1_4:
                                                     dec     eax
                                             .LBB1_5:
                                                     pop     rcx
                                                     ret
                                             .LBB1_6:
                                                     lea     rdi, [rip + .L__unnamed_2]
                                                     call    qword ptr [rip + panic...]
                                             .LBB1_7:
                                                     lea     rdi, [rip + .L__unnamed_2]
                                                     call    qword ptr [rip + panic...]
```

And on Aarch64:

```asm
_div_floor_new:                                   _div_floor_old:
        stp     x29, x30, [sp, #-16]!                     stp     x29, x30, [sp, #-16]!
        mov     x29, sp                                   mov     x29, sp
        cbz     w1, LBB0_4                                cbz     w1, LBB1_9
        mov     w8, #-2147483648                          mov     x8, x0
        cmp     w0, w8                                    mov     w9, #-2147483648
        b.ne    LBB0_3                                    cmp     w0, w9
        cmn     w1, #1                                    b.ne    LBB1_3
        b.eq    LBB0_5                                    cmn     w1, #1
LBB0_3:                                                   b.eq    LBB1_10
        sdiv    w8, w0, w1                        LBB1_3:
        msub    w9, w8, w1, w0                            sdiv    w0, w8, w1
        eor     w10, w1, w0                               msub    w8, w0, w1, w8
        asr     w10, w10, rust-lang#31                             tbz     w1, rust-lang#31, LBB1_5
        cmp     w9, #0                                    cmp     w8, #0
        csel    w9, wzr, w10, eq                          b.gt    LBB1_7
        add     w0, w9, w8                        LBB1_5:
        ldp     x29, x30, [sp], #16                       cmp     w1, #1
        ret                                               b.lt    LBB1_8
LBB0_4:                                                   tbz     w8, rust-lang#31, LBB1_8
        adrp    x0, l___unnamed_1@PAGE            LBB1_7:
        add     x0, x0, l___unnamed_1@PAGEOFF             sub     w0, w0, #1
        bl      panic...                          LBB1_8:
LBB0_5:                                                   ldp     x29, x30, [sp], #16
        adrp    x0, l___unnamed_1@PAGE                    ret
        add     x0, x0, l___unnamed_1@PAGEOFF     LBB1_9:
        bl      panic...                                  adrp    x0, l___unnamed_2@PAGE
                                                          add     x0, x0, l___unnamed_2@PAGEOFF
                                                          bl      panic...
                                                  LBB1_10:
                                                          adrp    x0, l___unnamed_2@PAGE
                                                          add     x0, x0, l___unnamed_2@PAGEOFF
                                                          bl      panic...
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants