[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In https://reviews.llvm.org/D32167#1026779, @davide wrote:

> In https://reviews.llvm.org/D32167#1026762, @jankratochvil wrote:
>
> > In https://reviews.llvm.org/D32167#1024546, @labath wrote:
> >
> > > I personally don't think having a new debug info flavour is a good idea. Tests written specifically to test this functionality will be easier to maintain and debug when they break. And keeping adding new flavours for every compiler option is not a viable long term strategy.
> >
> >
> > Here I disagree, from GDB testsuite experience I find more wider testing always better as it will better catch bugs even in unrelated new functionalities being implemented in the future. One can never guess all the combinations in advance - if one could then one needs no regression testing at all. There are more than enough CPU cycles available for testing.  I find rather a serious problem racy==unreliable test results which waste human efforts investigating the results and with more parallel testing (make -j) and virtualized infrastructure the racy results happen even more often.  As an example one racy testcase I idenitifed is:
> >
> >   packages/Python/lldbsuite/test/functionalities/signal/handle-segv/TestHandleSegv.py
> >   line 33: AssertionError: 4 != 5 (eStateLaunching != eStateStopped)
> >   
> >
> > I think at least these test are racy:
> >
> >   BreakpointAfterJoinTestCase-test
> >    CreateDuringStepTestCase-test_step_inst_with
> >    HandleSegvTestCase-test_inferior_handle_sigsegv
> >    ReturnValueTestCase-test_vector_values
> >    TestTargetSymbolsBuildidSymlink-test_target_symbols_buildid
> >    ThreadSpecificBreakPlusConditionTestCase-test_python
> >    ThreadSpecificBreakTestCase-test_python
> >    ThreadStateTestCase-test_process_interrupt
> >   
>
>
> I'm not necessarily convinced having another column in the matrix is something that can substitute targeted testing.


If I would have done targeted testing I wouldn't have caught the breakpoint issue that came up with this patch. So I agree with jankratochvil that it is best to test this all the way by adding another row. There is no way we can guess what certain compilers will do and and all the different permutations this can cause. Not sure where ".debug_types" lands in terms of popularity, but I would guess it comes in above other DWARF flavors (dwo, dwz).  Can I at least land this and then we can work on future improvements to the rows required. This is only currently enabled on linux where we tend to have fast buildbots. Darwin compilers don't support this with mach-o due to the 255 section limit that mach-o suffers from.


https://reviews.llvm.org/D32167