Commit Graph

18 Commits

Author SHA1 Message Date
Shoaib Meenai d806af3499 [CMake] Use PRIVATE in target_link_libraries for executables
We currently use target_link_libraries without an explicit scope
specifier (INTERFACE, PRIVATE or PUBLIC) when linking executables.
Dependencies added in this way apply to both the target and its
dependencies, i.e. they become part of the executable's link interface
and are transitive.

Transitive dependencies generally don't make sense for executables,
since you wouldn't normally be linking against an executable. This also
causes issues for generating install export files when using
LLVM_DISTRIBUTION_COMPONENTS. For example, clang has a lot of LLVM
library dependencies, which are currently added as interface
dependencies. If clang is in the distribution components but the LLVM
libraries it depends on aren't (which is a perfectly legitimate use case
if the LLVM libraries are being built static and there are therefore no
run-time dependencies on them), CMake will complain about the LLVM
libraries not being in export set when attempting to generate the
install export file for clang. This is reasonable behavior on CMake's
part, and the right thing is for LLVM's build system to explicitly use
PRIVATE dependencies for executables.

Unfortunately, CMake doesn't allow you to mix and match the keyword and
non-keyword target_link_libraries signatures for a single target; i.e.,
if a single call to target_link_libraries for a particular target uses
one of the INTERFACE, PRIVATE, or PUBLIC keywords, all other calls must
also be updated to use those keywords. This means we must do this change
in a single shot. I also fully expect to have missed some instances; I
tested by enabling all the projects in the monorepo (except dragonegg),
and configuring both with and without shared libraries, on both Darwin
and Linux, but I'm planning to rely on the buildbots for other
configurations (since it should be pretty easy to fix those).

Even after this change, we still have a lot of target_link_libraries
calls that don't specify a scope keyword, mostly for shared libraries.
I'm thinking about addressing those in a follow-up, but that's a
separate change IMO.

Differential Revision: https://reviews.llvm.org/D40823

llvm-svn: 319840
2017-12-05 21:49:56 +00:00
Mitch Phillips 2e7be2a65a [cfi-verify] Validate there are no register clobbers between CFI-check and instruction execution.
Summary:
This patch adds another failure mode for `validateCFIProtection(..)`, wherein any register that affects the indirect control flow instruction is clobbered to between the CFI-check and the instruction's execution.

Also includes a modification to make MCInstrDesc::hasDefOfPhysReg public.

Reviewers: vlad.tsyrklevich

Reviewed By: vlad.tsyrklevich

Subscribers: llvm-commits, pcc, kcc

Differential Revision: https://reviews.llvm.org/D39820

llvm-svn: 318238
2017-11-15 00:35:26 +00:00
Mitch Phillips 3b9ea32ef8 [cfi-verify] Made FileAnalysis operate on a GraphResult rather than build one and validate it.
Refactors the behaviour of building graphs out of FileAnalysis, allowing for analysis of the GraphResult by the callee without having to rebuild the graph. Means when we want to analyse the constructed graph (planned for later revisions), we don't do repeated work.

Also makes CFI verification in FileAnalysis now return an enum that allows us to differentiate why something failed, not just that it did/didn't fail.

Reviewers: vlad.tsyrklevich

Subscribers: kcc, pcc, llvm-commits

Differential Revision: https://reviews.llvm.org/D39764

llvm-svn: 317927
2017-11-10 21:00:22 +00:00
Mitch Phillips c15bdf5598 [cfi-verify] Add blacklist parsing for result filtering.
Adds blacklist parsing behaviour for filtering results into four categories:

 - Expected Protected: Things that are not in the blacklist and are protected.
 - Unexpected Protected: Things that are in the blacklist and are protected.
 - Expected Unprotected: Things that are in the blacklist and are unprotected.
 - Unexpected Unprotected: Things that are not in the blacklist and are unprotected.

 now can optionally be invoked with a second command line argument, which specifies the blacklist file that the binary was built with.

Current  statistics for chromium:

Reviewers: vlad.tsyrklevich

Subscribers: mgorny, llvm-commits, pcc, kcc

Differential Revision: https://reviews.llvm.org/D39525

llvm-svn: 317364
2017-11-03 20:54:26 +00:00
Mitch Phillips 189ebb6976 [cfi-verify] Add an interesting unit test where undef search length changes result.
Add an interesting unit test, found by changing --search-length-undef from the default. Program handles it correctly but good for ensuring correctness on further changes :)

Reviewers: pcc

Subscribers: mgorny, llvm-commits, kcc, vlad.tsyrklevich

Differential Revision: https://reviews.llvm.org/D38658

llvm-svn: 317355
2017-11-03 20:00:05 +00:00
Mitch Phillips 5ff01cdc59 Add FileVerifier::isCFIProtected().
Add a CFI protection check that is implemented by building a graph and inspecting the output to deduce if the indirect CF instruction is CFI protected. Also added the output of this instruction to printIndirectInstructions().

Reviewers: vlad.tsyrklevich

Subscribers: llvm-commits, kcc, pcc, mgorny

Differential Revision: https://reviews.llvm.org/D38428

llvm-svn: 316610
2017-10-25 21:21:16 +00:00
Sam Clegg dabd5be42d Fix LLVM_LINK_LLVM_DYLIB=On build of llvm-cfi-verify
Subscribers: mgorny, aheejin

Differential Revision: https://reviews.llvm.org/D39250

llvm-svn: 316493
2017-10-24 20:21:15 +00:00
Serguei Katkov 3356acacf7 Restore the fix rL316059 eliminated by rL316372
rL316059 fixed the potential build failure when compiling
with -DLLVM_BUILD_LLVM_DYLIB=ON -DLLVM_LINK_LLVM_DYLIB=ON.

rL316372 just reverted the part of the fix, so restore it.

llvm-svn: 316422
2017-10-24 09:42:55 +00:00
Mitch Phillips d9af383d58 Made llvm-cfi-verify not execute unit tests on non-x86 builds.
Patched out from D38427.

Reviewers: vlad.tsyrklevich

Reviewed By: vlad.tsyrklevich

Subscribers: llvm-commits, kcc, pcc, mgorny

Differential Revision: https://reviews.llvm.org/D39197

llvm-svn: 316375
2017-10-23 20:54:01 +00:00
Mitch Phillips 99fa140501 Graph builder implementation.
Implement a localised graph builder for indirect control flow
instructions. Main interface is through GraphBuilder::buildFlowGraph,
which will build a flow graph around an indirect CF instruction. Various
modifications to FileVerifier are also made to const-expose some members
needed for machine code analysis done by the graph builder.

Reviewers: vlad.tsyrklevich

Reviewed By: vlad.tsyrklevich

Subscribers: llvm-commits, kcc, pcc

Differential Revision: https://reviews.llvm.org/D38427

llvm-svn: 316372
2017-10-23 20:25:19 +00:00
Mitch Phillips e140b3d122 Accidently merged an incomplete upstream patch in 10e6ee563a6b5ca498f27972ca6dbe6c308f1ac2 - reverting the changes.
llvm-svn: 316359
2017-10-23 18:17:56 +00:00
Mitch Phillips f91a31b7a3 Patch in
llvm-svn: 316358
2017-10-23 18:11:31 +00:00
Vlad Tsyrklevich 4dc1c9735b Statically link llvm-cfi-verify's libraries.
Summary:
llvm-cfi-verify (D38379) introduced a potential build failure when compiling with `-DLLVM_BUILD_LLVM_DYLIB=ON -DLLVM_LINK_LLVM_DYLIB=ON`. Specific versions of cmake seem to treat the `add_subdirectory()` rule differently. It seems as if old versions of cmake BFS these rules, adding them to the fringe for expansion later. Newer versions of cmake seem to immediately execute CMakeFiles that are present in this subdirectory.

If the subdirectory is expanded through the fringe, the globbing resultant from `llvm_add_implicit_projects()` from `cmake/modules/AddLLVM.cmake:1012` means that `tools/llvm-shlib/CMakeFile.txt` gets executed before `tools/llvm-cfi-verify/lib/CMakeFile.txt`. As the latter CMakeFile adds a new library, this expansion order means that the library files required the unit tests in `unittests/tools/llvm-cfi-verify/` are not present in the dynamic library. This causes unit tests to fail as the required functions can't be found.

This change now ensures that the libraries created by `llvm-cfi-verify` are statically linked into the unit tests. As `tools/llvm-cfi-verify/lib` no longer adds anything to `llvm-shlib`, there should be no concern about the order-of-compilation.

Reviewers: skatkov, pcc

Reviewed By: skatkov, pcc

Subscribers: llvm-commits, kcc, pcc, aheejin, vlad.tsyrklevich, mgorny

Differential Revision: https://reviews.llvm.org/D39020

llvm-svn: 316059
2017-10-18 04:27:53 +00:00
Krzysztof Parzyszek c2e2e0a678 [unittests] Only build llvm-cfi-verify if X86 is in LLVM_TARGETS_TO_BUILD
The test requires a target for triple x86-64, and it fails in builds that
do not have the X86 backend.

llvm-svn: 315861
2017-10-15 16:55:23 +00:00
Vlad Tsyrklevich 1d47365259 [cfi-verify] Fix typo, actually check X86 target
The typo in r315556 disabled the cfi-verify unit tests from building
unconditionally, have it correctly check for the X86 target.

llvm-svn: 315581
2017-10-12 14:42:26 +00:00
Vlad Tsyrklevich 85171866be [cfi-verify] Fix unittest failures w/o x86 target
The llvm-cfi-verify unit tests fail if LLVM is built without the X86
target, disable the unit tests from being built unless X86 is enabled
for now.

llvm-svn: 315556
2017-10-12 04:17:33 +00:00
Vlad Tsyrklevich 0ee2632403 MC Helpers for llvm-cfi-verify.
Add instruction analysis and machinecode traversal helpers in
preparation for control flow graph generation implementation.

Reviewers: vlad.tsyrklevich

Reviewed By: vlad.tsyrklevich

Subscribers: mgorny, llvm-commits, pcc, kcc

Differential Revision: https://reviews.llvm.org/D38424

llvm-svn: 315528
2017-10-11 23:17:29 +00:00
Vlad Tsyrklevich 89c3c8c403 Reland 'Classify llvm-cfi-verify.'
Summary: Move llvm-cfi-verify into a class in preparation for CFI analysis to come.

Reviewers: vlad.tsyrklevich

Reviewed By: vlad.tsyrklevich

Subscribers: mgorny, llvm-commits, pcc, kcc

Differential Revision: https://reviews.llvm.org/D38379

llvm-svn: 315504
2017-10-11 20:35:01 +00:00