Copilot commented on code in PR #3495:
URL: https://github.com/apache/brpc/pull/3495#discussion_r3868195217
##########
src/bthread/context.cpp:
##########
@@ -347,20 +347,40 @@ __asm (
" pushq %r14 \n"
" pushq %r13 \n"
" pushq %r12 \n"
-" leaq -0x8(%rsp), %rsp\n"
+" leaq -0xa8(%rsp), %rsp\n"
+" movups %xmm6, 0x00(%rsp)\n"
+" movups %xmm7, 0x10(%rsp)\n"
+" movups %xmm8, 0x20(%rsp)\n"
+" movups %xmm9, 0x30(%rsp)\n"
Review Comment:
This change introduces new correctness-critical behavior (saving/restoring
XMM6–XMM15) but there’s no unit test asserting SIMD register preservation
across bthread_jump_fcontext. Consider extending test/bthread_unittest.cpp
(which already exercises make/jump fcontext) with a small test that sets known
patterns in XMM6–XMM15, context-switches, and verifies the values round-trip.
##########
src/bthread/task_tracer.cpp:
##########
@@ -280,16 +280,16 @@ unw_cursor_t TaskTracer::MakeCursor(bthread_fcontext_t
fcontext) {
// Only need RBP, RIP, RSP on x86_64.
// The base pointer (RBP).
- if (unw_set_reg(&cursor, UNW_X86_64_RBP, regs[6]) != 0) {
+ if (unw_set_reg(&cursor, UNW_X86_64_RBP, regs[26]) != 0) {
LOG(ERROR) << "Fail to set RBP";
}
// The instruction pointer (RIP).
- if (unw_set_reg(&cursor, UNW_REG_IP, regs[7]) != 0) {
+ if (unw_set_reg(&cursor, UNW_REG_IP, regs[27]) != 0) {
Review Comment:
The new context-frame layout change makes these hard-coded indices
(26/27/28) very brittle. Please replace the magic numbers with named constants
and add a brief note tying them to the linux_x86_64 frame layout in
src/bthread/context.cpp, so future frame adjustments don’t silently break stack
tracing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
