This is an archive of the discontinued LLVM Phabricator instance.

[cfi] Cross-DSO CFI diagnostic mode (clang part)
ClosedPublic

Authored by eugenis on Dec 21 2015, 1:06 PM.

Details

Reviewers
kcc
pcc
Summary
  • Runtime diagnostic data for cfi-icall changed to match the rest of cfi checks
  • Layout of all CFI diagnostic data changed to put Kind at the beginning. There is no ABI stability promise yet.
  • call cfi_slowpath_diag instead of cfi_slowpath when needed.
  • emit __cfi_check_fail function, which dispatches a CFI check faliure according to trap/recover settings of the current module.
  • a tiny driver change to match the way the new handlers are done in compiler-rt.

Diff Detail

Repository
rL LLVM

Event Timeline

eugenis updated this revision to Diff 43394.Dec 21 2015, 1:06 PM
eugenis retitled this revision from to [cfi] Cross-DSO CFI diagnostic mode (clang part).
eugenis updated this object.
eugenis added reviewers: pcc, kcc.
eugenis set the repository for this revision to rL LLVM.
eugenis added a subscriber: cfe-commits.
pcc added inline comments.Jan 12 2016, 5:14 PM
lib/CodeGen/CGExpr.cpp
2622

This looks like CodeGenFunction::EmitTrapCheck.

2656–2680

Can't you replace all this code with a call to EmitCheck passing icmps for each of the check kinds? Sure, it wouldn't be a switch, but we don't care about the performance of this code anyway.

lib/CodeGen/CodeGenFunction.h
1386

No need to rename, these are still all type checks.

test/CodeGen/cfi-icall-cross-dso.c
53–54

Are you still testing the __cfi_slowpath code path?

eugenis updated this revision to Diff 44805.Jan 13 2016, 3:42 PM
eugenis marked 2 inline comments as done.
eugenis added inline comments.
lib/CodeGen/CGExpr.cpp
2656–2680

Done. The code became a bit simpler, but the test looks awful now.

test/CodeGen/cfi-icall-cross-dso.c
53–54

added a test for the non-diag path

eugenis updated this revision to Diff 45292.Jan 19 2016, 12:08 PM
pcc accepted this revision.Jan 25 2016, 1:58 PM
pcc edited edge metadata.

LGTM

test/CodeGen/cfi-check-fail.c
18

Does this match in a -asserts build?

This revision is now accepted and ready to land.Jan 25 2016, 1:58 PM
eugenis updated this revision to Diff 45920.Jan 25 2016, 2:48 PM
eugenis edited edge metadata.
eugenis added inline comments.
test/CodeGen/cfi-check-fail.c
19

Right.
It fails.
I've removed the ":" after all label names.