Stop using the BKPT ARM instruction

src/include/arm_sandbox.h

@@ -17,6 +17,14 @@
 #ifndef NATIVE_CLIENT_SRC_INCLUDE_ARM_SANDBOX_H_
 #define NATIVE_CLIENT_SRC_INCLUDE_ARM_SANDBOX_H_ 1
 
+/*
+ * UDF #0x0010: generic breakpoint.
+ *
+ * The Linux kernel treats 0x10  specially - generates SIGTRAP instead of SIGILL.
+ * Usable statically by users or dynamically by the runtime.
+ */
+#define NACL_INSTR_ARM_BREAKPOINT        0xE7F001F0
+
 /*
  * Specially chosen BKPT and UDF instructions that also correspond to
  * BKPT and UDF when decoded as Thumb instructions.
@@ -28,6 +36,9 @@
  *
  * Treated as a roadblock by the validator: all words that follow it in
  * a bundle aren't validated and can't be branched to.
+ * TODO: also change this to a UDF instruction. But it also has to be changed
+ * in the toolchain (linker?). It is only generated when there is a jump
+ * of distance over 32MB.
  */
 #define NACL_INSTR_ARM_LITERAL_POOL_HEAD 0xE125BE70
 
@@ -41,13 +52,6 @@
  * validates the instructions that follow.
  */
 
-/*
- * BKPT #0x5BEF: generic breakpoint.
- *
- * Usable statically by users or dynamically by the runtime.
- */
-#define NACL_INSTR_ARM_BREAKPOINT        0xE125BE7F
-
 /*
  * UDF #0xEDEF: halt-fill.
  *

src/trusted/debug_stub/abi.cc

@@ -266,8 +266,7 @@ static Abi::BPDef breakpoint_x86 = {
   breakpoint_code_x86
 };
 
-// We use an illegal instruction rather than BKPT because BKPT cannot
-// be caught under qemu-arm whereas illegal instructions can.
+// We use an illegal instruction for debugger breakpoints.
 static uint32_t breakpoint_code_arm[] = { NACL_INSTR_ARM_ABORT_NOW };
 static Abi::BPDef breakpoint_arm = {
   sizeof(breakpoint_code_arm),

src/trusted/debug_stub/target.cc

@@ -137,7 +137,7 @@ bool Target::AddBreakpoint(uint32_t user_address) {
   if (!IPlatform::GetMemory(sys_bundle_addr, sizeof(uint32_t), &bundle_head))
     return false;
 
-  if (nacl_arm_dec::IsBreakPointAndConstantPoolHead(bundle_head)) {
+  if (nacl_arm_dec::IsConstantPoolHead(bundle_head)) {
     NaClLog(LOG_ERROR,
             "Failed to set breakpoint at 0x%x, within constant pool\n",
             user_address);

src/trusted/validator/validator_debug_stub_test.cc

@@ -398,21 +398,21 @@ static const uint32_t offset_pseudo_arm[kCodeSize/4] = {
   NACL_INSTR_ARM_NOP
 };
 
-// 2 Constant pools using both markers.
+// 2 Constant pools.
 static const uint32_t constant_pools_arm[kCodeSize/4] = {
   NACL_INSTR_ARM_LITERAL_POOL_HEAD,
   0xEF000000,  // SVC #0
   0xEF000000,
   0xEF000000,
-  NACL_INSTR_ARM_BREAKPOINT,
+  NACL_INSTR_ARM_LITERAL_POOL_HEAD,
   0xEF000000,
   0xEF000000,
   0xEF000000,
 };
 
 // Constant pool followed by a code bundle.
 static const uint32_t constant_pool_first_arm[kCodeSize/4] = {
-  NACL_INSTR_ARM_BREAKPOINT,
+  NACL_INSTR_ARM_LITERAL_POOL_HEAD,
   0xEF000000,  // SVC #0
   0xEF000000,
   0xEF000000,
@@ -428,7 +428,7 @@ static const uint32_t constant_pool_second_arm[kCodeSize/4] = {
   NACL_INSTR_ARM_NOP,  // 04
   NACL_INSTR_ARM_NOP,  // 08
   NACL_INSTR_ARM_NOP,  // 0c
-  NACL_INSTR_ARM_BREAKPOINT,
+  NACL_INSTR_ARM_LITERAL_POOL_HEAD,
   0xEF000000,  // SVC #0
   0xEF000000,
   0xEF000000

src/trusted/validator_arm/arm_helpers.h

@@ -75,7 +75,7 @@ inline uint32_t LiteralPoolHeadConstant() {
 // Returns true if the UDF instruction matches encoding values we've chosen
 // to be safe.
 inline bool IsUDFNaClSafe(uint32_t inst_bits) {
-  return inst_bits == kHaltFill || inst_bits == kAbortNow;
+  return inst_bits == kHaltFill || inst_bits == kAbortNow || inst_bits == kBreakpoint;
 }
 
 }  // namespace nacl_arm_dec

src/trusted/validator_arm/armv7.table

@@ -947,7 +947,7 @@
    defs := {};
    uses := {};
    safety := cond=~1110 => UNPREDICTABLE &
-             not IsBreakPointAndConstantPoolHead(inst) => FORBIDDEN_OPERANDS;
+             not IsConstantPoolHead(inst) => FORBIDDEN_OPERANDS;
    arch := v5T;
 +--
 | op2(6:4) B(9) op(22:21) op1(19:16)

src/trusted/validator_arm/build.scons

@@ -182,8 +182,8 @@ def _generate_source(filename, table=None):
                       'dgen_actuals.py',
                       'dgen_baselines.py',
                       ],
-              action=['${SOURCES[1].abspath} ${SOURCES[0].abspath} '
-                      '${TARGET.abspath} Arm32DecoderState ' +
+              action=['${SOURCES[1]} ${SOURCES[0]} '
+                      '${TARGET} Arm32DecoderState ' +
                       cl_args])
 
 for source in GEN_OTHER_LIST:
@@ -221,7 +221,7 @@ gtest_env.ComponentProgram('address_set_test_binary',
 
 address_set_test = gtest_env.Command(target='address_set_test.out',
                                      source=['address_set_test_binary'],
-                                     action=['${SOURCES[0].abspath}'])
+                                     action=['${SOURCES[0]}'])
 
 # TODO(cbiffle): get this wrapped in QEMU.
 #env.AddNodeToTestSuite(address_set_test, ['small_tests'], 'address_set_test')

src/trusted/validator_arm/dgen_input.py

@@ -152,7 +152,7 @@ def __repr__(self):
 _STRING_PATTERN = re.compile(r'^\'(.*)\'$')
 
 # When true, catch all bugs when parsing and report line.
-_CATCH_EXCEPTIONS = True
+_CATCH_EXCEPTIONS = False
 
 # List of file level decoder actions that must be specified in every
 # specification file, because they are used somewhere else than in table rows.

src/trusted/validator_arm/dgen_test_output.py

@@ -1,5 +1,3 @@
-#!/usr/bin/python
-#
 # Copyright (c) 2012 The Native Client Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.

src/trusted/validator_arm/gen/arm32_decode_actuals_1.cc

@@ -559,7 +559,7 @@ uses(Instruction inst) const {
 //    is_literal_pool_head: LiteralPoolHeadConstant()  ==
 //            inst,
 //    safety: [inst(31:28)=~1110 => UNPREDICTABLE,
-//      not IsBreakPointAndConstantPoolHead(inst) => FORBIDDEN_OPERANDS],
+//      not IsConstantPoolHead(inst) => FORBIDDEN_OPERANDS],
 //    uses: {},
 //    violations: [implied by 'is_literal_pool_head']}
 
@@ -587,8 +587,8 @@ safety(Instruction inst) const {
           0xE0000000)
     return UNPREDICTABLE;
 
-  // not IsBreakPointAndConstantPoolHead(inst) => FORBIDDEN_OPERANDS
-  if (!(nacl_arm_dec::IsBreakPointAndConstantPoolHead(inst.Bits())))
+  // not IsConstantPoolHead(inst) => FORBIDDEN_OPERANDS
+  if (!(nacl_arm_dec::IsConstantPoolHead(inst.Bits())))
     return FORBIDDEN_OPERANDS;
 
   return MAY_BE_SAFE;

src/trusted/validator_arm/gen/arm32_decode_actuals_1.h

@@ -1382,7 +1382,7 @@ class Actual_BIC_immediate_cccc0011110snnnnddddiiiiiiiiiiii_case_1
 //    is_literal_pool_head: LiteralPoolHeadConstant()  ==
 //            inst,
 //    safety: [inst(31:28)=~1110 => UNPREDICTABLE,
-//      not IsBreakPointAndConstantPoolHead(inst) => FORBIDDEN_OPERANDS],
+//      not IsConstantPoolHead(inst) => FORBIDDEN_OPERANDS],
 //    uses: {},
 //    violations: [implied by 'is_literal_pool_head']}
 //
@@ -1402,7 +1402,7 @@ class Actual_BIC_immediate_cccc0011110snnnnddddiiiiiiiiiiii_case_1
 //    pattern: cccc00010010iiiiiiiiiiii0111iiii,
 //    rule: BKPT,
 //    safety: [cond(31:28)=~1110 => UNPREDICTABLE,
-//      not IsBreakPointAndConstantPoolHead(inst) => FORBIDDEN_OPERANDS],
+//      not IsConstantPoolHead(inst) => FORBIDDEN_OPERANDS],
 //    uses: {},
 //    violations: [implied by 'is_literal_pool_head']}
 class Actual_BKPT_cccc00010010iiiiiiiiiiii0111iiii_case_1

src/trusted/validator_arm/gen/arm32_decode_baselines_1.cc

@@ -1208,7 +1208,7 @@ uses(Instruction inst) const {
 //    pattern: cccc00010010iiiiiiiiiiii0111iiii,
 //    rule: BKPT,
 //    safety: [cond(31:28)=~1110 => UNPREDICTABLE,
-//      not IsBreakPointAndConstantPoolHead(inst) => FORBIDDEN_OPERANDS],
+//      not IsConstantPoolHead(inst) => FORBIDDEN_OPERANDS],
 //    uses: {},
 //    violations: [implied by 'is_literal_pool_head']}
 RegisterList BKPT_cccc00010010iiiiiiiiiiii0111iiii_case_0::
@@ -1235,8 +1235,8 @@ safety(Instruction inst) const {
           0xE0000000)
     return UNPREDICTABLE;
 
-  // not IsBreakPointAndConstantPoolHead(inst) => FORBIDDEN_OPERANDS
-  if (!(nacl_arm_dec::IsBreakPointAndConstantPoolHead(inst.Bits())))
+  // not IsConstantPoolHead(inst) => FORBIDDEN_OPERANDS
+  if (!(nacl_arm_dec::IsConstantPoolHead(inst.Bits())))
     return FORBIDDEN_OPERANDS;
 
   return MAY_BE_SAFE;

src/trusted/validator_arm/gen/arm32_decode_baselines_1.h

@@ -685,7 +685,7 @@ class BIC_register_shifted_register_cccc0001110snnnnddddssss0tt1mmmm_case_0
 //    pattern: cccc00010010iiiiiiiiiiii0111iiii,
 //    rule: BKPT,
 //    safety: [cond(31:28)=~1110 => UNPREDICTABLE,
-//      not IsBreakPointAndConstantPoolHead(inst) => FORBIDDEN_OPERANDS],
+//      not IsConstantPoolHead(inst) => FORBIDDEN_OPERANDS],
 //    uses: {},
 //    violations: [implied by 'is_literal_pool_head']}
 class BKPT_cccc00010010iiiiiiiiiiii0111iiii_case_0

src/trusted/validator_arm/gen/arm32_decode_miscellaneous_instructions_tests.cc

@@ -613,7 +613,7 @@ ::PassesParsePreconditions(
 //       pattern: cccc00010010iiiiiiiiiiii0111iiii,
 //       rule: BKPT,
 //       safety: [cond(31:28)=~1110 => UNPREDICTABLE,
-//         not IsBreakPointAndConstantPoolHead(inst) => FORBIDDEN_OPERANDS],
+//         not IsConstantPoolHead(inst) => FORBIDDEN_OPERANDS],
 //       uses: {},
 //       violations: [implied by 'is_literal_pool_head']}
 class BKPT_cccc00010010iiiiiiiiiiii0111iiii_case_0TesterCase12
@@ -988,7 +988,7 @@ class ERET_cccc0001011000000000000001101110_case_0Tester_Case11
 //       pattern: cccc00010010iiiiiiiiiiii0111iiii,
 //       rule: BKPT,
 //       safety: [cond(31:28)=~1110 => UNPREDICTABLE,
-//         not IsBreakPointAndConstantPoolHead(inst) => FORBIDDEN_OPERANDS],
+//         not IsConstantPoolHead(inst) => FORBIDDEN_OPERANDS],
 //       uses: {},
 //       violations: [implied by 'is_literal_pool_head']}
 class BKPT_cccc00010010iiiiiiiiiiii0111iiii_case_0Tester_Case12
@@ -1286,7 +1286,7 @@ TEST_F(Arm32DecoderStateTests,
 //       pattern: cccc00010010iiiiiiiiiiii0111iiii,
 //       rule: BKPT,
 //       safety: [cond(31:28)=~1110 => UNPREDICTABLE,
-//         not IsBreakPointAndConstantPoolHead(inst) => FORBIDDEN_OPERANDS],
+//         not IsConstantPoolHead(inst) => FORBIDDEN_OPERANDS],
 //       uses: {},
 //       violations: [implied by 'is_literal_pool_head']}
 TEST_F(Arm32DecoderStateTests,

src/trusted/validator_arm/generate_decoder.py

@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python2
 #
 # Copyright 2012 The Native Client Authors.  All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
@@ -45,15 +45,7 @@
 import dgen_decoder
 
 def _localize_filename(filename):
-  """ Strips off directories above 'native_client', returning
-      a location neutral name for the file
-  """
-  m = re.match(r'.*/(native_client/.*)', filename)
-  if m:
-    return m.group(1)
-  else:
-    # Don't know localized
-    return filename
+  return 'native_client/' + filename
 
 def install_actuals_and_baselines(decoder, cl_args):
   if not decoder.primary:

src/trusted/validator_arm/model.h

@@ -404,14 +404,13 @@ class Instruction {
 };
 
 // Checks if instruction is a valid constant pool head.
-inline bool IsBreakPointAndConstantPoolHead(Instruction i) {
-  return ((i.Bits(31, 0) == kLiteralPoolHead) ||
-          (i.Bits(31, 0) == kBreakpoint));
+inline bool IsConstantPoolHead(Instruction i) {
+  return i.Bits(31, 0) == kLiteralPoolHead;
 }
 
 // Same as above, but with integer contents of instruction as argument.
-inline bool IsBreakPointAndConstantPoolHead(uint32_t i) {
-  return IsBreakPointAndConstantPoolHead(Instruction(i));
+inline bool IsConstantPoolHead(uint32_t i) {
+  return IsConstantPoolHead(Instruction(i));
 }
 
 }  // namespace nacl_arm_dec

src/trusted/validator_arm/validator.cc

@@ -435,7 +435,7 @@ bool SfiValidator::is_valid_inst_boundary(const CodeSegment& code,
       code.base() + offset)[0];
 
   // Check if addr falls within a constant pool.
-  if (nacl_arm_dec::IsBreakPointAndConstantPoolHead(instr))
+  if (nacl_arm_dec::IsConstantPoolHead(instr))
     return false;
 
   nacl_arm_dec::ViolationSet violations =

src/trusted/validator_arm/validator_small_tests.cc

@@ -1254,13 +1254,6 @@ TEST_F(ValidatorTests, LiteralPoolHeadIsBreakpoint) {
           "it needs to act as a roadblock");
 }
 
-TEST_F(ValidatorTests, Breakpoint) {
-  EXPECT_EQ(nacl_arm_dec::kBreakpoint & 0xFFF000F0,
-            0xE1200070)  // BKPT #0
-      << ("the breakpoint instruction should be a breakpoint: "
-          "it needs to trap");
-}
-
 TEST_F(ValidatorTests, HaltFill) {
   EXPECT_EQ(nacl_arm_dec::kHaltFill & 0xFFF000F0,
             0xE7F000F0)  // UDF #0
@@ -1286,10 +1279,10 @@ TEST_F(ValidatorTests, UDFAndBKPTValidateAsExpected) {
     arm_inst bkpt_inst = 0xE1200070 | ((i & 0xFFF0) << 4) | (i & 0xF);
     arm_inst udf_inst  = 0xE7F000F0 | ((i & 0xFFF0) << 4) | (i & 0xF);
     EXPECT_EQ(validate(&bkpt_inst, 1, kDefaultBaseAddr),
-              ((bkpt_inst == kLiteralPoolHead) ||
-               (bkpt_inst == nacl_arm_dec::kBreakpoint)));
+              (bkpt_inst == kLiteralPoolHead));
     EXPECT_EQ(validate(&udf_inst, 1, kDefaultBaseAddr),
               ((udf_inst == nacl_arm_dec::kHaltFill) ||
+               (udf_inst == nacl_arm_dec::kBreakpoint) ||
                (udf_inst == nacl_arm_dec::kAbortNow)));
     // Tautological note: kFailValidation should fail validation.
   }

tests/debug_stub/debugger_test.c

@@ -305,10 +305,10 @@ void test_arm_breakpoint(void) {
           ".word " NACL_TO_STRING(NACL_INSTR_ARM_NOP) "\n"
           ".word " NACL_TO_STRING(NACL_INSTR_ARM_NOP) "\n"
           ".word " NACL_TO_STRING(NACL_INSTR_ARM_NOP) "\n"
-          /* These are the two constant pool markers. */
+          /* These are the constant pool marker. */
           ".word " NACL_TO_STRING(NACL_INSTR_ARM_LITERAL_POOL_HEAD) "\n"
           ".p2align 4\n"
-          ".word " NACL_TO_STRING(NACL_INSTR_ARM_BREAKPOINT) "\n"
+          ".word " NACL_TO_STRING(NACL_INSTR_ARM_LITERAL_POOL_HEAD) "\n"
           ".p2align 4\n");
 #endif
 }