[PATCH] D88717: [obj2yaml] [yaml2obj] Add yaml support for SHT_LLVM_BB_ADDR_MAP section.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 26 23:41:43 PDT 2020


grimar added inline comments.


================
Comment at: llvm/test/tools/obj2yaml/ELF/bb-addr-map.yaml:18
+## Verify that the 'Address' field is omitted when it's zero.
+# VALID-NOT:  Address
+# VALID-NEXT:         BBEntries:
----------------
You don't need this line, since you are always using `-NEXT` and dump all output.


================
Comment at: llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml:3
+
+## Test the following cases"
+## 1) We can produce a .llvm_bb_addr_map section from a description with section
----------------
I'd move these  1-5 comments to YAML description though:

```
Sections:
## 1) We can produce a .llvm_bb_addr_map section from a description with section
##    content.
  - Name:    '.llvm_bb_addr_map (1)'
    Type:    SHT_LLVM_BB_ADDR_MAP
    Content: "000000000000000001010203"
## 2) We can produce an empty .llvm_bb_addr_map section from a description
##    swith empty section content.
  - Name:    '.llvm_bb_addr_map (2)'
    Type:    SHT_LLVM_BB_ADDR_MAP
...
```


================
Comment at: llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml:112
+##  Specify Content and Entries
+  - Name:    '.llvm_bb_addr_map (1)'
+    Type:    SHT_LLVM_BB_ADDR_MAP
----------------
You don't need `(1)` for this test.


================
Comment at: llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml:114
+    Type:    SHT_LLVM_BB_ADDR_MAP
+    Entries:
+      - BBEntries:
----------------
I think you should be able to write just `Entries: []`. No need to provide more input data than needed.
The same for Context: a single byte should be enough for this test.


================
Comment at: llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml:139
+            Metadata:         0x00000003
+    Size:    12
----------------
I'd merge last two YAMLs into one with the use of macros. I think something like the
following should work:


```
--- !ELF
FileHeader:
  Class: ELFCLASS64
  Data:  ELFDATA2LSB
  Type:  ET_EXEC
Sections:
##  Specify Content and Entries
  - Name:    '.llvm_bb_addr_map'
    Type:    SHT_LLVM_BB_ADDR_MAP
    Entries: []
    Content: [[CONTENT=<none>]]
    Size:    [[SIZE=<none>]]
```

And then you can:

```
not yaml2obj --docnum=2 -DCONTENT="00"...
not yaml2obj --docnum=2 -DSIZE=0...
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88717



More information about the llvm-commits mailing list