Re: [PATCH] RISC-V: Fix gdbserver problem with handling arch strings.

["Maciej W. Rozycki" <macro at wdc dot com>]

Hi Jim,

 Cc-ing <gdb@sourceware.org> for wider audience.

> >  I think it's an acceptable solution short-term; after all it's not going
> > to regress functionality.  However ultimately I think we ought to actually
> > interpret these suffix letters and arm the disassembler accordingly.
> 
> The strings are checked when used as options, and when used in elf
> attributes.  I'm not sure if they need to be checked here, but it is
> probably a good idea to do that eventually.  Checking the strings for
> correctness is complicated, as there are many different possible
> correct answers, which is why I didn't try to do it in the first
> version of the patch.  it would be nice to reuse some of the other
> support, maybe the attribute merging support, but it would have to be
> rewritten a bit to make this work.  I'd rather worry about that later,
> or ask someone else to do it.

 Sure.

 I have test results now and they are looking good; there are several 
progressions from failures like:

Remote 'g' packet reply is too long (expected 264 bytes, got 532 bytes): 00000000000000002a6f61551500000040faffff3f0000000028010000000000b03857551500000060ad5f5515000000906e615515000000802401000000000050faffff3f00000000000000000000000100000000000000a8fbffff3f000000b8fbffff3f000000000000000000000078faffff3f0000006e05010000000000589c6f55150000004e4b024946434a2f180957551500000080e1baaa2a0000000000000000000000c000bbaa2a00000080e1baaa2a0000006804baaa2a000000000000000000000000000000000000007053baaa2a0000008252b2aa2a00000090fe01000000000048e056551500000004000000000000004000000000000000760501000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000xxxxxxxxxxxxxxxx00000000
(gdb) FAIL: gdb.base/break-unload-file.exp: cmdline: always-inserted off: break: file

which is obviously a sign of non-XML support heuristics failing.  There's 
some usual noise from gdb.threads/ tests too; not too much though.

 For the record overall results without and with your change are:

		=== gdb Summary ===

# of expected passes		59831
# of unexpected failures	767
# of unexpected successes	2
# of expected failures		47
# of unknown successes		5
# of known failures		101
# of unresolved testcases	9
# of untested testcases		122
# of unsupported tests		235

and:

		=== gdb Summary ===

# of expected passes		59858
# of unexpected failures	737
# of unexpected successes	2
# of expected failures		47
# of unknown successes		5
# of known failures		101
# of unresolved testcases	9
# of untested testcases		122
# of unsupported tests		235

respectively.  Also native results for a reference:

		=== gdb Summary ===

# of expected passes		57572
# of unexpected failures	1756
# of expected failures		58
# of unknown successes		3
# of known failures		85
# of unresolved testcases	118
# of untested testcases		161
# of unsupported tests		399

These are significantly worse as you can see (and don't cover gdb.ada/ 
tests, which all scored UNSUPPORTED status due to the lack of an Ada 
compiler), and my brief look has indicated that the additional failures 
are in many cases in tests that are not run in a remote setup, and in 
other cases they are genuine regressions such as (remote):

p/d check_arg_struct_01_01 (ref_val_struct_01_01)
$1 = 1
(gdb) PASS: gdb.base/infcall-nested-structs.exp: l=c++: types-tf: p/d check_arg_struct_01_01 (ref_val_struct_01_01)

vs (native):

p/d check_arg_struct_01_01 (ref_val_struct_01_01)
$1 = 0
(gdb) FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-tf: p/d check_arg_struct_01_01 (ref_val_struct_01_01)

which might be a compiler problem (old `gcc (GCC) 8.2.1 20181105 (Red Hat 
8.2.1-5)' in the native installation here, vs `riscv64-linux-gnu-gcc (GCC) 
10.0.0 20191109 (experimental)' in the remote one), which I'll look into 
getting sorted independently, perhaps by wiring a native compiler build 
into my toolchain compilation.

 So I think we're good to go.  I'll post updated `gdbserver' patches 
shortly.

 NB I think eventually we ought to get rid of the heuristics.  We are now 
well after Y2004, which is when XML target description support was added, 
and support for the RISC-V target has also been added well after then.  
So we should have enforced XML support in stubs since the beginning.

 Now mistakes happen and we didn't for one reason or another, but I think 
we ought to fix it.  If someone still insists on living in non-XML world 
with their debug stub, then they can use:

(gdb) set tdesc filename <whatever>

to get GDB set up for their long obsolete world.

 I won't rush implementing that requirement as I have other priorities 
now, but I mean to do this sometime unless someone beats me to it.  All 
the heuristics can go then.

  Maciej