Extract the load/store type verification to a separate function.

Summary:
Added isLoadableOrStorableType to PointerType.

We were doing some checks in some places, occasionally assert()ing instead
of telling the caller. With this patch, I'm putting all type checking in
the same place for load/store type instructions, and verifying the same
thing every time.

I also added a check for load/store of a function type.

Applied extracted check to Load, Store, and Cmpxcg.

I don't have exhaustive tests for all of these, but all Error() calls in
TypeCheckLoadStoreInst are being tested (in invalid.test).

Reviewers: dblaikie, rafael

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D9785

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@237619 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/IR/DerivedTypes.h b/include/llvm/IR/DerivedTypes.h
index 76f806434f44f5307d6c75161fd0579712669696..38f1af0d70dab9f76c686e38e7d4484aa0084e54 100644 (file)
--- a/include/llvm/IR/DerivedTypes.h
+++ b/include/llvm/IR/DerivedTypes.h
@@ -464,6 +464,9 @@ public:
   /// element type.
   static bool isValidElementType(Type *ElemTy);
 
+  /// Return true if we can load or store from a pointer to this type.
+  static bool isLoadableOrStorableType(Type *ElemTy);
+
   /// @brief Return the address space of the Pointer type.
   inline unsigned getAddressSpace() const { return getSubclassData(); }
 

diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp
index e0800916c8cd6d0c8caa5b57b2aaa8fd30959a96..86c61bdf66b33035527114c225588106b25de8d4 100644 (file)
--- a/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -401,6 +401,12 @@ static std::error_code Error(DiagnosticHandlerFunction DiagnosticHandler,
   return Error(DiagnosticHandler, EC, EC.message());
 }
 
+static std::error_code Error(DiagnosticHandlerFunction DiagnosticHandler,
+                             const Twine &Message) {
+  return Error(DiagnosticHandler,
+               make_error_code(BitcodeError::CorruptedBitcode), Message);
+}
+
 std::error_code BitcodeReader::Error(BitcodeError E, const Twine &Message) {
   return ::Error(DiagnosticHandler, make_error_code(E), Message);
 }
@@ -3290,6 +3296,20 @@ std::error_code BitcodeReader::ParseMetadataAttachment(Function &F) {
   }
 }
 
+static std::error_code TypeCheckLoadStoreInst(DiagnosticHandlerFunction DH,
+                                              Type *ValType, Type *PtrType) {
+  if (!isa<PointerType>(PtrType))
+    return Error(DH, "Load/Store operand is not a pointer type");
+  Type *ElemType = cast<PointerType>(PtrType)->getElementType();
+
+  if (ValType && ValType != ElemType)
+    return Error(DH, "Explicit load/store type does not match pointee type of "
+                     "pointer operand");
+  if (!PointerType::isLoadableOrStorableType(ElemType))
+    return Error(DH, "Cannot load/store from pointer");
+  return std::error_code();
+}
+
 /// ParseFunctionBody - Lazily parse the specified function body block.
 std::error_code BitcodeReader::ParseFunctionBody(Function *F) {
   if (Stream.EnterSubBlock(bitc::FUNCTION_BLOCK_ID))
@@ -4071,13 +4091,11 @@ std::error_code BitcodeReader::ParseFunctionBody(Function *F) {
       Type *Ty = nullptr;
       if (OpNum + 3 == Record.size())
         Ty = getTypeByID(Record[OpNum++]);
-      if (!isa<PointerType>(Op->getType()))
-        return Error("Load operand is not a pointer type");
+      if (std::error_code EC =
+              TypeCheckLoadStoreInst(DiagnosticHandler, Ty, Op->getType()))
+        return EC;
       if (!Ty)
         Ty = cast<PointerType>(Op->getType())->getElementType();
-      else if (Ty != cast<PointerType>(Op->getType())->getElementType())
-        return Error("Explicit load type does not match pointee type of "
-                     "pointer operand");
 
       unsigned Align;
       if (std::error_code EC = parseAlignmentValue(Record[OpNum], Align))
@@ -4098,6 +4116,11 @@ std::error_code BitcodeReader::ParseFunctionBody(Function *F) {
       Type *Ty = nullptr;
       if (OpNum + 5 == Record.size())
         Ty = getTypeByID(Record[OpNum++]);
+      if (std::error_code EC =
+              TypeCheckLoadStoreInst(DiagnosticHandler, Ty, Op->getType()))
+        return EC;
+      if (!Ty)
+        Ty = cast<PointerType>(Op->getType())->getElementType();
 
       AtomicOrdering Ordering = GetDecodedOrdering(Record[OpNum+2]);
       if (Ordering == NotAtomic || Ordering == Release ||
@@ -4112,10 +4135,6 @@ std::error_code BitcodeReader::ParseFunctionBody(Function *F) {
         return EC;
       I = new LoadInst(Op, "", Record[OpNum+1], Align, Ordering, SynchScope);
 
-      (void)Ty;
-      assert((!Ty || Ty == I->getType()) &&
-             "Explicit type doesn't match pointee type of the first operand");
-
       InstructionList.push_back(I);
       break;
     }
@@ -4131,6 +4150,10 @@ std::error_code BitcodeReader::ParseFunctionBody(Function *F) {
                           Val)) ||
           OpNum + 2 != Record.size())
         return Error("Invalid record");
+
+      if (std::error_code EC = TypeCheckLoadStoreInst(
+              DiagnosticHandler, Val->getType(), Ptr->getType()))
+        return EC;
       unsigned Align;
       if (std::error_code EC = parseAlignmentValue(Record[OpNum], Align))
         return EC;
@@ -4152,6 +4175,9 @@ std::error_code BitcodeReader::ParseFunctionBody(Function *F) {
           OpNum + 4 != Record.size())
         return Error("Invalid record");
 
+      if (std::error_code EC = TypeCheckLoadStoreInst(
+              DiagnosticHandler, Val->getType(), Ptr->getType()))
+        return EC;
       AtomicOrdering Ordering = GetDecodedOrdering(Record[OpNum+2]);
       if (Ordering == NotAtomic || Ordering == Acquire ||
           Ordering == AcquireRelease)
@@ -4187,6 +4213,9 @@ std::error_code BitcodeReader::ParseFunctionBody(Function *F) {
         return Error("Invalid record");
       SynchronizationScope SynchScope = GetDecodedSynchScope(Record[OpNum+2]);
 
+      if (std::error_code EC = TypeCheckLoadStoreInst(
+              DiagnosticHandler, Cmp->getType(), Ptr->getType()))
+        return EC;
       AtomicOrdering FailureOrdering;
       if (Record.size() < 7)
         FailureOrdering =

diff --git a/lib/IR/Type.cpp b/lib/IR/Type.cpp
index d8baf7c9d332b2c1c9ce3d77675186a955f1afff..1c405168ae217e6bb81d2e3f02994ed7e38405b3 100644 (file)
--- a/lib/IR/Type.cpp
+++ b/lib/IR/Type.cpp
@@ -765,3 +765,7 @@ bool PointerType::isValidElementType(Type *ElemTy) {
   return !ElemTy->isVoidTy() && !ElemTy->isLabelTy() &&
          !ElemTy->isMetadataTy();
 }
+
+bool PointerType::isLoadableOrStorableType(Type *ElemTy) {
+  return isValidElementType(ElemTy) && !ElemTy->isFunctionTy();
+}

diff --git a/test/Bitcode/Inputs/invalid-load-ptr-type.bc b/test/Bitcode/Inputs/invalid-load-ptr-type.bc
new file mode 100644 (file)
index 0000000..5207ed9
Binary files /dev/null and b/test/Bitcode/Inputs/invalid-load-ptr-type.bc differ

diff --git a/test/Bitcode/invalid.test b/test/Bitcode/invalid.test
index fbd1cb9f4d90ad77b3fede438d543fe0ad0affc3..7dd97a4f49a8b2a4db5bc9142cf2f26bf7db8598 100644 (file)
--- a/test/Bitcode/invalid.test
+++ b/test/Bitcode/invalid.test
@@ -35,7 +35,7 @@ BAD-TYPE-TABLE-FORWARD-REF: Invalid TYPE table: Only named structs can be forwar
 BAD-BITWIDTH: Bitwidth for integer type out of range
 BAD-ALIGN: Invalid alignment value
 MISMATCHED-EXPLICIT-GEP: Explicit gep type does not match pointee type of pointer operand
-MISMATCHED-EXPLICIT-LOAD: Explicit load type does not match pointee type of pointer operand
+MISMATCHED-EXPLICIT-LOAD: Explicit load/store type does not match pointee type of pointer operand
 MISMATCHED-EXPLICIT-GEP-OPERATOR: Explicit gep operator type does not match pointee type of pointer operand
 MISMATCHED-EXPLICIT-CALL: Explicit call type does not match pointee type of callee operand
 NON-FUNCTION-EXPLICIT-CALL: Explicit call type is not a function type
@@ -121,7 +121,7 @@ HUGE-FWDREF: Invalid record
 RUN: not llvm-dis -disable-output %p/Inputs/invalid-load-pointer-type.bc 2>&1 | \
 RUN:   FileCheck --check-prefix=LOAD-BAD-TYPE %s
 
-LOAD-BAD-TYPE: Load operand is not a pointer type
+LOAD-BAD-TYPE: Load/Store operand is not a pointer type
 
 RUN: not llvm-dis -disable-output %p/Inputs/invalid-GCTable-overflow.bc 2>&1 | \
 RUN:   FileCheck --check-prefix=GCTABLE-OFLOW %s
@@ -137,3 +137,8 @@ RUN: not llvm-dis -disable-output %p/Inputs/invalid-extract-0-indices.bc 2>&1 |
 RUN:   FileCheck --check-prefix=EXTRACT-0-IDXS %s
 
 EXTRACT-0-IDXS: EXTRACTVAL: Invalid instruction with 0 indices
+
+RUN: not llvm-dis -disable-output %p/Inputs/invalid-load-ptr-type.bc 2>&1 | \
+RUN:   FileCheck --check-prefix=BAD-LOAD-PTR-TYPE %s
+
+BAD-LOAD-PTR-TYPE: Cannot load/store from pointer