Page MenuHomePhabricator

xiangzhangllvm (Xiang Zhang)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 25 2018, 11:14 PM (55 w, 4 d)

Recent Activity

Tue, Jan 14

xiangzhangllvm added a comment to D59780: Support Intel Control-flow Enforcement Technology.

Thank you again!!

Tue, Jan 14, 12:07 AM · Restricted Project

Mon, Jan 13

xiangzhangllvm added a comment to D59780: Support Intel Control-flow Enforcement Technology.

LGTM

Mon, Jan 13, 10:53 PM · Restricted Project
xiangzhangllvm added a comment to D59780: Support Intel Control-flow Enforcement Technology.

ping @ruiu Hi, Do you have time to review? Thanks!

Mon, Jan 13, 5:27 PM · Restricted Project

Fri, Jan 10

xiangzhangllvm added inline comments to D59780: Support Intel Control-flow Enforcement Technology.
Fri, Jan 10, 3:22 AM · Restricted Project

Thu, Jan 9

xiangzhangllvm added inline comments to D59780: Support Intel Control-flow Enforcement Technology.
Thu, Jan 9, 6:40 PM · Restricted Project

Wed, Jan 1

xiangzhangllvm added a comment to D71917: [X86] Optimization of inserting vxi1 sub vector into vXi1 vector .

update the patch, and I tested in my local performance test, it really have a little small improvement.

Wed, Jan 1, 5:51 PM · Restricted Project
xiangzhangllvm updated the diff for D71917: [X86] Optimization of inserting vxi1 sub vector into vXi1 vector .
Wed, Jan 1, 5:43 PM · Restricted Project

Sun, Dec 29

xiangzhangllvm updated the diff for D71917: [X86] Optimization of inserting vxi1 sub vector into vXi1 vector .
Sun, Dec 29, 5:51 PM · Restricted Project
xiangzhangllvm added inline comments to D71917: [X86] Optimization of inserting vxi1 sub vector into vXi1 vector .
Sun, Dec 29, 5:33 PM · Restricted Project

Thu, Dec 26

xiangzhangllvm created D71917: [X86] Optimization of inserting vxi1 sub vector into vXi1 vector .
Thu, Dec 26, 6:05 PM · Restricted Project

Dec 12 2019

xiangzhangllvm added a comment to D59780: Support Intel Control-flow Enforcement Technology.

For MPX prefix:
GCC have not supported the MPX from GCC 9. And intel will not support MPX code too. So we don’t consider MPX for CET in LLD.

I know that GCC has removed MPX and the Linux kernel is removing MPX (user-visible APIs and self-tests have been removed). I asked because I haven't seen a change on binutils-gdb side that will support a .plt.sec scheme without the BND prefix. So, I wonder what kind of changes are considered divergence from x86-64 psABI. After the removal of the BND prefix, the .plt entry will get the leeway of 2 bytes. If, say, in the future, a new security enhanced feature is proposed which requires a new instruction that will take more than 2 bytes, the 16-byte .plt entry no longer works, and toolchains will have to migrate a third PLT scheme, different from traditional PLT and the .plt.sec scheme.

As to the option name question, are you happy with -z force-ibt and -z shstk? (My understanding is that they should be very similar to -z force-bti and -z pac-plt, respectively.)

Dec 12 2019, 6:43 PM · Restricted Project

Dec 11 2019

xiangzhangllvm added a comment to D59780: Support Intel Control-flow Enforcement Technology.

For MPX prefix:
GCC have not supported the MPX from GCC 9. And intel will not support MPX code too. So we don’t consider MPX for CET in LLD.

Dec 11 2019, 11:48 PM · Restricted Project
xiangzhangllvm added a comment to D59780: Support Intel Control-flow Enforcement Technology.

Apply two patches

-  uint64_t Plt = In.Plt->getVA();
+  uint64_t Plt = In.IBTPlt ? In.IBTPlt->getVA() : In.Plt->getVA();
--- i/lld/ELF/Driver.cpp
+++ w/lld/ELF/Driver.cpp
@@ -1705,2 +1705,4 @@ template <class ELFT> static uint32_t getAndFeatures() {
-    } else if (!features && config->requireCET)
-      error(toString(f) + ": --require-cet: file is not compatible with CET");
+    } else if (config->requireCET && !(features & GNU_PROPERTY_X86_FEATURE_1_IBT)) {
+      warn(toString(f) + ": --require-cet: file is not compatible with CET");
+      features |= GNU_PROPERTY_X86_FEATURE_1_IBT;
+    }

@xiangzhangllvm The previous version I uploaded definitely did not work. I just applied a fix. Both i386 and x86_64 seem to work now, but there are still problems with the tests.

You need either arc patch D59780 or curl -L 'https://reviews.llvm.org/D59780?download=true' | patch -p1 to apply the latest diff in your local repository.

I have a separate patch to rename lld --force-bti to -z force-bti. I think --force-ibt or -z force-ibt may make more sense than --require-cet.

Dec 11 2019, 6:07 PM · Restricted Project

Dec 10 2019

xiangzhangllvm added a comment to D59780: Support Intel Control-flow Enforcement Technology.

Remove the "-plugin xxx", also work.

Dec 10 2019, 11:37 PM · Restricted Project
xiangzhangllvm added a comment to D59780: Support Intel Control-flow Enforcement Technology.
> gcc -fcf-protection=full -c a.c
> gcc -fcf-protection=full a.c -o a '-###'   # Retrieve linker command line, replace ld with
Dec 10 2019, 11:28 PM · Restricted Project
xiangzhangllvm added inline comments to D59780: Support Intel Control-flow Enforcement Technology.
Dec 10 2019, 9:47 PM · Restricted Project
xiangzhangllvm added a comment to D59780: Support Intel Control-flow Enforcement Technology.

This patch is outdated and needs rebasing. I'll rebase, upload it again and then submit.

Hi ruiu, just remind, if you have no time, I can rebase it.

I can do that, too, and cleaning the test.

Dec 10 2019, 5:23 PM · Restricted Project
xiangzhangllvm added a comment to D59780: Support Intel Control-flow Enforcement Technology.

This patch is outdated and needs rebasing. I'll rebase, upload it again and then submit.

Dec 10 2019, 4:46 PM · Restricted Project

Nov 14 2019

xiangzhangllvm added a comment to D70157: Align branches within 32-Byte boundary(NOP padding).

On x86, the preferred function alignment is 16 (https://github.com/llvm/llvm-project/blob/arcpatch-D70157/llvm/lib/Target/X86/X86ISelLowering.cpp#L1893), which is the default function alignment in text sections. If the cross-boundary decision is made with alignment=32 (--x86-align-branch-boundary=32) in mind, and the section alignment is still 16 (not increased to 32 or higher), the linker may place the section at an address which equals 16 modulo 32, the section contents will thus shift by 16. The instructions that do not cross the boundary in the object files may cross the boundary in the linker output. Have you considered increasing the section alignment to 32?

Shall we default to -mbranches-within-32B-boundaries if the specified -march= or -mtune= may be affected by the erratum?

Nov 14 2019, 5:49 PM · Restricted Project, Restricted Project

Nov 12 2019

xiangzhangllvm added inline comments to D70157: Align branches within 32-Byte boundary(NOP padding).
Nov 12 2019, 7:35 PM · Restricted Project, Restricted Project
xiangzhangllvm added inline comments to D70157: Align branches within 32-Byte boundary(NOP padding).
Nov 12 2019, 7:25 PM · Restricted Project, Restricted Project

Aug 10 2019

xiangzhangllvm updated the diff for D65840: [X86] Support -march=tigerlake.
Aug 10 2019, 1:20 AM · Restricted Project
xiangzhangllvm added inline comments to D65840: [X86] Support -march=tigerlake.
Aug 10 2019, 1:19 AM · Restricted Project
xiangzhangllvm updated the diff for D65840: [X86] Support -march=tigerlake.
Aug 10 2019, 12:08 AM · Restricted Project
xiangzhangllvm added a comment to D65840: [X86] Support -march=tigerlake.

Done again, thank you again!!

Aug 10 2019, 12:08 AM · Restricted Project
xiangzhangllvm added inline comments to D65840: [X86] Support -march=tigerlake.
Aug 10 2019, 12:00 AM · Restricted Project

Aug 9 2019

xiangzhangllvm added a comment to D65840: [X86] Support -march=tigerlake.

All changed, Thank you very much!

Aug 9 2019, 2:04 AM · Restricted Project
xiangzhangllvm updated the diff for D65840: [X86] Support -march=tigerlake.
Aug 9 2019, 2:03 AM · Restricted Project

Aug 8 2019

xiangzhangllvm added inline comments to D65840: [X86] Support -march=tigerlake.
Aug 8 2019, 7:40 PM · Restricted Project

Aug 6 2019

xiangzhangllvm retitled D65840: [X86] Support -march=tigerlake from Tigerlake to [X86] Support -march=tigerlake.
Aug 6 2019, 6:22 PM · Restricted Project
xiangzhangllvm created D65840: [X86] Support -march=tigerlake.
Aug 6 2019, 6:11 PM · Restricted Project

May 30 2019

xiangzhangllvm added a comment to D62367: [X86] VP2INTERSECT clang.

Done, Thank you very much!

May 30 2019, 8:02 PM · Restricted Project, Restricted Project
xiangzhangllvm updated the diff for D62367: [X86] VP2INTERSECT clang.
May 30 2019, 7:57 PM · Restricted Project, Restricted Project
xiangzhangllvm added a reviewer for D62367: [X86] VP2INTERSECT clang: smaslov.
May 30 2019, 6:38 PM · Restricted Project, Restricted Project
xiangzhangllvm added a reviewer for D62366: [X86] VP2INTERSECT llvm: smaslov.
May 30 2019, 6:36 PM · Restricted Project
xiangzhangllvm added a comment to D62366: [X86] VP2INTERSECT llvm.

Hi Dear friends, could you help merge this patch? Thank you very much!

May 30 2019, 6:31 PM · Restricted Project
xiangzhangllvm added a comment to D62367: [X86] VP2INTERSECT clang.

Hi Dear friends, could you help merge this patch? Thank you very much!

May 30 2019, 6:31 PM · Restricted Project, Restricted Project
xiangzhangllvm updated the diff for D62366: [X86] VP2INTERSECT llvm.

rebase

May 30 2019, 6:28 PM · Restricted Project
xiangzhangllvm updated the diff for D62367: [X86] VP2INTERSECT clang.

rebase

May 30 2019, 6:28 PM · Restricted Project, Restricted Project

May 28 2019

xiangzhangllvm added a comment to D59780: Support Intel Control-flow Enforcement Technology.

I'm sorry, I reflect the problem to some leaders and experts, but the PLT ABI is still undecided. The most concern come from the unknow risk of difference with GNU.

@xiangzhangllvm If it is still undecided, then there is still opportunity to fix :) IMHO tooling is still immature and currently very few tools understand .plt.sec.

I'd really like to see more discussions about this, e.g. https://groups.google.com/forum/#!topic/x86-64-abi/Z6GqQDy_NaI
If PLT entries should be aligned by 16 to maximize performance, shall we switch to 32-byte PLT entry? Xiang, can you raise a topic on https://sourceware.org/ml/gnu-gabi/2019-q2 or https://sourceware.org/ml/binutils/2019-05/

May 28 2019, 8:09 PM · Restricted Project
xiangzhangllvm added a comment to D59780: Support Intel Control-flow Enforcement Technology.

I'm sorry, I reflect the problem to some leaders and experts, but the PLT ABI is still undecided. The most concern come from the unknow risk of difference with GNU.

May 28 2019, 7:01 PM · Restricted Project
xiangzhangllvm updated the diff for D62367: [X86] VP2INTERSECT clang.

rebase

May 28 2019, 6:26 PM · Restricted Project, Restricted Project
xiangzhangllvm updated the diff for D62366: [X86] VP2INTERSECT llvm.

rebase

May 28 2019, 6:22 PM · Restricted Project
xiangzhangllvm added inline comments to D62367: [X86] VP2INTERSECT clang.
May 28 2019, 4:41 AM · Restricted Project, Restricted Project

May 27 2019

xiangzhangllvm updated the diff for D62366: [X86] VP2INTERSECT llvm.
May 27 2019, 11:59 PM · Restricted Project
xiangzhangllvm updated the diff for D62367: [X86] VP2INTERSECT clang.
May 27 2019, 11:59 PM · Restricted Project, Restricted Project
xiangzhangllvm added inline comments to D62367: [X86] VP2INTERSECT clang.
May 27 2019, 10:40 PM · Restricted Project, Restricted Project
xiangzhangllvm updated the diff for D62367: [X86] VP2INTERSECT clang.
May 27 2019, 9:54 PM · Restricted Project, Restricted Project
xiangzhangllvm updated the diff for D62366: [X86] VP2INTERSECT llvm.
May 27 2019, 9:52 PM · Restricted Project

May 24 2019

xiangzhangllvm updated the diff for D62366: [X86] VP2INTERSECT llvm.

Rename tests in test/CodeGen/X86

May 24 2019, 1:40 AM · Restricted Project
xiangzhangllvm updated the diff for D62366: [X86] VP2INTERSECT llvm.

rebase

May 24 2019, 1:12 AM · Restricted Project
xiangzhangllvm updated the diff for D62367: [X86] VP2INTERSECT clang.

rebase

May 24 2019, 1:11 AM · Restricted Project, Restricted Project

May 23 2019

xiangzhangllvm created D62367: [X86] VP2INTERSECT clang.
May 23 2019, 11:55 PM · Restricted Project, Restricted Project
xiangzhangllvm created D62366: [X86] VP2INTERSECT llvm.
May 23 2019, 11:54 PM · Restricted Project

May 21 2019

xiangzhangllvm added a reviewer for D61881: Deal with return-twice function such as vfork, setjmp when CET-IBT enabled: pengfei.
May 21 2019, 5:20 PM · Restricted Project

May 20 2019

xiangzhangllvm added inline comments to D62115: fix a issue that clang is incompatible with gcc with -H option..
May 20 2019, 12:14 AM · Restricted Project

May 16 2019

xiangzhangllvm updated the diff for D61881: Deal with return-twice function such as vfork, setjmp when CET-IBT enabled.
May 16 2019, 7:14 PM · Restricted Project
xiangzhangllvm added inline comments to D61881: Deal with return-twice function such as vfork, setjmp when CET-IBT enabled.
May 16 2019, 6:28 PM · Restricted Project
xiangzhangllvm added inline comments to D61881: Deal with return-twice function such as vfork, setjmp when CET-IBT enabled.
May 16 2019, 6:22 PM · Restricted Project

May 15 2019

xiangzhangllvm updated the diff for D61881: Deal with return-twice function such as vfork, setjmp when CET-IBT enabled.
May 15 2019, 7:36 PM · Restricted Project
xiangzhangllvm added inline comments to D61881: Deal with return-twice function such as vfork, setjmp when CET-IBT enabled.
May 15 2019, 6:42 PM · Restricted Project

May 14 2019

xiangzhangllvm added a comment to D61881: Deal with return-twice function such as vfork, setjmp when CET-IBT enabled.

Thank You very much! Fangrui

May 14 2019, 7:58 PM · Restricted Project
xiangzhangllvm updated the diff for D61881: Deal with return-twice function such as vfork, setjmp when CET-IBT enabled.
May 14 2019, 7:55 PM · Restricted Project
xiangzhangllvm added inline comments to D61881: Deal with return-twice function such as vfork, setjmp when CET-IBT enabled.
May 14 2019, 6:02 PM · Restricted Project

May 13 2019

xiangzhangllvm created D61881: Deal with return-twice function such as vfork, setjmp when CET-IBT enabled.
May 13 2019, 11:00 PM · Restricted Project

Apr 18 2019

xiangzhangllvm added a comment to D59780: Support Intel Control-flow Enforcement Technology.

Hi friends, I write a simple test to test the performance:
To simply test the performance affect I changed the PLT size from 16 Bytes to 24Bytes in LLVM 8.0.0.

[xiangzh1@scels74 /export/iusers/xiangzh1/LLVM/LLVMORG]$diff llvm800/tools/lld/ELF/Arch/X86_64.cpp llvm800-PLT24/tools/lld/ELF/Arch/X86_64.cpp
67c67
<   PltEntrySize = 16;
---
>   PltEntrySize = 24; //16-->24
157a158,159
>       0x66, 0x90,                   //nop
>       0x66, 0x0f, 0x1f, 0x44, 0, 0, // nop
[xiangzh1@scels74 /export/iusers/xiangzh1/LLVM/LLVMORG]$

Then I write a simple case like following:

Apr 18 2019, 8:20 PM · Restricted Project

Apr 17 2019

xiangzhangllvm added a comment to D59780: Support Intel Control-flow Enforcement Technology.

We are Ok with with the 2-PLT scheme.

Apr 17 2019, 6:20 PM · Restricted Project

Apr 16 2019

xiangzhangllvm added a comment to D59780: Support Intel Control-flow Enforcement Technology.

Hi Ruiu & Fangrui
Your idea is also make sense, but It is really too too late to change ABI now.
I wish we can reach a consensus as soon as possible.
Thank you all, very much!

Apr 16 2019, 1:11 AM · Restricted Project

Apr 2 2019

xiangzhangllvm added a comment to D59780: Support Intel Control-flow Enforcement Technology.

Hi MaskRay, I tested your idea by changing the patch:

Apr 2 2019, 7:48 PM · Restricted Project

Apr 1 2019

xiangzhangllvm added inline comments to D59780: Support Intel Control-flow Enforcement Technology.
Apr 1 2019, 4:22 AM · Restricted Project
xiangzhangllvm added inline comments to D59780: Support Intel Control-flow Enforcement Technology.
Apr 1 2019, 1:09 AM · Restricted Project

Mar 31 2019

xiangzhangllvm added inline comments to D59780: Support Intel Control-flow Enforcement Technology.
Mar 31 2019, 8:35 PM · Restricted Project

Mar 28 2019

xiangzhangllvm added inline comments to D59780: Support Intel Control-flow Enforcement Technology.
Mar 28 2019, 10:09 PM · Restricted Project

Mar 26 2019

xiangzhangllvm added inline comments to D59780: Support Intel Control-flow Enforcement Technology.
Mar 26 2019, 7:16 PM · Restricted Project
xiangzhangllvm added a comment to D59780: Support Intel Control-flow Enforcement Technology.

So on the compiler driver side, -fcf-protection is the single option end users are concerned. I believe this option hasn't been assigned linker option semantics in either GCC or clang. If assigning it the linker option semantics is favorable, we should make it compatible in GCC/clang and ideally use the same options in lld/ld.bfd/gold.

The current form of this patch just implements --force-cet, thus I believe the use cases are:

gcc a.o -o a # create `.splt` (or `.plt.sec`) if all of a.o and other stdlib have IBT bit set
             # I worry that someone may not want .splt for this case
  ld.lld a.o ... -o a

If all the files contain IBT, it means all the files compiled with -fcf-protection(=branch) ,there is no reason for user not to enable the IBT.

Mar 26 2019, 6:53 PM · Restricted Project
xiangzhangllvm added a comment to D59780: Support Intel Control-flow Enforcement Technology.

Is --force-cet the best option name? H. J. Lu pointed out that gold has -z ibtplt and other options, but I couldn't find these options in the binutils' repository. Is the option really implemneted to gold?

The binutils-gdb repository hosts two linkers: ld/ld-new (GNU ld) and gold/ld-new (GNU gold). -z ibtplt and -z ibt are ld.bfd options, not gold's. I asked in https://reviews.llvm.org/D58102 if there is a proposal to add support to gold but get no response so far...

I've done some experiments and let me clarify things. @xiangzhangllvm Please correct me I missed something.

  1. On the compiler side

    Support for Intel CET requires GCC 8 (https://gcc.gnu.org/gcc-8/changes.html).

    -fcf-protection= is used to define __CET__ (branch: 1, return: 2, full: 3) and emit .note.gnu.property in object files.

    In some older releases of GCC 8 (before Apr 20, 2018), CET requires two other options -mibt and -mshstk, which enable the macros __IBT__ __SHSTK__, respectively. -mcet implies both -mibt and -mshstk and defines __CET__. -mshstk enables several builtin functions.

    -mibt is deleted in some newer GCC 8 release. -mshstk controls only the availability of some builtin options. If you are using some older releases of GCC 8:

    ` gcc a.c -mibt -c # __IBT__ is defined but no .note.gnu.property gcc a.c -fcf-protection=branch -mibt -c # NT_GNU_PROPERTY_X86_FEATURE_IBT tag gcc a.c -fcf-protection=return -mshstk -c # NT_GNU_PROPERTY_X86_FEATURE_SHSTK tag gcc a.c -fcf-protection=full -mcet -c # -mcet is short for -mibt -mshstk, IBT + SHSTK `

    In newer GCC, forget about -mibt -mshstk and just use -fcf-protection=. All these options do no affect codegen.
  2. On the linker side

    ibt is what concerns linkers. shstk has no influence other than the output note section. The feature is only implemented in ld.bfd, not in gold.

    -z ibtplt generates the second PLT .plt.sec (not .splt). -z ibt implies -z ibtplt. The option does not check the input NT_GNU_PROPERTY_X86_FEATURE_IBT tag.

    -z ibt emits NT_GNU_PROPERTY_X86_FEATURE_IBT but plain -z ibtplt doesn't.

    GCC 5~8 support Intel MPX and you shall see the bnd prefix in some jump instructions. Note that GCC 9/Linux kernel 4.18 drop MPX. Fortunately it seems MPX doesn't change the size of PLT entries. Thus in lld we don't have to support MPX, but we are still flexible enough to add related support if it revives in the future.
Mar 26 2019, 6:11 PM · Restricted Project
xiangzhangllvm added inline comments to D59780: Support Intel Control-flow Enforcement Technology.
Mar 26 2019, 12:55 AM · Restricted Project

Mar 25 2019

xiangzhangllvm added inline comments to D59780: Support Intel Control-flow Enforcement Technology.
Mar 25 2019, 9:40 PM · Restricted Project
xiangzhangllvm added a comment to D59780: Support Intel Control-flow Enforcement Technology.
Is --force-cet the best option name? H. J. Lu pointed out that gold has -z ibtplt and other options, but I couldn't find these options in the binutils' repository. Is the option really implemneted to gold?
What is the compiler flag to enable CET? I'd like to write it in a comment for reference.

Hi, Ruiu, You may need update ld to 2.31+ version:
xiangzh1@xiangzh1:~$ ld -v
GNU ld (GNU Binutils) 2.32
xiangzh1@xiangzh1:~$ ld --help
elf_x86_64:

 ...
-z ibtplt                   Generate IBT-enabled PLT entries
-z ibt                      Generate GNU_PROPERTY_X86_FEATURE_1_IBT
-z shstk                    Generate GNU_PROPERTY_X86_FEATURE_1_SHSTK

All these options in gold is just for test.

Mar 25 2019, 7:04 PM · Restricted Project
xiangzhangllvm updated the diff for D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

Update test to check splt's context

Mar 25 2019, 1:29 AM · Restricted Project, lld

Mar 21 2019

xiangzhangllvm updated the diff for D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

rebase

Mar 21 2019, 8:37 PM · Restricted Project, lld
xiangzhangllvm added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

The update:

Mar 21 2019, 7:56 PM · Restricted Project, lld
xiangzhangllvm updated the diff for D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

Hi peter, this update will remove redundant .splt in your upper ifunc test.
The reason of redundant .splt is that we scan the IFUNC symbol after we collected the CET feature. So the splt was created first and without being removed after we found IFUNC.
Anyway this patch is just evade ifunc, I planed to hand ifunc+cet after this patch.
Thank your test! thank you too

Mar 21 2019, 7:46 PM · Restricted Project, lld
xiangzhangllvm updated the diff for D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

Bug fix for 32-bit, the Desc size is not same between 32-bit and 64-bit

Mar 21 2019, 3:53 AM · Restricted Project, lld

Mar 20 2019

xiangzhangllvm added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

I've checked that it does work, although at the moment it does leave a redundant .splt section in the binary. I think that this can be removed by accounting for this in the SPltSection::empty(), .........

Mar 20 2019, 6:45 PM · Restricted Project, lld
xiangzhangllvm added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

I'm having problems getting this to compile on at least GCC 5.4.0 on Linux:

lld/ELF/SyntheticSections.cpp:3104:28: warning: ISO C++ forbids zero-size array ‘Desc’ [-Wpedantic]
   GnuPropertyDescType Desc[]

lld/ELF/SyntheticSections.cpp:3110:55: error: invalid conversion from ‘llvm::cast_retty<GnuPropertyType, const unsigned int*>::ret_type {aka const GnuPropertyType*}’ to ‘GnuPropertyType*’ [-fpermissive]
   GnuPropertyType *GnuProperty = cast<GnuPropertyType>(Data.data());

The cast<> is an LLVM cast http://llvm.org/docs/ProgrammersManual.html#the-isa-cast-and-dyn-cast-templates and isn't something you can do on an arbitrary struct.

Assuming (x86) that your input data is always little endian then you'll need to use something like ulittle32_t instead of uint32_t or your tests will fail when run on a big-endian machine like PPC or some Mips machines.

Mar 20 2019, 5:40 PM · Restricted Project, lld
xiangzhangllvm updated the diff for D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

I rewrited the collecting feature functions.
It is very simple now!

Mar 20 2019, 1:02 AM · Restricted Project, lld

Mar 19 2019

xiangzhangllvm added inline comments to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.
Mar 19 2019, 11:35 PM · Restricted Project, lld
xiangzhangllvm added inline comments to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.
Mar 19 2019, 10:18 PM · Restricted Project, lld
xiangzhangllvm updated the diff for D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

Fine tuning the patch, and still need to refine the "code style".

Mar 19 2019, 10:17 PM · Restricted Project, lld
xiangzhangllvm added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

Overall this patch still look very different from the existing lld code base and does many things by hand without using LLVM libObject. You really need to read the existing code first and then try to mimic what the existing code is doing. What this patch is trying to do is not that new -- your patch should and could follow the existing patterns that I believe not too hard to find by reading existing code.

It is perhaps faster to actually show how it should look like than pointing out all the details in the review thread, so I'll apply your patch locally, modify that, and then upload it so that you can read it.

Mar 19 2019, 7:12 PM · Restricted Project, lld
xiangzhangllvm added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

As I understand it, what you have in this review (not the patch attached as a comment).

  • Driver.cpp calls mergeAggregateMetaData() to set the Config->SpltSectionEnabled and Config->X86Feature1AND
    • We know that all input objects have the feature property set.
  • Writer.cpp creates the In.SpltSection() and the In.GnuProperty() section as we know that will be needed.
  • If we encounter an ifunc then we disable IBT by setting Config->SpltSectionEnabled to false and clearing Config->X86Feature1AND

    One thing I don't understand yet, apologies I've not had time to go into the details of how the splt section works, is that if you discover an ifunc mid-way through processing then you'll have inserted many entries into the In.Splt section, then disabled it, from then on you'll just use the In.Plt. Will this result in a broken PLT as it looks like we'll call get getPltVA() if Config->SpltSectionEnabled and I'm not sure if that will return the right thing if we've previously added an entry into the In.Splt section. I don't see any existing tests for ifunc handling, can you add a test case that does something like:
  • Enable SpltSection
  • Generate a In.SpltSection entry
  • encounter an Ifunc
  • Disable SpltSection
  • Generate a In.Plt entry for some other symbol
  • Check that the generated PLT makes sense.

    If this bit is broken then I think you'll need to find a way of handling this case, which won't be easy. I can think of a few approaches:
  • Do a pre-scan for likely ifuncs, before calling scanRelocs. This could be simpler but more pessimistic than the existing code.
  • Have a way that the In.SpltSection decays into an In.PltSection gracefully if an ifunc is detected.

    Going back to your original question, how about:
  • Writer.cpp at construction of In.GnuPropertySection calls mergeAggregateMetaData() and sets GnuPropertySection::Feature1AND and possibly SpltSection::enable
    • As you've implemented empty() these sections can be unconditionally created, although it doesn't matter too much if you conditionally create them, in that case In.GnuPltSection == nullptr is implicity Feature1AND=0 and In.SpltSection == nullptr is implicitly enableSpltSection == false
  • If we encounter an ifunc then we disable IBT by setting In.SpltSection::Enable to false and setting In.GnuPropertySection::X86Feature1AND to 0. If you've not created the sections then you'll obviously need to test that they exist first. If I've understood correctly if the sections don't exist then there is nothing to disable anyway.

    Please let me know if I've misunderstood something critical?
Mar 19 2019, 6:43 PM · Restricted Project, lld
xiangzhangllvm added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

I do this changes in my local:
I do the feature collection work in the GnuPropertySection's Constructor which will be called in createSyntheticSections().
For the SPlt, I keep its generation condition, because the GnuPropertySection's Constructor will set the Config->SPltSectionEnable.

Mar 19 2019, 1:36 AM · Restricted Project, lld
xiangzhangllvm added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

I guess you mean "I tried", not "I tend" in 1). I think that you can do accomplish 1.) via unconditionally creating the In.GnuPropertySection and SPltSection and by implementing the empty() member function, ensure that they are removed by removeUnusedSyntheticSections() if there isn't anything that uses them. Unconditionally creating the sections means that you don't need to call mergeAggregateMetaData from Driver.cpp prior to createSyntheticSections(). For the case of ifunc where you need to turn off the feature then just inform In.GnuPropertySection and In.SPltSection as they will always be there.

Mar 19 2019, 1:26 AM · Restricted Project, lld

Mar 18 2019

xiangzhangllvm added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

We are using a special CET SDV for CET enabling. We have been enabling CET in Fedora piece by piece by adding -fcf-protection to CFLAGS.

How do we get access to such a simulator so that we can do the same in FreeBSD?

Mar 18 2019, 8:41 PM · Restricted Project, lld
xiangzhangllvm added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

But gold already has options, -z ibt and -z ibtplt. You should be consistent with these options, shouldn't we?

Yes, I think we should, and I think it's better to support these test options in a independent patch, and it's easy to support.

Mar 18 2019, 8:38 PM · Restricted Project, lld
xiangzhangllvm added inline comments to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.
Mar 18 2019, 6:38 PM · Restricted Project, lld
xiangzhangllvm updated the diff for D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.
Mar 18 2019, 1:59 AM · Restricted Project, lld
xiangzhangllvm updated the diff for D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

hi dears,
I updated the patch: most of changes refer to peter and ruiu's suggestions, but not of all.
The main change is:

  1. Read all input files, then collect the CET features, and discard the .note.gnu.property sections, do these in SyntheticSections.cpp not in Driver.cpp.
  2. If CET features checked successful, then create the SyntheticSection's sections (GnuPropertySection, SPltSection) in Writer's createSyntheticSections().

instead of directly creating .note.gnu.property section in Writer.

  1. Adjust the test cases to remove redundant code check.
Mar 18 2019, 1:36 AM · Restricted Project, lld
xiangzhangllvm added reviewers for D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD: maskray0, peter.smith.
Mar 18 2019, 1:32 AM · Restricted Project, lld