[PATCH] D70606: LLD: CET shadow stack support on Windows

Thu Feb 13 01:35:53 PST 2020

jhenderson added a comment.

In D70606#1873185 <https://reviews.llvm.org/D70606#1873185>, @penzn wrote:

> Expanding on my previous comment. In its current state `yaml2obj` is not aware of the section contents, and I think adding this functionality would be a bit more involved than the code we have in this review so far.
>
> Quick rundown of what would be needed for `yaml2obj` change:
>
> - Define YAML syntax for debug entries
> - Support debug directory and its contents inside section mapping in ObjectYAML
>   - Need to represent the structure from Debug section down to individual debug records
> - Change other tests which depend on debug directory structure
>
>   We should probably discuss details. For example, can debug directory support in ObjectYAML take inspiration from LLD or would that make testing moot? Would ObjectYAML need any tests for this new functionality?
>
>   Do you mind if we move ObjectYAML discussion elsewhere? What would be a good place for it, llvm-dev?
>
>   As for this change, another option is to use `yaml2obj`, but output debug directory as a hex section in the YAML file. However this would inconsistent with `codeview` tests, maybe it would be better to change everything at once when `yaml2obj` is ready.

I'm not sure I really know enough about the intricacies here to provide good feedback. I do think splitting out any yaml2obj functionality into a separate change makes sense though. When it comes to using a hex section/adding yaml2obj support for that section type, my opinion is that if the section only needs to be a few bytes long, it's fine, but if it's more than that and the hex contents are essentially unreadable, then there's no benefit in using yaml2obj at all. If you want to continue the discussion separately, llvm-dev is probably the best place for it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70606