Page MenuHomePhabricator

xiangzhangllvm (Xiang Zhang)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 25 2018, 11:14 PM (42 w, 6 d)

Recent Activity

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

Mar 13 2019

xiangzhangllvm added a comment to D56990: Bugfix for Replacement of tied operand of inline asm.

LGTM; I'll merge it tonight or tomorrow.

Mar 13 2019, 8:03 PM · Restricted Project

Mar 12 2019

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

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.

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

As an earlier comment mentioned, there is quite a lot of code that depends on the details of the .note.gnu.property section added very early to Driver.c which is not where we'd normally expect to see it. It also looks like you using quite a lot of code to get an InputSection to represent the output .note.gnu.property section. Given that there will be multiple Input .note.gnu.property sections but only one Output .note.gnu.property section, I think it would be cleaner to use a SyntheticSection to represent the .note.gnu.property section. SyntheticSections can implement all sorts of custom functionality that could move the parsing and merging code out of Driver.cpp. My suggestion is to:

  • Create a new SyntheticSection (see SyntheticSections.h), something like NotePropertySyntheticSection. This will represent our single output .note.gnu.property section. It can store the status of the FEATUREAND bits.
  • Add an entry to InStruct for a single instance of it that we will use for the output.
  • After InStruct instance is created in Writer.c but before the relocations are scanned to create PLT entries iterate through InputSections and pass each .note.gnu.property to the NotePropertySyntheticSection. Then erase these InputSections from the InputSections. The combineEhFrameSections<ELFT> function in Writer.cpp is one way of doing something like that.
  • When you add the .note.gnu.property InputSection to the NotePropertySyntheticSection you can update the FEATUREAND bits
  • The writeTo() member function of the NotePropertySyntheticSection can output the data needed to make up the output.

    I think that this should allow you to integrate the .note.gnu.property in a similar way to other "merge" features such as EHFrame, and SHF_MERGE sections.

    The ld.bfd AArch64 BTI patches have just gone up on https://www.sourceware.org/ml/binutils/2019-03/msg00021.html so I'll hopefully be starting work on another feature that uses .note.gnu.property soon.
Mar 12 2019, 6:01 PM · Restricted Project, lld
xiangzhangllvm added a comment to D56990: Bugfix for Replacement of tied operand of inline asm.

We found may tests failed about this issue.
I hope It can be committed.
Thank you very much!

Mar 12 2019, 1:19 AM · Restricted Project

Mar 11 2019

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

static void setConfigs(opt::InputArgList &Args);

Mar 11 2019, 10:22 PM · Restricted Project, lld
xiangzhangllvm added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.
This macros is unnecessary. You can use alignTo. See ELF/LinkerScript.cpp for examples.
Mar 11 2019, 8:03 PM · Restricted Project, lld
xiangzhangllvm added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

Gold auto enabled CET in default

What kind of CET support does gold have now? I think it can merge .note.gnu.property now but I don't find code related to second PLT or ibtplt. (I have a question about ibtplt, see below).

I add --auto-cet and --force-cet options, and disable CET in default. Now there will be no influence to the LLD when not using these options.

I find that ld.bfd has three related options -z ibt -z ibtplt -z shstk (are there others?). What do they do?

-z ibtplt                   Generate IBT-enabled PLT entries\n"));
fprintf (file, _("\
-z ibt                      Generate GNU_PROPERTY_X86_FEATURE_1_IBT\n"));
fprintf (file, _("\
-z shstk                    Generate GNU_PROPERTY_X86_FEATURE_1_SHSTK\n"));

Is -z ibtplt similar to the second PLT? Does -z ibt generate GNU_PROPERTY_X86_FEATURE_1_IBT even if there is an input object file with the GNU_PROPERTY_X86_FEATURE_1_IBT bit cleared? Or when it is specified while -z shstk is not, merge just ibt but not shstk? I'm puzzled as you said the note merging behavior is automatic in ld.bfd.

--auto-cet and --force-cet

We can also make one option --cet={none,auto,force}, but I'd like to understand first what -z ibt -z shstk -z ibtplt do...

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

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?

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

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

Mar 11 2019, 6:33 PM · Restricted Project, lld

Mar 10 2019

xiangzhangllvm added a comment to D56990: Bugfix for Replacement of tied operand of inline asm.

Hi, efriedma
could you help he commit this patch?
Thank you very much!

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

Hi friends:
I add --auto-cet and --force-cet options, and disable CET in default. Now there will be no influence to the LLD when not using these options.
I did these because I want to quickly finish the options discussion, I think it is easy to refine in the future.
Thank you very much!

Mar 10 2019, 8:45 PM · Restricted Project, lld

Mar 8 2019

xiangzhangllvm added a comment to D59120: [ELF] Sort notes by alignment in decreasing order.

if just .note.gnu.property require 64-bit alignment, I think it time to change the alignment of this section.

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

Yes, MaskRay, let's get consensus on Auto vs Manual soon,
If you and @ruiu do not buy the reasones of Auto way, I tend to switch to the Manual way right now, after all, it is easy to 'refine' in future.

Mar 8 2019, 11:10 PM · Restricted Project, lld
xiangzhangllvm updated the diff for D56990: Bugfix for Replacement of tied operand of inline asm.
Mar 8 2019, 6:37 PM · Restricted Project
xiangzhangllvm added a comment to D56990: Bugfix for Replacement of tied operand of inline asm.

Thank you, efriedma
But but the LLVM and Clang are different projects, I can commit the change at one time.
I 'll update the patch for clang first.

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

I think https://bugs.llvm.org/show_bug.cgi?id=41000 "PT_NOTE segment not packed correctly" has some relevance to this review. In particular the .note.gnu.property sections on 64-bit machines have 8-byte alignment, merging these with 4-byte aligned note sections such as build-id can cause problems for consumers of PT_NOTE (see also https://sourceware.org/ml/libc-alpha/2018-08/msg00340.html for a discussion on the glibc mailing list). My understanding is that the eventual solution was to collate 4 and 8 byte SHT_NOTE sections separately and output a PT_NOTE section for each. I think that we should follow that and have tests for it.

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

Rebase

Mar 8 2019, 12:08 AM · Restricted Project, lld

Mar 7 2019

xiangzhangllvm updated the summary of D56990: Bugfix for Replacement of tied operand of inline asm.
Mar 7 2019, 10:12 PM · Restricted Project
xiangzhangllvm added a comment to D56990: Bugfix for Replacement of tied operand of inline asm.

!!! Hi, dear efriedma, very sorry! I just saw your reply.
line 2093: getTargetHooks().adjustInlineAsmType(... InputConstraint,...) will just deal with the constrain string, and it can't check the TiedOperand in the function.
So, this will make inconsistent adjust for the operand and its tied operand.
The error will not be found in the IR files, it will cause back end error. like:
"error in backend: Unsupported asm: input constraint with a matching output constraint of incompatible type!"

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

From the linker's perspective, .splt *is* PLT, which inter-module calls jump to. If .splt is enabled, .plt is no longer a PLT in the regular sense, but it just contains code to lazy-resolve symbols. So, I think you should handle .splt as the PLT despite its name throughout this patch if .splt is enabled. For example, In.Plt should be > the .splt and Sym.getPltVA() should return its address in .splt.
(I feel that the spec made a mistake when naming .splt -- it should've been .plt and the current "first" PLT should be .splt, but it's probably too late to complain.)

Mar 7 2019, 6:55 PM · Restricted Project, lld