From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23173 invoked by alias); 19 Nov 2001 23:15:16 -0000 Mailing-List: contact gdb-patches-help@sourceware.cygnus.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 23090 invoked from network); 19 Nov 2001 23:15:10 -0000 Received: from unknown (HELO nevyn.them.org) (128.2.145.6) by sourceware.cygnus.com with SMTP; 19 Nov 2001 23:15:10 -0000 Received: from drow by nevyn.them.org with local (Exim 3.32 #1 (Debian)) id 165xd8-0007kc-00; Mon, 19 Nov 2001 18:15:10 -0500 Date: Wed, 07 Nov 2001 18:19:00 -0000 From: Daniel Jacobowitz To: Andrew Cagney Cc: gdb-patches@sources.redhat.com Subject: Re: [rfa] Mips heuristic_proc_desc vs. the stack pointer. Message-ID: <20011119181510.A25606@nevyn.them.org> Mail-Followup-To: Andrew Cagney , gdb-patches@sources.redhat.com References: <20011116162423.A30736@nevyn.them.org> <3BF979F7.4090207@cygnus.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3BF979F7.4090207@cygnus.com> User-Agent: Mutt/1.3.23i X-SW-Source: 2001-11/txt/msg00132.txt.bz2 On Mon, Nov 19, 2001 at 04:30:31PM -0500, Andrew Cagney wrote: > >As HJ noticed, we try to read the stack pointer in heuristic_proc_desc. > >I'm > >not sure why this normally works and fails with linuxthread support, but > >I'm > >convinced it's sometimes wrong. If we are called from after_prologue(), > >the > >stack pointer has nothing to do with the function we're trying to generate > >a > >desc for. We shouldn't try to read it in this case. The uses of it in > >*_heuristic_proc_desc are harmless. > > Regarding the threads, are you saying things still sometimes break with > your patch applied? I suspect there was the usual GDB internal thread > coherency problem where different parts of GDB were debugging different > threads. Things won't break with the patch applied. We build saved argument stack offsets in *_heuristic_proc_desc, even if we don't have a stack pointer; they'll be bogus, but nothing uses them. > >Is this OK, Andrew? > > Yes, but can you please adjust the following before committing: > > > * mips-tdep.c (find_proc_desc): Add read_sp argument. Update all > >callers. > > Given the updates were not identical / mechanical, could you please list > each making it clear that after_prologue() was the exception. > > Can you please add a comment to after_prologue() explaining why the SP > shouldn't be fetched in that case. > > Can the argument be called read_sp_p (say?) rather than read_sp. There > is a global function read_sp that is hiding behind that variable. Oops, I didn't even notice. Isn't there a -Wshadow somewhere? Here's the patch I committed to the trunk. -- Daniel Jacobowitz Carnegie Mellon University MontaVista Software Debian GNU/Linux Developer Index: ChangeLog =================================================================== RCS file: /cvs/src/src/gdb/ChangeLog,v retrieving revision 1.1797 diff -u -p -r1.1797 ChangeLog --- ChangeLog 2001/11/19 21:44:46 1.1797 +++ ChangeLog 2001/11/19 22:11:21 @@ -1,3 +1,13 @@ +2001-11-19 Daniel Jacobowitz + + * mips-tdep.c (find_proc_desc): Add cur_frame argument. Pass + cur_frame to heuristic_proc_desc. + (heuristic_proc_desc): Add cur_frame argument. Do not read SP + if cur_frame == 0. + (after_prologue): Pass cur_frame == 0 to find_proc_desc. + (mips_frame_chain): Pass cur_frame == 1 to find_proc_desc. + (mips_init_extra_frame_info): Likewise. + 2001-11-19 Andrew Cagney * defs.h (return_to_top_level): Comment. Index: mips-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/mips-tdep.c,v retrieving revision 1.60 diff -u -p -r1.60 mips-tdep.c --- mips-tdep.c 2001/10/15 18:18:29 1.60 +++ mips-tdep.c 2001/11/19 22:11:21 @@ -239,7 +239,7 @@ int gdb_print_insn_mips (bfd_vma, disass static void mips_print_register (int, int); static mips_extra_func_info_t -heuristic_proc_desc (CORE_ADDR, CORE_ADDR, struct frame_info *); +heuristic_proc_desc (CORE_ADDR, CORE_ADDR, struct frame_info *, int); static CORE_ADDR heuristic_proc_start (CORE_ADDR); @@ -252,7 +252,7 @@ static void mips_show_processor_type_com static void reinit_frame_cache_sfunc (char *, int, struct cmd_list_element *); static mips_extra_func_info_t -find_proc_desc (CORE_ADDR pc, struct frame_info *next_frame); +find_proc_desc (CORE_ADDR pc, struct frame_info *next_frame, int cur_frame); static CORE_ADDR after_prologue (CORE_ADDR pc, mips_extra_func_info_t proc_desc); @@ -561,8 +561,13 @@ after_prologue (CORE_ADDR pc, struct symtab_and_line sal; CORE_ADDR func_addr, func_end; + /* Pass cur_frame == 0 to find_proc_desc. We should not attempt + to read the stack pointer from the current machine state, because + the current machine state has nothing to do with the information + we need from the proc_desc; and the process may or may not exist + right now. */ if (!proc_desc) - proc_desc = find_proc_desc (pc, NULL); + proc_desc = find_proc_desc (pc, NULL, 0); if (proc_desc) { @@ -1858,10 +1863,15 @@ restart: static mips_extra_func_info_t heuristic_proc_desc (CORE_ADDR start_pc, CORE_ADDR limit_pc, - struct frame_info *next_frame) + struct frame_info *next_frame, int cur_frame) { - CORE_ADDR sp = read_next_frame_reg (next_frame, SP_REGNUM); + CORE_ADDR sp; + if (cur_frame) + sp = read_next_frame_reg (next_frame, SP_REGNUM); + else + sp = 0; + if (start_pc == 0) return NULL; memset (&temp_proc_desc, '\0', sizeof (temp_proc_desc)); @@ -1919,7 +1929,7 @@ non_heuristic_proc_desc (CORE_ADDR pc, C static mips_extra_func_info_t -find_proc_desc (CORE_ADDR pc, struct frame_info *next_frame) +find_proc_desc (CORE_ADDR pc, struct frame_info *next_frame, int cur_frame) { mips_extra_func_info_t proc_desc; CORE_ADDR startaddr; @@ -1951,7 +1961,7 @@ find_proc_desc (CORE_ADDR pc, struct fra { mips_extra_func_info_t found_heuristic = heuristic_proc_desc (PROC_LOW_ADDR (proc_desc), - pc, next_frame); + pc, next_frame, cur_frame); if (found_heuristic) proc_desc = found_heuristic; } @@ -1975,7 +1985,7 @@ find_proc_desc (CORE_ADDR pc, struct fra startaddr = heuristic_proc_start (pc); proc_desc = - heuristic_proc_desc (startaddr, pc, next_frame); + heuristic_proc_desc (startaddr, pc, next_frame, cur_frame); } return proc_desc; } @@ -2007,7 +2017,7 @@ mips_frame_chain (struct frame_info *fra saved_pc = tmp; /* Look up the procedure descriptor for this PC. */ - proc_desc = find_proc_desc (saved_pc, frame); + proc_desc = find_proc_desc (saved_pc, frame, 1); if (!proc_desc) return 0; @@ -2033,7 +2043,7 @@ mips_init_extra_frame_info (int fromleaf /* Use proc_desc calculated in frame_chain */ mips_extra_func_info_t proc_desc = - fci->next ? cached_proc_desc : find_proc_desc (fci->pc, fci->next); + fci->next ? cached_proc_desc : find_proc_desc (fci->pc, fci->next, 1); fci->extra_info = (struct frame_extra_info *) frame_obstack_alloc (sizeof (struct frame_extra_info));