[PATCH] D129337: [llvm-objcopy][ELF] Add --set-section-type

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp:686-688
+      auto It2 = Config.SetSectionType.find(Sec.Name);
+      if (It2 != Config.SetSectionType.end())
+        Sec.Type = It2->second;
----------------
jhenderson wrote:
> As --set-section-flags can potentially change the type, we might want to add a specific test to document the behaviour that --set-section-type overrides whatever --set-section-flags does.
Added a test `## --set-section-flags does not specify "readonly", so the output gets SHF_WRITE.`


================
Comment at: llvm/test/tools/llvm-objcopy/ELF/set-section-type.test:20
+
+## sh_type is an uint32_t. There is no diagnostic for an overflow value.
+# RUN: llvm-objcopy --set-section-type=.foo=0x10000000a %t %t.2 2>&1 | count 0
----------------
jhenderson wrote:
> I pronounce `uint32_t` as "you-int-thir-ty-two-tee", which would mean it should be "a" not "an". I'm curious if you say it differently?
I say "you-int-thir-ty-two-tee", too... Fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129337