From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cagney To: gdb-patches@sources.redhat.com Subject: Re: [patch/rfc] Use insert_step_resume_breakpoint everywhere Date: Tue, 11 May 2004 23:29:00 -0000 Message-id: <40A161F4.6050801@gnu.org> References: <40A01953.7060906@gnu.org> X-SW-Source: 2004-05/msg00352.html Hello, More sigtramp fallout. This patch eliminates step_over_function, instead using insert_step_resume_breakpoint (get_prev_frame ()). This way we're closer to a single function that manipulates the step-resume global (few cases still need to be deleted). When testing this I found a bug in the asser vis: + /* Remember, if the call instruction is the last in the step range, + the breakpoint will land just beyond that. Hence ``<= + step_range_end''. */ gdb_assert (get_frame_pc (step_frame) >= step_range_start - && get_frame_pc (step_frame) < step_range_end); + && get_frame_pc (step_frame) <= step_range_end); Further testing revealed an additional bug in this assertion (next vs nexti) vis: + /* Remember, if the call instruction is the last in the step range, + the breakpoint will land just beyond that. Hence ``<= + step_range_end''. Also, ignore check when "nexti". */ + gdb_assert (step_range_start == step_range_end + || (get_frame_pc (step_frame) >= step_range_start + && get_frame_pc (step_frame) <= step_range_end)); So I've tweaked that and checked it in. Andrew 2004-05-10 Andrew Cagney * infrun.c (step_over_function): Delete function. (handle_step_into_function): Use insert_step_resume_breakpoint. (insert_step_resume_breakpoint): Fix assertion. 2004-05-11 Andrew Cagney * infrun.c (step_over_function): Delete function. (handle_step_into_function): Use insert_step_resume_breakpoint. (insert_step_resume_breakpoint): Fix assertion. Index: infrun.c =================================================================== RCS file: /cvs/src/src/gdb/infrun.c,v retrieving revision 1.155 diff -p -u -r1.155 infrun.c --- infrun.c 11 May 2004 16:57:04 -0000 1.155 +++ infrun.c 11 May 2004 23:10:52 -0000 @@ -968,7 +968,6 @@ static void handle_step_into_function (s void handle_inferior_event (struct execution_control_state *ecs); static void step_into_function (struct execution_control_state *ecs); -static void step_over_function (struct execution_control_state *ecs); static void insert_step_resume_breakpoint (struct frame_info *step_frame, struct execution_control_state *ecs); static void stop_stepping (struct execution_control_state *ecs); @@ -1204,8 +1203,10 @@ handle_step_into_function (struct execut if (step_over_calls == STEP_OVER_ALL || IGNORE_HELPER_CALL (stop_pc)) { - /* We're doing a "next". */ - step_over_function (ecs); + /* We're doing a "next", set a breakpoint at callee's return + address (the address at which the caller will resume). */ + insert_step_resume_breakpoint (get_prev_frame (get_current_frame ()), + ecs); keep_going (ecs); return; } @@ -1249,7 +1250,9 @@ handle_step_into_function (struct execut return; } - step_over_function (ecs); + /* Set a breakpoint at callee's return address (the address at which + the caller will resume). */ + insert_step_resume_breakpoint (get_prev_frame (get_current_frame ()), ecs); keep_going (ecs); return; } @@ -2662,8 +2665,12 @@ insert_step_resume_breakpoint (struct fr /* This is only used within the step-resume range/frame. */ gdb_assert (frame_id_eq (step_frame_id, get_frame_id (step_frame))); gdb_assert (step_range_end != 0); - gdb_assert (get_frame_pc (step_frame) >= step_range_start - && get_frame_pc (step_frame) < step_range_end); + /* Remember, if the call instruction is the last in the step range, + the breakpoint will land just beyond that. Hence ``<= + step_range_end''. Also, ignore check when "nexti". */ + gdb_assert (step_range_start == step_range_end + || (get_frame_pc (step_frame) >= step_range_start + && get_frame_pc (step_frame) <= step_range_end)); init_sal (&sr_sal); /* initialize to zeros */ @@ -2675,83 +2682,6 @@ insert_step_resume_breakpoint (struct fr step_resume_breakpoint = set_momentary_breakpoint (sr_sal, get_frame_id (step_frame), bp_step_resume); - - if (breakpoints_inserted) - insert_breakpoints (); -} - -/* We've just entered a callee, and we wish to resume until it returns - to the caller. Setting a step_resume breakpoint on the return - address will catch a return from the callee. - - However, if the callee is recursing, we want to be careful not to - catch returns of those recursive calls, but only of THIS instance - of the caller. - - To do this, we set the step_resume bp's frame to our current - caller's frame (obtained by doing a frame ID unwind). */ - -static void -step_over_function (struct execution_control_state *ecs) -{ - struct symtab_and_line sr_sal; - struct frame_id sr_id; - - init_sal (&sr_sal); /* initialize to zeros */ - - /* NOTE: cagney/2003-04-06: - - At this point the equality get_frame_pc() == get_frame_func() - should hold. This may make it possible for this code to tell the - frame where it's function is, instead of the reverse. This would - avoid the need to search for the frame's function, which can get - very messy when there is no debug info available (look at the - heuristic find pc start code found in targets like the MIPS). */ - - /* NOTE: cagney/2003-04-06: - - The intent of DEPRECATED_SAVED_PC_AFTER_CALL was to: - - - provide a very light weight equivalent to frame_unwind_pc() - (nee FRAME_SAVED_PC) that avoids the prologue analyzer - - - avoid handling the case where the PC hasn't been saved in the - prologue analyzer - - Unfortunately, not five lines further down, is a call to - get_frame_id() and that is guarenteed to trigger the prologue - analyzer. - - The `correct fix' is for the prologe analyzer to handle the case - where the prologue is incomplete (PC in prologue) and, - consequently, the return pc has not yet been saved. It should be - noted that the prologue analyzer needs to handle this case - anyway: frameless leaf functions that don't save the return PC; - single stepping through a prologue. - - The d10v handles all this by bailing out of the prologue analsis - when it reaches the current instruction. */ - - if (DEPRECATED_SAVED_PC_AFTER_CALL_P ()) - sr_sal.pc = ADDR_BITS_REMOVE (DEPRECATED_SAVED_PC_AFTER_CALL (get_current_frame ())); - else - sr_sal.pc = ADDR_BITS_REMOVE (frame_pc_unwind (get_current_frame ())); - sr_sal.section = find_pc_overlay (sr_sal.pc); - - check_for_old_step_resume_breakpoint (); - - /* NOTE: cagney/2004-03-31: Code using the current value of - "step_frame_id", instead of unwinding that frame ID, removed. On - s390 GNU/Linux, after taking a signal, the program is directly - resumed at the signal handler and, consequently, the PC would - point at at the first instruction of that signal handler but - STEP_FRAME_ID would [incorrectly] at the interrupted code when it - should point at the signal trampoline. By always and locally - doing a frame ID unwind, it's possible to assert that the code is - always using the correct ID. */ - sr_id = frame_unwind_id (get_current_frame ()); - - step_resume_breakpoint = set_momentary_breakpoint (sr_sal, sr_id, bp_step_resume); if (breakpoints_inserted) insert_breakpoints (); >From cagney@gnu.org Tue May 11 23:34:00 2004 From: Andrew Cagney To: Nick Roberts Cc: gdb-patches@sources.redhat.com Subject: Re: [PATCH] Pending breakpoints Date: Tue, 11 May 2004 23:34:00 -0000 Message-id: <40A162F5.8070601@gnu.org> References: <16543.59280.786760.919884@nick.uklinux.net> <40A0F1C0.6090205@gnu.org> <16545.10303.526588.598463@nick.uklinux.net> <40A15558.8090803@gnu.org> <16545.24079.489553.335457@nick.uklinux.net> X-SW-Source: 2004-05/msg00353.html Content-length: 496 > > > Also ok for the branch > > > > For the 6.1.1 respin right? How do I do that? > > Yes. Off the top of my head: > > $ cvs -d:ext:sources.redhat.com:/cvs/src co -r gdb_6_1-branch gdb > ... And download millions of files? I'm not falling for that again! I did: cvs up -r gdb_6_1-branch Yes, good point. For bonus points: $ tar cf src | ( cd newdir && tar xpf - && cd src && cvs update -r ) It appears to be in, thanks. Andrew You might like to check I've not goofed. Nick