From 56db9e66601ba696d517f7c38d53d8d2e830b287 Mon Sep 17 00:00:00 2001 From: Pietro Fezzardi Date: Thu, 13 Apr 2023 15:54:32 +0200 Subject: [PATCH] Make getExpectedModelType more strict on integers This commit teaches getExpectedModelType about the fact that various bitwise operations are only allowed to have integer operands. It also updates VMA, which uses getExpectedModelType, to take this into account. --- lib/Support/ModelHelpers.cpp | 76 +++++++++++++++++++ .../TypeFlowGraph.cpp | 44 ++++++----- .../TypeFlowNode.cpp | 4 + lib/ValueManipulationAnalysis/VMAPipeline.cpp | 40 +++++----- tests/unit/ValueManipulationAnalysis.cpp | 31 ++++---- 5 files changed, 142 insertions(+), 53 deletions(-) diff --git a/lib/Support/ModelHelpers.cpp b/lib/Support/ModelHelpers.cpp index 7b406b4cb..4853f85bf 100644 --- a/lib/Support/ModelHelpers.cpp +++ b/lib/Support/ModelHelpers.cpp @@ -8,6 +8,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/IR/Constant.h" #include "llvm/IR/DerivedTypes.h" +#include "llvm/IR/InstrTypes.h" #include "llvm/IR/Instruction.h" #include "llvm/IR/Instructions.h" #include "llvm/IR/Value.h" @@ -559,6 +560,81 @@ getExpectedModelType(FunctionMetadataCache &Cache, } } else if (auto *Ret = dyn_cast(User)) { return handleReturnValue(ParentFunc()->Prototype(), Model); + } else if (auto *BinaryOp = dyn_cast(User)) { + using namespace model::PrimitiveTypeKind; + auto Opcode = BinaryOp->getOpcode(); + switch (Opcode) { + + case llvm::Instruction::SDiv: + case llvm::Instruction::SRem: { + model::QualifiedType Result; + auto BitWidth = U->get()->getType()->getIntegerBitWidth(); + revng_assert(BitWidth >= 8 and std::has_single_bit(BitWidth)); + auto Bytes = BitWidth / 8; + Result.UnqualifiedType() = Model.getPrimitiveType(Signed, Bytes); + return { Result }; + } break; + + case llvm::Instruction::UDiv: + case llvm::Instruction::URem: { + model::QualifiedType Result; + auto BitWidth = U->get()->getType()->getIntegerBitWidth(); + revng_assert(BitWidth >= 8 and std::has_single_bit(BitWidth)); + auto Bytes = BitWidth / 8; + Result.UnqualifiedType() = Model.getPrimitiveType(Unsigned, Bytes); + return { Result }; + } break; + + case llvm::Instruction::AShr: + case llvm::Instruction::LShr: + case llvm::Instruction::Shl: { + model::QualifiedType Result; + auto BitWidth = U->get()->getType()->getIntegerBitWidth(); + revng_assert(BitWidth >= 8 and std::has_single_bit(BitWidth)); + auto Bytes = BitWidth / 8; + + if (U->getOperandNo() == 0) { + switch (Opcode) { + case llvm::Instruction::AShr: + Result.UnqualifiedType() = Model.getPrimitiveType(Signed, Bytes); + break; + + case llvm::Instruction::LShr: + Result.UnqualifiedType() = Model.getPrimitiveType(Unsigned, Bytes); + break; + + case llvm::Instruction::Shl: + Result.UnqualifiedType() = Model.getPrimitiveType(Number, Bytes); + break; + + default: + revng_abort(); + } + } + + if (U->getOperandNo() == 1) + Result.UnqualifiedType() = Model.getPrimitiveType(Unsigned, Bytes); + + return { Result }; + } break; + + case llvm::Instruction::Mul: + case llvm::Instruction::And: + case llvm::Instruction::Or: + case llvm::Instruction::Xor: { + model::QualifiedType Result; + auto BitWidth = U->get()->getType()->getIntegerBitWidth(); + revng_assert(std::has_single_bit(BitWidth) + and (BitWidth == 1 or BitWidth >= 8)); + auto Bytes = (BitWidth == 1) ? 1 : BitWidth / 8; + Result.UnqualifiedType() = Model.getPrimitiveType(Number, Bytes); + return { Result }; + } break; + + default: + // no strict requirement for others + ; + } } return {}; diff --git a/lib/ValueManipulationAnalysis/TypeFlowGraph.cpp b/lib/ValueManipulationAnalysis/TypeFlowGraph.cpp index ac2bf5f9c..386029a26 100644 --- a/lib/ValueManipulationAnalysis/TypeFlowGraph.cpp +++ b/lib/ValueManipulationAnalysis/TypeFlowGraph.cpp @@ -69,8 +69,14 @@ static ColorSet getAcceptedColors(FunctionMetadataCache &Cache, // handling when there is no model. if (not DeducedTypes.empty()) { - if (DeducedTypes.size() == 1) - return QTToColor(DeducedTypes.back()); + if (DeducedTypes.size() == 1) { + ColorSet Result = QTToColor(DeducedTypes.back()); + if (isUse(Content) + and getUse(Content)->get()->getType()->isIntegerTy(1)) + Result.addColor(BOOLNESS); + + return Result; + } // There are cases in which we can associate to an LLVM value (typically // an aggregate) more than one model type, e.g. for values returned by @@ -164,27 +170,20 @@ static ColorSet getAcceptedColors(FunctionMetadataCache &Cache, case Instruction::AShr: if (IsContentInst or getOpNo(Content) == 0) - return SIGNEDNESS; + return SIGNEDNESS | NUMBERNESS; if (getOpNo(Content) == 1) return ~(FLOATNESS | POINTERNESS); break; case Instruction::LShr: if (IsContentInst or getOpNo(Content) == 0) - // TODO: rule on first operand too strict? - return UNSIGNEDNESS; + return UNSIGNEDNESS | NUMBERNESS; if (getOpNo(Content) == 1) return ~(FLOATNESS | POINTERNESS); break; case Instruction::Shl: - if (IsContentInst) - return ~(FLOATNESS | POINTERNESS); - if (getOpNo(Content) == 0) - // TODO: rule on first operand too strict? - return (SIGNEDNESS | UNSIGNEDNESS | BOOLNESS); - if (getOpNo(Content) == 1) - return ~(FLOATNESS | POINTERNESS); + return (SIGNEDNESS | UNSIGNEDNESS | BOOLNESS | NUMBERNESS); break; case Instruction::Mul: @@ -203,12 +202,10 @@ static ColorSet getAcceptedColors(FunctionMetadataCache &Cache, return BOOLNESS; break; - case Instruction::Trunc: case Instruction::And: case Instruction::Or: case Instruction::Xor: - // TODO: Restrict more what can be accepted by bitwise operations? - return ~NUMBERNESS; + return NUMBERNESS | UNSIGNEDNESS | SIGNEDNESS | BOOLNESS; break; case Instruction::GetElementPtr: @@ -372,7 +369,6 @@ static bool connect(TypeFlowNode *N1, TypeFlowNode *N2) { break; } - case Instruction::Mul: case Instruction::Sub: return AddBidirectionalEdge(UseNode, ValNode, @@ -393,13 +389,19 @@ static bool connect(TypeFlowNode *N1, TypeFlowNode *N2) { return AddBidirectionalEdge(UseNode, ValNode, ~NUMBERNESS); break; + case Instruction::Mul: case Instruction::And: case Instruction::Or: - case Instruction::Xor: - return AddBidirectionalEdge(UseNode, - ValNode, - ~(FLOATNESS | POINTERNESS | NUMBERNESS)); - break; + case Instruction::Xor: { + auto Colors = ~(FLOATNESS | POINTERNESS | NUMBERNESS); + if (not I->getType()->isIntegerTy(1)) + Colors &= ~BOOLNESS; + return AddBidirectionalEdge(UseNode, ValNode, Colors); + } break; + + // Freeze is transparent + case Instruction::Freeze: + return AddBidirectionalEdge(UseNode, ValNode, ALL_COLORS); } if (auto *Call = llvm::dyn_cast(I)) { diff --git a/lib/ValueManipulationAnalysis/TypeFlowNode.cpp b/lib/ValueManipulationAnalysis/TypeFlowNode.cpp index 16da6c40a..5c5631715 100644 --- a/lib/ValueManipulationAnalysis/TypeFlowNode.cpp +++ b/lib/ValueManipulationAnalysis/TypeFlowNode.cpp @@ -32,6 +32,10 @@ RecursiveCoroutine vma::QTToColor(const model::QualifiedType &QT) { rc_return POINTERNESS; if (QT.is(model::TypeKind::TypedefType)) { + + if (not QT.isScalar()) + rc_return NO_COLOR; + auto *UnqualT = QT.UnqualifiedType().getConst(); auto *TypedefT = llvm::cast(UnqualT); diff --git a/lib/ValueManipulationAnalysis/VMAPipeline.cpp b/lib/ValueManipulationAnalysis/VMAPipeline.cpp index 65e43c8c3..98ca7c886 100644 --- a/lib/ValueManipulationAnalysis/VMAPipeline.cpp +++ b/lib/ValueManipulationAnalysis/VMAPipeline.cpp @@ -126,12 +126,18 @@ static ColorSet getInitialCandidates(const UseOrValue &Content) { case Instruction::SDiv: case Instruction::SRem: - return SIGNEDNESS | NUMBERNESS; + if (IsContentInst) + return SIGNEDNESS | NUMBERNESS; + else + return SIGNEDNESS; break; case Instruction::UDiv: case Instruction::URem: - return UNSIGNEDNESS | NUMBERNESS; + if (IsContentInst) + return UNSIGNEDNESS | NUMBERNESS; + else + return UNSIGNEDNESS; break; case Instruction::Alloca: @@ -154,34 +160,28 @@ static ColorSet getInitialCandidates(const UseOrValue &Content) { break; case Instruction::AShr: - if (IsContentInst or getOpNo(Content) == 0) + if (IsContentInst) + return SIGNEDNESS | NUMBERNESS; + if (getOpNo(Content) == 0) return SIGNEDNESS; if (getOpNo(Content) == 1) return UNSIGNEDNESS; break; case Instruction::LShr: - if (IsContentInst or getOpNo(Content) == 0) - // TODO: rule on first operand too strict? - return UNSIGNEDNESS; - if (getOpNo(Content) == 1) + if (IsContentInst) + return UNSIGNEDNESS | NUMBERNESS; + else return UNSIGNEDNESS; break; case Instruction::Shl: - if (IsContentInst) - return NO_COLOR; - if (getOpNo(Content) == 0) - // TODO: rule on first operand too strict? + if (IsContentInst or getOpNo(Content) == 0) return NO_COLOR; if (getOpNo(Content) == 1) return UNSIGNEDNESS; break; - case Instruction::Mul: - return NUMBERNESS; - break; - case Instruction::Br: if (isUse(Content) && cast(I)->isConditional() && getUse(Content)->get() == cast(I)->getCondition()) @@ -195,11 +195,17 @@ static ColorSet getInitialCandidates(const UseOrValue &Content) { break; case Instruction::Trunc: + return NO_COLOR; + break; + case Instruction::And: case Instruction::Or: case Instruction::Xor: - // TODO: Restrict more what can be accepted by bitwise operations? - return NO_COLOR; + case Instruction::Mul: + if (IsContentInst) + return NUMBERNESS; + else + return NO_COLOR; break; case Instruction::GetElementPtr: diff --git a/tests/unit/ValueManipulationAnalysis.cpp b/tests/unit/ValueManipulationAnalysis.cpp index f661f44de..9de360e4e 100644 --- a/tests/unit/ValueManipulationAnalysis.cpp +++ b/tests/unit/ValueManipulationAnalysis.cpp @@ -85,7 +85,8 @@ static void checkShape(const TypeFlowGraph &TG, const ExpectedShape &Expected) { const TypeCounter ActualType = countTypes(TG); const ColorCounter ActualColor = countColors(TG); - dbgs() << " What Expected Actual\n"; + dbgs() << "========================\n"; + dbgs() << "What Expected Actual\n"; // Check that there are the expected number of nodes of each type for (size_t I = 0; I < MAX_TYPES; I++) { @@ -298,7 +299,7 @@ BOOST_AUTO_TEST_CASE(TestTGInit) { /*bools=*/0, /*signed=*/2, /*floats=*/0, - /*numbers=*/2 }, + /*numbers=*/1 }, /*casts=*/2, /*undecided=*/0 }, /*After propagation=*/ @@ -309,7 +310,7 @@ BOOST_AUTO_TEST_CASE(TestTGInit) { /*bools=*/0, /*signed=*/9, /*floats=*/0, - /*numbers=*/2 }, + /*numbers=*/1 }, /*casts=*/0, /*undecided=*/5 }, /*Final=*/ @@ -388,7 +389,7 @@ BOOST_AUTO_TEST_CASE(TestTGInit) { /*bools=*/0, /*signed=*/2, /*floats=*/0, - /*numbers=*/4 }, + /*numbers=*/2 }, /*casts=*/2, /*undecided=*/0 }, /*After propagation=*/ @@ -399,7 +400,7 @@ BOOST_AUTO_TEST_CASE(TestTGInit) { /*bools=*/0, /*signed=*/7, /*floats=*/0, - /*numbers=*/4 }, + /*numbers=*/2 }, /*casts=*/0, /*undecided=*/5 }, /*Final=*/ @@ -481,7 +482,7 @@ BOOST_AUTO_TEST_CASE(TestTGInit) { /*bools=*/0, /*signed=*/0, /*floats=*/0, - /*numbers=*/3 }, + /*numbers=*/1 }, /*casts=*/1, /*undecided=*/0 }, /*After propagation=*/ @@ -492,7 +493,7 @@ BOOST_AUTO_TEST_CASE(TestTGInit) { /*bools=*/0, /*signed=*/0, /*floats=*/0, - /*numbers=*/3 }, + /*numbers=*/1 }, /*casts=*/2, /*undecided=*/0 }, /*Final=*/ @@ -529,7 +530,7 @@ BOOST_AUTO_TEST_CASE(TestTGInit) { /*bools=*/0, /*signed=*/0, /*floats=*/0, - /*numbers=*/3 }, + /*numbers=*/1 }, /*casts=*/1, /*undecided=*/0 }, /*After propagation=*/ @@ -540,7 +541,7 @@ BOOST_AUTO_TEST_CASE(TestTGInit) { /*bools=*/0, /*signed=*/0, /*floats=*/0, - /*numbers=*/3 }, + /*numbers=*/1 }, /*casts=*/1, /*undecided=*/0 }, /*Final=*/ @@ -574,7 +575,7 @@ BOOST_AUTO_TEST_CASE(TestTGInit) { /*bools=*/2, /*signed=*/0, /*floats=*/0, - /*numbers=*/0 }, + /*numbers=*/1 }, /*casts=*/2, /*undecided=*/0 }, /*After propagation=*/ @@ -585,7 +586,7 @@ BOOST_AUTO_TEST_CASE(TestTGInit) { /*bools=*/5, /*signed=*/0, /*floats=*/0, - /*numbers=*/0 }, + /*numbers=*/1 }, /*casts=*/0, /*undecided=*/0 }, /*Final=*/ @@ -662,24 +663,24 @@ BOOST_AUTO_TEST_CASE(TestTGInit) { /*bools=*/0, /*signed=*/0, /*floats=*/0, - /*numbers=*/0 }, + /*numbers=*/1 }, /*casts=*/1, /*undecided=*/0 }, /*Final=*/ { { /*values=*/6, /*uses=*/4 }, - { /*pointers=*/7, + { /*pointers=*/6, /*unsigned=*/0, /*bools=*/0, /*signed=*/0, /*floats=*/0, - /*numbers=*/0 }, + /*numbers=*/1 }, /*casts=*/1, /*undecided=*/0 }, /*After propagation=*/ { { /*values=*/6, /*uses=*/4 }, - { /*pointers=*/7, + { /*pointers=*/6, /*unsigned=*/0, /*bools=*/0, /*signed=*/0,