Re: [patch, testsuite] Clean up gdb.trace results

[Sandra Loosemore <sandra at codesourcery dot com>]

On 10/10/2018 10:08 PM, Simon Marchi wrote:
> On 2018-10-10 22:01, Sandra Loosemore wrote:
> > Thanks, I tried that and made a couple tweaks to the patch to fix
> > those errors.  Is this version OK?
> >
> > -Sandra
>
> Hi Sandra,
>
> I'd just like to understand the situation a bit more.  From what I 
> understand, trace-common.h is only useful for fast tracepoint tests. 
> Let's take the actions.exp test, for example.  It uses actions.c, which 
> includes trace-common.h, therefore you added a 
> gdb_trace_common_supports_arch check.  However, actions.exp does not 
> rely on fast tracepoint support at all.  actions.c happens to be used by 
> another test that tests fast tracepoints.  The result is that if an 
> architecture supports tracepoints, but not fast tracepoints, the 
> actions.exp test will be skipped even though it would be relevant.

If the architecture isn't one of those supported by trace-common.h, then 
actions.c (or any other program that includes trace-common.h) won't 
compile.  E.g. here is a recent gdb.sum extract for nios2-elf (using gdb 
8.2 branch):

Running 
/scratch/sandra/nios2-elf-fall-preview/src/gdb-master-8.2/gdb/testsuite/gdb.trace/actions.exp 
...
gdb compile failed, In file included from 
/scratch/sandra/nios2-elf-fall-preview/src/gdb-master-8.2/gdb/testsuite/gdb.trace/actions.c:24:
/scratch/sandra/nios2-elf-fall-preview/src/gdb-master-8.2/gdb/testsuite/gdb.trace/trace-common.h:61:2: 
error: #error "unsupported architecture for trace tests"
 #error "unsupported architecture for trace tests"
  ^~~~~
/scratch/sandra/nios2-elf-fall-preview/src/gdb-master-8.2/gdb/testsuite/gdb.trace/actions.c: 
In function 'main':
/scratch/sandra/nios2-elf-fall-preview/src/gdb-master-8.2/gdb/testsuite/gdb.trace/actions.c:146:26: 
error: 'fast_tracepoint_loc' undeclared (first use in this function)
   FAST_TRACEPOINT_LABEL (fast_tracepoint_loc);
                          ^~~~~~~~~~~~~~~~~~~
/scratch/sandra/nios2-elf-fall-preview/src/gdb-master-8.2/gdb/testsuite/gdb.trace/actions.c:146:26: 
note: each undeclared identifier is reported only once for each function 
it appears in
UNTESTED: gdb.trace/actions.exp: failed to compile

My patch does not change what targets the tests work on, it only makes 
it fail gracefully with UNSUPPORTED instead of spewing a bunch of 
compilation errors into the gdb.sum file and reporting UNTESTED.

> Trying to read between the lines: if your target did not support 
> tracepoints at all, then the gdb_target_supports_trace calls would be 
> enough.  If your target did support regular and fast tracepoints, then 
> you would just add support for it in trace-common.h, because you would 
> want to run the fast tracepoint tests.  So the remaining combination is 
> that your target supports regular tracepoints, but not fast tracepoints. 
>   Is it the case? 
>
> If so, I think the right fix would be to untangle fast tracepoints tests 
> from regular tracepoints tests.  The goal being to make it possible to 
> run as many tests as possible against targets that only support regular 
> tracepoints.
>
> Or maybe I'm completely lost, in which case can you clarify what your 
> use case is?

Well, the particular use case I've been looking at are nios2-linux-gnu 
and nios2-elf, and seeing my gdb.sum files full of random compilation 
errors and TCL ERRORs where I think it should just be reporting 
UNSUPPORTED.  Most of the compilation errors are coming from 
trace-common.h, and as I said, I'm under the impression that making this 
work involves adding target hooks to gdb and/or gdbserver and not just 
adding some stub for the arch to trace-common.h to prevent it from 
hitting the preprocessor #error and undefined symbol errors.  I'm really 
not even clear on the difference between "tracepoints" and "fast 
tracepoints" is, or which things which testcases are trying to test.

IMO the problem here is that these tests were written with the 
assumption that the all support is present -- not just the tracepoint 
support, but things like shared libraries and signals that typically 
aren't supported on bare-metal targets, so they're just failing in 
really ugly ways when the necessary support isn't there.

-Sandra