From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20726 invoked by alias); 21 Apr 2004 19:18:04 -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 20621 invoked from network); 21 Apr 2004 19:18:02 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sources.redhat.com with SMTP; 21 Apr 2004 19:18:02 -0000 Received: from drow by nevyn.them.org with local (Exim 4.32 #1 (Debian)) id 1BGNEP-0005nA-I1; Wed, 21 Apr 2004 15:18:01 -0400 Date: Wed, 21 Apr 2004 19:18:00 -0000 From: Daniel Jacobowitz To: Michael Snyder Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA] Prevent runaway (infinite loop) in mips-tdep.c Message-ID: <20040421191801.GA12161@nevyn.them.org> Mail-Followup-To: Michael Snyder , gdb-patches@sources.redhat.com References: <4085BE65.3000402@redhat.com> <20040421005257.GA11993@nevyn.them.org> <4086B631.9050304@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4086B631.9050304@redhat.com> User-Agent: Mutt/1.5.5.1+cvs20040105i X-SW-Source: 2004-04/txt/msg00500.txt.bz2 On Wed, Apr 21, 2004 at 05:58:09PM +0000, Michael Snyder wrote: > Daniel Jacobowitz wrote: > >On Wed, Apr 21, 2004 at 12:20:53AM +0000, Michael Snyder wrote: > > > >>Hi Andrew, > >> > >>This change will prevent the caller(s) of mips_mdebug_frame_id from > >>infinite-looping when we get badly lost on the stack frame. > >> > > > > > >>2004-04-21 Michael Snyder > >> > >> * mips-tdep.c (mips_mdebug_frame_cache): Call error to prevent > >> infinite looping by caller. > >> (heuristic_proc_start): Warning() already prefixes "Warning: ". > > > > > >Since I have some patches to make this "I'm not sure how" case into a > >working part of the unwinder, I don't much like this. They got hung up > >on the question of how to trust proc_desc's when we might be in the > >prologue. > > > >One time where this case occurs is in backtracing from a NULL pointer > >dereference, which happens in the GDB testsuite now. > > OK -- can I see your patches? Maybe they'll solve the runaway condition > I'm experiencing, and if not, maybe I can fix it in a way that's > consistant with what you're doing. What I've attached is a diff against GDB 6.1. No promises about quality or "correctness". diff -urp -x objdir -x '*~' gdb-2004-03-01-cvs.orig/gdb/mips-linux-tdep.c gdb-2004-03-01-cvs/gdb/mips-linux-tdep.c --- gdb-2004-03-01-cvs.orig/gdb/mips-linux-tdep.c 2004-03-02 16:56:42.000000000 -0500 +++ gdb-2004-03-01-cvs/gdb/mips-linux-tdep.c 2004-03-03 18:29:25.000000000 -0500 @@ -85,7 +85,7 @@ mips_linux_get_longjmp_target (CORE_ADDR buf, TARGET_PTR_BIT / TARGET_CHAR_BIT)) return 0; - *pc = extract_unsigned_integer (buf, TARGET_PTR_BIT / TARGET_CHAR_BIT); + *pc = extract_signed_integer (buf, TARGET_PTR_BIT / TARGET_CHAR_BIT); return 1; } @@ -379,7 +379,7 @@ mips64_linux_get_longjmp_target (CORE_AD buf, TARGET_PTR_BIT / TARGET_CHAR_BIT)) return 0; - *pc = extract_unsigned_integer (buf, TARGET_PTR_BIT / TARGET_CHAR_BIT); + *pc = extract_signed_integer (buf, TARGET_PTR_BIT / TARGET_CHAR_BIT); return 1; } @@ -866,11 +866,8 @@ static void set_reg_offset (struct trad_frame_saved_reg *saved_regs, int regno, CORE_ADDR offset) { - if (saved_regs[regno].addr == 0) - { - saved_regs[regno + 0 * NUM_REGS].addr = offset; - saved_regs[regno + 1 * NUM_REGS].addr = offset; - } + saved_regs[regno + 0 * NUM_REGS].addr = offset; + saved_regs[regno + 1 * NUM_REGS].addr = offset; } /* There are two possible layouts for a signal frame: @@ -945,7 +942,7 @@ mips_linux_sigframe_find_saved_regs (COR int sigframe_kind) { int ireg, reg_position; - CORE_ADDR sigcontext_base = frame - SIGFRAME_CODE_OFFSET; + CORE_ADDR sigcontext_base = frame; const struct mips_regnum *regs; if (sigframe_kind == 1) @@ -1067,7 +1064,7 @@ mips64_linux_sigframe_find_saved_regs (C { const struct mips_regnum *regs; int ireg, reg_position; - CORE_ADDR sigcontext_base = frame - SIGFRAME_CODE_OFFSET; + CORE_ADDR sigcontext_base = frame; if (sigframe_kind == 3) sigcontext_base += N64_SIGFRAME_SIGCONTEXT_OFFSET; @@ -1191,7 +1188,7 @@ mips_make_sigtramp_cache (struct frame_i cache = frame_obstack_zalloc (sizeof (struct mips_prologue_cache)); - cache->prev_sp = frame_unwind_register_unsigned (next_frame, SP_REGNUM); + cache->prev_sp = frame_unwind_register_signed (next_frame, NUM_REGS + SP_REGNUM); cache->saved_regs = trad_frame_alloc_saved_regs (next_frame); @@ -1201,10 +1198,14 @@ mips_make_sigtramp_cache (struct frame_i mips_linux_sigframe_find_saved_regs (cache->prev_sp, cache->saved_regs, cache->kind); +#if 0 /* Is this right? */ cache->prev_sp = read_memory_integer (cache->saved_regs[SP_REGNUM].addr, - DEPRECATED_REGISTER_RAW_SIZE (SP_REGNUM)); + register_size (current_gdbarch, SP_REGNUM)); + trad_frame_set_value (cache->saved_regs, SP_REGNUM, cache->prev_sp); + /* What about NUM_REGS + SP_REGNUM? */ +#endif return cache; } diff -urp -x objdir -x '*~' gdb-2004-03-01-cvs.orig/gdb/mips-tdep.c gdb-2004-03-01-cvs/gdb/mips-tdep.c --- gdb-2004-03-01-cvs.orig/gdb/mips-tdep.c 2004-03-02 16:56:42.000000000 -0500 +++ gdb-2004-03-01-cvs/gdb/mips-tdep.c 2004-03-04 14:51:06.000000000 -0500 @@ -1503,19 +1505,26 @@ mips_mdebug_frame_cache (struct frame_in /* Get the mdebug proc descriptor. */ proc_desc = find_proc_desc (frame_pc_unwind (next_frame), next_frame, 1); if (proc_desc == NULL) - /* I'm not sure how/whether this can happen. Normally when we - can't find a proc_desc, we "synthesize" one using - heuristic_proc_desc and set the saved_regs right away. */ - return cache; - - /* Extract the frame's base. */ - cache->base = (frame_unwind_register_signed (next_frame, NUM_REGS + PROC_FRAME_REG (proc_desc)) - + PROC_FRAME_OFFSET (proc_desc) - PROC_FRAME_ADJUST (proc_desc)); + { + /* This can happen at least at PC == 0. */ + cache->base = frame_unwind_register_signed (next_frame, NUM_REGS + SP_REGNUM); + cache->saved_regs[NUM_REGS + mips_regnum (current_gdbarch)->pc] + = cache->saved_regs[NUM_REGS + RA_REGNUM]; + return cache; + } + + /* Extract the frame's base. Assume that the first instruction sets it + up. */ + if (frame_pc_unwind (next_frame) == PROC_LOW_ADDR (proc_desc)) + cache->base = frame_unwind_register_signed (next_frame, NUM_REGS + SP_REGNUM); + else + cache->base = (frame_unwind_register_signed (next_frame, NUM_REGS + PROC_FRAME_REG (proc_desc)) + + PROC_FRAME_OFFSET (proc_desc) - PROC_FRAME_ADJUST (proc_desc)); kernel_trap = PROC_REG_MASK (proc_desc) & 1; gen_mask = kernel_trap ? 0xFFFFFFFF : PROC_REG_MASK (proc_desc); float_mask = kernel_trap ? 0xFFFFFFFF : PROC_FREG_MASK (proc_desc); - + /* In any frame other than the innermost or a frame interrupted by a signal, we assume that all registers have been saved. This assumes that all register saves in a function happen before the @@ -1681,7 +1690,8 @@ mips_mdebug_frame_this_id (struct frame_ glibc syscall stubs does not work. Eventually MIPS will use allow us to solve this with DWARF unwind information instead. */ if (frame_relative_level (next_frame) >= 0 - && !trad_frame_addr_p (info->saved_regs, PC_REGNUM)) + && get_frame_type (next_frame) == NORMAL_FRAME + && !trad_frame_addr_p (info->saved_regs, NUM_REGS + PC_REGNUM)) { (*this_id) = null_frame_id; return; @@ -2471,7 +2481,8 @@ find_proc_desc (CORE_ADDR pc, struct fra */ /* If the next frame is a signal handler trampoline, then we also need to do this, because we might be in the prologue. */ - if (next_frame == NULL || get_frame_type (next_frame) == SIGTRAMP_FRAME) + if (frame_relative_level (next_frame) < 0 + || get_frame_type (next_frame) != NORMAL_FRAME) { struct symtab_and_line val; struct symbol *proc_symbol = @@ -2489,7 +2500,10 @@ find_proc_desc (CORE_ADDR pc, struct fra heuristic_proc_desc (PROC_LOW_ADDR (proc_desc), pc, next_frame, cur_frame); if (found_heuristic) - proc_desc = found_heuristic; + { + PROC_REG_OFFSET (found_heuristic) = PROC_REG_OFFSET (proc_desc); + proc_desc = found_heuristic; + } } } } @@ -3123,6 +3137,11 @@ mips_n32n64_push_dummy_call (struct gdba val = (char *) VALUE_CONTENTS (arg); + /* FIXME drow/2004-03-04: One unimplemented rule from the + N32/N64 ABI is that any 64-bit chunk of a structure which + consists of a double-precision floating point field (but not + a double member of a union) will be passed in an FP reg + instead of an integer reg. */ if (fp_register_arg_p (typecode, arg_type) && float_argreg <= MIPS_LAST_FP_ARG_REGNUM) { @@ -3147,13 +3166,6 @@ mips_n32n64_push_dummy_call (struct gdba /* Copy the argument to general registers or the stack in register-sized pieces. Large arguments are split between registers and stack. */ - /* Note: structs whose size is not a multiple of - mips_regsize() are treated specially: Irix cc passes them - in registers where gcc sometimes puts them on the stack. - For maximum compatibility, we will put them in both - places. */ - int odd_sized_struct = ((len > mips_saved_regsize (tdep)) - && (len % mips_saved_regsize (tdep) != 0)); /* Note: Floating-point values that didn't fit into an FP register are only written to memory. */ while (len > 0) @@ -3169,7 +3181,6 @@ mips_n32n64_push_dummy_call (struct gdba /* Write this portion of the argument to the stack. */ if (argreg > MIPS_LAST_ARG_REGNUM - || odd_sized_struct || fp_register_arg_p (typecode, arg_type)) { /* Should shorter than int integer values be @@ -3209,15 +3220,10 @@ mips_n32n64_push_dummy_call (struct gdba } write_memory (addr, val, partial_len); } - - /* Note!!! This is NOT an else clause. Odd sized - structs may go thru BOTH paths. Floating point - arguments will not. */ - /* Write this portion of the argument to a general - purpose register. */ - if (argreg <= MIPS_LAST_ARG_REGNUM - && !fp_register_arg_p (typecode, arg_type)) + else { + /* Write this portion of the argument to a general + purpose register. */ LONGEST regval = extract_unsigned_integer (val, partial_len); @@ -3290,10 +3296,10 @@ mips_n32n64_return_value (struct gdbarch void *readbuf, const void *writebuf) { struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch); - if (TYPE_CODE (type) == TYPE_CODE_STRUCT - || TYPE_CODE (type) == TYPE_CODE_UNION - || TYPE_CODE (type) == TYPE_CODE_ARRAY - || TYPE_LENGTH (type) > 2 * mips_saved_regsize (tdep)) + if ((TYPE_CODE (type) == TYPE_CODE_STRUCT + || TYPE_CODE (type) == TYPE_CODE_UNION + || TYPE_CODE (type) == TYPE_CODE_ARRAY) + && TYPE_LENGTH (type) > 2 * mips_saved_regsize (tdep)) return RETURN_VALUE_STRUCT_CONVENTION; else if (TYPE_CODE (type) == TYPE_CODE_FLT && TYPE_LENGTH (type) == 16 @@ -3366,6 +3372,29 @@ mips_n32n64_return_value (struct gdbarch mips_xfer_lower. */ int offset; int regnum; + + /* This is... rough. If we have a non-trivial copy constructor or + destructor, use reference. This should be in common code. */ + int i, j; + for (i = 0; i < TYPE_NFN_FIELDS (type); i++) + for (j = 0; j < TYPE_FN_FIELDLIST_LENGTH (type, i); j++) + { + struct fn_field *fn = TYPE_FN_FIELDLIST1 (type, i); + if (TYPE_FN_FIELD_ARTIFICIAL (fn, j)) + continue; + if (TYPE_FN_FIELDLIST_NAME (type, i)[0] == '~') + return RETURN_VALUE_STRUCT_CONVENTION; + if (strncmp (TYPE_NAME (type), + TYPE_FN_FIELDLIST_NAME (type, i), + strlen (TYPE_NAME (type))) == 0 + && (TYPE_FN_FIELDLIST_NAME (type, i)[strlen (TYPE_NAME (type))] == '\0' + || TYPE_FN_FIELDLIST_NAME (type, i)[strlen (TYPE_NAME (type))] == '<') + && TYPE_NFIELDS (TYPE_FN_FIELD_TYPE (fn, j)) == 2 + && TYPE_CODE (TYPE_FIELD_TYPE (TYPE_FN_FIELD_TYPE (fn, j), 1)) == TYPE_CODE_REF + && TYPE_TARGET_TYPE (TYPE_FIELD_TYPE (TYPE_FN_FIELD_TYPE (fn, j), 1)) == type) + return RETURN_VALUE_STRUCT_CONVENTION; + } + for (offset = 0, regnum = V0_REGNUM; offset < TYPE_LENGTH (type); offset += register_size (current_gdbarch, regnum), regnum++) -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer