[PATCH] D108825: [yaml2obj] Allow variable number of directories

[James Henderson via Phabricator via llvm-commits]

jhenderson added a subscriber: mstorsjo.
jhenderson added a comment.

@mstorsjo, do you have any particular thoughts on this patch at all?



================
Comment at: llvm/lib/ObjectYAML/COFFEmitter.cpp:474-477
+    if (NumDir < CP.Obj.OptionalHeader->Header.NumberOfRvaAndSize) {
+      OS << zeros(uint32_t(0));
+      OS << zeros(uint32_t(0));
+    }
----------------
Don't you need to repeat this multiple times potentially, in case the `NumberOfRvaAndSize` value is more than one bigger than the number of `DataDirectories`?


================
Comment at: llvm/test/tools/yaml2obj/COFF/variable-number-rva.yaml:1
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readobj --file-headers %t | FileCheck %s
----------------
This test case is insufficient. You need the following cases:

1) `NumberOfRvaAndSize` is unspecified (value is the default value).
2) `NumberOfRvaAndSize` is specified and matches the default value.
3) `NumberOfRvaAndSize` is specified and is lower than the default value (need to check that the relevant DataDirectory content is not written too).
4) `NumberOfRvaAndSize` is specified and is higher (more than 1 higher, preferably) than the default value (need to show that the additional zeros are written too).

You may find it useful to use the yaml2obj preprocessor-like functionality. This allows you to specify `-D VAR=0`, and yaml2obj will replace patterns in the input like the following before processing the document:
```
# RUN: yaml2obj ... -DVAR=0

---! COFF
<snip>
NumberOfRvaAndSize = [[VAR]]
```
becomes
```
NumberOfRvaAndSize = 0
```
You can also specify a default value using the following: `NumberOfRvaAndSize = [[VAR=0]]` which would use the value 0 if VAR is not defined on the command-line.

Finally, I believe, but am not certain, you may be able to use the pattern `NumberOfRvaAndSize = [[VAR=<none>]]`, as NumberOfRvaAndSize is a default option. In this case, if VAR is not defined on the command-line, it is as if NumberOfRvaAndSize does not appear in the YAML document and therefore a default value is used (i.e. test case 1 above).

By using the -D option, you should be able to use the same YAML doc in all test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108825