Linux sandbox: implement new BPF testing macros

sandbox/linux/seccomp-bpf/bpf_tests.h

Index: sandbox/linux/seccomp-bpf/bpf_tests.h
diff --git a/sandbox/linux/seccomp-bpf/bpf_tests.h b/sandbox/linux/seccomp-bpf/bpf_tests.h
index ccfc651c012d238777a5c3f603f53bf74e20aa6f..68f6874dc8ca9b4179ff6fc92211b863c862d431 100644
--- a/sandbox/linux/seccomp-bpf/bpf_tests.h
+++ b/sandbox/linux/seccomp-bpf/bpf_tests.h
@@ -5,17 +5,9 @@
 #ifndef SANDBOX_LINUX_SECCOMP_BPF_BPF_TESTS_H__
 #define SANDBOX_LINUX_SECCOMP_BPF_BPF_TESTS_H__
 
-#include <fcntl.h>
-#include <sys/stat.h>
-#include <sys/types.h>
-
-#include "base/debug/leak_annotations.h"
-#include "base/memory/scoped_ptr.h"
+#include "base/basictypes.h"
 #include "build/build_config.h"
-#include "sandbox/linux/seccomp-bpf/sandbox_bpf.h"
-#include "sandbox/linux/seccomp-bpf/sandbox_bpf_compatibility_policy.h"
-#include "sandbox/linux/seccomp-bpf/sandbox_bpf_test_runner.h"
-#include "sandbox/linux/tests/sandbox_test_runner.h"
+#include "sandbox/linux/seccomp-bpf/bpf_tester_compatibility_delegate.h"
 #include "sandbox/linux/tests/unit_tests.h"
 
 namespace sandbox {
@@ -25,94 +17,106 @@ namespace sandbox {
 // macros from unit_tests.h to specify the expected error condition.
 // A BPF_DEATH_TEST is always disabled under ThreadSanitizer, see
 // crbug.com/243968.
-#define BPF_DEATH_TEST(test_case_name, test_name, death, policy, aux...) \
-  void BPF_TEST_##test_name(                                             \
-      sandbox::BPFTesterSimpleDelegate<aux>::AuxType* BPF_AUX);          \
-  TEST(test_case_name, DISABLE_ON_TSAN(test_name)) {                     \
-    sandbox::SandboxBPFTestRunner bpf_test_runner(                       \
-        new sandbox::BPFTesterSimpleDelegate<aux>(BPF_TEST_##test_name,  \
-                                                  policy));              \
-    sandbox::UnitTests::RunTestInProcess(&bpf_test_runner, death);       \
-  }                                                                      \
-  void BPF_TEST_##test_name(                                             \
-      sandbox::BPFTesterSimpleDelegate<aux>::AuxType* BPF_AUX)
-
-// BPF_TEST() is a special version of SANDBOX_TEST(). It turns into a no-op,
-// if the host does not have kernel support for running BPF filters.
-// Also, it takes advantage of the Die class to avoid calling LOG(FATAL), from
-// inside our tests, as we don't need or even want all the error handling that
-// LOG(FATAL) would do.
+#define BPF_DEATH_TEST(test_case_name, test_name, death, policy, aux...)       \
+  void BPF_TEST_##test_name(                                                   \
+      sandbox::BPFTesterCompatibilityDelegate<aux>::AuxType* BPF_AUX);         \
+  TEST(test_case_name, DISABLE_ON_TSAN(test_name)) {                           \
+    sandbox::SandboxBPFTestRunner bpf_test_runner(                             \
+        new sandbox::BPFTesterCompatibilityDelegate<aux>(BPF_TEST_##test_name, \
+                                                         policy));             \
+    sandbox::UnitTests::RunTestInProcess(&bpf_test_runner, death);             \
+  }                                                                            \
+  void BPF_TEST_##test_name(                                                   \
+      sandbox::BPFTesterCompatibilityDelegate<aux>::AuxType* BPF_AUX)
+
+// This form of BPF_TEST is now discouraged (but still allowed) in favor of

[mdempsky, 2014/05/07 00:57:39]

Doesn't need to be addressed in this CL, but the order that things are defined
and documented in is getting a bit confusing, I think.  E.g., if you start
reading from the top, you first learn that BPF_DEATH_TEST is like BPF_TEST, but
you don't know what that is yet... then scrolling down to find out what BPF_TEST
does, you find out it's deprecated, and you should use other macros that are
even further down.

Since I don't believe macro definition order matters, I'd probably reorganize a
bit so the more commonly used and easier explained things (e.g., BPF_TEST_C) are
defined first, and then after that we can start explaining more
advanced/flexible functions in terms of it (e.g., BPF_DEATH_TEST_C allows
specifying a custom death predicate; BPF_TEST_D allows specifying an entirely
custom delegate implementation; ...).

[jln (very slow on Chromium), 2014/05/07 01:07:37]

Done, thanks!

+// BPF_TEST_D and BPF_TEST_C.
+// BPF_TEST() is a special version of SANDBOX_TEST(). It runs a test function
+// in a sub-process, under a seccomp-bpf policy specified in |policy| without
+// failing on configurations that are allowed to not support seccomp-bpf in
+// their kernels.
+// The |policy| parameter should be a SyscallEvaluator function pointer
+// (which is now a deprecated way of expressing policies).
 // BPF_TEST() takes a C++ data type as an optional fourth parameter. If
 // present, this sets up a variable that can be accessed as "BPF_AUX". This
 // variable will be passed as an argument to the "policy" function. Policies
 // would typically use it as an argument to SandboxBPF::Trap(), if they want to
 // communicate data between the BPF_TEST() and a Trap() function. The life-time
-// of this object is the same as the life-time of the process.
+// of this object is the same as the life-time of the process running under the
+// seccomp-bpf policy.
 // The type specified in |aux| and the last parameter of the policy function
 // must be compatible. If |aux| is not specified, the policy function must
 // take a void* as its last parameter (that is, must have the EvaluateSyscall
 // type).
-#define BPF_TEST(test_case_name, test_name, policy, aux...) \
+#define BPF_TEST(test_case_name, test_name, policy, aux...)               \
   BPF_DEATH_TEST(test_case_name, test_name, DEATH_SUCCESS(), policy, aux)
 
-// Assertions are handled exactly the same as with a normal SANDBOX_TEST()
-#define BPF_ASSERT SANDBOX_ASSERT
-#define BPF_ASSERT_EQ(x, y) BPF_ASSERT((x) == (y))
-#define BPF_ASSERT_NE(x, y) BPF_ASSERT((x) != (y))
-#define BPF_ASSERT_LT(x, y) BPF_ASSERT((x) < (y))
-#define BPF_ASSERT_GT(x, y) BPF_ASSERT((x) > (y))
-#define BPF_ASSERT_LE(x, y) BPF_ASSERT((x) <= (y))
-#define BPF_ASSERT_GE(x, y) BPF_ASSERT((x) >= (y))
+#define BPF_DEATH_TEST_D(                                          \
+    test_case_name, test_name, death, bpf_tester_delegate_class)   \
+  TEST(test_case_name, DISABLE_ON_TSAN(test_name)) {               \
+    sandbox::SandboxBPFTestRunner bpf_test_runner(                 \
+        new bpf_tester_delegate_class());                          \
+    sandbox::UnitTests::RunTestInProcess(&bpf_test_runner, death); \
+  }
 
-template <class Aux = void>
+// This form of BPF_TEST is a little verbose and should be reserved for complex
+// tests where a lot of control is required.
+// |bpf_tester_delegate_class| must be a classname implementing the
+// BPFTesterDelegate interface.
+#define BPF_TEST_D(test_case_name, test_name, bpf_tester_delegate_class)     \
+  BPF_DEATH_TEST_D(                                                          \
+      test_case_name, test_name, DEATH_SUCCESS(), bpf_tester_delegate_class)
+
+// This class takes a simple function pointer as a constructor parameter and a
+// class name as a template parameter to implement the BPFTesterDelegate
+// interface which can be used to build BPF unittests with
+// the SandboxBPFTestRunner class.
+template <class PolicyClass>
 class BPFTesterSimpleDelegate : public BPFTesterDelegate {
  public:
-  typedef Aux AuxType;
-  BPFTesterSimpleDelegate(
-      void (*test_function)(AuxType*),
-      typename CompatibilityPolicy<AuxType>::SyscallEvaluator policy_function)
-      : aux_pointer_for_policy_(NULL),
-        test_function_(test_function),
-        policy_function_(policy_function) {
-    // This will be NULL iff AuxType is void.
-    aux_pointer_for_policy_ = NewAux();
-  }
-
-  virtual ~BPFTesterSimpleDelegate() { DeleteAux(aux_pointer_for_policy_); }
+  explicit BPFTesterSimpleDelegate(void (*test_function)(void))
+      : test_function_(test_function) {}
+  virtual ~BPFTesterSimpleDelegate() {}
 
   virtual scoped_ptr<SandboxBPFPolicy> GetSandboxBPFPolicy() OVERRIDE {
-    // The current method is guaranteed to only run in the child process
-    // running the test. In this process, the current object is guaranteed
-    // to live forever. So it's ok to pass aux_pointer_for_policy_ to
-    // the policy, which could in turn pass it to the kernel via Trap().
-    return scoped_ptr<SandboxBPFPolicy>(new CompatibilityPolicy<AuxType>(
-        policy_function_, aux_pointer_for_policy_));
+    return scoped_ptr<SandboxBPFPolicy>(new PolicyClass());
   }
-
   virtual void RunTestFunction() OVERRIDE {
-    // Run the actual test.
-    // The current object is guaranteed to live forever in the child process
-    // where this will run.
-    test_function_(aux_pointer_for_policy_);
+    DCHECK(test_function_);
+    test_function_();
   }
 
  private:
-  // Allocate an object of type Aux. This is specialized to return NULL when
-  // trying to allocate a void.
-  static Aux* NewAux() { return new Aux(); }
-  static void DeleteAux(Aux* aux) { delete aux; }
-
-  AuxType* aux_pointer_for_policy_;
-  void (*test_function_)(AuxType*);
-  typename CompatibilityPolicy<AuxType>::SyscallEvaluator policy_function_;
+  void (*test_function_)(void);
   DISALLOW_COPY_AND_ASSIGN(BPFTesterSimpleDelegate);
 };
 
-// Specialization of NewAux that returns NULL;
-template <>
-void* BPFTesterSimpleDelegate<void>::NewAux();
-template <>
-void BPFTesterSimpleDelegate<void>::DeleteAux(void* aux);
+#define BPF_DEATH_TEST_C(                                          \
+    test_case_name, test_name, death, bpf_policy_class_name)       \
+  void BPF_TEST_C_##test_name();                                   \
+  TEST(test_case_name, DISABLE_ON_TSAN(test_name)) {               \
+    sandbox::SandboxBPFTestRunner bpf_test_runner(                 \
+        new BPFTesterSimpleDelegate<bpf_policy_class_name>(        \
+            BPF_TEST_C_##test_name));                              \
+    sandbox::UnitTests::RunTestInProcess(&bpf_test_runner, death); \
+  }                                                                \
+  void BPF_TEST_C_##test_name()
+
+// This is the prefered format for new BPF tests. It takes a class name

[mdempsky, 2014/05/07 00:57:39]

Nit: Preferred has two 'r's.

[jln (very slow on Chromium), 2014/05/07 01:07:37]

Done.

+// that implements the policy (which will be default-constructed) and
+// the test function's body can simply follow.
+#define BPF_TEST_C(test_case_name, test_name, bpf_policy_class_name)     \
+  BPF_DEATH_TEST_C(                                                      \
+      test_case_name, test_name, DEATH_SUCCESS(), bpf_policy_class_name)
+
+// Assertions are handled exactly the same as with a normal SANDBOX_TEST()
+#define BPF_ASSERT SANDBOX_ASSERT
+#define BPF_ASSERT_EQ(x, y) BPF_ASSERT((x) == (y))
+#define BPF_ASSERT_NE(x, y) BPF_ASSERT((x) != (y))
+#define BPF_ASSERT_LT(x, y) BPF_ASSERT((x) < (y))
+#define BPF_ASSERT_GT(x, y) BPF_ASSERT((x) > (y))
+#define BPF_ASSERT_LE(x, y) BPF_ASSERT((x) <= (y))
+#define BPF_ASSERT_GE(x, y) BPF_ASSERT((x) >= (y))
 
 }  // namespace sandbox