This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [rfa] mips heuristic_proc_start fix
>
> 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.
Yes, I looked at this, played with it a little and gave up. It is nasty :-)
> 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.
I'd do both. I think it is better to spell out the intent of each thing
so that you can introduce a few invariants.
> 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;
Suggest adding:
gdb_assert ((pc % instlen) == 0);
Should that also be ``pc <= instlen'' as otherwize:
pc - instlen - instlen
can underflow. I suspect it depends on the for loop.
> + start_pc = pc - instlen;
> + fence = start_pc - heuristic_fence_post;
I think this should still have the underflow check as otherwize you're
not quite sure what fence is upto.
> if (heuristic_fence_post == UINT_MAX
> || fence < VM_MIN_ADDRESS)
> fence = VM_MIN_ADDRESS;
>
> - instlen = pc_is_mips16 (pc) ? MIPS16_INSTLEN : MIPS_INSTLEN;
> -
gdb_assert (fence >= VM_MIN_ADDRESS);
gdb_assert (start_pc >= instlen);
Hmm, what happens if VM_MIN_ADDRESS < instlen.
> /* search back for previous return */
> - for (start_pc -= instlen;; start_pc -= instlen)
> + for (;; start_pc -= instlen)
Er, if VM_MIN_ADDRESS == 0 (hence fence == 0) then this ain't going to work.
what a rats nest. Would be better to change the for loop to:
for (; start_pc > fence; start_pc -= instlen;
reversing the exit condition?
Alternativly, should fence be guarenteed to be >= instlen.
Andrew
> 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
>