This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [1/2] C-SKY Port
On 21/10/18 04:22, Simon Marchi wrote:
> On 2018-10-20 11:04 p.m., Hafiz Abid Qadeer wrote:
>> On 20/10/18 19:55, Simon Marchi wrote:
>>> On 2018-07-25 6:54 a.m., Hafiz Abid Qadeer wrote:
>>
>>> Hi Hafiz,
>>>
>>> I noticed the "cooked_read" selftest fails with c-sky (ever since the c-sky support was added):
>>>
>>> (gdb) maintenance selftest cooked_read
>>> Running selftest regcache::cooked_read_test.
>>> ...
>>> Self test failed: arch csky: self-test failed at /home/simark/src/binutils-gdb/gdb/regcache.c:1697
>>> Self test failed: arch csky:ck510: self-test failed at /home/simark/src/binutils-gdb/gdb/regcache.c:1697
>>> Self test failed: arch csky:ck610: self-test failed at /home/simark/src/binutils-gdb/gdb/regcache.c:1697
>>> Self test failed: arch csky:ck801: self-test failed at /home/simark/src/binutils-gdb/gdb/regcache.c:1697
>>> Self test failed: arch csky:ck802: self-test failed at /home/simark/src/binutils-gdb/gdb/regcache.c:1697
>>> Self test failed: arch csky:ck803: self-test failed at /home/simark/src/binutils-gdb/gdb/regcache.c:1697
>>> Self test failed: arch csky:ck807: self-test failed at /home/simark/src/binutils-gdb/gdb/regcache.c:1697
>>> Self test failed: arch csky:ck810: self-test failed at /home/simark/src/binutils-gdb/gdb/regcache.c:1697
>>> Self test failed: arch csky:any: self-test failed at /home/simark/src/binutils-gdb/gdb/regcache.c:1697
>>> ...
>>> Self test failed: self-test failed at /home/simark/src/binutils-gdb/gdb/selftest-arch.c:86
>>> Ran 1 unit tests, 1 failed
>>>
>>> Could you take a look?
>> Hi Simon,
>> Thanks for letting me know. I am traveling at the moment. Will take a
>> look at it when I am back in a few days.
>
> Ok, thanks!
>
> A quick investigation shows it's because some raw registers are not in the
> save reggroup. So csky should probably be added to that big if in the test:
>
> if (bfd_arch == bfd_arch_frv || bfd_arch == bfd_arch_h8300
> || bfd_arch == bfd_arch_m32c || bfd_arch == bfd_arch_sh
> || bfd_arch == bfd_arch_alpha || bfd_arch == bfd_arch_v850
> || bfd_arch == bfd_arch_msp430 || bfd_arch == bfd_arch_mep
> || bfd_arch == bfd_arch_mips || bfd_arch == bfd_arch_v850_rh850
> || bfd_arch == bfd_arch_tic6x || bfd_arch == bfd_arch_mn10300
> || bfd_arch == bfd_arch_rl78 || bfd_arch == bfd_arch_score
> || bfd_arch == bfd_arch_riscv)
> {
> /* Raw registers. If raw registers are not in save_reggroup,
> their status are unknown. */
> if (gdbarch_register_reggroup_p (gdbarch, regnum, save_reggroup))
> SELF_CHECK (status == REG_VALID);
> else
> SELF_CHECK (status == REG_UNKNOWN);
> }
> else
> SELF_CHECK (status == REG_VALID);
>
When I was reviewing the test results for csky, I saw this fail in
unittest.exp and fixed it by adding csky to the big if list as you
mentioned above. Somehow I missed that patch when upstreaming the port.
So if you are happy with that change then I will push it.
> But I'm tempted to replace all of that with simply;
>
> /* Raw registers. If raw registers are not in save_reggroup,
> their status are unknown. */
> if (gdbarch_register_reggroup_p (gdbarch, regnum, save_reggroup))
> SELF_CHECK (status == REG_VALID);
> else
> SELF_CHECK (status == REG_UNKNOWN);
>
> We won't have to maintain this big list, and I don't think we lose any testing
> coverage/safety. For architectures for which all raw registers are in the
> save_reggroup (all arches not listed above), we will still always assert that
> status == REG_VALID.
I have not looked at this code much but your suggestion makes sense to me.
Regards,
--
Hafiz Abid Qadeer
Mentor Embedded/CodeSourcery