Add test suite for the Control Flow Integrity feature.

[jfb]

I'm not familiar with how these tests usually work: is the intent just to see if they don't crash? IIUC there's no correctness check, right?

test/cfi/overwrite.cpp

18 ↗

(On Diff #20224)

Could you explain this test?

[pcc]

The purpose of this test suite is mostly to check that the enforcement mechanism works, i.e. that certain things (e.g. calls through classes of the wrong type, or calls through things that look like vtables but aren't) cause the program to crash. I'll add a few comments to each test case to explain what it is testing.

test/cfi/overwrite.cpp

18 ↗

(On Diff #20224)

The test overwrites a's virtual function pointer with a pointer to fake_vtable and attempts to make a call through it. If the enforcement mechanism is working, the program will crash (checked via not --crash above).

[kcc]

Lovely!

+samsonov for cmake code.
Questions:

can we make these tests run with -O0 and -O2 w/o adding extra RUN lines? 
what will happen with the tests if LTO is not supported?

test/cfi/anon-namespace.cpp

4 ↗

(On Diff #20224)

Hm... I would actually like to have a way to validate that we have an expected crash, not some unrelated crash (e.g. due to miscompile).
The simplest way is to add two printfs, one before and one after the vcall, and check that only the first one is there

6 ↗

(On Diff #20224)

Why does it require that now? Is that an LTO limitation?

test/cfi/overwrite.cpp

2 ↗

(On Diff #20224)

I would like to run this test with and w/o CFI, so that w/o CFI we actually see foo() called.

test/cfi/simple-fail.cpp

2 ↗

(On Diff #20224)

Same here and for all other --crash tests: I would like to also run these tests w/o CFI to see that they "work".
I't ok that the test has undefined behavior according to the standard. We are testing the expected behavior of the particular compiler.

test/cfi/simple-pass.cpp

50 ↗

(On Diff #20224)

we need to ensure that the calls are not devirtualized.
e.g. check break_optimization in test/asan/TestCases/Linux/sized_delete_test.cc

[pcc]

• Add comments and a test for virtual inheritance
• Address Kostya's comments

test/cfi/anon-namespace.cpp

4 ↗

(On Diff #20224)

Done

6 ↗

(On Diff #20224)

Added a comment explaining.

test/cfi/overwrite.cpp

2 ↗

(On Diff #20224)

Done

test/cfi/simple-fail.cpp

2 ↗

(On Diff #20224)

Done

test/cfi/simple-pass.cpp

50 ↗

(On Diff #20224)

Done

[pcc]

can we make these tests run with -O0 and -O2 w/o adding extra RUN lines?

Probably not, but I don't think it would make a difference at the moment. As far as I know, the optimization level chosen at compile time does not affect the level used at link time.

what will happen with the tests if LTO is not supported?

There is logic in lit.cfg to disable the tests if LTO is not supported.

[kcc]

Please also add tests with multiple inheritance

[pcc]

• Add multiple inheritance test

[jfb]

The NCFI tests are checking that UB doesn't cause a crash. It would be nice for the test to point out exactly where the UB is so that if the test starts failing it's easy to figure out why.

[kcc]

Looks great.
Once Alexey LGTMs the cmake code you have my LGTM too.
Please, whenever you get any kind of question about the CFI add a test here that answers the question.
Of course, this patch depends on the other two.

The question about -O0 vs -O2 is more to Alexey.
I think we do have similar cmake magic somewhere, don't we?
It would be great to run all these tests with both -O0 and -O2

test/cfi/anon-namespace.cpp

43 ↗

(On Diff #20246)

#endif // TU1
here and below

test/cfi/multiple-inheritance.cpp

35 ↗

(On Diff #20246)

If you use stderr you will not need fflush

[pcc]

• Mark UB
• Use stderr
• endif comments

test/cfi/anon-namespace.cpp

43 ↗

(On Diff #20246)

Done

test/cfi/multiple-inheritance.cpp

35 ↗

(On Diff #20246)

Done

[jfb]

lgtm too.

[samsonov]

test/cfi/CMakeLists.txt

11 ↗

(On Diff #20358)

Would this work if LLVM_BINUTILS_INCDIR is unavailable?

test/cfi/lit.cfg

8 ↗

(On Diff #20358)

This should be set in test/lit.common.cfg

9 ↗

(On Diff #20358)

os.path.dirname(__file__)

?

10 ↗

(On Diff #20358)

You probably don't need test_exec_root here.

11 ↗

(On Diff #20358)

There is no such directory at the moment

29 ↗

(On Diff #20358)

We have config.clang for that

32 ↗

(On Diff #20358)

ditto

38 ↗

(On Diff #20358)

This should be handled by lit.common.cfg as well: we add llvm_tools_dir to PATH

[pcc]

• Address review comments

test/cfi/CMakeLists.txt

11 ↗

(On Diff #20358)

Probably not, fixed.

test/cfi/lit.cfg

8 ↗

(On Diff #20358)

Done

9 ↗

(On Diff #20358)

Done

10 ↗

(On Diff #20358)

Done

11 ↗

(On Diff #20358)

Done

29 ↗

(On Diff #20358)

Done

32 ↗

(On Diff #20358)

Done

38 ↗

(On Diff #20358)

Done

[samsonov]

LGTM

[kcc]

None of these tests have 32+ or 64+ vfunctions.
In a separate commit please add some larger tests to cover the case with more vfunctions.

[pcc]

r230116

compiler-rt/trunk/test/CMakeLists.txt

if(COMPILER_RT_HAS_SANITIZER_COMMON)
     add_subdirectory(sanitizer_common)
   endif()
   if(COMPILER_RT_HAS_TSAN)
     add_subdirectory(tsan)
   endif()
   if(COMPILER_RT_HAS_UBSAN)
     add_subdirectory(ubsan)
   endif()
+  add_subdirectory(cfi)
 endif()
 
 if(COMPILER_RT_STANDALONE_BUILD)
   # Now that we've traversed all the directories and know all the lit testsuites,
   # introduce a rule to run to run all of them.
   get_property(LLVM_LIT_TESTSUITES GLOBAL PROPERTY LLVM_LIT_TESTSUITES)
   get_property(LLVM_LIT_DEPENDS GLOBAL PROPERTY LLVM_LIT_DEPENDS)
   add_lit_target(check-all
     "Running all regression tests"
     ${LLVM_LIT_TESTSUITES}
     DEPENDS ${LLVM_LIT_DEPENDS})
 endif()

compiler-rt/trunk/test/cfi/CMakeLists.txt

+configure_lit_site_cfg(
+  ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in
+  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg
+  )
+
+set(CFI_TEST_DEPS)
+if(NOT COMPILER_RT_STANDALONE_BUILD)
+  list(APPEND CFI_TEST_DEPS
+    FileCheck
+    clang
+    not
+  )
+  if(LLVM_ENABLE_PIC AND LLVM_BINUTILS_INCDIR)
+    list(APPEND CFI_TEST_DEPS
+      LLVMgold
+    )
+  endif()
+endif()
+
+add_lit_testsuite(check-cfi "Running the cfi regression tests"
+  ${CMAKE_CURRENT_BINARY_DIR}
+  DEPENDS ${CFI_TEST_DEPS})
+set_target_properties(check-cfi PROPERTIES FOLDER "Tests")

compiler-rt/trunk/test/cfi/anon-namespace.cpp

+// RUN: %clangxx_cfi -c -DTU1 -o %t1.o %s
+// RUN: %clangxx_cfi -c -DTU2 -o %t2.o %S/../cfi/anon-namespace.cpp
+// RUN: %clangxx_cfi -o %t %t1.o %t2.o
+// RUN: not --crash %t 2>&1 | FileCheck --check-prefix=CFI %s
+
+// RUN: %clangxx -c -DTU1 -o %t1.o %s
+// RUN: %clangxx -c -DTU2 -o %t2.o %S/../cfi/anon-namespace.cpp
+// RUN: %clangxx -o %t %t1.o %t2.o
+// RUN: %t 2>&1 | FileCheck --check-prefix=NCFI %s
+
+// Tests that the CFI mechanism treats classes in the anonymous namespace in
+// different translation units as having distinct identities. This is done by
+// compiling two translation units TU1 and TU2 containing a class named B in an
+// anonymous namespace, and testing that the program crashes if TU2 attempts to
+// use a TU1 B as a TU2 B.
+
+// FIXME: This test should not require that the paths supplied to the compiler
+// are different. It currently does so because bitset names have global scope
+// so we have to mangle the file path into the bitset name.
+
+#include <stdio.h>
+
+struct A {
+  virtual void f() = 0;
+};
+
+namespace {
+
+struct B : A {
+  virtual void f() {}
+};
+
+}
+
+A *mkb();
+
+#ifdef TU1
+
+A *mkb() {
+  return new B;
+}
+
+#endif  // TU1
+
+#ifdef TU2
+
+int main() {
+  A *a = mkb();
+
+  // CFI: 1
+  // NCFI: 1
+  fprintf(stderr, "1\n");
+
+  ((B *)a)->f(); // UB here
+
+  // CFI-NOT: 2
+  // NCFI: 2
+  fprintf(stderr, "2\n");
+}
+
+#endif  // TU2

compiler-rt/trunk/test/cfi/lit.cfg

+import lit.formats
+import os
+import subprocess
+import sys
+
+config.name = 'cfi'
+config.suffixes = ['.cpp']
+config.test_source_root = os.path.dirname(__file__)
+
+def is_darwin_lto_supported():
+  return os.path.exists(os.path.join(config.llvm_shlib_dir, 'libLTO.dylib'))
+
+def is_linux_lto_supported():
+  if not os.path.exists(os.path.join(config.llvm_shlib_dir, 'LLVMgold.so')):
+    return False
+
+  ld_cmd = subprocess.Popen([config.gold_executable, '--help'], stdout = subprocess.PIPE)
+  ld_out = ld_cmd.stdout.read().decode()
+  ld_cmd.wait()
+
+  if not '-plugin' in ld_out:
+    return False
+
+  return True
+
+clangxx = ' '.join([config.clang] + config.cxx_mode_flags)
+
+config.substitutions.append((r"%clangxx ", clangxx + ' '))
+
+if sys.platform == 'darwin' and is_darwin_lto_supported():
+  config.substitutions.append((r"%clangxx_cfi ", 'env DYLD_LIBRARY_PATH=' + config.llvm_shlib_dir + ' ' + clangxx + ' -fsanitize=cfi '))
+elif sys.platform.startswith('linux') and is_linux_lto_supported():
+  config.substitutions.append((r"%clangxx_cfi ", clangxx + ' -fuse-ld=gold -fsanitize=cfi '))
+else:
+  config.unsupported = True

compiler-rt/trunk/test/cfi/lit.site.cfg.in

+lit_config.load_config(config, "@COMPILER_RT_BINARY_DIR@/test/lit.common.configured")
+lit_config.load_config(config, "@CMAKE_CURRENT_SOURCE_DIR@/lit.cfg")

compiler-rt/trunk/test/cfi/multiple-inheritance.cpp

+// RUN: %clangxx_cfi -o %t %s
+// RUN: not --crash %t 2>&1 | FileCheck --check-prefix=CFI %s
+// RUN: not --crash %t x 2>&1 | FileCheck --check-prefix=CFI %s
+
+// RUN: %clangxx -o %t %s
+// RUN: %t 2>&1 | FileCheck --check-prefix=NCFI %s
+// RUN: %t x 2>&1 | FileCheck --check-prefix=NCFI %s
+
+// Tests that the CFI mechanism is sensitive to multiple inheritance and only
+// permits calls via virtual tables for the correct base class.
+
+#include <stdio.h>
+
+struct A {
+  virtual void f() = 0;
+};
+
+struct B {
+  virtual void g() = 0;
+};
+
+struct C : A, B {
+  virtual void f(), g();
+};
+
+void C::f() {}
+void C::g() {}
+
+int main(int argc, char **argv) {
+  C *c = new C;
+
+  // CFI: 1
+  // NCFI: 1
+  fprintf(stderr, "1\n");
+
+  if (argc > 1) {
+    A *a = c;
+    ((B *)a)->g(); // UB here
+  } else {
+    B *b = c;
+    ((A *)b)->f(); // UB here
+  }
+
+  // CFI-NOT: 2
+  // NCFI: 2
+  fprintf(stderr, "2\n");
+}

compiler-rt/trunk/test/cfi/overwrite.cpp

+// RUN: %clangxx_cfi -o %t %s
+// RUN: not --crash %t 2>&1 | FileCheck --check-prefix=CFI %s
+
+// RUN: %clangxx -o %t %s
+// RUN: %t 2>&1 | FileCheck --check-prefix=NCFI %s
+
+// Tests that the CFI mechanism crashes the program when a virtual table is
+// replaced with a compatible table of function pointers that does not belong to
+// any class, by manually overwriting the virtual table of an object and
+// attempting to make a call through it.
+
+#include <stdio.h>
+
+struct A {
+  virtual void f();
+};
+
+void A::f() {}
+
+void foo() {
+  fprintf(stderr, "foo\n");
+}
+
+void *fake_vtable[] = { (void *)&foo };
+
+int main() {
+  A *a = new A;
+  *((void **)a) = fake_vtable; // UB here
+
+  // CFI: 1
+  // NCFI: 1
+  fprintf(stderr, "1\n");
+
+  // CFI-NOT: foo
+  // NCFI: foo
+  a->f();
+
+  // CFI-NOT: 2
+  // NCFI: 2
+  fprintf(stderr, "2\n");
+}

compiler-rt/trunk/test/cfi/simple-fail.cpp

+// RUN: %clangxx_cfi -o %t %s
+// RUN: not --crash %t 2>&1 | FileCheck --check-prefix=CFI %s
+
+// RUN: %clangxx -o %t %s
+// RUN: %t 2>&1 | FileCheck --check-prefix=NCFI %s
+
+// Tests that the CFI mechanism crashes the program when making a virtual call
+// to an object of the wrong class but with a compatible vtable, by casting a
+// pointer to such an object and attempting to make a call through it.
+
+#include <stdio.h>
+
+struct A {
+  virtual void f();
+};
+
+void A::f() {}
+
+struct B {
+  virtual void f();
+};
+
+void B::f() {}
+
+int main() {
+  A *a = new A;
+
+  // CFI: 1
+  // NCFI: 1
+  fprintf(stderr, "1\n");
+
+  ((B *)a)->f(); // UB here
+
+  // CFI-NOT: 2
+  // NCFI: 2
+  fprintf(stderr, "2\n");
+}

compiler-rt/trunk/test/cfi/simple-pass.cpp

+// RUN: %clangxx_cfi -o %t %s
+// RUN: %t
+
+// Tests that the CFI mechanism does not crash the program when making various
+// kinds of valid calls involving classes with various different linkages and
+// types of inheritance.
+
+inline void break_optimization(void *arg) {
+  __asm__ __volatile__("" : : "r" (arg) : "memory");
+}
+
+struct A {
+  virtual void f();
+};
+
+void A::f() {}
+
+struct A2 : A {
+  virtual void f();
+};
+
+void A2::f() {}
+
+struct B {
+  virtual void f() {}
+};
+
+struct B2 : B {
+  virtual void f() {}
+};
+
+namespace {
+
+struct C {
+  virtual void f();
+};
+
+void C::f() {}
+
+struct C2 : C {
+  virtual void f();
+};
+
+void C2::f() {}
+
+struct D {
+  virtual void f() {}
+};
+
+struct D2 : D {
+  virtual void f() {}
+};
+
+}
+
+struct E {
+  virtual void f() {}
+};
+
+struct E2 : virtual E {
+  virtual void f() {}
+};
+
+int main() {
+  A *a = new A;
+  break_optimization(a);
+  a->f();
+  a = new A2;
+  break_optimization(a);
+  a->f();
+
+  B *b = new B;
+  break_optimization(b);
+  b->f();
+  b = new B2;
+  break_optimization(b);
+  b->f();
+
+  C *c = new C;
+  break_optimization(c);
+  c->f();
+  c = new C2;
+  break_optimization(c);
+  c->f();
+
+  D *d = new D;
+  break_optimization(d);
+  d->f();
+  d = new D2;
+  break_optimization(d);
+  d->f();
+
+  E *e = new E;
+  break_optimization(e);
+  e->f();
+  e = new E2;
+  break_optimization(e);
+  e->f();
+}

compiler-rt/trunk/test/cfi/vdtor.cpp

+// RUN: %clangxx_cfi -o %t %s
+// RUN: not --crash %t 2>&1 | FileCheck --check-prefix=CFI %s
+
+// RUN: %clangxx -o %t %s
+// RUN: %t 2>&1 | FileCheck --check-prefix=NCFI %s
+
+// Tests that the CFI enforcement also applies to virtual destructor calls made
+// via 'delete'.
+
+#include <stdio.h>
+
+struct A {
+  virtual ~A();
+};
+
+A::~A() {}
+
+struct B {
+  virtual ~B();
+};
+
+B::~B() {}
+
+int main() {
+  A *a = new A;
+
+  // CFI: 1
+  // NCFI: 1
+  fprintf(stderr, "1\n");
+
+  delete (B *)a; // UB here
+
+  // CFI-NOT: 2
+  // NCFI: 2
+  fprintf(stderr, "2\n");
+}

compiler-rt/trunk/test/lit.common.configured.in

 set_default("target_arch", "@COMPILER_RT_TEST_TARGET_ARCH@")
 set_default("host_os", "@HOST_OS@")
 set_default("llvm_build_mode", "@LLVM_BUILD_MODE@")
 set_default("llvm_src_root", "@LLVM_SOURCE_DIR@")
 set_default("llvm_obj_root", "@LLVM_BINARY_DIR@")
 set_default("compiler_rt_src_root", "@COMPILER_RT_SOURCE_DIR@")
 set_default("compiler_rt_obj_root", "@COMPILER_RT_BINARY_DIR@")
 set_default("llvm_tools_dir", "@LLVM_TOOLS_DIR@")
+set_default("llvm_shlib_dir", "@SHLIBDIR@")
+set_default("gold_executable", "@GOLD_EXECUTABLE@")
 set_default("clang", "@COMPILER_RT_TEST_COMPILER@")
 set_default("compiler_id", "@COMPILER_RT_TEST_COMPILER_ID@")
 set_default("compiler_rt_arch", "@COMPILER_RT_SUPPORTED_ARCH@")
 set_default("python_executable", "@PYTHON_EXECUTABLE@")
 set_default("compiler_rt_debug", @COMPILER_RT_DEBUG_PYBOOL@)
 set_default("compiler_rt_libdir", "@COMPILER_RT_LIBRARY_OUTPUT_DIR@")
 set_default("emulator", "@COMPILER_RT_EMULATOR@")