[Lldb-commits] [PATCH] D94063: lldb: Add support for DW_AT_ranges on DW_TAG_subprograms

[Pavel Labath via lldb-commits]

(sorry for inactivity -- I mostly tuned out of this discussion as I 
couldn't keep up with its pace)

On 08/01/2021 01:55, Jim Ingham via lldb-commits wrote:
 > 3) However, we try to push as many tests as possible all the way
 > through the compiler, since the lldb test suite is also one of the
 > significant test harnesses for compiler debug output.  .s files are
 > exposed to much less of the compiler, and so don't catch new compiler
 > debug output bugs as well.  So unless you have a good reason to use a
 > .s file, you shouldn't.

On 11/01/2021 02:37, David Blaikie via lldb-commits wrote:
> Thanks for all the context - so sounds like mostly based on (3) the 
> recommendation would be for this to be an API test

Well... as Jim said, there are different takes on this :), and I would 
not be so quick to convert this to an API test. While that does make 
sure that the compiler part is tested, it also introduces the opposite 
problem -- symmetric bugs in clang & lldb cancelling each other out, 
which can lead to lldb only being able to debug binaries built by a 
matching compiler. While this can be mitigated by running the tests 
against an older compiler (API tests can do that, shell tests cannot), 
that requires that:
a) someone actually runs the tests this way
b) those tests work with the older compiler -- unstable/experimental 
flags, or relying on very specific details of the compiler output make 
that tricky

This is why I was saying (way back) that, if anything, we should have 
two tests, one that would be very explicit about what input it is 
testing, and a second one, which tests the integration with the compiler.

I am not saying that every patch should have two kinds of test (most of 
the time, I am happy that a patch has _any_ test). However, I am not 
sure that the second test is really useful (except as a learning 
exercise) in this case -- that's because the -mllvm switch is 
(deliberately) very targeted, so it's hard to see what else would be 
tested by it (the codegen is presumably already tested by an llvm test). 
Also, now that you already have written the assembly test, I'd be sad to 
see it thrown away.

What would make the second test really interesting (to me) is if could 
be phrased in a way that's as independent as possible from specific 
clang flags, and ideally even DWARF. Like, if it was phrased as "test 
that we can (break, step, view variables) in discontinuous functions", 
then we could run the same tests against gcc or msvc (pdb) and check 
that we do something reasonable.

(Note that the msvc part is just a pipe dream, as we currently don't 
have any infrastructure to compile test inferiors with msvc, and even if 
we did, I expect most of them would fail.)

Also note that, I am not saying that we should never use -mllvm options 
in API tests. However, doing that lowers their value in my mind. The 
reason why it's not possible to make a clear cut (-mllvm does not go 
into API tests, only Shell ones), is that there are also other competing 
criteria at play here -- some things just cannot be tested with a Shell 
test due to their non-interactivity, even though they are testing a very 
specific aspect of debug info (or they are not even testing debug info 
at all).


cheers,
pl

PS: I don't know if you've seen this, but we also have an "official" 
description of the various test types here: 
<https://lldb.llvm.org/resources/test.html>.