[llvm-objcopy] Don't remove .gnu_debuglink section when using --strip-all

[felixkellenbenz]

This fixes the issue mentioned here: #57407
It prevents llvm-objcopy from removing the .gnu _debuglink section when used with the --strip-all flag. Since --strip-all is the default of llvm-strip the patch also prevents llvm-strip from removing the .gnu_debuglink section.

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-llvm-binary-utilities

Author: Felix Kellenbenz (felixkellenbenz)

Changes

This fixes #57407 and therefore makes llvm-strip not remove the .gnu_debuglink section when used as described in #57407.

---

Full diff: https://github.com/llvm/llvm-project/pull/78919.diff

1 Files Affected:

• (modified) llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp (+2)

diff --git a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
index daf03810fd7bff..b6d77d17bae36c 100644
--- a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
+++ b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
@@ -450,6 +450,8 @@ static Error replaceAndRemoveSections(const CommonConfig &Config,
         return false;
       if (StringRef(Sec.Name).starts_with(".gnu.warning"))
         return false;
+      if (StringRef(Sec.Name).starts_with(".gnu_debuglink"))
+        return false;
       // We keep the .ARM.attribute section to maintain compatibility
       // with Debian derived distributions. This is a bug in their
       // patchset as documented here:

[jh7370]

Hi, thanks for the contribution.

Please could you add a new lit test case to show that this behaviour works as intended.

I note that this code as-is only impacts --strip-all behaviour. This means that if you use --strip-debug, the section will get removed. Is this intentional? What do GNU strip or objcopy do in these cases?

[felixkellenbenz]

Hey thanks for the review,

I will add a test case in a moment.

That's true, my change only impacts --strip-all. I tested llvm-strip with the --strip-debug flag and it didn't remove the .gnu_debuglink section. This is also seen when using GNU strip.

Here are the commands I used when testing the --strip-debug flag:

gcc -g -xc /dev/null -ffreestanding -shared -o a.out
llvm-objcopy --only-keep-debug a.out a.out.dbg
llvm-objcopy --add-gnu-debuglink=a.out.dbg a.out
llvm-strip --strip-debug -o a.out.stripped a.out
readelf -wk a.out

EDIT:
I also tried it with clang-14 instead of gcc, the result was the same.

[jh7370]

The code change looks good to me. Before this can be merged, please tweak your PR title and description slightly, as this is used for the final commt message that is merged into main:

1. Add [llvm-objcopy] to the title line (llvm-strip is basically just an alias of llvm-objcopy with some slightly different command-line options and we tend to use the llvm-objcopy tag more often than llvm-strip).
2. The title should mention --strip-all, so something like [llvm-objcopy] Don't remove .gnu_debuglink section with --strip-all
3. In the description, make sure you're using the actual links to the issue, rather than the GitHub shorthand of #NNNNNN: this is because if somebody is looking at the git log on the command-line, #NNNNNN isn't a useful link.
4. In the description, mention llvm-objcopy as well as llvm-strip and --strip-all (e.g. "and therefore makes llvm-strip (and llvm-objcopy) not remove the .gnu_debuglink section when used with --strip-all"). Note that --strip-all is on by default for llvm-strip, but not llvm-objcopy, hence why the behaviour works as it does.

When you've made those changes, ping me and I'll merge the patch for you.

[dwblaikie]

(FWIW, as a minor issue, I disagree with (3) "In the description, make sure you're using the actual links to the issue, rather than the GitHub shorthand of #NNNNNN: this is because if somebody is looking at the git log on the command-line, #NNNNNN isn't a useful link." - before github we used "PRNNNN" in bugs, not generally full links to bugs.llvm.org, and I think by-and-large we probably have way more #NNNNN than full links to github issues these days (github certainly encourages it by linking them, etc) so for consistency I think we should probably go with that - or at least I wouldn't actively discourage it (& I probably wouldn't actively discourage someone who put a full link in either, to be fair - but I guess I'm actively discouraging the active discouraging of #NNNN at least... if that makes any sense))

[felixkellenbenz]

@jh7370 I made the requested changes.

I replaced the #NNNNN with a link but it seems link git hub automatically converts it to #NNNNN.

[jh7370]

@felixkellenbenz, I've gone ahead and merged this. Please keep an eye out for test failure emails from the CI build bots that look like they might be to do with your change (note that you may get some failure emails that have nothing to do with your change - you can safely ignore those ones).

[jh7370]

Also, please resolve the issue when you are ready.

[MaskRay]

@felixkellenbenz, I've gone ahead and merged this. Please keep an eye out for test failure emails from the CI build bots that look like they might be to do with your change (note that you may get some failure emails that have nothing to do with your change - you can safely ignore those ones).

This commit has an author field of xxx@users.noreply.github.com and a committer field of GitHub <noreply@github.com>. Build bots cannot deal with this situation (technically it could leave a comment, but this is not implemented; and build bots have too high false positives right now..)

https://discourse.llvm.org/t/hidden-emails-on-github-should-we-do-something-about-it/74223

I have run check-llvm-tools and it passed.

[felixkellenbenz]

Thanks @jh7370 for merging the PR, also thanks @MaskRay for running the checks.

Since the checks that @MaskRay performed passed, and the I could not find something in the test failures of the build bots related to my changes, I will contact the person who raised the issue and ask them to close it.