[PATCH] D75833: [RISCV] Support RISC-V ELF attribute section in llvm-readobj

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Mar 28 18:16:18 PDT 2020


MaskRay added inline comments.


================
Comment at: llvm/lib/Support/ELFAttributeParser.cpp:132
 
-  // Ignore unrecognized vendor-name.
+  // Ignore unrecognized vendor name.
   if (vendorName.lower() != vendor)
----------------
I use vendor-name because

```
<format-version>
[ <section-length> "vendor-name"
      [ <file-tag> <size> <attribute>*
      | <section-tag> <size> <section-number>* 0 <attribute>*
      | <symbol-tag> <size> <symbol-number>* 0 <attribute>*
      ]+
]*
```

I hope this change can be dropped.


================
Comment at: llvm/lib/Support/ELFAttributeParser.cpp:150
       return createStringError(errc::invalid_argument,
-                               "invalid attribute size " + Twine(size) +
+                               "invalid subsection size " + Twine(size) +
                                    " at offset 0x" +
----------------
subsection is not very appropriate. The original `attribute` seems to capture the collection (file/section/symbol) better.


================
Comment at: llvm/lib/Support/ELFAttributeParser.cpp:220
       return createStringError(errc::invalid_argument,
-                               "invalid subsection length " +
+                               "invalid section size " +
                                    Twine(sectionLength) + " at offset 0x" +
----------------
`subsection`->`section` is fine, but `length`->`size` is not appropriate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75833





More information about the llvm-commits mailing list