[PATCH] D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD

Mon Mar 11 11:15:55 PDT 2019

ruiu added a comment.

`--auto-cet` and `--force-cet` is perhaps fine. That should ensure users are aware of what they are doing (so the feature is not enabled too automatically), and it also provides a way to pin it so that it won't be accidentally turned off.

Was there any reason to choose that option names? H. J. Lu mentioned that GNU linkers supports several `-z` options for CET. If there's no strong reason to invent new option names, we should use the same command line options as GNU.

I think there are still several issues with this patch. I pointed out some of them before, but to summarize, let me iterate:

- Your patch collects `X86Feature1` bits from input files to copy that to a .note section in the output file. I don't think you need to do that. You only need to make sure that the input files have a desired bit in `X86Feature1` bits, and generate a .note section containing `GNU_PROPERTY_X86_FEATURE_1_IBT` bit. In other words, please remove `X86Feature1AND` from `Config`.
- You add fair amount of new code to Driver.cpp, but which feels wrong. It should not be part of a driver. But I think the real problem is that the code to read .note section does just too much.
- Do you really have to read a .note section containing multiple descriptions? I think that compiler-created .note section contains only one description per a section, so you don't need to care abou it.

Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D58102