[PATCH] D63267: [yaml2obj] - Allow setting custom section types for implicit sections.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 14 03:42:54 PDT 2019


grimar added inline comments.


================
Comment at: test/tools/yaml2obj/elf-symtab-shinfo.yaml:27-37
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
 Sections:
-  - Name:          .symtab
-    Info:          42
-    Type:          SHT_SYMTAB
-  - Name:          .dynsym
-    Info:          26
-    Type:          SHT_SYMTAB
+  - Name: .symtab
+    Info: 42
----------------
jhenderson wrote:
> This change doesn't seem related?
It is related. This test has a following check lines:

```
# CHECK:      Name: .dynsym
# CHECK-NEXT: Type: SHT_DYNSYM
```

We set SHT_DYNSYM by default before this patch.
But YAML seems contains `SHT_SYMTAB` by mistake.

This patch revealed this errors in this and other test cases, so I fixed them.


================
Comment at: test/tools/yaml2obj/explicit-dynsym-no-dynstr.yaml:22
   - Name: .dynsym
-    Type: SHT_SYMTAB
+    Type: SHT_DYNSYM
----------------
jhenderson wrote:
> Unrelated?
The same.


================
Comment at: tools/yaml2obj/yaml2elf.cpp:471
   SHeader.sh_name = DotShStrtab.getOffset(Name);
-  SHeader.sh_type = ELF::SHT_STRTAB;
+  SHeader.sh_type = YAMLSec ? (uint32_t)YAMLSec->Type : ELF::SHT_STRTAB;
   SHeader.sh_addralign = YAMLSec ? (uint64_t)YAMLSec->AddressAlign : 1;
----------------
jhenderson wrote:
> Is the cast needed? It isn't above.
Nope. Removed.


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

https://reviews.llvm.org/D63267





More information about the llvm-commits mailing list