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

Mon Oct 21 10:46:17 PDT 2019

MaskRay added inline comments.

================
Comment at: llvm/test/tools/llvm-objcopy/ELF/strip-preserve-arm-attributes.test:1
+# RUN: yaml2obj %s > %t
+# RUN: cp %t %t3
----------------
We need a `## ` file-level comments that explains the purpose of the test. You can find many examples in the same directory.

================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:506
         return false;
+      // While keeping this section around shouldn't be needed
+      // there are older version of glibc and eglibc that requires
----------------
We should document the upstream glibc bug (as an example, put a link `http://sourceware.org/PR16773`) and the first version of glibc that fixed the bug, so that we can retire the exemption at some point (I think we usually don't care about toolchains that are more than 5 years old).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69188