Merge branch feature/promote-init-csv-to-undef

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

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<llvm::UndefValue>(Operand)) {
+    revng_assert(Undef->getType()->isIntOrPtrTy());
+    TokenMap[Operand] = "0 /* undef */";
+    rc_return true;
+  }
+
+  if (auto *Null = dyn_cast<llvm::ConstantPointerNull>(Operand)) {
+    TokenMap[Operand] = "NULL";
+    rc_return true;
+  }
+
+  if (auto *Glob = dyn_cast<llvm::GlobalVariable>(Operand)) {
+    TokenMap[Operand] = Glob->getNameOrAsOperand();
+    rc_return true;
+  }
 
   if (auto *Const = dyn_cast<llvm::ConstantInt>(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<llvm::ConstantPointerNull>(Operand)) {
-    TokenMap[Operand] = "NULL";
+    rc_return true;
+  }
 
-  } else if (auto *Glob = dyn_cast<llvm::GlobalVariable>(Operand)) {
-    TokenMap[Operand] = Glob->getNameOrAsOperand();
-
-  } else if (auto *ConstExpr = dyn_cast<llvm::ConstantExpr>(Operand)) {
+  if (auto *ConstExpr = dyn_cast<llvm::ConstantExpr>(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<llvm::SelectInst>(&I)) {
 

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<llvm::UndefValue>(U.get())
+            or isa<llvm::ConstantPointerNull>(U.get())) {
+          revng_log(ModelGEPLog, "Skipping null pointer address");
+          continue;
+        }
+
         ModelGEPSummation GEPSum = GEPSumCache.getGEPSummation(U, PointerTypes);
 
         revng_log(ModelGEPLog, "GEPSum " << GEPSum);

lib/InitModelTypes/InitModelTypes.cpp

@@ -128,10 +128,11 @@ static RecursiveCoroutine<bool> addOperandType(const llvm::Value *Operand,
       }
     }
   } else if (isa<llvm::ConstantInt>(Operand)
-             or isa<llvm::GlobalVariable>(Operand)) {
+             or isa<llvm::GlobalVariable>(Operand)
+             or isa<llvm::UndefValue>(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<bool> addOperandType(const llvm::Value *Operand,
     }
     rc_return true;
 
-  } else if (isa<llvm::ConstantPointerNull>(Operand)) {
-    if (not PointersOnly)
-      TypeMap.insert({ Operand, {} });
+  } else if (auto *NullPtr = dyn_cast<llvm::ConstantPointerNull>(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;
   }
 

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);
 
     //

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

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<CallInst>(&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<PromoteInitCSVToUndefPass>;
+static Reg X(Flag, "Promotes calls to init_* functions for CSV to undefs");

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<CallInst>(&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;
 }
 

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<TypedefType>(NewQT.UnqualifiedType.get())) {
-    rc_return rc_recur dropPointer(TD->UnderlyingType);
+    rc_return NewQT;
   }
 
-  rc_return NewQT;
+  if (auto *TD = dyn_cast<TypedefType>(NewQT.UnqualifiedType.getConst()))
+    rc_return rc_recur dropPointer(TD->UnderlyingType);
+
+  revng_abort("Cannot dropPointer, QT does not have pointer qualifiers");
+
+  rc_return{};
 }

share/revng/pipelines/remove-lifting-artifacts.yml

@@ -11,4 +11,5 @@ Steps:
        UsedContainers: [module.ll]
        Passes:
          - remove-lifting-artifacts
+         - promote-init-csv-to-undef