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

Tue Mar 12 18:54:54 PDT 2019

xiangzhangllvm added a comment.

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

> Please keep in mind that "let's think about that later" is not a correct approach for command line option names. That's a public interface that cannot be changed once released. I think that is also true to gold. Even if the intention to add the `-z` options to gold was for testing, if the options have been added in a user-visible way, it is very unlikely that they'd be removed. So you need to think carefully as to what is the most consistent set of option names both for gold and lld once you implement all the features you want. That the situation that lld has `--{auto,force}-cet` and gold has `-z` options for the same functionality seems pretty inconsistent. Probably the importance of option names should be taken more seriously.

lld's `--{auto,force}-cet`  is different with gold's `-z` options, "-z" options directly add CET info into output files, not care the input files is CETed or not, they just designed for testing, it is not right to use them in normal build.
`--{auto,force}-cet` depend on the input files to enable CET or not.
The **key difference** is that gold auto enabled CET in default (equal with LLD's --auto-cet). 
I tended to keep it same with gold, but seems hard to persuade you ☺

> What I recommended is to always discard .note section after checking the GNU_PROPERTY_X86_FEATURE_1_IBT *bit*. There's no .note section merging or anything. When you parse a file, just read a single bit from a .note section (if exists), and discard it.
>  As a result, after you read all input files, you have only one bit -- which indicates whether or not all input files support CET.
>  Then, if CET is supported by all input files, create a synthetic section for a .note section to enable CET for the output.

OK, as peter's suggestion, I'll recheck here code, thank you！

> I'm skeptical if people actually hand-write a .note section in assembly. In particular, is it really normal or even correct to write a note section containing multiple descriptions in assembly by hand?

The assemble syntax support multiple descriptions.
And suppose that, one day, not only CET feature will put into .note.gnu.property, (now ABI have defined several other kind features such as GNU_PROPERTY_X86_ISA_1_USED ... ). So I think it will be normal to see one section with multiple descriptions (each description contains a different feature).

> When you enable the CET for an assembly file, you would pass a command line option to the assembler to let the assembler to add that section, no? I seems that you are overthinking. I'd just consider the case where there is only one note description in a .note section.

Yes, no, we write the .note.gnu.property in the assembly file not use options.

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

https://reviews.llvm.org/D58102