Skip to content

Commit

Permalink
py/nlrthumb: Make non-Thumb2 long-jump workaround opt-in.
Browse files Browse the repository at this point in the history
Although the original motivation given for the workaround[1] is correct,
nlr.o and nlrthumb.o are linked with a small enough distance that the
problem does not occur, and the workaround isn't necessary. The distance
between the b instruction and its target (nlr_push_tail) is just 64
bytes[2], well within the ±2046 byte range addressable by an
unconditional branch instruction in Thumb mode.

The workaround induces a relocation in the text section (textrel), which
isn't supported everywhere, notably not on musl-libc[3], where it causes
a crash on start-up. With the workaround removed, micropython works on an
ARMv5T Linux system built with musl-libc.

This commit changes nlrthumb.c to use a direct jump by default, but
leaves the long jump workaround as an option for those cases where it's
actually needed.

[1]: commit dd376a2

Author: Damien George <damien.p.george@gmail.com>
Date:   Fri Sep 1 15:25:29 2017 +1000

    py/nlrthumb: Get working again on standard Thumb arch (ie not Thumb2).

    "b" on Thumb might not be long enough for the jump to nlr_push_tail so
    it must be done indirectly.

[2]: Excerpt from objdump -d micropython:

000095c4 <nlr_push_tail>:
    95c4:       b510            push    {r4, lr}
    95c6:       0004            movs    r4, r0
    95c8:       f02d fd42       bl      37050 <mp_thread_get_state>
    95cc:       6943            ldr     r3, [r0, #20]
    95ce:       6023            str     r3, [r4, #0]
    95d0:       6144            str     r4, [r0, #20]
    95d2:       2000            movs    r0, #0
    95d4:       bd10            pop     {r4, pc}

000095d6 <nlr_pop>:
    95d6:       b510            push    {r4, lr}
    95d8:       f02d fd3a       bl      37050 <mp_thread_get_state>
    95dc:       6943            ldr     r3, [r0, #20]
    95de:       681b            ldr     r3, [r3, #0]
    95e0:       6143            str     r3, [r0, #20]
    95e2:       bd10            pop     {r4, pc}

000095e4 <nlr_push>:
    95e4:       60c4            str     r4, [r0, #12]
    95e6:       6105            str     r5, [r0, #16]
    95e8:       6146            str     r6, [r0, #20]
    95ea:       6187            str     r7, [r0, #24]
    95ec:       4641            mov     r1, r8
    95ee:       61c1            str     r1, [r0, #28]
    95f0:       4649            mov     r1, r9
    95f2:       6201            str     r1, [r0, #32]
    95f4:       4651            mov     r1, sl
    95f6:       6241            str     r1, [r0, #36]   @ 0x24
    95f8:       4659            mov     r1, fp
    95fa:       6281            str     r1, [r0, #40]   @ 0x28
    95fc:       4669            mov     r1, sp
    95fe:       62c1            str     r1, [r0, #44]   @ 0x2c
    9600:       4671            mov     r1, lr
    9602:       6081            str     r1, [r0, #8]
    9604:       e7de            b.n     95c4 <nlr_push_tail>

[3]: https://www.openwall.com/lists/musl/2020/09/25/4

Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
  • Loading branch information
neuschaefer authored and dpgeorge committed Apr 25, 2024
1 parent 49af8ca commit 7b050b3
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
6 changes: 6 additions & 0 deletions py/mpconfig.h
Expand Up @@ -587,6 +587,12 @@
/*****************************************************************************/
/* Python internal features */

// Use a special long jump in nlrthumb.c, which may be necessary if nlr.o and
// nlrthumb.o are linked far apart from each other.
#ifndef MICROPY_NLR_THUMB_USE_LONG_JUMP
#define MICROPY_NLR_THUMB_USE_LONG_JUMP (0)
#endif

// Whether to enable import of external modules
// When disabled, only importing of built-in modules is supported
// When enabled, a port must implement mp_import_stat (among other things)
Expand Down
10 changes: 9 additions & 1 deletion py/nlrthumb.c
Expand Up @@ -38,6 +38,14 @@

__attribute__((naked)) unsigned int nlr_push(nlr_buf_t *nlr) {

// If you get a linker error here, indicating that a relocation doesn't
// fit, try the following (in that order):
//
// 1. Ensure that nlr.o nlrthumb.o are linked closely together, i.e.
// there aren't too many other files between them in the linker list
// (PY_CORE_O_BASENAME in py/py.mk)
// 2. Set -DMICROPY_NLR_THUMB_USE_LONG_JUMP=1 during the build
//
__asm volatile (
"str r4, [r0, #12] \n" // store r4 into nlr_buf
"str r5, [r0, #16] \n" // store r5 into nlr_buf
Expand Down Expand Up @@ -71,7 +79,7 @@ __attribute__((naked)) unsigned int nlr_push(nlr_buf_t *nlr) {
"str lr, [r0, #8] \n" // store lr into nlr_buf
#endif

#if !defined(__thumb2__)
#if MICROPY_NLR_THUMB_USE_LONG_JUMP
"ldr r1, nlr_push_tail_var \n"
"bx r1 \n" // do the rest in C
".align 2 \n"
Expand Down

0 comments on commit 7b050b3

Please sign in to comment.