Re: [patch] Re: longjmp handling vs. glibc LD_POINTER_GUARD problems

Pedro Alves
2008-05-21T22:01:52Z

[-- Attachment #1: Type: text/plain, Size: 4027 bytes --]

A Wednesday 21 May 2008 20:38:02, Pedro Alves wrote:
> A Wednesday 21 May 2008 20:20:19, Ulrich Weigand wrote:
>
> > Another issue with your patch is the use of frame_id_inner ... I'd rather
> > get rid of this function instead of adding new uses, because this really
> > requires that it is possible to compare two stack (frame) addresses
> > along a linear order. This breaks for me in multi-architecture
> > scenarios, but even on existing targets it may not always work OK (e.g.
> > if signal handlers run on a different frame, or if the code uses some
> > sort of user-level threading or coroutine library ...). Maybe instead of
> > comparing frame_ids, it would be better to check whether or not a frame
> > with the given ID still exists in the current backtrace?

> OK. That may work too. I'll give it a try.

Hmmm, I got blocked getting that to work. It always stops stepping
before getting to the other side. It looks like going out of
longjmp, the frame id of the longjmp's caller starts
being computed differently, or badly...

Using the longjmp.c testcase, on x86_64:

(gdb) n
99 longjmp (env, 1); /* patt1 */
(gdb) n

(about thirty or so single-steps go ok, but then it stops,)

0x00007f0480e4af33 in ?? () from /lib/libc.so.6
(gdb) bt
#0 0x00007f413d0dff33 in ?? () from /lib/libc.so.6
#1 0x000000000040067d in main ()
at ../../../src/gdb/testsuite/gdb.base/longjmp.c:96

(gdb) disassemble ($pc-20) ($pc + 28)
Dump of assembler code from 0x7f26b04fbf1f to 0x7f26b04fbf4f:
0x00007f26b04fbf1f: xor 0x30,%ecx
0x00007f26b04fbf26: ror $0x11,%rdx << look, pointer unmangling!
0x00007f26b04fbf2a: xor %fs:0x30,%rdx << " "
0x00007f26b04fbf33: mov (%rdi),%rbx << here, things go bad...
0x00007f26b04fbf36: mov 0x10(%rdi),%r12
0x00007f26b04fbf3a: mov 0x18(%rdi),%r13
0x00007f26b04fbf3e: mov 0x20(%rdi),%r14
0x00007f26b04fbf42: mov 0x28(%rdi),%r15
0x00007f26b04fbf46: mov %esi,%eax
0x00007f26b04fbf48: mov %r8,%rsp
0x00007f26b04fbf4b: mov %r9,%rbp
0x00007f26b04fbf4e: jmpq *%rdx

(gdb) i
(top-gdb) c
Continuing.
(gdb) up
#1 0x000000000040067d in main ()
at ../../../src/gdb/testsuite/gdb.base/longjmp.c:96
96 if (setjmp (env) == 0)
(gdb) i
(top-gdb) p /x get_frame_id (selected_frame)
$3 = {stack_addr = 0xe706f6f457b7fe24, code_addr = 0x400664, special_addr =
0x0, stack_addr_p = 0x1,
code_addr_p = 0x1, special_addr_p = 0x0}

When the longjmp breakpoint was hit it looked like:
Breakpoint 3, handle_inferior_event (ecs=0x7fffcdfb48f0)
at ../../src/gdb/infrun.c:2605
2605 ecs->stepping_through_longjmp = 1;
(top-gdb) p /x ecs->longjmp_caller_frame
$1 = {stack_addr = 0x7fff087e7b20, code_addr = 0x400664, special_addr = 0x0,
stack_addr_p = 0x1, code_addr_p = 0x1,
special_addr_p = 0x0}

Notice that stack_addr is now different of the original 0x7fff087e7b20,
although, the $sp is unwinds ok,
(top-gdb) c
Continuing.
(gdb) p $sp
$1 = (void *) 0x7fff087e7b00

x86 is even worse:

99 longjmp (env, 1); /* patt1 */
(gdb) n
0xf7da41c3 in ?? () from /lib32/libc.so.6
(gdb) bt
#0 0xf7da41c3 in ?? () from /lib32/libc.so.6
#1 0xffb327a8 in ?? ()
#2 0xf7da413c in siglongjmp () from /lib32/libc.so.6
Backtrace stopped: frame did not save the PC
(gdb) disassemble ($pc-10) ($pc+20)
Dump of assembler code from 0xf7da41b9 to 0xf7da41d7:
0xf7da41b9: nop
0xf7da41ba: nop
0xf7da41bb: nop
0xf7da41bc: nop
0xf7da41bd: nop
0xf7da41be: nop
0xf7da41bf: nop
0xf7da41c0: push %ebp
0xf7da41c1: mov %esp,%ebp
0xf7da41c3: push %ebx <<< fails here, just while
<<< setting up a new frame...
0xf7da41c4: call 0xf7d8f290 <_Unwind_Find_FDE@plt+120>
0xf7da41c9: add $0x11fe2b,%ebx
0xf7da41cf: sub $0x8,%esp
0xf7da41d2: mov 0x340c(%ebx),%eax
End of assembler dump.
(gdb)

Any ideas?

--
Pedro Alves

[-- Attachment #2: longjmp.diff --]
[-- Type: text/x-diff, Size: 11535 bytes --]

2008-05-21 Pedro Alves <pedro@codesourcery.com>

* infrun.c (struct execution_control_state): Add
stepping_through_longjmp and longjmp_caller_frame.
(init_execution_control_state): Clear them.
(context_switch): Context switch them.
(handle_inferior_event): If stepping through longjmp, a SIGTRAP is
not random. When a longjmp breakpoint is hit, prepare to step
until the other end. Keep stepping while in an inner frame
relative to the longjmp caller. When coming out on the other end,
check if the step-resume breakpoint frame is not needed anymore.
(currently_stepping): Return true if stepping through longjmp.
* breakpoint.c (set_longjmp_breakpoint): Always set longjmp
breakpoints.

* gdbthread.h (struct thread_info): Add stepping_through_longjmp
and longjmp_caller_frame.
(save_infrun_state, load_infrun_state): Context switch them.
* thread.c (save_infrun_state, load_infrun_state): Ditto.

---
gdb/breakpoint.c | 13 ++----
gdb/gdbthread.h | 14 ++++++-
gdb/infrun.c | 106 +++++++++++++++++++++++++++++++++++++++++--------------
gdb/thread.c | 12 +++++-
4 files changed, 106 insertions(+), 39 deletions(-)

Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c 2008-05-21 20:27:11.000000000 +0100
+++ src/gdb/infrun.c 2008-05-21 22:31:15.000000000 +0100
@@ -1386,6 +1386,13 @@ struct execution_control_state
int step_after_step_resume_breakpoint;
int stepping_through_solib_after_catch;
bpstat stepping_through_solib_catchpoints;
+
+ /* True if a longjmp call was detected while stepping, and we're
+ single-stepping until the other end. */
+ int stepping_through_longjmp;
+ /* The frame that called longjmp. */
+ struct frame_id longjmp_caller_frame;
+
int new_thread_event;
struct target_waitstatus tmpstatus;
enum infwait_states infwait_state;
@@ -1556,6 +1563,8 @@ init_execution_control_state (struct exe
ecs->infwait_state = infwait_normal_state;
ecs->waiton_ptid = pid_to_ptid (-1);
ecs->wp = &(ecs->ws);
+ ecs->stepping_through_longjmp = 0;
+ ecs->longjmp_caller_frame = null_frame_id;
}

/* Return the cached copy of the last pid/waitstatus returned by
@@ -1605,7 +1614,9 @@ context_switch (struct execution_control
ecs->stepping_over_breakpoint,
ecs->stepping_through_solib_after_catch,
ecs->stepping_through_solib_catchpoints,
- ecs->current_line, ecs->current_symtab);
+ ecs->current_line, ecs->current_symtab,
+ ecs->stepping_through_longjmp,
+ ecs->longjmp_caller_frame);

/* Load infrun state for the new thread. */
load_infrun_state (ecs->ptid, &prev_pc,
@@ -1615,7 +1626,9 @@ context_switch (struct execution_control
&ecs->stepping_over_breakpoint,
&ecs->stepping_through_solib_after_catch,
&ecs->stepping_through_solib_catchpoints,
- &ecs->current_line, &ecs->current_symtab);
+ &ecs->current_line, &ecs->current_symtab,
+ &ecs->stepping_through_longjmp,
+ &ecs->longjmp_caller_frame);
}

switch_to_thread (ecs->ptid);
@@ -2449,7 +2462,8 @@ handle_inferior_event (struct execution_
ecs->random_signal
= !(bpstat_explains_signal (stop_bpstat)
|| stepping_over_breakpoint
- || (step_range_end && step_resume_breakpoint == NULL));
+ || (step_range_end && step_resume_breakpoint == NULL)
+ || ecs->stepping_through_longjmp);
else
{
ecs->random_signal = !bpstat_explains_signal (stop_bpstat);
@@ -2562,7 +2576,6 @@ process_event_stop_test:

/* Handle cases caused by hitting a breakpoint. */
{
- CORE_ADDR jmp_buf_pc;
struct bpstat_what what;

what = bpstat_what (stop_bpstat);
@@ -2575,35 +2588,21 @@ process_event_stop_test:
switch (what.main_action)
{
case BPSTAT_WHAT_SET_LONGJMP_RESUME:
- /* If we hit the breakpoint at longjmp while stepping, we
- install a momentary breakpoint at the target of the
- jmp_buf. */
-
+ /* If we hit the breakpoint at longjmp while stepping, prepare
+ to step all the way through it. */
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog,
"infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME\n");

+ /* Step over this longjmp breakpoint. */
ecs->stepping_over_breakpoint = 1;

- if (!gdbarch_get_longjmp_target_p (current_gdbarch)
- || !gdbarch_get_longjmp_target (current_gdbarch,
- get_current_frame (), &jmp_buf_pc))
- {
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog, "\
-infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (!gdbarch_get_longjmp_target)\n");
- keep_going (ecs);
- return;
- }
-
- /* We're going to replace the current step-resume breakpoint
- with a longjmp-resume breakpoint. */
- if (step_resume_breakpoint != NULL)
- delete_step_resume_breakpoint (&step_resume_breakpoint);
-
- /* Insert a breakpoint at resume address. */
- insert_longjmp_resume_breakpoint (jmp_buf_pc);
+ /* Store the frame id of the caller. We'll single-step until
+ we detect an outer frame becoming current. */
+ ecs->longjmp_caller_frame = frame_unwind_id (get_current_frame ());

+ /* See you on the other side! */
+ ecs->stepping_through_longjmp = 1;
keep_going (ecs);
return;

@@ -2819,6 +2818,60 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (
return;
}

+ if (ecs->stepping_through_longjmp)
+ {
+ struct frame_info *frame = get_current_frame ();
+ struct frame_id frame_id = get_frame_id (frame);
+ struct gdbarch *gdbarch = get_frame_arch (frame);
+
+ /* If the current frame is the longjmp caller's, then we're
+ done. We're also done if the longjmp caller's frame is not
+ found on the frame stack. */
+ if (!frame_id_eq (frame_id, ecs->longjmp_caller_frame)
+ && frame_find_by_id (ecs->longjmp_caller_frame) != NULL)
+ {
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog,
+ "infrun: stepping through longjmp\n");
+#if 0
+ /* REMOVE ME: For debugging purposes only. */
+ print_stack_frame (frame, 0, SRC_AND_LOC);
+#endif
+ /* Still not there. */
+ keep_going (ecs);
+ return;
+ }
+
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog, "infrun: got out of longjmp\n");
+
+ /* We made it. */
+ ecs->stepping_through_longjmp = 0;
+
+ /* If there's a step-resume breakpoint set, decide if we should
+ keep stepping to the step-resume breakpoint, or if the
+ longjmp took us outermost already, hence the step-resume
+ breakpoint will never be hit, and we should stop now. */
+ if (step_resume_breakpoint)
+ {
+ if (!frame_id_eq (frame_id, step_resume_breakpoint->frame_id)
+ && frame_find_by_id (step_resume_breakpoint->frame_id) != NULL)
+ {
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog,
+ "\
+infrun: step-resume frame found on the stack\n");
+ }
+ else
+ {
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog,
+ "infrun: step-resume overran by longjmp\n");
+ delete_step_resume_breakpoint (&step_resume_breakpoint);
+ }
+ }
+ }
+
if (step_resume_breakpoint)
{
if (debug_infrun)
@@ -3183,6 +3236,7 @@ currently_stepping (struct execution_con
{
return (((step_range_end && step_resume_breakpoint == NULL)
|| stepping_over_breakpoint)
+ || ecs->stepping_through_longjmp
|| ecs->stepping_through_solib_after_catch
|| bpstat_should_step ());
}
Index: src/gdb/gdbthread.h
===================================================================
--- src.orig/gdb/gdbthread.h 2008-05-21 20:27:11.000000000 +0100
+++ src/gdb/gdbthread.h 2008-05-21 20:42:06.000000000 +0100
@@ -63,6 +63,12 @@ struct thread_info
when we finally do stop stepping. */
bpstat stepping_through_solib_catchpoints;

+ /* True if a longjmp call was detected while stepping, and we're
+ single-stepping until the other end. */
+ int stepping_through_longjmp;
+ /* The frame that called longjmp. */
+ struct frame_id longjmp_caller_frame;
+
/* Private data used by the target vector implementation. */
struct private_thread_info *private;
};
@@ -126,7 +132,9 @@ extern void save_infrun_state (ptid_t pt
int stepping_through_solib_after_catch,
bpstat stepping_through_solib_catchpoints,
int current_line,
- struct symtab *current_symtab);
+ struct symtab *current_symtab,
+ int stepping_through_longjmp,
+ struct frame_id longjmp_caller_frame);

/* infrun context switch: load the debugger state previously saved
for the given thread. */
@@ -141,7 +149,9 @@ extern void load_infrun_state (ptid_t pt
int *stepping_through_solib_affter_catch,
bpstat *stepping_through_solib_catchpoints,
int *current_line,
- struct symtab **current_symtab);
+ struct symtab **current_symtab,
+ int *stepping_through_longjmp,
+ struct frame_id *longjmp_caller_frame);

/* Switch from one thread to another. */
extern void switch_to_thread (ptid_t ptid);
Index: src/gdb/thread.c
===================================================================
--- src.orig/gdb/thread.c 2008-05-21 20:27:11.000000000 +0100
+++ src/gdb/thread.c 2008-05-21 20:42:06.000000000 +0100
@@ -325,7 +325,9 @@ load_infrun_state (ptid_t ptid,
int *stepping_through_solib_after_catch,
bpstat *stepping_through_solib_catchpoints,
int *current_line,
- struct symtab **current_symtab)
+ struct symtab **current_symtab,
+ int *stepping_through_longjmp,
+ struct frame_id *longjmp_caller_frame)
{
struct thread_info *tp;

@@ -348,6 +350,8 @@ load_infrun_state (ptid_t ptid,
tp->stepping_through_solib_catchpoints;
*current_line = tp->current_line;
*current_symtab = tp->current_symtab;
+ *stepping_through_longjmp = tp->stepping_through_longjmp;
+ *longjmp_caller_frame = tp->longjmp_caller_frame;
}

/* Save infrun state for the thread PID. */
@@ -364,7 +368,9 @@ save_infrun_state (ptid_t ptid,
int stepping_through_solib_after_catch,
bpstat stepping_through_solib_catchpoints,
int current_line,
- struct symtab *current_symtab)
+ struct symtab *current_symtab,
+ int stepping_through_longjmp,
+ struct frame_id longjmp_caller_frame)
{
struct thread_info *tp;

@@ -385,6 +391,8 @@ save_infrun_state (ptid_t ptid,
tp->stepping_through_solib_catchpoints = stepping_through_solib_catchpoints;
tp->current_line = current_line;
tp->current_symtab = current_symtab;
+ tp->stepping_through_longjmp = stepping_through_longjmp;
+ tp->longjmp_caller_frame = longjmp_caller_frame;
}

/* Return true if TP is an active thread. */
Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c 2008-05-21 20:27:11.000000000 +0100
+++ src/gdb/breakpoint.c 2008-05-21 20:42:06.000000000 +0100
@@ -4470,15 +4470,10 @@ create_longjmp_breakpoint (char *func_na
void
set_longjmp_breakpoint (void)
{
- struct breakpoint *b;
-
- if (gdbarch_get_longjmp_target_p (current_gdbarch))
- {
- create_longjmp_breakpoint ("longjmp");
- create_longjmp_breakpoint ("_longjmp");
- create_longjmp_breakpoint ("siglongjmp");
- create_longjmp_breakpoint ("_siglongjmp");
- }
+ create_longjmp_breakpoint ("longjmp");
+ create_longjmp_breakpoint ("_longjmp");
+ create_longjmp_breakpoint ("siglongjmp");
+ create_longjmp_breakpoint ("_siglongjmp");
}

/* Delete all longjmp breakpoints from THREAD. */