[PATCH] D65799: [yaml2obj/obj2yaml][MachO] Allow setting custom section data

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Have you rebased recently? I feel like the content in yaml2macho.cpp may have moved to the ObjectYAML library recently.



================
Comment at: llvm/lib/Object/MachOObjectFile.cpp:1948
 
+ArrayRef<uint8_t> MachOObjectFile::getSectionContents(uint32_t Offset,
+                                                      uint64_t Size) const {
----------------
A 32-bit Offset but 64-bit Size seems off to me. What are the limitations on the two fields in Mach-O's file format?


================
Comment at: llvm/test/ObjectYAML/MachO/section_data.yaml:1
+## Case 1: The size of content is greater than the section size.
+# RUN: not yaml2obj --docnum=1 %s -o %t1 2>&1 | FileCheck %s --check-prefix=CASE1
----------------
Probably worth a header comment summarizing the purpose of the test, e.g. "Show that yaml2obj supports custom section data for Mach-O YAML inputs."


================
Comment at: llvm/test/ObjectYAML/MachO/section_data.yaml:3
+# RUN: not yaml2obj --docnum=1 %s -o %t1 2>&1 | FileCheck %s --check-prefix=CASE1
+--- !mach-o
+FileHeader:
----------------
Nit: add a blank line before the YAML.


================
Comment at: llvm/test/ObjectYAML/MachO/section_data.yaml:40
+
+# CASE1: error: Section size must be greater than or equal to the content size
+
----------------
Move the check before the yaml. Flow should be:


```
## Comment
# RUN:
# CHECK:
Yaml

## Comment
# RUN:
# CHECK:
Yaml
```


================
Comment at: llvm/test/ObjectYAML/MachO/section_data.yaml:42
+
+## Case 2: The size of content equals to the section size.
+# RUN: yaml2obj --docnum=2 %s > %t2
----------------
The size of content -> The content size

equals to -> equals


================
Comment at: llvm/test/ObjectYAML/MachO/section_data.yaml:101
+
+## Case 3: The size of content is less than the section size. In this case, the area
+## after the custom content are filled by zero.
----------------
size of content -> content size



================
Comment at: llvm/test/ObjectYAML/MachO/section_data.yaml:102
+## Case 3: The size of content is less than the section size. In this case, the area
+## after the custom content are filled by zero.
+# RUN: yaml2obj --docnum=3 %s > %t3
----------------
are filled -> is filled
by zero -> with zeroes


================
Comment at: llvm/test/ObjectYAML/MachO/virtual_section.yaml:178
 # CHECK-NEXT:        reserved3:       0x00000000
-# CHECK-NEXT:      - sectname:        __data
+# CHECK:           - sectname:        __data
 # CHECK-NEXT:        segname:         __DATA
----------------
Why are these no longer CHECK-NEXT? Should you be testing the new stuff before it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65799