From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18459 invoked by alias); 30 Aug 2004 19:32:09 -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 18429 invoked from network); 30 Aug 2004 19:32:05 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org with SMTP; 30 Aug 2004 19:32:05 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.10/8.12.10) with ESMTP id i7UJW0S2004496 for ; Mon, 30 Aug 2004 15:32:00 -0400 Received: from localhost.redhat.com (porkchop.devel.redhat.com [172.16.58.2]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id i7UJVr319509; Mon, 30 Aug 2004 15:31:54 -0400 Received: from gnu.org (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 0297F2B9D; Mon, 30 Aug 2004 15:30:43 -0400 (EDT) Message-ID: <41338063.7020500@gnu.org> Date: Mon, 30 Aug 2004 19:32:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-GB; rv:1.4.1) Gecko/20040801 MIME-Version: 1.0 To: Joel Brobecker Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA/RFC] fix problems with unwinder on mips-irix References: <20040723011059.GI20596@gnat.com> <410994BD.5040506@gnu.org> <20040803044358.GA18163@gnat.com> <411039F3.1020102@gnu.org> <20040830181829.GC969@gnat.com> In-Reply-To: <20040830181829.GC969@gnat.com> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2004-08/txt/msg00773.txt.bz2 > But you want a "major overhaul" of the mips unwinder as a precondition > to fixing the problem at hand. Could you explain a bit more what > overhaul you are interested in? I don't see what needs to be done. > > In particular, you said: > > >>> Two key things to know: >>> >>> - with three unwinders handling three different cases previously handled >>> by one, there's a lot of dead code paths. For instance, >>> mips32_heuristic_proc_desc is now only called by mips_insn32_frame_cache >>> and hence can be inlined there, making it possible for your problem to >>> be solved more locally. > > > mips32_heuristic_proc_desc is called by heuristic_proc_desc. (And > frankly I find inlining often counter productive, as we end up > with giant function just as we did with decode_line_1). Yes, sometimes inlineing doesn't help, here it does. There's really no value in trying to preserve this code so be brutal. If find_proc_desc is inlined: > static mips_extra_func_info_t > find_proc_desc (CORE_ADDR pc, struct frame_info *next_frame, int cur_frame) > { > mips_extra_func_info_t proc_desc; > CORE_ADDR startaddr = 0; then everything from here .... > proc_desc = non_heuristic_proc_desc (pc, &startaddr); > > if (proc_desc) > { > /* IF this is the topmost frame AND > * (this proc does not have debugging information OR > * the PC is in the procedure prologue) > * THEN create a "heuristic" proc_desc (by analyzing > * the actual code) to replace the "official" proc_desc. > */ > if (next_frame == NULL) > { > struct symtab_and_line val; > struct symbol *proc_symbol = > PROC_DESC_IS_DUMMY (proc_desc) ? 0 : PROC_SYMBOL (proc_desc); > > if (proc_symbol) > { > val = find_pc_line (BLOCK_START > (SYMBOL_BLOCK_VALUE (proc_symbol)), 0); > val.pc = val.end ? val.end : pc; > } > if (!proc_symbol || pc < val.pc) > { > mips_extra_func_info_t found_heuristic = > heuristic_proc_desc (PROC_LOW_ADDR (proc_desc), > pc, next_frame, cur_frame); > if (found_heuristic) > proc_desc = found_heuristic; > } > } > } > else > { > /* Is linked_proc_desc_table really necessary? It only seems to be used > by procedure call dummys. However, the procedures being called ought > to have their own proc_descs, and even if they don't, > heuristic_proc_desc knows how to create them! */ > > struct linked_proc_info *link; > > for (link = linked_proc_desc_table; link; link = link->next) > if (PROC_LOW_ADDR (&link->info) <= pc > && PROC_HIGH_ADDR (&link->info) > pc) > return &link->info; ... to here is simply dead. That is all for the non-heuristic case. That leaves: > if (startaddr == 0) > startaddr = heuristic_proc_start (pc); > > proc_desc = heuristic_proc_desc (startaddr, pc, next_frame, cur_frame); > } > return proc_desc; > } If heuristic_proc_descr is then inlined we find: > static mips_extra_func_info_t > heuristic_proc_desc (CORE_ADDR start_pc, CORE_ADDR limit_pc, > struct frame_info *next_frame, int cur_frame) > { > CORE_ADDR sp; > > if (cur_frame) > sp = read_next_frame_reg (next_frame, NUM_REGS + MIPS_SP_REGNUM); > else > sp = 0; > > if (start_pc == 0) > return NULL; > memset (&temp_proc_desc, '\0', sizeof (temp_proc_desc)); > temp_saved_regs = xrealloc (temp_saved_regs, SIZEOF_FRAME_SAVED_REGS); > memset (temp_saved_regs, '\0', SIZEOF_FRAME_SAVED_REGS); > PROC_LOW_ADDR (&temp_proc_desc) = start_pc; > PROC_FRAME_REG (&temp_proc_desc) = MIPS_SP_REGNUM; > PROC_PC_REG (&temp_proc_desc) = RA_REGNUM; > if (start_pc + 200 < limit_pc) > limit_pc = start_pc + 200; > if (pc_is_mips16 (start_pc)) this mips16 call is dead. > mips16_heuristic_proc_desc (start_pc, limit_pc, next_frame, sp); > else > mips32_heuristic_proc_desc (start_pc, limit_pc, next_frame, sp); > return &temp_proc_desc; > } If we then inline the mips32_heuristic ... call into mips_insn32...: > static void > mips32_heuristic_proc_desc (CORE_ADDR start_pc, CORE_ADDR limit_pc, > struct frame_info *next_frame, CORE_ADDR sp) > { > CORE_ADDR cur_pc; > CORE_ADDR frame_addr = 0; /* Value of $r30. Used by gcc for frame-pointer */ > restart: > temp_saved_regs = xrealloc (temp_saved_regs, SIZEOF_FRAME_SAVED_REGS); > memset (temp_saved_regs, '\0', SIZEOF_FRAME_SAVED_REGS); > PROC_FRAME_OFFSET (&temp_proc_desc) = 0; > PROC_FRAME_ADJUST (&temp_proc_desc) = 0; /* offset of FP from SP */ > for (cur_pc = start_pc; cur_pc < limit_pc; cur_pc += MIPS_INSTLEN) > { > unsigned long inst, high_word, low_word; > int reg; > > /* Fetch the instruction. */ > inst = (unsigned long) mips_fetch_instruction (cur_pc); > > /* Save some code by pre-extracting some useful fields. */ > high_word = (inst >> 16) & 0xffff; > low_word = inst & 0xffff; > reg = high_word & 0x1f; > > if (high_word == 0x27bd /* addiu $sp,$sp,-i */ > || high_word == 0x23bd /* addi $sp,$sp,-i */ > || high_word == 0x67bd) /* daddiu $sp,$sp,-i */ > { > if (low_word & 0x8000) /* negative stack adjustment? */ > PROC_FRAME_OFFSET (&temp_proc_desc) += 0x10000 - low_word; > else > /* Exit loop if a positive stack adjustment is found, which > usually means that the stack cleanup code in the function > epilogue is reached. */ > break; > } > else if ((high_word & 0xFFE0) == 0xafa0) /* sw reg,offset($sp) */ > { > PROC_REG_MASK (&temp_proc_desc) |= 1 << reg; > set_reg_offset (temp_saved_regs, reg, sp + low_word); > } > else if ((high_word & 0xFFE0) == 0xffa0) /* sd reg,offset($sp) */ > { > /* Irix 6.2 N32 ABI uses sd instructions for saving $gp and > $ra. */ > PROC_REG_MASK (&temp_proc_desc) |= 1 << reg; > set_reg_offset (temp_saved_regs, reg, sp + low_word); > } > else if (high_word == 0x27be) /* addiu $30,$sp,size */ > { > /* Old gcc frame, r30 is virtual frame pointer. */ > if ((long) low_word != PROC_FRAME_OFFSET (&temp_proc_desc)) > frame_addr = sp + low_word; > else if (PROC_FRAME_REG (&temp_proc_desc) == MIPS_SP_REGNUM) > { > unsigned alloca_adjust; > PROC_FRAME_REG (&temp_proc_desc) = 30; > frame_addr = read_next_frame_reg (next_frame, NUM_REGS + 30); > alloca_adjust = (unsigned) (frame_addr - (sp + low_word)); > if (alloca_adjust > 0) > { > /* FP > SP + frame_size. This may be because > * of an alloca or somethings similar. > * Fix sp to "pre-alloca" value, and try again. > */ > sp += alloca_adjust; > goto restart; > } > } > } > /* move $30,$sp. With different versions of gas this will be either > `addu $30,$sp,$zero' or `or $30,$sp,$zero' or `daddu 30,sp,$0'. > Accept any one of these. */ > else if (inst == 0x03A0F021 || inst == 0x03a0f025 || inst == 0x03a0f02d) > { > /* New gcc frame, virtual frame pointer is at r30 + frame_size. */ > if (PROC_FRAME_REG (&temp_proc_desc) == MIPS_SP_REGNUM) > { > unsigned alloca_adjust; > PROC_FRAME_REG (&temp_proc_desc) = 30; > frame_addr = read_next_frame_reg (next_frame, NUM_REGS + 30); > alloca_adjust = (unsigned) (frame_addr - sp); > if (alloca_adjust > 0) > { > /* FP > SP + frame_size. This may be because > * of an alloca or somethings similar. > * Fix sp to "pre-alloca" value, and try again. > */ > sp += alloca_adjust; > goto restart; > } > } > } > else if ((high_word & 0xFFE0) == 0xafc0) /* sw reg,offset($30) */ > { > PROC_REG_MASK (&temp_proc_desc) |= 1 << reg; > set_reg_offset (temp_saved_regs, reg, frame_addr + low_word); > } > } > } > and compare it against these parts of the body of mips_insn32's heuristic ..., we also find: > static struct mips_frame_cache * > mips_insn32_frame_cache (struct frame_info *next_frame, void **this_cache) > { > mips_extra_func_info_t proc_desc; > struct mips_frame_cache *cache; > struct gdbarch *gdbarch = get_frame_arch (next_frame); > struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); > /* r0 bit means kernel trap */ > int kernel_trap; > /* What registers have been saved? Bitmasks. */ > unsigned long gen_mask, float_mask; > > if ((*this_cache) != NULL) > return (*this_cache); > cache = FRAME_OBSTACK_ZALLOC (struct mips_frame_cache); > (*this_cache) = cache; > cache->saved_regs = trad_frame_alloc_saved_regs (next_frame); this is has been inlined down to mips32_... > /* 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 kernel_trap stuff isn't applicable here. > 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); THe following is just duplicating that inlined mips32... so the pair can be folded into one. > /* 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 > first function call. */ > if (in_prologue (frame_pc_unwind (next_frame), PROC_LOW_ADDR (proc_desc)) > /* Not sure exactly what kernel_trap means, but if it means the > kernel saves the registers without a prologue doing it, we > better not examine the prologue to see whether registers > have been saved yet. */ > && !kernel_trap) > { > /* We need to figure out whether the registers that the > proc_desc claims are saved have been saved yet. */ > > CORE_ADDR addr; > > /* Bitmasks; set if we have found a save for the register. */ > unsigned long gen_save_found = 0; > unsigned long float_save_found = 0; > int mips16; > > /* If the address is odd, assume this is MIPS16 code. */ > addr = PROC_LOW_ADDR (proc_desc); > mips16 = pc_is_mips16 (addr); > > /* Scan through this function's instructions preceding the > current PC, and look for those that save registers. */ > while (addr < frame_pc_unwind (next_frame)) > { > if (mips16) > { > mips16_decode_reg_save (mips16_fetch_instruction (addr), > &gen_save_found); > addr += MIPS16_INSTLEN; > } > else > { > mips32_decode_reg_save (mips32_fetch_instruction (addr), > &gen_save_found, &float_save_found); > addr += MIPS_INSTLEN; > } > } > gen_mask = gen_save_found; > float_mask = float_save_found; > } > > /* Fill in the offsets for the registers which gen_mask says were > saved. */ > { > CORE_ADDR reg_position = (cache->base > + PROC_REG_OFFSET (proc_desc)); > int ireg; > for (ireg = MIPS_NUMREGS - 1; gen_mask; --ireg, gen_mask <<= 1) > if (gen_mask & 0x80000000) > { > cache->saved_regs[NUM_REGS + ireg].addr = reg_position; > reg_position -= mips_abi_regsize (gdbarch); > } > } this is dead ... > /* The MIPS16 entry instruction saves $s0 and $s1 in the reverse > order of that normally used by gcc. Therefore, we have to fetch > the first instruction of the function, and if it's an entry > instruction that saves $s0 or $s1, correct their saved addresses. */ > if (pc_is_mips16 (PROC_LOW_ADDR (proc_desc))) > { > ULONGEST inst = mips16_fetch_instruction (PROC_LOW_ADDR (proc_desc)); > if ((inst & 0xf81f) == 0xe809 && (inst & 0x700) != 0x700) > /* entry */ > { > int reg; > int sreg_count = (inst >> 6) & 3; > > /* Check if the ra register was pushed on the stack. */ > CORE_ADDR reg_position = (cache->base > + PROC_REG_OFFSET (proc_desc)); > if (inst & 0x20) > reg_position -= mips_abi_regsize (gdbarch); > > /* Check if the s0 and s1 registers were pushed on the > stack. */ > /* NOTE: cagney/2004-02-08: Huh? This is doing no such > check. */ > for (reg = 16; reg < sreg_count + 16; reg++) > { > cache->saved_regs[NUM_REGS + reg].addr = reg_position; > reg_position -= mips_abi_regsize (gdbarch); > } > } > } this is redundant (We've got the same loop (or equivalent)) in the mips32_heuristic_proc_desc. > /* Fill in the offsets for the registers which float_mask says were > saved. */ > { > CORE_ADDR reg_position = (cache->base > + PROC_FREG_OFFSET (proc_desc)); > int ireg; > /* Fill in the offsets for the float registers which float_mask > says were saved. */ > for (ireg = MIPS_NUMREGS - 1; float_mask; --ireg, float_mask <<= 1) > if (float_mask & 0x80000000) > { > if (mips_abi_regsize (gdbarch) == 4 > && TARGET_BYTE_ORDER == BFD_ENDIAN_BIG) > { > /* On a big endian 32 bit ABI, floating point registers > are paired to form doubles such that the most > significant part is in $f[N+1] and the least > significant in $f[N] vis: $f[N+1] ||| $f[N]. The > registers are also spilled as a pair and stored as a > double. > > When little-endian the least significant part is > stored first leading to the memory order $f[N] and > then $f[N+1]. > > Unfortunately, when big-endian the most significant > part of the double is stored first, and the least > significant is stored second. This leads to the > registers being ordered in memory as firt $f[N+1] and > then $f[N]. > > For the big-endian case make certain that the > addresses point at the correct (swapped) locations > $f[N] and $f[N+1] pair (keep in mind that > reg_position is decremented each time through the > loop). */ > if ((ireg & 1)) > cache->saved_regs[NUM_REGS + mips_regnum (current_gdbarch)->fp0 + ireg] > .addr = reg_position - mips_abi_regsize (gdbarch); > else > cache->saved_regs[NUM_REGS + mips_regnum (current_gdbarch)->fp0 + ireg] > .addr = reg_position + mips_abi_regsize (gdbarch); > } > else > cache->saved_regs[NUM_REGS + mips_regnum (current_gdbarch)->fp0 + ireg] > .addr = reg_position; > reg_position -= mips_abi_regsize (gdbarch); > } > > cache->saved_regs[NUM_REGS + mips_regnum (current_gdbarch)->pc] > = cache->saved_regs[NUM_REGS + RA_REGNUM]; > } > > /* SP_REGNUM, contains the value and not the address. */ > trad_frame_set_value (cache->saved_regs, NUM_REGS + MIPS_SP_REGNUM, cache->base); > > return (*this_cache); > } and so it goes have fun! Andrew