From 67c6c81e536dfd9ac254bf924046a444057771f9 Mon Sep 17 00:00:00 2001 From: Pietro Fezzardi Date: Wed, 11 May 2022 18:04:31 +0200 Subject: [PATCH 1/7] MakeModelGEP: ignore undef and null addresses --- lib/IRCanonicalization/MakeModelGEPPass.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/IRCanonicalization/MakeModelGEPPass.cpp b/lib/IRCanonicalization/MakeModelGEPPass.cpp index 0d914934c..d05b89137 100644 --- a/lib/IRCanonicalization/MakeModelGEPPass.cpp +++ b/lib/IRCanonicalization/MakeModelGEPPass.cpp @@ -1927,6 +1927,16 @@ makeGEPReplacements(llvm::Function &F, const model::Binary &Model) { } } + // Skip null pointer constants, and undefs, since they cannot be valid + // addresses + // TODO: if we ever need to support memory mapped at address 0 we can + // probably work around this, but this is not top priority for now. + if (isa(U.get()) + or isa(U.get())) { + revng_log(ModelGEPLog, "Skipping null pointer address"); + continue; + } + ModelGEPSummation GEPSum = GEPSumCache.getGEPSummation(U, PointerTypes); revng_log(ModelGEPLog, "GEPSum " << GEPSum); From 35e0e688d22e1c83bd4bd727d73bd8687680901f Mon Sep 17 00:00:00 2001 From: Pietro Fezzardi Date: Wed, 11 May 2022 18:19:45 +0200 Subject: [PATCH 2/7] Support emission in C of integer or pointers undef --- lib/Backend/DecompileFunction.cpp | 5 +++++ lib/InitModelTypes/InitModelTypes.cpp | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/Backend/DecompileFunction.cpp b/lib/Backend/DecompileFunction.cpp index 1a19c0f3c..06f2afa4d 100644 --- a/lib/Backend/DecompileFunction.cpp +++ b/lib/Backend/DecompileFunction.cpp @@ -467,6 +467,11 @@ CCodeGenerator::addOperandToken(const llvm::Value *Operand) { } revng_assert(not Operand->getType()->isVoidTy()); + if (auto *Undef = dyn_cast(Operand)) { + revng_assert(Undef->getType()->isIntOrPtrTy()); + TokenMap[Operand] = "0 /* undef */"; + rc_return true; + } if (auto *Const = dyn_cast(Operand)) { llvm::APInt Value = Const->getValue(); diff --git a/lib/InitModelTypes/InitModelTypes.cpp b/lib/InitModelTypes/InitModelTypes.cpp index 176bde3af..86d191dc3 100644 --- a/lib/InitModelTypes/InitModelTypes.cpp +++ b/lib/InitModelTypes/InitModelTypes.cpp @@ -128,10 +128,11 @@ static RecursiveCoroutine addOperandType(const llvm::Value *Operand, } } } else if (isa(Operand) - or isa(Operand)) { + or isa(Operand) + or isa(Operand)) { // For constants and globals, fallback to the LLVM type - revng_assert(not Operand->getType()->isVoidTy()); + revng_assert(Operand->getType()->isIntOrPtrTy()); auto ConstType = llvmIntToModelType(Operand->getType(), Model); // Skip if it's not a pointer and we are only interested in pointers if (not PointersOnly or ConstType.isPointer()) { From 4ed636cbba37bf14b78dd88f8df38e242e66b2d4 Mon Sep 17 00:00:00 2001 From: Pietro Fezzardi Date: Tue, 24 May 2022 16:27:47 +0200 Subject: [PATCH 3/7] Support emission in C of null pointers --- assets/compile_flags.txt.in | 1 + lib/Backend/DecompileFunction.cpp | 110 ++++++++++++++++---------- lib/InitModelTypes/InitModelTypes.cpp | 12 ++- lib/Support/ModelHelpers.cpp | 10 ++- 4 files changed, 85 insertions(+), 48 deletions(-) diff --git a/assets/compile_flags.txt.in b/assets/compile_flags.txt.in index 5c66b6217..5eb9413ad 100644 --- a/assets/compile_flags.txt.in +++ b/assets/compile_flags.txt.in @@ -106,3 +106,4 @@ ${CMAKE_BINARY_DIR}/${ASSETS_DIR}/recompilation-headers/ -Wno-incompatible-pointer-types -Wno-null-pointer-arithmetic -Wno-non-literal-null-conversion +-Wno-null-dereference diff --git a/lib/Backend/DecompileFunction.cpp b/lib/Backend/DecompileFunction.cpp index 06f2afa4d..8404feabc 100644 --- a/lib/Backend/DecompileFunction.cpp +++ b/lib/Backend/DecompileFunction.cpp @@ -192,22 +192,18 @@ static StringToken buildAddressExpr(const llvm::Twine &Expr) { static StringToken buildCastExpr(StringRef ExprToCast, const model::QualifiedType &SrcType, const model::QualifiedType &DestType) { + StringToken Result = ExprToCast; if (SrcType == DestType or not SrcType.UnqualifiedType.isValid() or not SrcType.UnqualifiedType.isValid()) - return ExprToCast; + return Result; - StringToken CastString; + revng_assert((SrcType.isScalar() or SrcType.isPointer()) + and (DestType.isScalar() or DestType.isPointer())); - if ((SrcType.isScalar() or SrcType.isPointer()) - and (DestType.isScalar() or DestType.isPointer())) - CastString = (addParentheses(getTypeName(DestType)) - + addParentheses(ExprToCast)) - .str(); - else - CastString = buildDerefExpr(addParentheses(getTypeName(DestType) + " *") - + buildAddressExpr(ExprToCast)); + Result.assign(addParentheses(getTypeName(DestType))); + Result.append(addParentheses(ExprToCast)); - return CastString; + return Result; } /// Return a string that represents a C assignment: @@ -473,6 +469,16 @@ CCodeGenerator::addOperandToken(const llvm::Value *Operand) { rc_return true; } + if (auto *Null = dyn_cast(Operand)) { + TokenMap[Operand] = "NULL"; + rc_return true; + } + + if (auto *Glob = dyn_cast(Operand)) { + TokenMap[Operand] = Glob->getNameOrAsOperand(); + rc_return true; + } + if (auto *Const = dyn_cast(Operand)) { llvm::APInt Value = Const->getValue(); if (Value.isIntN(64)) { @@ -505,13 +511,10 @@ CCodeGenerator::addOperandToken(const llvm::Value *Operand) { TokenMap[Operand] = addParentheses(CompositeConstant).str(); } - } else if (auto *Null = dyn_cast(Operand)) { - TokenMap[Operand] = "NULL"; + rc_return true; + } - } else if (auto *Glob = dyn_cast(Operand)) { - TokenMap[Operand] = Glob->getNameOrAsOperand(); - - } else if (auto *ConstExpr = dyn_cast(Operand)) { + if (auto *ConstExpr = dyn_cast(Operand)) { // A constant expression might have its own uninitialized constant operands for (const llvm::Value *Op : ConstExpr->operand_values()) rc_recur addOperandToken(Op); @@ -537,11 +540,10 @@ CCodeGenerator::addOperandToken(const llvm::Value *Operand) { rc_return false; } - } else { - rc_return false; + rc_return true; } - rc_return true; + rc_return false; } StringToken CCodeGenerator::handleSpecialFunction(const llvm::CallInst *Call) { @@ -882,32 +884,56 @@ StringToken CCodeGenerator::buildExpression(const llvm::Instruction &I) { const llvm::Value *PointerOp = Store->getPointerOperand(); const llvm::Value *ValueOp = Store->getValueOperand(); - // The LHS side of a store has the same type of the object pointed by the - // pointer operand - const QualifiedType PointedType = dropPointer(TypeMap.at(PointerOp)); + const QualifiedType &PointerType = TypeMap.at(PointerOp); const QualifiedType &StoredType = TypeMap.at(ValueOp); - if (StoredType.UnqualifiedType.isValid() and StoredType.isScalar() - and not PointedType.is(model::TypeKind::PrimitiveType)) { - // If we are storing a scalar value into a pointer to a struct, union or - // pointer, cast the LHS to the scalar type before assigning it - QualifiedType StoredPtrType = StoredType; - addPointerQualifier(StoredPtrType, Model); - StringToken CastedToken = buildCastExpr(TokenMap.at(PointerOp), - TypeMap.at(PointerOp), - StoredPtrType); - Expression = buildAssignmentExpr(StoredType, - buildDerefExpr(CastedToken), - StoredType, - TokenMap.at(ValueOp), - /*WithDeclaration=*/false); + StringToken PointerOperandExpr = TokenMap.at(PointerOp); + + QualifiedType DerefPointedType; + StringToken PointerExprToDeref; + + // This may be false if the pointer is null, because in that case we emit + // NULL, whose type is integer + bool PointerOperandIsInteger = not PointerType.isPointer(); + if (PointerOperandIsInteger) { + // The pointer operand does not contain enough information to figure out + // the type of the pointee. In this sense the pointer is integer, because + // it's just a bag of bytes without useful type information. In this case + // we want the stored value to determine the type pointed-to by the + // pointer. + DerefPointedType = StoredType; + QualifiedType DerefPointerType = DerefPointedType; + addPointerQualifier(DerefPointerType, Model); + revng_assert(DerefPointerType.verify()); + PointerExprToDeref = buildCastExpr(PointerOperandExpr, + PointerType, + DerefPointerType); } else { - Expression = buildAssignmentExpr(PointedType, - buildDerefExpr(TokenMap.at(PointerOp)), - StoredType, - TokenMap.at(ValueOp), - /*WithDeclaration=*/false); + // Otherwise the type pointed-to by the pointer operand can be obtained + // just dropping the pointer qualifier. + DerefPointedType = dropPointer(PointerType); + PointerExprToDeref = PointerOperandExpr; + + if (StoredType.UnqualifiedType.isValid() and StoredType.isScalar() + and not DerefPointedType.is(model::TypeKind::PrimitiveType)) { + // If we are storing a scalar value into a pointer to a struct, union or + // pointer, cast the LHS to the scalar type before assigning it + DerefPointedType = StoredType; + QualifiedType DerefPointerType = StoredType; + addPointerQualifier(DerefPointerType, Model); + revng_assert(DerefPointerType.verify()); + PointerExprToDeref = buildCastExpr(PointerOperandExpr, + PointerType, + DerefPointerType); + } } + revng_assert(DerefPointedType.verify()); + + Expression = buildAssignmentExpr(DerefPointedType, + buildDerefExpr(PointerExprToDeref), + StoredType, + TokenMap.at(ValueOp), + /*WithDeclaration=*/false); } else if (auto *Select = dyn_cast(&I)) { diff --git a/lib/InitModelTypes/InitModelTypes.cpp b/lib/InitModelTypes/InitModelTypes.cpp index 86d191dc3..8c8f88213 100644 --- a/lib/InitModelTypes/InitModelTypes.cpp +++ b/lib/InitModelTypes/InitModelTypes.cpp @@ -140,9 +140,15 @@ static RecursiveCoroutine addOperandType(const llvm::Value *Operand, } rc_return true; - } else if (isa(Operand)) { - if (not PointersOnly) - TypeMap.insert({ Operand, {} }); + } else if (auto *NullPtr = dyn_cast(Operand)) { + if (not PointersOnly) { + auto PtrSize = model::Architecture::getPointerSize(Model.Architecture); + auto NullPointerType = model::QualifiedType{ + Model.getPrimitiveType(model::PrimitiveTypeKind::Generic, PtrSize), + /*Qualifiers*/ {} + }; + TypeMap.insert({ Operand, NullPointerType }); + } rc_return true; } diff --git a/lib/Support/ModelHelpers.cpp b/lib/Support/ModelHelpers.cpp index 17e94f1ca..4fabacbfc 100644 --- a/lib/Support/ModelHelpers.cpp +++ b/lib/Support/ModelHelpers.cpp @@ -138,9 +138,13 @@ dropPointer(const model::QualifiedType &QT) { if (It != NewQT.Qualifiers.rend()) { std::erase(NewQT.Qualifiers, *It); - } else if (auto *TD = dyn_cast(NewQT.UnqualifiedType.get())) { - rc_return rc_recur dropPointer(TD->UnderlyingType); + rc_return NewQT; } - rc_return NewQT; + if (auto *TD = dyn_cast(NewQT.UnqualifiedType.getConst())) + rc_return rc_recur dropPointer(TD->UnderlyingType); + + revng_abort("Cannot dropPointer, QT does not have pointer qualifiers"); + + rc_return{}; } From d02f51c0f08cd3a9d61a5bd556245afe008fee72 Mon Sep 17 00:00:00 2001 From: Pietro Fezzardi Date: Tue, 31 May 2022 11:01:34 +0200 Subject: [PATCH 4/7] SegregateStackAccessesPass: handle dead code Before this commit, SegregateStackAccessesPass always expected to find calls stack_size_at_call_site markers injected by InjectStackSizeProbesAtCallSitesPass. This is not always true, because aggressive LLVM optimizations can remove dead code. As an example, if the user or some analysis earlier in the pipeline wrongly marks registers as non-arguments, the optimization pipeline will throw away and eliminate everything that descends from the initial values of those registers. This commit enables SegregateStackAccessesPass to cope with calls to stack_size_at_call_site that were eliminated, and keep going just considering the calls that are still there. --- lib/PromoteStackPointer/SegregateStackAccessesPass.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/PromoteStackPointer/SegregateStackAccessesPass.cpp b/lib/PromoteStackPointer/SegregateStackAccessesPass.cpp index 300bfced9..ad783f079 100644 --- a/lib/PromoteStackPointer/SegregateStackAccessesPass.cpp +++ b/lib/PromoteStackPointer/SegregateStackAccessesPass.cpp @@ -580,7 +580,12 @@ private: // Find old call instruction CallInst *OldCall = findAssociatedCall(SSACSCall); - revng_assert(OldCall != nullptr); + + if (not OldCall) { + // We can't find the original call, it might have been DCE'd away + return; + } + IRBuilder<> Builder(OldCall); // From 229648a6cbdb9bc8686bc627ea901e18a7976960 Mon Sep 17 00:00:00 2001 From: Pietro Fezzardi Date: Tue, 31 May 2022 12:37:03 +0200 Subject: [PATCH 5/7] Backend: fix casts in assignments expressions Before this commit source and destination types were swapped. --- lib/Backend/DecompileFunction.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Backend/DecompileFunction.cpp b/lib/Backend/DecompileFunction.cpp index 8404feabc..0b2b6f519 100644 --- a/lib/Backend/DecompileFunction.cpp +++ b/lib/Backend/DecompileFunction.cpp @@ -227,7 +227,7 @@ static StringToken buildAssignmentExpr(const model::QualifiedType &LHSType, AssignmentStr += LHSToken; AssignmentStr += " = "; - AssignmentStr += buildCastExpr(RHSToken, LHSType, RHSType); + AssignmentStr += buildCastExpr(RHSToken, RHSType, LHSType); return AssignmentStr; } From 18b9e481c85304c1ec3f3b826b995ee93973a922 Mon Sep 17 00:00:00 2001 From: Pietro Fezzardi Date: Tue, 31 May 2022 12:37:51 +0200 Subject: [PATCH 6/7] Backend: fix emission of integer constants Before this commit we were using a suboptimal API that often resulted in expressions with many zeros for the 0 constant, e.g. 0000000 instead of just 0. This commit fixes the problem. --- lib/Backend/DecompileFunction.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/Backend/DecompileFunction.cpp b/lib/Backend/DecompileFunction.cpp index 0b2b6f519..70ad1d27f 100644 --- a/lib/Backend/DecompileFunction.cpp +++ b/lib/Backend/DecompileFunction.cpp @@ -483,10 +483,7 @@ CCodeGenerator::addOperandToken(const llvm::Value *Operand) { llvm::APInt Value = Const->getValue(); if (Value.isIntN(64)) { // TODO: Decide how to print constants - Value.toString(TokenMap[Operand], - /*radix=*/10, - /*signed=*/false, - /*formatAsCLiteral=*/true); + TokenMap[Operand] = to_string(Value.getLimitedValue()); } else { // In C, even if you can have 128-bit variables, you cannot have 128-bit // literals, so we need this hack to assign a big constant value to a From f62cdc118112a8db519b3d080970a77501a153c6 Mon Sep 17 00:00:00 2001 From: Pietro Fezzardi Date: Wed, 11 May 2022 11:19:49 +0200 Subject: [PATCH 7/7] RemoveLiftingArtifacts: promote init_* to undef --- lib/RemoveLiftingArtifacts/CMakeLists.txt | 2 +- .../PromoteInitCSVToUndef.cpp | 60 +++++++++++++++++++ .../RemoveLiftingArtifacts.cpp | 23 +++++++ .../pipelines/remove-lifting-artifacts.yml | 1 + 4 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 lib/RemoveLiftingArtifacts/PromoteInitCSVToUndef.cpp diff --git a/lib/RemoveLiftingArtifacts/CMakeLists.txt b/lib/RemoveLiftingArtifacts/CMakeLists.txt index ab4affd8f..54fb7a238 100644 --- a/lib/RemoveLiftingArtifacts/CMakeLists.txt +++ b/lib/RemoveLiftingArtifacts/CMakeLists.txt @@ -3,7 +3,7 @@ # revng_add_analyses_library(revngcRemoveLiftingArtifacts revngc - RemoveLiftingArtifacts.cpp) + PromoteInitCSVToUndef.cpp RemoveLiftingArtifacts.cpp) target_link_libraries( revngcRemoveLiftingArtifacts revngcSupport revng::revngModel diff --git a/lib/RemoveLiftingArtifacts/PromoteInitCSVToUndef.cpp b/lib/RemoveLiftingArtifacts/PromoteInitCSVToUndef.cpp new file mode 100644 index 000000000..5cc5ec995 --- /dev/null +++ b/lib/RemoveLiftingArtifacts/PromoteInitCSVToUndef.cpp @@ -0,0 +1,60 @@ +// +// Copyright (c) rev.ng Labs Srl. See LICENSE.md for details. +// + +#include "llvm/ADT/StringRef.h" +#include "llvm/IR/Attributes.h" +#include "llvm/IR/Function.h" +#include "llvm/IR/IntrinsicInst.h" +#include "llvm/IR/Module.h" + +#include "revng/Model/LoadModelPass.h" +#include "revng/Support/FunctionTags.h" + +#include "revng-c/Support/FunctionTags.h" +#include "revng-c/Support/IRHelpers.h" + +using namespace llvm; + +static bool makeInitRegsUndef(Function &F) { + bool Changed = false; + + for (auto &BB : F) { + for (auto &I : BB) { + auto *Call = dyn_cast(&I); + if (not Call) + continue; + + auto *Callee = Call->getCalledFunction(); + if (not Callee or not FunctionTags::OpaqueCSVValue.isTagOf(Callee)) + continue; + + Call->replaceAllUsesWith(llvm::UndefValue::get(Call->getType())); + + Changed = true; + } + } + + return Changed; +} + +struct PromoteInitCSVToUndefPass : public FunctionPass { +public: + static char ID; + + PromoteInitCSVToUndefPass() : FunctionPass(ID) {} + + bool runOnFunction(Function &F) override { + if (FunctionTags::Isolated.isTagOf(&F)) + return makeInitRegsUndef(F); + + return false; + } +}; + +char PromoteInitCSVToUndefPass::ID = 0; + +static constexpr const char *Flag = "promote-init-csv-to-undef"; + +using Reg = RegisterPass; +static Reg X(Flag, "Promotes calls to init_* functions for CSV to undefs"); diff --git a/lib/RemoveLiftingArtifacts/RemoveLiftingArtifacts.cpp b/lib/RemoveLiftingArtifacts/RemoveLiftingArtifacts.cpp index b041bc397..094148a8e 100644 --- a/lib/RemoveLiftingArtifacts/RemoveLiftingArtifacts.cpp +++ b/lib/RemoveLiftingArtifacts/RemoveLiftingArtifacts.cpp @@ -131,10 +131,33 @@ static bool makeEnvNull(Function &F) { return Changed; } +static bool makeInitRegsUndef(Function &F) { + bool Changed = false; + + for (auto &BB : F) { + for (auto &I : BB) { + auto *Call = dyn_cast(&I); + if (not Call) + continue; + + auto *Callee = Call->getCalledFunction(); + if (not Callee or not FunctionTags::OpaqueCSVValue.isTagOf(Callee)) + continue; + + Call->replaceAllUsesWith(llvm::UndefValue::get(Call->getType())); + + Changed = true; + } + } + + return Changed; +} + static bool removeLiftingArtifacts(Function &F) { bool Changed = removeCallsToArtifacts(F); Changed |= removeStoresToCPULoopExiting(F); Changed |= makeEnvNull(F); + Changed |= makeInitRegsUndef(F); return Changed; } diff --git a/share/revng/pipelines/remove-lifting-artifacts.yml b/share/revng/pipelines/remove-lifting-artifacts.yml index 616234ddf..a2c32c2e2 100644 --- a/share/revng/pipelines/remove-lifting-artifacts.yml +++ b/share/revng/pipelines/remove-lifting-artifacts.yml @@ -11,4 +11,5 @@ Steps: UsedContainers: [module.ll] Passes: - remove-lifting-artifacts + - promote-init-csv-to-undef