Page MenuHomePhabricator

LLDB Test Suite: Provide an Option to run all tests with Dwarf Package Format (DWP).
AbandonedPublic

Authored by plotfi on Jun 29 2018, 10:59 AM.

Details

Summary

Right now there is no testing as far as I know for DWP. DWP is a package format that links DWO files (split dwarf) together into one file. The idea is what when compiling for ELF on Linux you can pass -gsplit-dwarf=1 to produce a .dwo for every .o file, and later use llvm-dwp (or GNU dwp) to link the .dwo files into one .dwp file. This patch allows for specifying the location of llvm-dwp and for linking the dwo files into one dwp file and then removing the dwo files so that we force a given lldb test in the suite to use the dwp file. By default this should be turned off. I want some feedback on setting this up as a new testing flavor (but one that is off by default).

Diff Detail

Event Timeline

plotfi created this revision.Jun 29 2018, 10:59 AM

Is your plan to add dwp as another dimension in the test matrix (an equal citizen of DWARF, dSYM, DWO) or something that would be on or off for an entire run of the suite, or something only exercised by few specialized testcases?

aprantl added inline comments.Jun 29 2018, 11:11 AM
packages/Python/lldbsuite/test/make/Makefile.rules
238

Is the fact this this is *llvm-*dwp critical, or are llvm-dwp and GNU dwp interchangeable? In the latter case, I'd prefer to drop the LLVM part from the variable.

Is your plan to add dwp as another dimension in the test matrix (an equal citizen of DWARF, dSYM, DWO) or something that would be on or off for an entire run of the suite, or something only exercised by few specialized testcases?

Judging by the title, I would say it's the first option.

With that in mind I have reiterate I don't agree with the proliferation of the test formats. Besides the 4 existing ones, we now have pending patches to add three more (DWZ, type units, this patch). If I really wanted to I could add a a fourth one: .debug_names mode. Or even debug_names and debug_names&debug_types... Or debug_names + debug_names&debug_types + debug_names&debug_types&DWO... It just doesn't scale.

I am not denying that there is value in running the dotest suite in all of these modes. In fact, I think that (the fact that we can use the same tests to exercise a lot of different scenarios) is one of the strengths of our test suite. However, I don't believe all of these modes should be inflicted onto everyone running lldb tests or be our first and only line of defense against regressions.

For this particular case (DWP), I'd recommend trying to write as many regression tests as possible using lldb-test. The symbols command can locate the debug info, parse it and dump it out. It has a bunch of arguments to restrict the kind of output it produces. These were mainly geared for testing symbol lookup, but they may prove useful for this as well. And we can also add other arguments to help testing this (though I suspect that here it may be most useful to tweak the format in which the output is printed). The best part about this is the tests will be targeted, fast, and run on all platforms.

Then, for the integration test part, I propose to come up with a more generic way to specify the kind of debug info to generate. I don't have this fully thought out yet, but I have been thinking of something that could wrap the compiler invocation with some user specified script. Then we could use the same mechanism to run DWP and DWZ with any kind of crazy compiler flags we think of (which is definitely useful when bringing up support for a new kind of debug info format). If someone has a particular interest in a specific combo, he can set up a bot which runs tests in this mode (details about who would be responsible for fixing failures TBD)

alexshap added inline comments.
packages/Python/lldbsuite/test/make/Makefile.rules
238

llvm-dwp and dwp should both work

@labath

I am not denying that there is value in running the dotest suite in all of these modes. In fact, I think that (the fact that we can use the same tests to exercise a lot of different scenarios) is one of the strengths ?>of our test suite. However, I don't believe all of these modes should be inflicted onto everyone running lldb tests or be our first and only line of defense against regressions.

for what it's worth - not sure how much you care about my opinion, but i think it's an important point but it doesn't actually contradict or prevent your second point regarding adding regression tests using lldb-test, however i think those should be added over time (sadly no tests were added when the support for .dwp was implemented / introduced) (not in this patch).
I think that the approach of this patch is still useful, this mode can be off by default, but if smb needs to run all the tests with dwps - it's easy to do by passing or setting a variable (for example).

Then, for the integration test part, I propose to come up with a more generic way to specify the kind of debug info to generate. I don't have this fully thought out yet, but I have been thinking of something that could wrap the compiler invocation with some user specified script. Then we could use the same mechanism to run DWP and DWZ with any kind of crazy compiler flags we think of (which is definitely useful when bringing up support for a new kind of debug info format). If someone has a particular interest in a specific combo, he can set up a bot which runs tests in this mode (details about who would be responsible for fixing failures TBD)

I really like that idea. It sounds similar to the EXPENSIVE_CHECKS we have for LLVM. Personally I'd love to have an overnight job that runs the test suite with DWARF5 for example.

plotfi added a comment.Jul 1 2018, 5:29 PM

@labath

I am not denying that there is value in running the dotest suite in all of these modes. In fact, I think that (the fact that we can use the same tests to exercise a lot of different scenarios) is one of the strengths ?>of our test suite. However, I don't believe all of these modes should be inflicted onto everyone running lldb tests or be our first and only line of defense against regressions.

for what it's worth - not sure how much you care about my opinion, but i think it's an important point but it doesn't actually contradict or prevent your second point regarding adding regression tests using lldb-test, however i think those should be added over time (sadly no tests were added when the support for .dwp was implemented / introduced) (not in this patch).
I think that the approach of this patch is still useful, this mode can be off by default, but if smb needs to run all the tests with dwps - it's easy to do by passing or setting a variable (for example).

yes, thats the near term solution.

Is your plan to add dwp as another dimension in the test matrix (an equal citizen of DWARF, dSYM, DWO) or something that would be on or off for an entire run of the suite, or something only exercised by few specialized testcases?

another dimension, off by default

Then, for the integration test part, I propose to come up with a more generic way to specify the kind of debug info to generate. I don't have this fully thought out yet, but I have been thinking of something that could wrap the compiler invocation with some user specified script. Then we could use the same mechanism to run DWP and DWZ with any kind of crazy compiler flags we think of (which is definitely useful when bringing up support for a new kind of debug info format). If someone has a particular interest in a specific combo, he can set up a bot which runs tests in this mode (details about who would be responsible for fixing failures TBD)

I really like that idea. It sounds similar to the EXPENSIVE_CHECKS we have for LLVM. Personally I'd love to have an overnight job that runs the test suite with DWARF5 for example.

Oh, I see. Have some generic option for debug format to make this problem simpler every time it arises.

packages/Python/lldbsuite/test/make/Makefile.rules
238

Ah good. I will do that.

plotfi marked 3 inline comments as done.Jul 1 2018, 5:29 PM

@labath

I am not denying that there is value in running the dotest suite in all of these modes. In fact, I think that (the fact that we can use the same tests to exercise a lot of different scenarios) is one of the strengths ?>of our test suite. However, I don't believe all of these modes should be inflicted onto everyone running lldb tests or be our first and only line of defense against regressions.

for what it's worth - not sure how much you care about my opinion, but i think it's an important point but it doesn't actually contradict or prevent your second point regarding adding regression tests using lldb-test, however i think those should be added over time (sadly no tests were added when the support for .dwp was implemented / introduced) (not in this patch).
I think that the approach of this patch is still useful, this mode can be off by default, but if smb needs to run all the tests with dwps - it's easy to do by passing or setting a variable (for example).

yes, thats the near term solution.

What's the medium-to-long term solution?

Then, for the integration test part, I propose to come up with a more generic way to specify the kind of debug info to generate. I don't have this fully thought out yet, but I have been thinking of something that could wrap the compiler invocation with some user specified script. Then we could use the same mechanism to run DWP and DWZ with any kind of crazy compiler flags we think of (which is definitely useful when bringing up support for a new kind of debug info format). If someone has a particular interest in a specific combo, he can set up a bot which runs tests in this mode (details about who would be responsible for fixing failures TBD)

I really like that idea. It sounds similar to the EXPENSIVE_CHECKS we have for LLVM. Personally I'd love to have an overnight job that runs the test suite with DWARF5 for example.

Oh, I see. Have some generic option for debug format to make this problem simpler every time it arises.

The thing is, we *already* have multiple needs for that. Maybe you could work with @jankratochvil to come up with a solution that would work for both DWP and DWZ? It sounds to me like both of these formats (and even dSYM) could be described by "there is some tool which runs after we link the main executable/library". If we could have a generic way to specify this as an argument to test suite, then we could solve both problems together. As the tools are unlikely to take the same arguments (and IIRC, for DWZ there are multiple commands we need to issue), we can have wrapper scripts (checked into the utils folder or somewhere), which hides these differences.

Does that make sense?

The DWZ-testmode Makefile.rules part.
The DWZ-mode is written as another test matrix dimension. Compared to this DWP-mode it autodetects if all the tools are available on the host for that testsuite mode.
I agree that some EXPENSIVE_CHECKS would be appropriate for both DWZ and DWP as general LLVM/LLDB development should not break only specifically these modes.
I do not think it matters whether there is an external wrapper script or whether the commands are specified as a Makefile rule/macro, sure I will adapt it if anyone requests an external command.

jankratochvil added inline comments.Jul 3 2018, 11:04 AM
packages/Python/lldbsuite/test/make/Makefile.rules
520

Shouldn't be $(LLVM_DWP) even here? DWZ mode has its command even here.

546

Shouldn't be $(LLVM_DWP) even here? DWZ mode has its command even here.

plotfi updated this revision to Diff 155452.Jul 13 2018, 12:05 PM

What's the medium-to-long term solution?

Havn't fully fleshed that out yet.

packages/Python/lldbsuite/test/make/Makefile.rules
520

Ah, I can add it. I wasn't sure if dwp was something that works with dylibs or not. Thought there was something like dsym for those.

546

I'm not sure about this one.

I suppose we can add an off-by-default DWP mode so that it can be used for integration testing.

However, I wouldn't consider any future DWP changes "tested" if the code is only exercised via this mode. As I said above, I think the majority of DWP code can be exercised without even running a process via lldb-test. If there are any odd corner casees that can be only reached by running the full pipeline, then we add a couple of specialized tests which explicitly set MAKE_DWP=YES, and are run unconditionally (similar to how we have two debug_names tests run via dotest because the functionality was not accessible via lldb-test).

After some thought, I don't think the script idea I came up with initially is necessary here, though the situation may be different for DWZ, as the dwz utility seems to be quite temperamental -- IIRC it will error out if the input file happens to not contain a .debug_info section. For the purposes of integration testing I think it would be better to just silently accept these so that all tests work "out of the box" with dwz. It might be nice to hide these details in a wrapper script.

jankratochvil requested changes to this revision.Jul 18 2018, 8:16 AM
jankratochvil added inline comments.
packages/Python/lldbsuite/test/make/Makefile.rules
238

I was thinking DWP should be specifiable by user but make check-lldb DWP=/usr/bin/dwp has no effect. And DWP is not set by any build*( functions in packages/Python/lldbsuite/test/plugins/builder_base.py, sorry but I do not find that obvious.

528

The patch as exported by Download Raw Diff says:

../../../make/Makefile.rules:529: *** missing separator.  Stop.

(after forcing it by DWP=/usr/bin/dwp in that Makefile.rules file)

This revision now requires changes to proceed.Jul 18 2018, 8:16 AM

For the purposes of integration testing I think it would be better to just silently accept these so that all tests work "out of the box" with dwz. It might be nice to hide these details in a wrapper script.

FYI the LLDB testsuite uses DWZ in a weird way so that the DWZ common file (external file in /usr/lib/debug/.dwz/ with DIEs shared by multiple .debug files) can be tested by LLDB testsuite. It makes a copy of the .debug file so that the DWZ-tool thinks most DIEs are used by two files and makes a DWZ common file for them. This is why such script has no use outside of the run by LLDB testsuite. This is why the script would not be useful on its own. Also currently the DWZ is ran always only on one file (two files - the one file being duplicated) while in practice one runs DWZ on multiple similar files (all files of a rpm package being built) - which is never done by the LLDB testsuite (the file duplication tests the DWZ common files more thoroughly IMO).

plotfi abandoned this revision.Aug 20 2019, 9:53 AM