DWARF-5 support in debugedit #1537
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
New functions edit_strp, skip_form and edit_attributes_str_comp_dir called by edit_attributes. Split part of read_dwarf2_line into a read_dwarf4_line function. New function edit_dwarf2_any_str called by edit_dwarf2 at the end of phase 0 to rebuild .debug_str.
Recognize the various new DWARF5 .debug sections. Parse and skip new DWARF5 forms in read_abbrev and skip_form. Read DWARF5 unit headers for compile and partial units in edit_info. This is enough to be able to process gcc -gdwarf-5 produced binaries without the new DWARF5 .debug_line format (which isn't produced with binutils < 2.36). Patches slightly edited/merged by Mark Wielaard <mark@klomp.org>
Handle the new DWARF5 .debug_line tables and the new DW_FORM_line_strp. DWARF5 tables are handled separately from the earlier tables. They will never change size, but they do need updates to the .debug_str or .debug_line_str references. Based on a patch from Jan Kratochvil <jan.kratochvil@redhat.com>
Adjust some existing tests so they are run with an explicit -gdwarf-4 and add an -gdwarf-5 variant to make testing independent of the gcc default DWARF version. The tests that might generate a DWARF5 line table work for both version 4 and 5 and some also ignore stderr output when using -p.debug_line_str because some combinations of gcc/binutils generate DWARF5 debug info with DWARF4 debug line tables, even when -gdwarf-5 is given. Tested against GCC10, which defaults to DWARF4 and GCC11, which defaults to DWARF5.
|
How do I test this "in the real world"? The code looks fine (insomuch as debugedit code usually does...)... |
|
AIUI this has been real-world tested in rawhide for a few weeks now... |
|
So... some of this has been reviewed in #1332 months ago and the rest has been battle-tested in rawhide for several weeks now, I don't think this PR will get any better by "letting it breath" here some more 😅 |
|
Thanks for the patches @jankratochvil and Mark! |
|
The patchset regressed against my version as it no longer tests rpmbuild compatibility with LLVM product as required by paying customers of Red Hat company. |
|
On Mon, 2021-02-15 at 20:58 -0800, Neal Gompa (ニール・ゴンパ) wrote:
How do I test this "in the real world"?
See also the new testcases, which are really just like the old
testcases. Before the patch running debugedit on a binary compiled with
-gdwarf-5 would not work, or not correctly, missing some sources and/or
not rewriting base/dest paths. Now it does.
When using gcc 11 (pre-release) you get to test it immediately when
using -g. Because gcc 11 defaults to DWARF5. That is why the testsuite
now also has some explicit -gdwarf-4 testcases. So the code is tested
using either DWARF4 or DWARF5, independent of what the used compiler
defaults to.
Cheers,
Mark
|
|
On Mon, 2021-02-15 at 23:07 -0800, Panu Matilainen wrote:
AIUI this has been real-world tested in rawhide for a few weeks
now...
Yes, it has, but admittedly a slightly different patch set, because
that is using rpm 4.16.1.2
https://src.fedoraproject.org/rpms/rpm/tree/f34
https://code.wildebeest.org/git/user/mjw/rpm/log/?h=gcc-dwarf5-4.16.1.2
|
|
Hi Jan,
On Tue, 2021-02-16 at 00:16 -0800, Jan Kratochvil wrote:
The patchset regressed against my version as it no longer tests
rpmbuild compatibility with LLVM product as required by paying
customers of Red Hat company.
Unfortunately they did because I rewrote the tests so they didn't
require the llvm-toolset tools to run and made them independent of the
version of DWARF emitted by the compiler so that they work using either
older or newer gcc and binutils versions installed on the system.
Untested, but you can probably improve on that experience by changing
the gcc invocations inside the RPM_DEBUGEDIT_SETUP m4 macro (which is
just a shell script snippet) to use ${CC} instead. That should pick up
the CC setup by configure. That might also help people trying to build
and test rpm in a cross-compiler situation.
Cheers,
Mark
|
|
Could you rather point what was incomplete on the previous testcases than to screw it up? You removed the only part that matters. |
|
Hi Jan,
On Tue, 2021-02-16 at 01:37 -0800, Jan Kratochvil wrote:
Could you rather point what was incomplete on the previous testcases
than to screw it up? You removed the only part that matters.
I didn't screw up. I simply didn't use that part. But wrote some new
testcases based on the existing tests, just with explicit -gdwarf-4 and
-gdwarf-5 settings instead of tweaking the compiler used or needing new
tools in the tests. The reason I didn't use the testcases you wrote was
simply because they depended on llvm tooling being present and you said
you didn't care about making them work with gcc. I just happened to be
interested in testing with gcc, so I had to write some new testcases to
make sure the new functionality worked as expected.
|
gcc did not support the needed functionality the time when I was implementing it. I sure do not care at all about gcc (like the rest of the world) but that was not the reason why I did not include the testcases for gcc. Running the tests with both gcc and clang would be a trivial extension of my testcases instead of dropping them all and replacing them by your incomplete ones. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resubmitting patches posted on rpm-maint: http://lists.rpm.org/pipermail/rpm-maint/2021-February/016462.html