diff --git a/llvm/include/llvm/IR/Instructions.h b/llvm/include/llvm/IR/Instructions.h index 47693f40f64328..a94f5406f3b255 100644 --- a/llvm/include/llvm/IR/Instructions.h +++ b/llvm/include/llvm/IR/Instructions.h @@ -590,6 +590,18 @@ class AtomicCmpXchgInst : public Instruction { /// Transparently provide more efficient getOperand methods. DECLARE_TRANSPARENT_OPERAND_ACCESSORS(Value); + static bool isValidSuccessOrdering(AtomicOrdering Ordering) { + return Ordering != AtomicOrdering::NotAtomic && + Ordering != AtomicOrdering::Unordered; + } + + static bool isValidFailureOrdering(AtomicOrdering Ordering) { + return Ordering != AtomicOrdering::NotAtomic && + Ordering != AtomicOrdering::Unordered && + Ordering != AtomicOrdering::AcquireRelease && + Ordering != AtomicOrdering::Release; + } + /// Returns the success ordering constraint of this cmpxchg instruction. AtomicOrdering getSuccessOrdering() const { return getSubclassData(); @@ -597,8 +609,8 @@ class AtomicCmpXchgInst : public Instruction { /// Sets the success ordering constraint of this cmpxchg instruction. void setSuccessOrdering(AtomicOrdering Ordering) { - assert(Ordering != AtomicOrdering::NotAtomic && - "CmpXchg instructions can only be atomic."); + assert(isValidSuccessOrdering(Ordering) && + "invalid CmpXchg success ordering"); setSubclassData(Ordering); } @@ -609,8 +621,8 @@ class AtomicCmpXchgInst : public Instruction { /// Sets the failure ordering constraint of this cmpxchg instruction. void setFailureOrdering(AtomicOrdering Ordering) { - assert(Ordering != AtomicOrdering::NotAtomic && - "CmpXchg instructions can only be atomic."); + assert(isValidFailureOrdering(Ordering) && + "invalid CmpXchg failure ordering"); setSubclassData(Ordering); } diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp index 88fd26fdb7d5c8..3184aba838ac9b 100644 --- a/llvm/lib/AsmParser/LLParser.cpp +++ b/llvm/lib/AsmParser/LLParser.cpp @@ -32,6 +32,7 @@ #include "llvm/IR/GlobalIFunc.h" #include "llvm/IR/GlobalObject.h" #include "llvm/IR/InlineAsm.h" +#include "llvm/IR/Instructions.h" #include "llvm/IR/Intrinsics.h" #include "llvm/IR/LLVMContext.h" #include "llvm/IR/Metadata.h" @@ -7582,13 +7583,10 @@ int LLParser::parseCmpXchg(Instruction *&Inst, PerFunctionState &PFS) { parseOptionalCommaAlign(Alignment, AteExtraComma)) return true; - if (SuccessOrdering == AtomicOrdering::Unordered || - FailureOrdering == AtomicOrdering::Unordered) - return tokError("cmpxchg cannot be unordered"); - if (FailureOrdering == AtomicOrdering::Release || - FailureOrdering == AtomicOrdering::AcquireRelease) - return tokError( - "cmpxchg failure ordering cannot include release semantics"); + if (!AtomicCmpXchgInst::isValidSuccessOrdering(SuccessOrdering)) + return tokError("invalid cmpxchg success ordering"); + if (!AtomicCmpXchgInst::isValidFailureOrdering(FailureOrdering)) + return tokError("invalid cmpxchg failure ordering"); if (!Ptr->getType()->isPointerTy()) return error(PtrLoc, "cmpxchg operand must be a pointer"); if (!cast(Ptr->getType()) diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp index ea655f99de955b..7d23f3b5728f97 100644 --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp @@ -5143,6 +5143,10 @@ Error BitcodeReader::parseFunctionBody(Function *F) { ? AtomicCmpXchgInst::getStrongestFailureOrdering(SuccessOrdering) : getDecodedOrdering(Record[OpNum + 3]); + if (FailureOrdering == AtomicOrdering::NotAtomic || + FailureOrdering == AtomicOrdering::Unordered) + return error("Invalid record"); + const Align Alignment( TheModule->getDataLayout().getTypeStoreSize(Cmp->getType())); @@ -5192,9 +5196,8 @@ Error BitcodeReader::parseFunctionBody(Function *F) { const AtomicOrdering SuccessOrdering = getDecodedOrdering(Record[OpNum + 1]); - if (SuccessOrdering == AtomicOrdering::NotAtomic || - SuccessOrdering == AtomicOrdering::Unordered) - return error("Invalid record"); + if (!AtomicCmpXchgInst::isValidSuccessOrdering(SuccessOrdering)) + return error("Invalid cmpxchg success ordering"); const SyncScope::ID SSID = getDecodedSyncScopeID(Record[OpNum + 2]); @@ -5203,6 +5206,8 @@ Error BitcodeReader::parseFunctionBody(Function *F) { const AtomicOrdering FailureOrdering = getDecodedOrdering(Record[OpNum + 3]); + if (!AtomicCmpXchgInst::isValidFailureOrdering(FailureOrdering)) + return error("Invalid cmpxchg failure ordering"); const bool IsWeak = Record[OpNum + 4]; diff --git a/llvm/lib/IR/Instructions.cpp b/llvm/lib/IR/Instructions.cpp index 2415b9b244431a..f1df5001c134b5 100644 --- a/llvm/lib/IR/Instructions.cpp +++ b/llvm/lib/IR/Instructions.cpp @@ -1556,13 +1556,6 @@ void AtomicCmpXchgInst::Init(Value *Ptr, Value *Cmp, Value *NewVal, "Ptr must be a pointer to NewVal type!"); assert(getOperand(1)->getType() == getOperand(2)->getType() && "Cmp type and NewVal type must be same!"); - assert(SuccessOrdering != AtomicOrdering::NotAtomic && - "AtomicCmpXchg instructions must be atomic!"); - assert(FailureOrdering != AtomicOrdering::NotAtomic && - "AtomicCmpXchg instructions must be atomic!"); - assert(FailureOrdering != AtomicOrdering::Release && - FailureOrdering != AtomicOrdering::AcquireRelease && - "AtomicCmpXchg failure ordering cannot include release semantics"); } AtomicCmpXchgInst::AtomicCmpXchgInst(Value *Ptr, Value *Cmp, Value *NewVal, diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp index fb5c1a3088b92b..80f343365afda7 100644 --- a/llvm/lib/IR/Verifier.cpp +++ b/llvm/lib/IR/Verifier.cpp @@ -3826,29 +3826,7 @@ void Verifier::visitAllocaInst(AllocaInst &AI) { } void Verifier::visitAtomicCmpXchgInst(AtomicCmpXchgInst &CXI) { - - // FIXME: more conditions??? - Assert(CXI.getSuccessOrdering() != AtomicOrdering::NotAtomic, - "cmpxchg instructions must be atomic.", &CXI); - Assert(CXI.getFailureOrdering() != AtomicOrdering::NotAtomic, - "cmpxchg instructions must be atomic.", &CXI); - Assert(CXI.getSuccessOrdering() != AtomicOrdering::Unordered, - "cmpxchg instructions cannot be unordered.", &CXI); - Assert(CXI.getFailureOrdering() != AtomicOrdering::Unordered, - "cmpxchg instructions cannot be unordered.", &CXI); - Assert(CXI.getFailureOrdering() != AtomicOrdering::Release && - CXI.getFailureOrdering() != AtomicOrdering::AcquireRelease, - "cmpxchg failure ordering cannot include release semantics", &CXI); - - PointerType *PTy = dyn_cast(CXI.getOperand(0)->getType()); - Assert(PTy, "First cmpxchg operand must be a pointer.", &CXI); Type *ElTy = CXI.getOperand(1)->getType(); - Assert(PTy->isOpaqueOrPointeeTypeMatches(ElTy), - "Expected value type does not match pointer operand type!", &CXI, - ElTy); - Assert(ElTy == CXI.getOperand(2)->getType(), - "Stored value type does not match expected value operand type!", &CXI, - ElTy); Assert(ElTy->isIntOrPtrTy(), "cmpxchg operand must have integer or pointer type", ElTy, &CXI); checkAtomicMemAccessSize(ElTy, &CXI); @@ -3856,13 +3834,9 @@ void Verifier::visitAtomicCmpXchgInst(AtomicCmpXchgInst &CXI) { } void Verifier::visitAtomicRMWInst(AtomicRMWInst &RMWI) { - Assert(RMWI.getOrdering() != AtomicOrdering::NotAtomic, - "atomicrmw instructions must be atomic.", &RMWI); Assert(RMWI.getOrdering() != AtomicOrdering::Unordered, "atomicrmw instructions cannot be unordered.", &RMWI); auto Op = RMWI.getOperation(); - PointerType *PTy = dyn_cast(RMWI.getOperand(0)->getType()); - Assert(PTy, "First atomicrmw operand must be a pointer.", &RMWI); Type *ElTy = RMWI.getOperand(1)->getType(); if (Op == AtomicRMWInst::Xchg) { Assert(ElTy->isIntegerTy() || ElTy->isFloatingPointTy(), "atomicrmw " + @@ -3881,9 +3855,6 @@ void Verifier::visitAtomicRMWInst(AtomicRMWInst &RMWI) { &RMWI, ElTy); } checkAtomicMemAccessSize(ElTy, &RMWI); - Assert(PTy->isOpaqueOrPointeeTypeMatches(ElTy), - "Argument value type does not match pointer operand type!", &RMWI, - ElTy); Assert(AtomicRMWInst::FIRST_BINOP <= Op && Op <= AtomicRMWInst::LAST_BINOP, "Invalid binary operation!", &RMWI); visitInstruction(RMWI); diff --git a/llvm/test/Assembler/cmpxchg-ordering-2.ll b/llvm/test/Assembler/cmpxchg-ordering-2.ll new file mode 100644 index 00000000000000..5bbb7e8b103680 --- /dev/null +++ b/llvm/test/Assembler/cmpxchg-ordering-2.ll @@ -0,0 +1,7 @@ +; RUN: not llvm-as < %s 2>&1 | FileCheck %s + +define void @f(i32* %a, i32 %b, i32 %c) { +; CHECK: invalid cmpxchg failure ordering + %x = cmpxchg i32* %a, i32 %b, i32 %c acq_rel unordered + ret void +} diff --git a/llvm/test/Assembler/cmpxchg-ordering-3.ll b/llvm/test/Assembler/cmpxchg-ordering-3.ll new file mode 100644 index 00000000000000..fa760b217c11a7 --- /dev/null +++ b/llvm/test/Assembler/cmpxchg-ordering-3.ll @@ -0,0 +1,7 @@ +; RUN: not llvm-as < %s 2>&1 | FileCheck %s + +define void @f(i32* %a, i32 %b, i32 %c) { +; CHECK: invalid cmpxchg failure ordering + %x = cmpxchg i32* %a, i32 %b, i32 %c acq_rel acq_rel + ret void +} diff --git a/llvm/test/Assembler/cmpxchg-ordering-4.ll b/llvm/test/Assembler/cmpxchg-ordering-4.ll new file mode 100644 index 00000000000000..d33100daf0dc57 --- /dev/null +++ b/llvm/test/Assembler/cmpxchg-ordering-4.ll @@ -0,0 +1,7 @@ +; RUN: not llvm-as < %s 2>&1 | FileCheck %s + +define void @f(i32* %a, i32 %b, i32 %c) { +; CHECK: invalid cmpxchg failure ordering + %x = cmpxchg i32* %a, i32 %b, i32 %c acq_rel release + ret void +} diff --git a/llvm/test/Assembler/cmpxchg-ordering.ll b/llvm/test/Assembler/cmpxchg-ordering.ll new file mode 100644 index 00000000000000..62462ce3da1345 --- /dev/null +++ b/llvm/test/Assembler/cmpxchg-ordering.ll @@ -0,0 +1,7 @@ +; RUN: not llvm-as < %s 2>&1 | FileCheck %s + +define void @f(i32* %a, i32 %b, i32 %c) { +; CHECK: invalid cmpxchg success ordering + %x = cmpxchg i32* %a, i32 %b, i32 %c unordered monotonic + ret void +} diff --git a/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-2.bc b/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-2.bc new file mode 100644 index 00000000000000..76693f78c5aa75 Binary files /dev/null and b/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-2.bc differ diff --git a/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-3.bc b/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-3.bc new file mode 100644 index 00000000000000..2421e5bb47750e Binary files /dev/null and b/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-3.bc differ diff --git a/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-4.bc b/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-4.bc new file mode 100644 index 00000000000000..711a0a350be7b4 Binary files /dev/null and b/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering-4.bc differ diff --git a/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering.bc b/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering.bc new file mode 100644 index 00000000000000..c319f46f536dda Binary files /dev/null and b/llvm/test/Bitcode/Inputs/invalid-cmpxchg-ordering.bc differ diff --git a/llvm/test/Bitcode/invalid.test b/llvm/test/Bitcode/invalid.test index 90303586ec3b99..26bea8c2c7b1a9 100644 --- a/llvm/test/Bitcode/invalid.test +++ b/llvm/test/Bitcode/invalid.test @@ -235,3 +235,14 @@ RUN: not llvm-dis -disable-output %p/Inputs/invalid-fcmp-opnum.bc 2>&1 | \ RUN: FileCheck --check-prefix=INVALID-FCMP-OPNUM %s INVALID-FCMP-OPNUM: Invalid record: operand number exceeded available operands + +RUN: not llvm-dis -disable-output %p/Inputs/invalid-cmpxchg-ordering.bc 2>&1 | \ +RUN: FileCheck --check-prefix=CMPXCHG-ORDERING %s +RUN: not llvm-dis -disable-output %p/Inputs/invalid-cmpxchg-ordering-2.bc 2>&1 | \ +RUN: FileCheck --check-prefix=CMPXCHG-ORDERING %s +RUN: not llvm-dis -disable-output %p/Inputs/invalid-cmpxchg-ordering-3.bc 2>&1 | \ +RUN: FileCheck --check-prefix=CMPXCHG-ORDERING %s +RUN: not llvm-dis -disable-output %p/Inputs/invalid-cmpxchg-ordering-4.bc 2>&1 | \ +RUN: FileCheck --check-prefix=CMPXCHG-ORDERING %s + +CMPXCHG-ORDERING: Invalid cmpxchg {{failure|success}} ordering