OSDN Git Service

Add UBSAN build option and fix undefined behaviour errors.
authorAndrew Scull <ascull@google.com>
Wed, 9 Sep 2015 22:50:42 +0000 (15:50 -0700)
committerAndrew Scull <ascull@google.com>
Wed, 9 Sep 2015 22:50:42 +0000 (15:50 -0700)
1. The assembler tried to write to unaligned addresses but memcpy fixes this.
2. The InstKind and OperandKind enums allowed target specific kinds that did not
   fall in the defined range. Defining the maximum target kind in the enum sorts
   this problem.

BUG=
R=stichnot@chromium.org

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

Makefile.standalone
src/IceAssembler.h
src/IceConverter.cpp
src/IceInst.h
src/IceOperand.h
src/IceRNG.cpp

index 0210ed4..9469f81 100644 (file)
@@ -96,6 +96,18 @@ else
   OBJDIR := $(OBJDIR)+Asserts
 endif
 
+ifdef UBSAN
+  OBJDIR := $(OBJDIR)+UBSan
+  CXX_EXTRA += -fsanitize=undefined -fno-sanitize=vptr -fno-sanitize=nonnull-attribute
+  LD_EXTRA += -fsanitize=undefined
+endif
+
+ifdef UBSAN_TRAP
+  OBJDIR := $(OBJDIR)+UBSan_Trap
+  CXX_EXTRA += -fsanitize=undefined-trap -fsanitize-undefined-trap-on-error -fno-sanitize=vptr -fno-sanitize=nonnull-attribute
+  LD_EXTRA += -fsanitize=undefined-trap
+endif
+
 ifdef TSAN
   OBJDIR := $(OBJDIR)+TSan
   CXX_EXTRA += -fsanitize=thread
@@ -108,6 +120,12 @@ ifdef ASAN
   LD_EXTRA += -fsanitize=address
 endif
 
+ifdef MSAN
+  OBJDIR := $(OBJDIR)+MSan
+  CXX_EXTRA += -fsanitize=memory
+  LD_EXTRA += -fsanitize=memory
+endif
+
 SB_OBJDIR := $(OBJDIR)+Sandboxed
 
 $(info -----------------------------------------------)
index 7f5d909..5b07975 100644 (file)
@@ -97,24 +97,31 @@ public:
   AssemblerBuffer(Assembler &);
   ~AssemblerBuffer();
 
-  /// Basic support for emitting, loading, and storing.
+  /// \name Basic support for emitting, loading, and storing.
+  /// @{
+  // These use memcpy instead of assignment to avoid undefined behaviour of
+  // assigning to unaligned addresses. Since the size of the copy is known the
+  // compiler can inline the memcpy with simple moves.
   template <typename T> void emit(T Value) {
     assert(hasEnsuredCapacity());
-    *reinterpret_cast<T *>(Cursor) = Value;
+    memcpy(reinterpret_cast<void *>(Cursor), &Value, sizeof(T));
     Cursor += sizeof(T);
   }
 
   template <typename T> T load(intptr_t Position) const {
     assert(Position >= 0 &&
            Position <= (size() - static_cast<intptr_t>(sizeof(T))));
-    return *reinterpret_cast<T *>(Contents + Position);
+    T Value;
+    memcpy(&Value, reinterpret_cast<void *>(Contents + Position), sizeof(T));
+    return Value;
   }
 
   template <typename T> void store(intptr_t Position, T Value) {
     assert(Position >= 0 &&
            Position <= (size() - static_cast<intptr_t>(sizeof(T))));
-    *reinterpret_cast<T *>(Contents + Position) = Value;
+    memcpy(reinterpret_cast<void *>(Contents + Position), &Value, sizeof(T));
   }
+  /// @{
 
   /// Emit a fixup at the current location.
   void emitFixup(AssemblerFixup *Fixup) { Fixup->set_position(size()); }
index 1d10ba4..4450a79 100644 (file)
@@ -745,7 +745,7 @@ LLVM2ICEGlobalsConverter::convertGlobalsToIce(Module *Mod) {
       addGlobalInitializer(*VarDecl, Initializer);
     }
   }
-  return std::move(VariableDeclarations);
+  return VariableDeclarations;
 }
 
 void LLVM2ICEGlobalsConverter::addGlobalInitializer(
index 360135a..56c5d3a 100644 (file)
 #include "IceTypes.h"
 
 // TODO: The Cfg structure, and instructions in particular, need to be
-// validated for things like valid operand types, valid branch
-// targets, proper ordering of Phi and non-Phi instructions, etc.
-// Most of the validity checking will be done in the bitcode reader.
-// We need a list of everything that should be validated, and tests
-// for each.
+// validated for things like valid operand types, valid branch targets, proper
+// ordering of Phi and non-Phi instructions, etc. Most of the validity
+// checking will be done in the bitcode reader. We need a list of everything
+// that should be validated, and tests for each.
 
 namespace Ice {
 
-/// Base instruction class for ICE.  Inst has two subclasses:
-/// InstHighLevel and InstTarget.  High-level ICE instructions inherit
-/// from InstHighLevel, and low-level (target-specific) ICE
-/// instructions inherit from InstTarget.
+/// Base instruction class for ICE. Inst has two subclasses: InstHighLevel and
+/// InstTarget. High-level ICE instructions inherit from InstHighLevel, and
+/// low-level (target-specific) ICE instructions inherit from InstTarget.
 class Inst : public llvm::ilist_node<Inst> {
   Inst() = delete;
   Inst(const Inst &) = delete;
@@ -68,13 +66,14 @@ public:
     FakeUse,      // not part of LLVM/PNaCl bitcode
     FakeKill,     // not part of LLVM/PNaCl bitcode
     JumpTable,    // not part of LLVM/PNaCl bitcode
-    Target        // target-specific low-level ICE
-                  // Anything >= Target is an InstTarget subclass.
-                  // Note that the value-spaces are shared across targets.
-                  // To avoid confusion over the definition of shared values,
-                  // an object specific to one target should never be passed
-                  // to a different target.
+    // Anything >= Target is an InstTarget subclass. Note that the value-spaces
+    // are shared across targets. To avoid confusion over the definition of
+    // shared values, an object specific to one target should never be passed
+    // to a different target.
+    Target,
+    Target_Max = std::numeric_limits<uint8_t>::max(),
   };
+  static_assert(Target <= Target_Max, "Must not be above max.");
   InstKind getKind() const { return Kind; }
 
   InstNumberT getNumber() const { return Number; }
@@ -107,13 +106,13 @@ public:
   bool isLastUse(const Operand *Src) const;
   void spliceLivenessInfo(Inst *OrigInst, Inst *SpliceAssn);
 
-  /// Returns a list of out-edges corresponding to a terminator
-  /// instruction, which is the last instruction of the block.
-  /// The list must not contain duplicates.
+  /// Returns a list of out-edges corresponding to a terminator instruction,
+  /// which is the last instruction of the block. The list must not contain
+  /// duplicates.
   virtual NodeList getTerminatorEdges() const {
-    // All valid terminator instructions override this method.  For
-    // the default implementation, we assert in case some CfgNode
-    // is constructed without a terminator instruction at the end.
+    // All valid terminator instructions override this method. For the default
+    // implementation, we assert in case some CfgNode is constructed without a
+    // terminator instruction at the end.
     llvm_unreachable(
         "getTerminatorEdges() called on a non-terminator instruction");
     return NodeList();
@@ -131,19 +130,18 @@ public:
   virtual bool isSimpleAssign() const { return false; }
 
   void livenessLightweight(Cfg *Func, LivenessBV &Live);
-  /// Calculates liveness for this instruction.  Returns true if this
-  /// instruction is (tentatively) still live and should be retained,
-  /// and false if this instruction is (tentatively) dead and should be
-  /// deleted.  The decision is tentative until the liveness dataflow
-  /// algorithm has converged, and then a separate pass permanently
-  /// deletes dead instructions.
+  // Calculates liveness for this instruction.  Returns true if this
+  /// instruction is (tentatively) still live and should be retained, and false
+  /// if this instruction is (tentatively) dead and should be deleted. The
+  /// decision is tentative until the liveness dataflow algorithm has converged,
+  /// and then a separate pass permanently deletes dead instructions.
   bool liveness(InstNumberT InstNumber, LivenessBV &Live, Liveness *Liveness,
                 LiveBeginEndMap *LiveBegin, LiveBeginEndMap *LiveEnd);
 
-  /// Get the number of native instructions that this instruction
-  /// ultimately emits.  By default, high-level instructions don't
-  /// result in any native instructions, and a target-specific
-  /// instruction results in a single native instruction.
+  /// Get the number of native instructions that this instruction ultimately
+  /// emits. By default, high-level instructions don't result in any native
+  /// instructions, and a target-specific instruction results in a single native
+  /// instruction.
   virtual uint32_t getEmitInstCount() const { return 0; }
   // TODO(stichnot): Change Inst back to abstract once the g++ build
   // issue is fixed.  llvm::ilist<Ice::Inst> doesn't work under g++
@@ -960,6 +958,7 @@ protected:
   InstTarget(Cfg *Func, InstKind Kind, SizeT MaxSrcs, Variable *Dest)
       : Inst(Func, Kind, MaxSrcs, Dest) {
     assert(Kind >= Target);
+    assert(Kind <= Target_Max);
   }
 };
 
index 9638066..64ce01d 100644 (file)
@@ -8,11 +8,10 @@
 //===----------------------------------------------------------------------===//
 ///
 /// \file
-/// This file declares the Operand class and its target-independent
-/// subclasses.  The main classes are Variable, which represents an
-/// LLVM variable that is either register- or stack-allocated, and the
-/// Constant hierarchy, which represents integer, floating-point,
-/// and/or symbolic constants.
+/// This file declares the Operand class and its target-independent subclasses.
+/// The main classes are Variable, which represents an LLVM variable that is
+/// either register- or stack-allocated, and the Constant hierarchy, which
+/// represents integer, floating-point, and/or symbolic constants.
 ///
 //===----------------------------------------------------------------------===//
 
@@ -32,7 +31,7 @@ class Operand {
   Operand &operator=(const Operand &) = delete;
 
 public:
-  static const size_t MaxTargetKinds = 10;
+  static constexpr size_t MaxTargetKinds = 10;
   enum OperandKind {
     kConst_Base,
     kConstInteger32,
@@ -42,24 +41,25 @@ public:
     kConstRelocatable,
     kConstUndef,
     kConst_Target, // leave space for target-specific constant kinds
-    kConst_Num = kConst_Target + MaxTargetKinds,
+    kConst_Max = kConst_Target + MaxTargetKinds,
     kVariable,
     kVariable_Target, // leave space for target-specific variable kinds
-    kVariable_Num = kVariable_Target + MaxTargetKinds,
+    kVariable_Max = kVariable_Target + MaxTargetKinds,
     // Target-specific operand classes use kTarget as the starting
     // point for their Kind enum space. Note that the value-spaces are shared
     // across targets. To avoid confusion over the definition of shared
     // values, an object specific to one target should never be passed
     // to a different target.
-    kTarget
+    kTarget,
+    kTarget_Max = std::numeric_limits<uint8_t>::max(),
   };
+  static_assert(kTarget <= kTarget_Max, "Must not be above max.");
   OperandKind getKind() const { return Kind; }
   Type getType() const { return Ty; }
 
-  /// Every Operand keeps an array of the Variables referenced in
-  /// the operand.  This is so that the liveness operations can get
-  /// quick access to the variables of interest, without having to dig
-  /// so far into the operand.
+  /// Every Operand keeps an array of the Variables referenced in the operand.
+  /// This is so that the liveness operations can get quick access to the
+  /// variables of interest, without having to dig so far into the operand.
   SizeT getNumVars() const { return NumVars; }
   Variable *getVar(SizeT I) const {
     assert(I < getNumVars());
@@ -86,7 +86,10 @@ public:
   /// @}
 
 protected:
-  Operand(OperandKind Kind, Type Ty) : Ty(Ty), Kind(Kind) {}
+  Operand(OperandKind Kind, Type Ty) : Ty(Ty), Kind(Kind) {
+    // It is undefined behavior to have a larger value in the enum
+    assert(Kind <= kTarget_Max);
+  }
   virtual ~Operand() = default;
 
   const Type Ty;
@@ -118,7 +121,7 @@ public:
 
   static bool classof(const Operand *Operand) {
     OperandKind Kind = Operand->getKind();
-    return Kind >= kConst_Base && Kind <= kConst_Num;
+    return Kind >= kConst_Base && Kind <= kConst_Max;
   }
 
   /// Judge if this given immediate should be randomized or pooled
@@ -508,7 +511,7 @@ public:
 
   static bool classof(const Operand *Operand) {
     OperandKind Kind = Operand->getKind();
-    return Kind >= kVariable && Kind <= kVariable_Num;
+    return Kind >= kVariable && Kind <= kVariable_Max;
   }
 
 protected:
index 28efb8f..89b1893 100644 (file)
@@ -14,7 +14,7 @@
 
 #include "IceRNG.h"
 
-#include <time.h>
+#include <ctime>
 
 namespace Ice {