[PATCH] D69188: [llvm-objcopy] Preserve .ARM.attributes section when stripping files

Mon Oct 28 02:45:45 PDT 2019

thieta added a comment.

I totally defer to you guys on if this is something you want to accept or not. We are now patching our own toolchain with this patch to not create bad binaries. If I where to make a case for accepting this patch it would be something like:

Stripping with llvm-strip without any arguments currently creates binaries that doesn't work on any debian derriative distribution on arm32. And while yes this is totally a bug in a patch they ship, the upstream have had a bug opened against this for over 2 years without even triaging it, so I don't think we can see it be removed for a long time. What's worse is that the error is really hard to spot / understand - it just gives feedback that the shared object can't be loaded and it lead us to troubleshoot for hours to find the culprit.

The downside to accepting this is that we add around 100 bytes to files for other dists that doesn't really need it. Personally I think that's a small price to pay to get working binaries.

One option could be to add a new option that more aggressively removes things that can break compatibility and document those (could even drop .gnu.warnings). Or make `--strip-all-gnu` the default and `--strip-all` something you opt into.

>From my point of view I think argument less `llvm-strip` should retain high compatibility and only people that know what they are doing can opt for more aggressive removal.

But as stated above - I won't be crushed if this patch doesn't make it to the mainline. I think maybe the incompatibility should be documented in that case so it can be easier to find in the future without a lot of troubleshooting.

Thanks again for taking the time to review my first attempt at a LLVM patch!

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69188/new/

https://reviews.llvm.org/D69188