[PATCH] D65799: [yaml2obj/obj2yaml][MachO] Allow setting custom section data
Seiya Nuta via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 9 00:29:18 PDT 2019
seiya marked an inline comment as done.
seiya added inline comments.
================
Comment at: llvm/include/llvm/ObjectYAML/MachOYAML.h:203
static void mapping(IO &IO, MachOYAML::Section &Section);
+ static StringRef validate(IO &io, MachOYAML::Section &Section);
};
----------------
alexshap wrote:
> khm, to be honest, I would not place it here , these template specializations provide the static method "mapping" only. Somehow, I don't see where this method ("validate") is being used at the moment - maybe we can either delete it or hide it inside MachOYAML.cpp
`validate` is an optional method [[ https://github.com/llvm/llvm-project/blob/27ed1c5bb80c8da317cd80bdd50cb22189d1c6fb/llvm/include/llvm/Support/YAMLTraits.h#L63-L64 | in MappingTraits ]] and it's implemented in `MachOYAML.cpp`.
I think this should be here to validate obviously invalid input by YAMLTraits.h since it provides human-friendly error messages like:
```
YAML:30:9: error: Section size must be greater than or equal to the content size
- sectname: __data
^
yaml2obj: error: Failed to parse YAML input!
```
================
Comment at: llvm/lib/ObjectYAML/MachOYAML.cpp:294
+StringRef
+MappingTraits<MachOYAML::Section>::validate(IO &IO,
+ MachOYAML::Section &Section) {
----------------
Here.
================
Comment at: llvm/tools/obj2yaml/macho2yaml.cpp:21
+static bool isVirtualSection(uint8_t type) {
+ return (type == MachO::S_ZEROFILL || type == MachO::S_GB_ZEROFILL ||
----------------
jhenderson wrote:
> seiya wrote:
> > I'd like to move this function into somewhere else instead of copying from MachOEmitter.cpp but I couldn't locate the appropriate place. Any suggestions?
> Two suggestions:
> 1) MachOYAML.cpp/.h
> 2) BinaryFormat/MachO.h (assuming it could be useful in a wider context).
Moved into the BinaryFormat/MachO.h. Thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65799/new/
https://reviews.llvm.org/D65799
More information about the llvm-commits
mailing list