Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
compiler-rt/lib/hwasan/hwasan_setjmp_x86_64.S | ||
---|---|---|
37 | We should insert endbr64 for CET enable program. It has no effect for non-CET program. | |
49–54 | Why not use JB_xxx which defined in the compiler-rt ? That is more readable // Save callee save registers. movq %rbx, (JB_RBX*8)(%rdi) movq %rbp, (JB_RBP*8)(%rdi) movq %r12, (JB_R12*8)(%rdi) movq %r13, (JB_R13*8)(%rdi) movq %r14, (JB_R14*8)(%rdi) movq %r15, (JB_R15*8)(%rdi) | |
58 | here too | |
62 | here too | |
64 | we 'd better consider Shadow Stack for CET // Check if Shadow Stack is enabled. testl $X86_FEATURE_1_SHSTK, %fs:FEATURE_1_OFFSET jz .L_skip_ssp // Get the current Shadow-Stack-Pointer and save it. rdsspq %rax movq %rax, SHADOW_STACK_POINTER_OFFSET(%rdi) // Make a tail call to __sigjmp_save; it takes the same args. .L_skip_ssp: jmp __sigjmp_save |
compiler-rt/lib/hwasan/hwasan_interceptors.cpp | ||
---|---|---|
68 | Here should be no space |
compiler-rt/lib/hwasan/hwasan_interceptors.cpp | ||
---|---|---|
68 | This spaces are inserted by clang-format. Looks funny here, but not sure I want to override clang-format on every change. | |
compiler-rt/lib/hwasan/hwasan_setjmp_x86_64.S | ||
49–54 | I considered doing this, but it seems inline assembly can't use those macros, so we don't get to use them in our longjmp implementation. So rather than using macros in setjmp and constants in longjmp, I just used constants everywhere. FWIW, the aarch64 implementation doesn't use macros either. | |
64 | Is there a way I can test CET in QEMU? I don't want to complicate things without a way to verify it works. |
LGTM modulo the CET discussion
compiler-rt/lib/hwasan/hwasan_interceptors.cpp | ||
---|---|---|
68 | +1 we do what the computer tells us to do for code formatting |
compiler-rt/lib/hwasan/hwasan_setjmp_x86_64.S | ||
---|---|---|
64 | Not sure it can be tested in QEMU, I didn't do that test. but current standard setjmp/longjmp has support it in glibc. The C/C++ source code can be compiled with CET option, but asm code can not, we'd better consider it. Or, at least, add a "TODO" here, if you worry about the testing. |
compiler-rt/lib/hwasan/hwasan_setjmp_x86_64.S | ||
---|---|---|
64 | TODO is already present on line 29. |
compiler-rt/lib/hwasan/hwasan_setjmp_x86_64.S | ||
---|---|---|
64 | Other assembly for x86_64 will also need to be adapted for CET (e.g., vfork interceptor). I propose we wait until we can test properly and then update everything together. |
Hi @morehouse,
I see warnings like this with this patch:
23:54:36 [297/1283] Building ASM object compiler-rt/lib/hwasan/CMakeFiles/RTHwasan_dynamic.x86_64.dir/hwasan_setjmp_x86_64.S.o 23:54:36 <instantiation>:3:3: warning: __sigsetjmp changed binding to STB_WEAK 23:54:36 .weak __sigsetjmp 23:54:36 ^ 23:54:36 <instantiation>:3:3: warning: _setjmp changed binding to STB_WEAK 23:54:36 .weak _setjmp 23:54:36 ^
I use gcc 9.3.0 with glibc 2.17 and linux 3.10.
Is there anything that should/could be done about the warnings?
I'm unable to reproduce locally, but it looks like we've also had this warning on the aarch64 buildbots for a while.
Does https://reviews.llvm.org/D110178 silence the warning for you?
Here should be no space