Re: Fwd: Re: [patch] Add support for ARMv7M devices.

[Pedro Alves <palves at redhat dot com>]

On 03/09/2012 04:13 PM, Jonathan Larmour wrote:

> On 09/03/12 15:38, Yao Qi wrote:
>> On 03/09/2012 12:24 PM, Jonathan Larmour wrote:
>>> -		tdesc = tdesc_arm_with_m;
>>> +		is_m = 1;
>>
>> `tdesc' is used later in the function,
>>
>>   /* Check any target description for validity.  */
>>   if (tdesc_has_registers (tdesc))
>>
>> Is it correct to skip updating `tdesc'?
> 
> Yes we do not want that condition to match. We want the tdesc to still be
> undetermined by the point of the 'if' test if this is Cortex-M. The code
> prior to this change only set tdesc because it thought it knew exactly
> what register set to use by that point.
> 
> But you have made me think of one improvement: we should probably not call
> register_remote_g_packet_guess() if tdesc_has_registers (tdesc) - because
> if someone has directly supplied a target description, we should solely
> use that, and avoid any guessing. 


I think that's always true, irrespective of a g packet guess being
installed.  See target_find_description: it's always "file > target xml > g-guesses",

> This would be true for both my and Pedro's patch.
> 
> That's trivial to handle though, e.g. for Pedro's patch, just replace the
> call with:
> 
>   if (!tdesc_has_registers (tdesc))
>     arm_register_g_packet_guesses (gdbarch);


so if we unconditionally register the guesses, then even "set tdesc foo;
file foo; unset tdesc filename; tar rem ..." works correctly.

-- 
Pedro Alves