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

sizeof(jmp_buf) possibly wrong #6

Open
sorear opened this issue Feb 17, 2019 · 6 comments
Open

sizeof(jmp_buf) possibly wrong #6

sorear opened this issue Feb 17, 2019 · 6 comments

Comments

@sorear
Copy link
Contributor

sorear commented Feb 17, 2019

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

@jrtc27
Copy link

jrtc27 commented Feb 17, 2019

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.

@sorear
Copy link
Contributor Author

sorear commented Feb 17, 2019

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
Copy link
Contributor Author

sorear commented Feb 17, 2019

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

@sorear
Copy link
Contributor Author

sorear commented Feb 17, 2019

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

@richfelker
Copy link

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.

@Ruinland-ChuanTzu-Tsai
Copy link

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 .

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

No branches or pull requests

4 participants