Re: [rfa] mips heuristic_proc_start fix

Andrew Cagney
2001-07-12T14:11:00Z

>
> 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
>