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

Mon Mar 11 18:33:42 PDT 2019

xiangzhangllvm added a comment.

In D58102#1424938 <https://reviews.llvm.org/D58102#1424938>, @ruiu wrote:

> `--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.

Gold auto enabled CET in default, so it have no options corresponding to the  `--auto-cet` and `--force-cet`.
The '-z' options in gold is just for testing, For Simplify，It better to add them in a dependent patch.

> 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`.

Yes, ruiu, last time I also ask you about this point. please let me summarize your ideas here, then please check if I understand right?

1. If not all input files have `X86Feature1` bits, then discard the .note.gnu.property sections.
2. If all input files have `X86Feature1` bit**s**: then we need to 'calculate' the resulting `X86Feature1` bit**s**  from all the  `X86Feature1` bits ?           (yes or no ?) then delete all the input files' .note.gnu.property sections ?                                                                       (yes or no ?) then generate a .note.gnu.property section with the resulting `X86Feature1` bits  in input file level not in output file by Writer, so we need not to pass the `X86Feature1` bits to Writer ?                                                                          (yes or no ?)

> - 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 about it.

Yes, the compiler-created .note section contains only one description per a section, But we should consider the handwriting assemble files.

Thank you for your careful review!

Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D58102