Page MenuHomePhabricator

CET for Exception Handle
ClosedPublic

Authored by xiangzhangllvm on Mar 15 2020, 3:05 AM.

Details

Summary

Bug fix for https://bugs.llvm.org/show_bug.cgi?id=45182
Exception handle may indirectly jump to catch pad, So we should add ENDBR instruction before catch pad instructions.

Diff Detail

Event Timeline

xiangzhangllvm created this revision.Mar 15 2020, 3:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2020, 3:05 AM
LuoYuanke added inline comments.Mar 15 2020, 7:33 AM
llvm/lib/Target/X86/X86IndirectBranchTracking.cpp
132

Line 131 ~ 134 can be merged to "bool EHPadIBTNeeded = MBB.isEHPad();"

140

Does it looks better to insert endbr first for EHPAD block?

MachineBasicBlock::iterator I = MBB.begin();
if (MBB.isEHPad()) {
  for (; I != MBB.end(); ++I) {
    if (I->isDebugInstr()) continue;
  }
  Changed |= addENDBR(MBB, std::next(I));
  break;
}
for (; I != MBB.end(); ++I) {
  if (!I->isCall()) continue;
  ...
}
xiangzhangllvm marked 2 inline comments as done.Mar 15 2020, 6:15 PM
xiangzhangllvm added inline comments.
llvm/lib/Target/X86/X86IndirectBranchTracking.cpp
132

Yes, you are right, thank you!

140

I have thought that, but it may cost 2 times loop to do it.

xiangzhangllvm added a reviewer: pengfei.
This revision is now accepted and ready to land.Mar 17 2020, 8:39 PM
This revision was automatically updated to reflect the committed changes.
MaskRay added a subscriber: MaskRay.EditedMar 29 2020, 11:40 AM

@pengfei You can strip unneeded Phabricator tags with:

% which arcfilter
arcfilter () {
        git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: /,
"");print}' | git commit --amend -F -
}

Subscribers: and Reviewers: are just annoying. It is also not appropriate to use Patch By: Xiang Zhang (xiangzhangllvm). See https://llvm.org/docs/DeveloperPolicy.html#commit-messages " git commit --amend --author="John Doe <jdoe@llvm.org> to correct the author property"

@pengfei You can strip unneeded Phabricator tags with:

% which arcfilter
arcfilter () {
        git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: /,
"");print}' | git commit --amend -F -
}

Subscribers: and Reviewers: are just annoying. It is also not appropriate to use Patch By: Xiang Zhang (xiangzhangllvm). See https://llvm.org/docs/DeveloperPolicy.html#commit-messages " git commit --amend --author="John Doe <jdoe@llvm.org> to correct the author property"

Thanks for your information. It's very helpful.