Skip to content

Commit 7b050b3

Browse files
neuschaeferdpgeorge
authored andcommitted
py/nlrthumb: Make non-Thumb2 long-jump workaround opt-in.
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 <[email protected]> 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 <[email protected]>
1 parent 49af8ca commit 7b050b3

File tree

2 files changed

+15
-1
lines changed

2 files changed

+15
-1
lines changed

py/mpconfig.h

+6
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,12 @@
587587
/*****************************************************************************/
588588
/* Python internal features */
589589

590+
// Use a special long jump in nlrthumb.c, which may be necessary if nlr.o and
591+
// nlrthumb.o are linked far apart from each other.
592+
#ifndef MICROPY_NLR_THUMB_USE_LONG_JUMP
593+
#define MICROPY_NLR_THUMB_USE_LONG_JUMP (0)
594+
#endif
595+
590596
// Whether to enable import of external modules
591597
// When disabled, only importing of built-in modules is supported
592598
// When enabled, a port must implement mp_import_stat (among other things)

py/nlrthumb.c

+9-1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,14 @@
3838

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

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

74-
#if !defined(__thumb2__)
82+
#if MICROPY_NLR_THUMB_USE_LONG_JUMP
7583
"ldr r1, nlr_push_tail_var \n"
7684
"bx r1 \n" // do the rest in C
7785
".align 2 \n"

0 commit comments

Comments
 (0)