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 1a19c0f3c..70ad1d27f 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: @@ -231,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; } @@ -467,15 +463,27 @@ 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 *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)) { // 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 @@ -500,13 +508,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); @@ -532,11 +537,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) { @@ -877,32 +881,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/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); diff --git a/lib/InitModelTypes/InitModelTypes.cpp b/lib/InitModelTypes/InitModelTypes.cpp index 176bde3af..8c8f88213 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()) { @@ -139,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/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); // 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/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{}; } 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