Add x86 feature with IBT and/or SHSTK bits to ELF program property if they are enabled. Otherwise, contents in this header file are unused.
This file is mainly design for assembly source code which want to enable CET
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
clang/test/CodeGen/asm-cet.S | ||
---|---|---|
2 | Please replace -DIBT -DSHSTK with -fcf-protection and also test i386-pc-linux. |
clang/lib/Headers/cet.h | ||
---|---|---|
18 | Hello, H.J., I not much understand here, | |
23 | So here you mean: ? | |
clang/test/CodeGen/asm-cet.S | ||
2 | You mean use "-fcf-protection" to define macro "CET" internally in compiler? |
clang/lib/Headers/cet.h | ||
---|---|---|
18 | Oh, I understand it, you want define _CET_ENDBR for endbr64/endbr32, let user to use it when he/she write the assembly code. |
clang/lib/Headers/cet.h | ||
---|---|---|
18 | User can use _CET_ENDBR unconditionally. You should a CET enabled assembly | |
23 | Yes. | |
clang/test/CodeGen/asm-cet.S | ||
2 | Yes, | |
6 | Add a function label and use _CET_ENDBR. You should check endbr32/endbr64. |
Good thing is that: CET is already defined to 1/2/3 according to -fcf-protection=xxx at front end, So we directly used it for preprocessor.
I want to put this patch to llvm10.0.1, could you help review and then accept this patch?
clang/lib/Headers/cet.h | ||
---|---|---|
16 | Move it after __CET__ block and define it only if it isn't defined. |
clang/test/CodeGen/asm-cet.S | ||
---|---|---|
17 | This seems not test anything. Maybe test as below code. |
clang/lib/Headers/cet.h | ||
---|---|---|
16 | I think It's better outside of CET block, because user can write the _CET_ENDBR in their assembly code in any case (CET enable or disable). | |
clang/test/CodeGen/asm-cet.S | ||
17 | Oh! yes, sorry for this mistake. |
clang/lib/Headers/cet.h | ||
---|---|---|
16 | You have #define _CET_ENDBR ... #define _CET_ENDBR endbr64 Don't clang complain like x.c:2:9: warning: '_CET_ENDBR' macro redefined [-Wmacro-redefined] #define _CET_ENDBR endbr64 ^ x.c:1:9: note: previous definition is here #define _CET_ENDBR ^ 1 warning generated. |
clang/lib/Headers/cet.h | ||
---|---|---|
16 | Yes, I tested it directly use the llvm-lit before, so I miss the warning information. I add "#ifndef _CET_ENDBR" this time now. |
clang/lib/Headers/cet.h | ||
---|---|---|
16 | I test it, It do not have warning. |
clang/lib/Headers/cet.h | ||
---|---|---|
16 | I retest it, It really has warning!! Add "#undef xxx" now. |
clang/lib/Headers/cet.h | ||
---|---|---|
16 | Should we remove line 15? If _CET_ENDBR is defined in another file, we should report warning. |
clang/lib/Headers/cet.h | ||
---|---|---|
16 | I think you should do #ifdef __CET__ ... #define _CET_ENDBR ... #endif #ifndef _CET_ENDBR #define _CET_ENDBR #endif If _CET_ENDBR is defined by programmer, there will be a warnng, |
! In D79617#2029228, @hjl.tools wrote:
Works for me.
Hello, H.J., could you help accept this patch, I hope it can go to llvm 10.0.1.
Thank you!!
Hello @rsmith if you feel OK too, could you help accept it. Thank you!
Hello every friends, I plan to commit it in these days, if you do not oppose it, thank you!
Did @rsmith ever approve this patch? I was following the discussion at llvm.org/PR45484 but did not see an explicit approval.
I would like a specification for this header to be added somewhere. We shouldn't be implementing random things with no specification. (Suppose someone claims that our <cet.h> is wrong in some way. How would we know whether they're right?)
Ideally, I'd also like this header to be installed somewhere where we look for assembler-with-cpp preprocessing but not for regular compilation; it doesn't make sense to me to pollute the header namespace for all C and C++ compilations with a header that is not meaningful in C and C++. But knowing whether that change is correct depends on having, you know, a specification.
I don't think we should be porting this to Clang 10. It seems clear that this is a new feature, not a bug fix.
<cet.h> from GCC is as close as you can get for a reference implementation/specification.
Hello rsmith,
first, very sorry for have committed this patch before your reply, I waited 10 days, I thought you have agreed it.
I think the linux-ABI can be the specification of this head file. The context of this cet.h is according to the linux ABI about CET.
We explained in which case we should use this cet.h file. (line 2-4)
tks
Move it after __CET__ block and define it only if it isn't defined.