[PATCH] D123902: [llvm-objcopy][ObjectYAML][mips] Add MIPS specific ELF section indexes

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/ObjectYAML/ELFYAML.cpp:769
+
+  if (!IO.outputting() || Object->getMachine() == ELF::EM_MIPS) {
+    ECase(SHN_MIPS_ACOMMON);
----------------
I'd like to see test cases in yaml2obj and obj2yaml showing that the MIPS-specific values are understood under the right circumstances only. I think extending obj2yaml\ELF\special-symbol-indices.yaml should be a minimum for that.


================
Comment at: llvm/test/tools/llvm-objcopy/ELF/mips-symbol.test:1
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objcopy %t %t2
----------------
You may also wish to rename and extend `llvm-objcopy\ELF\hexagon-unsupported-on-x86.test`, or alternatively, modify this test to cover that case, by attempting the same run, but with a different value for the Machine field (you can use yaml2obj's -D option to parameterise the field without needing to duplicate the document).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123902