[PATCH][CRISv32] Add support for threaded debugging

[Ricard Wanderlof]

On Mon, 2 Sep 2013, Pedro Alves wrote:

> (fixing subject)

Thanks.

>> 2013-08-30  Ricard Wanderlof  <ricardw@axis.com>
>>
>>   	* cris-tdep.c: Fix typedef for elf_greg_t.
>>   	  (cris_gdbarch_init): Add call to
>>   	  set_gdbarch_fetch_tls_load_module_address.
>
> Indentation doesn't look right.  Should be indented
> with a single tab.

Thanks for pointing it out. Will fix.

>> Suggested-by: Edgar Iglisias <edgar.iglesias@gmail.com>
>
> (Note: surname/address don't match)

Thanks for spotting that. Edgar used to work at the same company I do but 
does not any more so I had to piece this together manually.

>> 2013-08-30  Ricard Wanderlof  <ricardw@axis.com>
>>
>>   	* cris-tdep.c: Fix typedef for elf_greg_t.
>
> This seems to be unrelated to threaded debugging support.
> It just looks like a host-dependency fix.  Please keep logically unrelated
> patches separate, and always send them as separate threads each with their
> own rationale.

Sure. That was the intention, like I said the patch as such has been 
applied locally for some time and I decided to submit it, not really 
considering all the details.

I will split this into two patches then.

> (Note that "fix" is not "what" changed, but "why" you changed it, so
> it's not the proper ChangeLog description of the change.)

Ok.

> In this case, the fix should just go the extra mile and remove the
> reliance on host alignment from this type, that is representing an
> external structure.  IOW, that typedef, if any, would better
> be:
>
>  typedef gdb_byte cris_elf_greg_t[4];
>
> See frv_linux_supply_gregset for example.

Ok, I'll do something similar then. I see that MIPS has a similar 
construct.

>>   	  (cris_gdbarch_init): Add call to
>>   	  set_gdbarch_fetch_tls_load_module_address.
>
> This part looks OK, though it did raise some eyebrows to have
> GNU/Linux-specific code in cris-tdep.c, rather than in a cris-linux-tdep.c
> file.  It seems there's no real support for cris bare-metal debugging?

Virtually all CRIS systems run Linux, and GDB is mostly used for 
user-space debugging. There is a GDB stub available for the Linux kernel, 
which I guess would qualify as bare metal debugging, but as far as I know 
it is virtually never used and the support in the kernel may even be 
broken by know.

So as you say the file should probably be split, but at this point in time 
I don't really see the need for it. It doesn't seem like it would be a 
huge effort (essentially the call to 
set_gdbarch_fetch_tls_load_module_address and also 
set_solib_svr4_fetch_link_map_offsets would be put in cris-linux-tdep.c), 
on the other hand I can't really test that it works as expected, so 
perhaps the best solution for now is to add a FIXME, and to split the 
files when the need comes up and the result can be tested properly?

>>
>> gdbserver
>>
>>     	* linux-crisv32-low.c (ps_get_thread_area): New function.
>
> This part is OK, though should mention also the addition of
> PTRACE_GET_THREAD_AREA, and,

Ok, will add that.

>> +  *base = (void *) ((char *)*base - idx);
>
> missing space after '(char *) '.

Ok, will fix that.

I'll fix the issues and submit a new patch (two patches).

/Ricard
-- 
Ricard Wolf Wanderlöf                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30