From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Jacobowitz To: Andrew Cagney Cc: gdb-patches@sources.redhat.com Subject: Re: [rfa] mips heuristic_proc_start fix Date: Thu, 12 Jul 2001 12:14:00 -0000 Message-id: <20010712121358.A25739@nevyn.them.org> References: <20010706112010.A5578@nevyn.them.org> <3B46030B.2010007@cygnus.com> <20010706113232.A6209@nevyn.them.org> <20010706114028.A6366@nevyn.them.org> <3B4D4FC6.6090003@cygnus.com> X-SW-Source: 2001-07/msg00303.html On Thu, Jul 12, 2001 at 03:20:38AM -0400, Andrew Cagney wrote: > Hmm, with this patch UINT_MAX would give a fence of VM_MIN_ADDRESS yet > UINT_MAX-1 would give the above warning. > > Could I suggest instead something like: > > pc = ADDR_BITS_REMOVE (pc); > start_pc = pc; > if (start_pc >= heuristic_fence_post) > fence = start_pc - heuristic_fence_post; > else > fence = 0; > if (start_pc == 0) > return 0; > > if (heuristic_fence_post == UINT_MAX > || fence < VM_MIN_ADDRESS) > fence = VM_MIN_ADDRESS; > > The test being moved to before the assignment to make what is happening > more transparent. I agree it would be clearer to check for overflow, but just that won't solve the problem. If start_pc is 2 and instlen is 4, it doesn't matter what fence gets set to. First time through the for loop we decrement start_pc by instlen, and that's where the overflow is. How's this instead? Instead of checking for pc == 0, check for pc < instlen. If fence overflows, that's fine, because start_pc will be less than fence; or I could explicitly check for that too. diff -u -r1.57 mips-tdep.c --- mips-tdep.c 2001/07/12 17:34:33 1.57 +++ mips-tdep.c 2001/07/12 19:12:20 @@ -1497,19 +1497,19 @@ int seen_adjsp = 0; pc = ADDR_BITS_REMOVE (pc); - start_pc = pc; - fence = start_pc - heuristic_fence_post; - if (start_pc == 0) + instlen = pc_is_mips16 (pc) ? MIPS16_INSTLEN : MIPS_INSTLEN; + + if (pc < instlen) return 0; + start_pc = pc - instlen; + fence = start_pc - heuristic_fence_post; if (heuristic_fence_post == UINT_MAX || fence < VM_MIN_ADDRESS) fence = VM_MIN_ADDRESS; - instlen = pc_is_mips16 (pc) ? MIPS16_INSTLEN : MIPS_INSTLEN; - /* search back for previous return */ - for (start_pc -= instlen;; start_pc -= instlen) + for (;; start_pc -= instlen) if (start_pc < fence) { /* It's not clear to me why we reach this point when -- Daniel Jacobowitz Carnegie Mellon University MontaVista Software Debian GNU/Linux Developer