Linux sandbox: Remove CodeGen::JoinInstructions.

Recent refactoring has made this function unnecessary.

BUG=414363

Review URL: https://codereview.chromium.org/565713004

Cr-Commit-Position: refs/heads/master@{#295423}

3 files changed, 25 insertions, 65 deletions

diff --git a/sandbox/linux/seccomp-bpf/codegen.cc b/sandbox/linux/seccomp-bpf/codegen.cc
index aa47155..2273caf 100644
--- a/sandbox/linux/seccomp-bpf/codegen.cc
+++ b/sandbox/linux/seccomp-bpf/codegen.cc
@@ -181,9 +181,7 @@ Instruction* CodeGen::MakeInstruction(uint16_t code,
   if (BPF_CLASS(code) != BPF_JMP || BPF_OP(code) == BPF_JA) {
     SANDBOX_DIE("Expected a BPF_JMP instruction");
   }
-  if (!jt && !jf) {
-    // We allow callers to defer specifying exactly one of the branch
-    // targets. It must then be set later by calling "JoinInstructions".
+  if (!jt || !jf) {
     SANDBOX_DIE("Branches must jump to a valid instruction");
   }
   Instruction* insn = new Instruction(code, k, jt, jf);
@@ -191,35 +189,6 @@ Instruction* CodeGen::MakeInstruction(uint16_t code,
   return insn;
 }
 
-void CodeGen::JoinInstructions(Instruction* head, Instruction* tail) {
-  // Merge two instructions, or set the branch target for an "always" jump.
-  // This function should be called, if the caller didn't initially provide
-  // a value for "next" when creating the instruction.
-  if (BPF_CLASS(head->code) == BPF_JMP) {
-    if (BPF_OP(head->code) == BPF_JA) {
-      if (head->jt_ptr) {
-        SANDBOX_DIE("Cannot append instructions in the middle of a sequence");
-      }
-      head->jt_ptr = tail;
-    } else {
-      if (!head->jt_ptr && head->jf_ptr) {
-        head->jt_ptr = tail;
-      } else if (!head->jf_ptr && head->jt_ptr) {
-        head->jf_ptr = tail;
-      } else {
-        SANDBOX_DIE("Cannot append instructions after a jump");
-      }
-    }
-  } else if (BPF_CLASS(head->code) == BPF_RET) {
-    SANDBOX_DIE("Cannot append instructions after a return statement");
-  } else if (head->next) {
-    SANDBOX_DIE("Cannot append instructions in the middle of a sequence");
-  } else {
-    head->next = tail;
-  }
-  return;
-}
-
 void CodeGen::Traverse(Instruction* instruction,
                        void (*fnc)(Instruction*, void*),
                        void* aux) {
diff --git a/sandbox/linux/seccomp-bpf/codegen.h b/sandbox/linux/seccomp-bpf/codegen.h
index d5c513b..078cf3c 100644
--- a/sandbox/linux/seccomp-bpf/codegen.h
+++ b/sandbox/linux/seccomp-bpf/codegen.h
@@ -30,20 +30,18 @@ typedef std::map<const BasicBlock*, int> IncomingBranches;
 // build a DAG of Instructions. They'll eventually call Compile() to convert
 // this DAG to a SandboxBPF::Program.
 //
-// Instructions can be chained at the time when they are created, or they
-// can be joined later by calling JoinInstructions().
-//
 //   CodeGen gen;
-//   Instruction *dag, *branch;
-//   dag =
-//     gen.MakeInstruction(BPF_LD+BPF_W+BPF_ABS,
-//                         offsetof(struct arch_seccomp_data, nr),
-//   branch =
-//     gen.MakeInstruction(BPF_JMP+BPF_EQ+BPF_K, __NR_getpid,
-//                         Trap(GetPidHandler, NULL), NULL);
-//   gen.JoinInstructions(branch,
+//   Instruction *allow, *branch, *dag;
+//
+//   allow =
 //     gen.MakeInstruction(BPF_RET+BPF_K,
 //                         ErrorCode(ErrorCode::ERR_ALLOWED).err()));
+//   branch =
+//     gen.MakeInstruction(BPF_JMP+BPF_EQ+BPF_K, __NR_getpid,
+//                         Trap(GetPidHandler, NULL), allow);
+//   dag =
+//     gen.MakeInstruction(BPF_LD+BPF_W+BPF_ABS,
+//                         offsetof(struct arch_seccomp_data, nr), branch);
 //
 //   // Simplified code follows; in practice, it is important to avoid calling
 //   // any C++ destructors after starting the sandbox.
@@ -74,11 +72,6 @@ class SANDBOX_EXPORT CodeGen {
                                Instruction* jt,
                                Instruction* jf);
 
-  // Join two (sequences of) instructions. This is useful, if the "next"
-  // parameter had not originally been given in the call to MakeInstruction(),
-  // or if a (conditional) jump still has an unsatisfied target.
-  void JoinInstructions(Instruction* head, Instruction* tail);
-
   // Traverse the graph of instructions and visit each instruction once.
   // Traversal order is implementation-defined. It is acceptable to make
   // changes to the graph from within the callback function. These changes
diff --git a/sandbox/linux/seccomp-bpf/codegen_unittest.cc b/sandbox/linux/seccomp-bpf/codegen_unittest.cc
index 3a5ca4b..5c1db24 100644
--- a/sandbox/linux/seccomp-bpf/codegen_unittest.cc
+++ b/sandbox/linux/seccomp-bpf/codegen_unittest.cc
@@ -84,28 +84,29 @@ Instruction* SampleProgramComplex(CodeGen* codegen, int* flags) {
   //    JUMP if eq 42 the $0 else $1     (insn6)
   // 0: LD 23                            (insn5)
   // 1: JUMP if eq 42 then $2 else $4    (insn4)
-  // 2: JUMP to $3                       (insn1)
-  // 3: LD 42                            (insn0)
-  //    RET 42                           (insn2)
+  // 2: JUMP to $3                       (insn2)
+  // 3: LD 42                            (insn1)
+  //    RET 42                           (insn0)
   // 4: LD 42                            (insn3)
   //    RET 42                           (insn3+)
   *flags = HAS_MERGEABLE_TAILS;
 
-  Instruction* insn0 = codegen->MakeInstruction(BPF_LD + BPF_W + BPF_ABS, 42);
+  Instruction* insn0 = codegen->MakeInstruction(BPF_RET + BPF_K, 42);
   SANDBOX_ASSERT(insn0);
-  SANDBOX_ASSERT(insn0->code == BPF_LD + BPF_W + BPF_ABS);
-  SANDBOX_ASSERT(insn0->k == 42);
+  SANDBOX_ASSERT(insn0->code == BPF_RET + BPF_K);
   SANDBOX_ASSERT(insn0->next == NULL);
 
-  Instruction* insn1 = codegen->MakeInstruction(BPF_JMP + BPF_JA, 0, insn0);
+  Instruction* insn1 =
+      codegen->MakeInstruction(BPF_LD + BPF_W + BPF_ABS, 42, insn0);
   SANDBOX_ASSERT(insn1);
-  SANDBOX_ASSERT(insn1->code == BPF_JMP + BPF_JA);
-  SANDBOX_ASSERT(insn1->jt_ptr == insn0);
+  SANDBOX_ASSERT(insn1->code == BPF_LD + BPF_W + BPF_ABS);
+  SANDBOX_ASSERT(insn1->k == 42);
+  SANDBOX_ASSERT(insn1->next == insn0);
 
-  Instruction* insn2 = codegen->MakeInstruction(BPF_RET + BPF_K, 42);
+  Instruction* insn2 = codegen->MakeInstruction(BPF_JMP + BPF_JA, 0, insn1);
   SANDBOX_ASSERT(insn2);
-  SANDBOX_ASSERT(insn2->code == BPF_RET + BPF_K);
-  SANDBOX_ASSERT(insn2->next == NULL);
+  SANDBOX_ASSERT(insn2->code == BPF_JMP + BPF_JA);
+  SANDBOX_ASSERT(insn2->jt_ptr == insn1);
 
   // We explicitly duplicate instructions so that MergeTails() can coalesce
   // them later.
@@ -115,16 +116,13 @@ Instruction* SampleProgramComplex(CodeGen* codegen, int* flags) {
       codegen->MakeInstruction(BPF_RET + BPF_K, 42));
 
   Instruction* insn4 =
-      codegen->MakeInstruction(BPF_JMP + BPF_JEQ + BPF_K, 42, insn1, insn3);
+      codegen->MakeInstruction(BPF_JMP + BPF_JEQ + BPF_K, 42, insn2, insn3);
   SANDBOX_ASSERT(insn4);
   SANDBOX_ASSERT(insn4->code == BPF_JMP + BPF_JEQ + BPF_K);
   SANDBOX_ASSERT(insn4->k == 42);
-  SANDBOX_ASSERT(insn4->jt_ptr == insn1);
+  SANDBOX_ASSERT(insn4->jt_ptr == insn2);
   SANDBOX_ASSERT(insn4->jf_ptr == insn3);
 
-  codegen->JoinInstructions(insn0, insn2);
-  SANDBOX_ASSERT(insn0->next == insn2);
-
   Instruction* insn5 =
       codegen->MakeInstruction(BPF_LD + BPF_W + BPF_ABS, 23, insn4);
   SANDBOX_ASSERT(insn5);