From 11bb8495f6f44ec842bed641f837df2e3cbc29fa Mon Sep 17 00:00:00 2001 From: Filipe Cabecinhas Date: Mon, 18 May 2015 21:48:55 +0000 Subject: [PATCH] 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 llvm-svn: 237619 --- llvm/include/llvm/IR/DerivedTypes.h | 3 ++ llvm/lib/Bitcode/Reader/BitcodeReader.cpp | 47 ++++++++++++++---- llvm/lib/IR/Type.cpp | 4 ++ .../Bitcode/Inputs/invalid-load-ptr-type.bc | Bin 0 -> 424 bytes llvm/test/Bitcode/invalid.test | 9 +++- 5 files changed, 52 insertions(+), 11 deletions(-) create mode 100644 llvm/test/Bitcode/Inputs/invalid-load-ptr-type.bc diff --git a/llvm/include/llvm/IR/DerivedTypes.h b/llvm/include/llvm/IR/DerivedTypes.h index 76f806434f44..38f1af0d70da 100644 --- a/llvm/include/llvm/IR/DerivedTypes.h +++ b/llvm/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/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp index e0800916c8cd..86c61bdf66b3 100644 --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/llvm/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(PtrType)) + return Error(DH, "Load/Store operand is not a pointer type"); + Type *ElemType = cast(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(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(Op->getType())->getElementType(); - else if (Ty != cast(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(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/llvm/lib/IR/Type.cpp b/llvm/lib/IR/Type.cpp index d8baf7c9d332..1c405168ae21 100644 --- a/llvm/lib/IR/Type.cpp +++ b/llvm/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/llvm/test/Bitcode/Inputs/invalid-load-ptr-type.bc b/llvm/test/Bitcode/Inputs/invalid-load-ptr-type.bc new file mode 100644 index 0000000000000000000000000000000000000000..5207ed9156e7229fd8ba068b8dfc11b0771cf278 GIT binary patch literal 424 zcmZ>AK5$Qwhk+rFfq{X$Nr8b0NDBcmd!zD1#}h1`Yyw7>lNeigR9QJB}F$U~Vl5k}h%XN#7@Jx&eml@;v8GYWa0 zG4Q_?;QP|RXUyXycj%z(xrH)m2CQIZ&C+L>ZBIDc_AuK5%_vl0U;vpXw!xy;#U+?k zM&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