From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9479 invoked by alias); 10 Mar 2004 20:32:55 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 9472 invoked from network); 10 Mar 2004 20:32:53 -0000 Received: from unknown (HELO faui10.informatik.uni-erlangen.de) (131.188.31.10) by sources.redhat.com with SMTP; 10 Mar 2004 20:32:53 -0000 Received: from faui1d.informatik.uni-erlangen.de (faui1d [131.188.31.34]) by faui10.informatik.uni-erlangen.de (8.9.3p3/8.1.9-FAU) with ESMTP id VAA09994 for ; Wed, 10 Mar 2004 21:32:52 +0100 (CET) From: Ulrich Weigand Received: (from weigand@localhost) by faui1d.informatik.uni-erlangen.de (8.9.3p3/8.1.6-FAU) id VAA05524 for gdb-patches@sources.redhat.com; Wed, 10 Mar 2004 21:32:52 +0100 (CET) Message-ID: <200403102032.VAA05524@faui1d.informatik.uni-erlangen.de> Subject: [PATCH] Fix signals.exp test case on S/390 To: gdb-patches@sources.redhat.com Date: Fri, 19 Mar 2004 00:09:00 -0000 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-SW-Source: 2004-03/txt/msg00250.txt.bz2 Message-ID: <20040319000900.hI4R3c4we_k6B_twaWU5n8jzT9-G6rc2z4yM0xn0Zp8@z> Hello, I was trying to get the signals.exp test case to pass on s390. This exposed a number of problems in the Linux kernel and gdb; in particular I think I've found a bug in gdb common code (step_over_function in infrun.c). However, I'm not completely sure I fully understand the intended flow of control throughout infrun.c, so I'd appreciate comments on what I found. Basically, the test case arranges for a signal (SIGALRM) to arrive while the inferior is stopped, and then does a 'next'. When the inferior is continued with PT_SINGLESTEP due to the 'next', the kernel tries to deliver the pending SIGALRM. This in turn causes another ptrace intercept. gdb gets active, notices that it doesn't want to do anything special with the signal, and continues with PT_SINGLESTEP giving the signal number. At this point, the Linux kernel delivers the signal, runs to the first instruction of the signal handler, and gets the single step trap there. (Note that there was a bug in the kernel that caused this not to work, but I've fixed that one.) Now the interesting aspect starts. handle_inferior_event needs to figure out what has happened here. There is special code in handle_inferior_event that tries to recognize the 'we've just stepped into a signal handler' situation: /* Did we just take a signal? */ if (pc_in_sigtramp (stop_pc) && !pc_in_sigtramp (prev_pc) && INNER_THAN (read_sp (), step_sp)) { /* We've just taken a signal; go until we are back to the point where we took it and one more. */ Note, however, that this fundamentally does not work on Linux because we use a signal trampoline only to *exit* from a signal handler -- the kernel *starts* signal handlers by jumping to them directly without any trampoline. So this test doesn't trigger, and the event turns out to be interpreted as call to a subroutine, and is passed on to handle_step_info_function, which decides to step over that function call using step_over_function. This would basically work just fine for this case -- we'd continue just after the signal handler terminates, which is what we want here. However, step_over_function continues to run not only until a specific PC has been reached, but at the same time a specific *frame* needs to be reached. In the situation we're in right now: PC frame 1st instruction of signal handler sig. handler frame 1st instruction of sigreturn trampoline sigtramp frame interrupted main routine main routine frame step_over_function tries to continue running until it reaches the current frame's return address (i.e. the 1st instruction of the sigreturn trampoline) while at the same time reaching the frame currently being stepped (i.e the main routine's frame). Of course, these two events never coincide, and hence gdb steps until the program terminates. Now, a similar situation occurs if there is a dynamic linker resolver frame between a called subroutine and the main routine, and there is special code in step_over_function to skip the intermediate frame in this case. The patch below extends this special handling to the case of an intermediate signal trampoline frame, which fixes the symptom for me -- the signals.exp test case now passes. Is this an acceptable solution or should the signal-handler code in handle_inferior_event be made smarter? Also, I've run into another problem when single-stepping *out* of a signal handler. In this case code in handle_step_into_function is supposed to recognize that we've just stepped into a signal trampoline and just continue on over the sigreturn system call. This doesn't work on s390 because of the comparison: && frame_id_inner (step_frame_id, frame_id_build (read_sp (), 0))) since our frame IDs contain the DWARF CFA as stack_addr, which is biased relative to the stack pointer at function entry. So comparing this CFA to a real SP value is bogus. Other places compare either two stack frame IDs or two real SP values, either of which should work. The patch below changes the comparison to && INNER_THAN (step_sp, read_sp ())) similar to most other places in infrun.c that manage signal trampolines. Finally, the patch below reintroduces a pc_in_sigtramp gdbarch callback to s390-tdep.c; I had thought this would be no longer necessary when using the new frame code, but apparently there's still other users ... The whole patch builds on s390-ibm-linux with no test suite regressions and fixes the signals.exp test case. Bye, Ulrich ChangeLog: * infrun.c (handle_step_into_function): Do not compare a frame ID stack_addr directly against a read_sp () stack pointer value. (step_over_function): Skip signal trampoline frame on function return. * s390-tdep.c (s390_sigtramp_frame_sniffer): Move core signal trampoline detection logic to ... (s390_pc_in_sigtramp): ... this new function. (s390_gdbarch_init): Call set_gdbarch_pc_in_sigtramp. Index: gdb/infrun.c =================================================================== RCS file: /cvs/src/src/gdb/infrun.c,v retrieving revision 1.138 diff -c -p -r1.138 infrun.c *** gdb/infrun.c 6 Mar 2004 00:10:06 -0000 1.138 --- gdb/infrun.c 10 Mar 2004 01:33:06 -0000 *************** handle_step_into_function (struct execut *** 1265,1272 **** /* We're doing a "next". */ if (pc_in_sigtramp (stop_pc) ! && frame_id_inner (step_frame_id, ! frame_id_build (read_sp (), 0))) /* We stepped out of a signal handler, and into its calling trampoline. This is misdetected as a subroutine call, but stepping over the signal --- 1265,1271 ---- /* We're doing a "next". */ if (pc_in_sigtramp (stop_pc) ! && INNER_THAN (step_sp, read_sp ())) /* We stepped out of a signal handler, and into its calling trampoline. This is misdetected as a subroutine call, but stepping over the signal *************** step_over_function (struct execution_con *** 2976,2982 **** check_for_old_step_resume_breakpoint (); if (frame_id_p (step_frame_id) ! && !IN_SOLIB_DYNSYM_RESOLVE_CODE (sr_sal.pc)) /* NOTE: cagney/2004-02-27: Use the global state's idea of the stepping frame ID. I suspect this is done as it is lighter weight than a call to get_prev_frame. */ --- 2975,2982 ---- check_for_old_step_resume_breakpoint (); if (frame_id_p (step_frame_id) ! && !IN_SOLIB_DYNSYM_RESOLVE_CODE (sr_sal.pc) ! && !pc_in_sigtramp (sr_sal.pc)) /* NOTE: cagney/2004-02-27: Use the global state's idea of the stepping frame ID. I suspect this is done as it is lighter weight than a call to get_prev_frame. */ Index: gdb/s390-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/s390-tdep.c,v retrieving revision 1.129 diff -c -p -r1.129 s390-tdep.c *** gdb/s390-tdep.c 26 Feb 2004 23:48:01 -0000 1.129 --- gdb/s390-tdep.c 10 Mar 2004 01:33:06 -0000 *************** static const struct frame_unwind s390_si *** 2237,2258 **** s390_sigtramp_frame_prev_register }; ! static const struct frame_unwind * ! s390_sigtramp_frame_sniffer (struct frame_info *next_frame) { - CORE_ADDR pc = frame_pc_unwind (next_frame); bfd_byte sigreturn[2]; - if (read_memory_nobpt (pc, sigreturn, 2)) ! return NULL; if (sigreturn[0] != 0x0a /* svc */) ! return NULL; if (sigreturn[1] != 119 /* sigreturn */ && sigreturn[1] != 173 /* rt_sigreturn */) return NULL; ! return &s390_sigtramp_frame_unwind; } --- 2237,2266 ---- s390_sigtramp_frame_prev_register }; ! static int ! s390_pc_in_sigtramp (CORE_ADDR pc, char *name) { bfd_byte sigreturn[2]; if (read_memory_nobpt (pc, sigreturn, 2)) ! return 0; if (sigreturn[0] != 0x0a /* svc */) ! return 0; if (sigreturn[1] != 119 /* sigreturn */ && sigreturn[1] != 173 /* rt_sigreturn */) + return 0; + + return 1; + } + + static const struct frame_unwind * + s390_sigtramp_frame_sniffer (struct frame_info *next_frame) + { + CORE_ADDR pc = frame_pc_unwind (next_frame); + if (!s390_pc_in_sigtramp (pc, NULL)) return NULL; ! return &s390_sigtramp_frame_unwind; } *************** s390_gdbarch_init (struct gdbarch_info i *** 3024,3029 **** --- 3032,3038 ---- set_gdbarch_return_value (gdbarch, s390_return_value); /* Frame handling. */ + set_gdbarch_pc_in_sigtramp (gdbarch, s390_pc_in_sigtramp); set_gdbarch_in_solib_call_trampoline (gdbarch, in_plt_section); dwarf2_frame_set_init_reg (gdbarch, s390_dwarf2_frame_init_reg); frame_unwind_append_sniffer (gdbarch, dwarf2_frame_sniffer); -- Dr. Ulrich Weigand weigand@informatik.uni-erlangen.de