Re: arm_addr_bits_remove

[Pedro Alves <pedro_alves@portugalmail.pt>]

Jim Blandy wrote:
> I'm not an ARM expert by any means, so I don't object to the patch,
> but I wonder if allowing arm_pc_is_thumb to return the wrong answer
> for the first address beyond the end of a function will cause other
> problems elsewhere.
> 

[FWIW, I'm not an ARM expert either]

> If it is valuable to make arm_pc_is_thumb accurate in this case, when
> it can't find a minsym at memaddr, and memaddr > 0, would it make
> sense to look for a minsym at memaddr - 1, and see if MSYMBOL_SIZE (m)
> != 0 && SYMBOL_VALUE_ADDRESS (m) + MSYMBOL_SIZE (m) == memaddr, and
> use m if so?
> 

With that you'll be certain that there is a symbol *before* the
address you want to check, and you'll be sure about it's mode,
and I'm sure that most of the times that mode will be the same as
the mode of memaddr, but you can't be sure, can you?

I'd say that relying on the mode of first address beyond the end
of a function to infer something about the function itself is broken.
In those cases the '- 1' should be applied explicitly on the
call to arm_pc_is_thumb (or earlier on the call stack).

The case I stumbled on the bug is a bit different from that
case you mentioned, because the line info doesn't refer to a possible
function which includes memaddr or ends before memaddr.  There was no
real code at the address the lookup was being performed, because
it refers to the end of the object file, where padding is being
performed, but real code in a different mode could be there.  If
there was code there, then the correct mode for it could be
inferred, and it could be different from memaddr-1 -- at
least that's my understanding.  I could be wrong though.  :-)

An earlier version of the patch changed arm_addr_bits_remove to
call a new function arm_mode_at_pc that returned arm, thumb or
unknown.  Only if that returned arm, I'd strip bit 1.  But then,
if that bit is not used for any encoding (unlike bit 0), we can
just never strip it, and save us the min sym lookup to begin with.
That is, the patch removes one usage of arm_pc_is_thumb when
it can't always provide a correct answer.

-- 
Pedro Alves