[PATCH] D68333: [yaml2obj/obj2yaml] - Add support for SHT_LLVM_ADDRSIG sections.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: include/llvm/ObjectYAML/ELFYAML.h:260-262
+  AddrsigSymbol(StringRef N) : Name(N), Index(None){};
+  AddrsigSymbol(llvm::yaml::Hex32 Ndx) : Name(None), Index(Ndx){};
+  AddrsigSymbol() : Name(None), Index(None){};
----------------
MaskRay wrote:
> jhenderson wrote:
> > clang-format?
> `{};` -> ` {}`
(It was formatted.. but because of excessive ';' which I've removed, formatting did't add a space between ')' and '{').


================
Comment at: test/tools/yaml2obj/elf-llvm-addrsig-section.yaml:117
+
+## Check that maximum "Symbol"->"Index" size is 32 bits.
+
----------------
jhenderson wrote:
> I'm not really sure what `"Symbol"->"Index"` is trying to say. Perhaps it could be written as follows: "Check that the maximum symbol index size is 32 bits."
> 
> This actually shows that an error is produced if the value is out of range. You should have a separate case without the error, using the maximum possible value.
> This actually shows that an error is produced if the value is out of range. You should have a separate case without the error, using the maximum possible value.

I've included it to `SYMBOL-INDEX` test.


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

https://reviews.llvm.org/D68333