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

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

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

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

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

test/cfi/overwrite.cpp

2

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

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

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

Done

6

Added a comment explaining.

test/cfi/overwrite.cpp

2

Done

test/cfi/simple-fail.cpp

2

Done

test/cfi/simple-pass.cpp

50

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

44

#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

44

Done

test/cfi/multiple-inheritance.cpp

35 ↗

(On Diff #20246)

Done

[jfb]

lgtm too.

[samsonov]

test/cfi/CMakeLists.txt

12

Would this work if LLVM_BINUTILS_INCDIR is unavailable?

test/cfi/lit.cfg

9

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

10

os.path.dirname(__file__)

?

11

You probably don't need test_exec_root here.

12

There is no such directory at the moment

30

We have config.clang for that

33

ditto

39

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

12

Probably not, fixed.

test/cfi/lit.cfg

9

Done

10

Done

11

Done

12

Done

30

Done

33

Done

39

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

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()

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
+    LLVMgold
+    not
+  )
+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")

test/cfi/anon-namespace.cpp

+// RUN: %clang_cfi -c -DTU1 -o %t1.o %s
+// RUN: %clang_cfi -c -DTU2 -o %t2.o %S/../cfi/anon-namespace.cpp
+// RUN: %clang_cfi -o %t %t1.o %t2.o
+// RUN: not --crash %t
+
+// FIXME: This test should not require that the paths supplied to the compiler
+// are different.
+
+struct A {
+  virtual void f() = 0;
+};
+
+namespace {
+
+struct B : A {
+  virtual void f() {}
+};
+
+}
+
+A *mkb();
+
+#ifdef TU1
+
+A *mkb() {
+  return new B;
+}
+
+#endif
+
+#ifdef TU2
+
+int main() {
+  A *a = mkb();
+  ((B *)a)->f();
+}
+
+#endif

test/cfi/lit.cfg

+import lit.formats
+import os
+import subprocess
+import sys
+
+config.name = 'cfi'
+config.suffixes = ['.cpp']
+config.test_format = lit.formats.ShTest()
+config.test_source_root = config.compiler_rt_src_root + '/test/cfi'
+config.test_exec_root = config.compiler_rt_obj_root + '/test/cfi'
+config.excludes = ['Inputs']
+
+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
+
+if sys.platform == 'darwin' and is_darwin_lto_supported():
+  config.substitutions.append((r"%clang_cfi", 'env DYLD_LIBRARY_PATH=' + config.llvm_obj_root + '/lib ' + config.llvm_obj_root + '/bin/clang++ -fsanitize=cfi'))
+elif sys.platform.startswith('linux') and is_linux_lto_supported():
+  config.substitutions.append((r"%clang_cfi", config.llvm_obj_root + '/bin/clang++ -fuse-ld=gold -fsanitize=cfi'))
+else:
+  config.unsupported = True
+
+config.substitutions.append((r"\bnot\b", config.llvm_obj_root + '/bin/not'))

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")

test/cfi/overwrite.cpp

+// RUN: %clang_cfi -o %t %s
+// RUN: not --crash %t
+
+struct A {
+  virtual void f();
+};
+
+void A::f() {}
+
+void foo() {
+}
+
+void *fake_vtable[] = { (void *)&foo };
+
+int main() {
+  A *a = new A;
+  *((void **)a) = fake_vtable;
+  a->f();
+}

test/cfi/simple-fail.cpp

+// RUN: %clang_cfi -o %t %s
+// RUN: not --crash %t
+
+struct A {
+  virtual void f();
+};
+
+void A::f() {}
+
+struct B {
+  virtual void f();
+};
+
+void B::f() {}
+
+int main() {
+  A *a = new A;
+  ((B *)a)->f();
+}

test/cfi/simple-pass.cpp

+// RUN: %clang_cfi -o %t %s
+// RUN: %t
+
+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() {}
+};
+
+}
+
+int main() {
+  A *a = new A;
+  a->f();
+  a = new A2;
+  a->f();
+
+  B *b = new B;
+  b->f();
+  b = new B2;
+  b->f();
+
+  C *c = new C;
+  c->f();
+  c = new C2;
+  c->f();
+
+  D *d = new D;
+  d->f();
+  d = new D2;
+  d->f();
+}

test/cfi/vdtor.cpp

+// RUN: %clang_cfi -o %t %s
+// RUN: not --crash %t
+
+struct A {
+  virtual ~A();
+};
+
+A::~A() {}
+
+struct B {
+  virtual ~B();
+};
+
+B::~B() {}
+
+int main() {
+  A *a = new A;
+  delete (B *)a;
+}

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@")