riscv-musl icon indicating copy to clipboard operation
riscv-musl copied to clipboard

sizeof(jmp_buf) possibly wrong

Open sorear opened this issue 6 years ago • 6 comments

jmp_buf is declared as an array of 28 register-sized values:

https://github.com/riscv/riscv-musl/blob/staging/arch/riscv64/bits/setjmp.h#L1

but we are actually saving 12 + 12 + 2 = 26 registers:

https://github.com/riscv/riscv-musl/blob/staging/src/setjmp/riscv64/setjmp.S#L10-L38

Moreover, fs3 is being saved twice. riscv32 has the same issue with fs3 and, once that is corrected, also allocates 8 bytes more than the setjmp implementation actually seems to use.

@ddevault this will be an ABI issue and should block upstreaming

sorear avatar Feb 17 '19 00:02 sorear

28 is correct, since sigjmp_buf is the same as jmp_buf, yet needs the extra 2 words. Similarly 40 for riscv32 (12 xlen + 12 flen + 2 xlen + extra 2 xlen for sigsetjmp). However the repeated save of fs3 is indeed a problem, and has a knock-on effect in sigsetjmp which writes 1 word past the end of the buffer.

jrtc27 avatar Feb 17 '19 01:02 jrtc27

My mistake earlier; the definition I pointed to was of __jmp_buf which is not the same as jmp_buf, and the global definition of jmp_buf adds enough space for the save.

The current jmp_buf (the actual, user-exposed type) is 360 bytes on riscv64 musl and 344 bytes on riscv64 glibc; Rich on IRC suggested these should match.


Looking in more detail at what aarch64 does:

  1. __jmp_buf is declared to hold 22 words = 176 bytes. jmp_buf is declared as a __jmp_buf followed by 17 additional words in __fl and __ss

  2. setjmp writes to, and longjmp reads from, the first 22 words of its argument only: the __jmp_buf portion. Word 12 is unused, apparently for alignment. Words 22 and above are untouched.

  3. sigsetjmp replaces the return address with a fixup routine and uses the saved x19 slot to hold the address of the jmp_buf object itself (preventing application code from moving jmp_buf objects; neither C11 or POSIX has a clear statement on the correctness of this). The application return address and x19 are stored in words 22 (__fl) and 24 (__ss[1]) respectively; 23 (__ss[0]) receives the saved signal mask.

  4. The interpretation of the initial 22 words is exactly the same between glibc and musl. This is not a documented requirement in the aarch64 psABI nor AAPCS64, it is unclear if another specification requires it. In contrast, riscv64 musl and glibc store registers in a completely different order. The usage of words 22 and above for signal masks is not compatible.

sorear avatar Feb 17 '19 06:02 sorear

Just noticed that the orderings on all the barriers are wrong, will address later

sorear avatar Feb 17 '19 07:02 sorear

Proposed changes for the above are in #7 ; I'll make a separate PR to fix the barriers

sorear avatar Feb 17 '19 17:02 sorear

It's probably off-topic here, but re: "moving" jmp_buf objects, the specification is "The longjmp function restores the environment saved by the most recent invocation of the setjmp macro in the same invocation of the program with the corresponding jmp_buf argument". A different object containing the same representation is not the same (corresponding to the setjmp call) jmp_buf that was used with setjmp.

richfelker avatar Mar 07 '19 16:03 richfelker

I apologize for bumping a stale topic. Yet I'm wondering how is the status of this issue on RV32 port ?

I resized jmp_buf[] in arch/riscv32/bits/setjmp.h so as to get libc-test's functional/setjmp work.

Orginally, __jmp_buf is set to 38 in arch/riscv32/bits/setjmp.h, which is 152 bytes. Yet in src/signal/riscv32/sigsetjmp.s, we're saving/loading s0 at 160(a0) and that is obviously accessing outside of __jb in include/setjmp.h .

Ruinland-ChuanTzu-Tsai avatar Dec 03 '19 04:12 Ruinland-ChuanTzu-Tsai