From 4767fa63c3ea588cfea36fe23385d4902bfafe64 Mon Sep 17 00:00:00 2001
From: Pietro Fezzardi <pietro@rev.ng>
Date: Thu, 14 Jan 2021 14:28:14 +0100
Subject: [PATCH] Fixed emission for local_sp and scev_barrier

This commit fixes a bug due to interacting behaviors between
MarkForSerialization, AddSCEVBarrierPass, and the emission in C of calls
to revng_init_local_sp.

These interacting behaviors caused the following quirks:
- At the beginning of Functions that contained a call to
  `revng_init_local_sp()`, that call was actually emitted twice.
  The first time was due to the actual call to `revng_init_local_sp()`,
  while the second was due to the first call being wrapped from a call
  to `revng_scev_barrier_*`.
  Now we properly emit only one call.
- The original call to `revng_init_local_sp()` was supposed to generate
  a local variable, to be used in various places across the function.
  However, due to the fact that the call was not properly labeled by
  MarkForSerialization, there was no local variable, causing calls to
  `revng_init_local_sp()` to be scattered around the body of the
  functions, follwed by various arithmetic operations.
  This behavior has been fixed as well, and we now emit the local
  variable correctly.
---
 lib/Decompiler/ASTBuildAnalysis.cpp     | 18 +++++++++++++++---
 lib/Decompiler/AddSCEVBarrierPass.cpp   |  2 +-
 lib/Decompiler/MarkForSerialization.cpp | 13 +++++++++++--
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/lib/Decompiler/ASTBuildAnalysis.cpp b/lib/Decompiler/ASTBuildAnalysis.cpp
index d9b5c840c..917bdf6e2 100644
--- a/lib/Decompiler/ASTBuildAnalysis.cpp
+++ b/lib/Decompiler/ASTBuildAnalysis.cpp
@@ -647,10 +647,22 @@ void StmtBuilder::createAST(llvm::Function &F, clang::FunctionDecl &FDecl) {
 
     for (Instruction &I : *BB) {
       // Skip calls to `revng_scev_barrier_*`
+      // FIXME: calls to revng_scev_barrier_* should eventually be removed after
+      // using them and before actually generating C code for them.
       if (auto *Call = dyn_cast<CallInst>(&I)) {
-        llvm::StringRef CalleeName = Call->getCalledFunction()->getName();
-        if (CalleeName.startswith("revng_scev_barrier_"))
-          continue;
+        if (Call->getType()->isIntOrPtrTy()) {
+
+          const llvm::Type *BarrierTy = Call->getType();
+          const std::string BarrierName = makeSCEVBarrierName(BarrierTy);
+          const llvm::Function *SCEVBarrier = Call->getCalledFunction();
+
+          if (SCEVBarrier->getName().str() == BarrierName) {
+            revng_assert(SCEVBarrier->arg_size() == 1);
+            InstrStmts[&I] = getExprForValue(Call->getArgOperand(0));
+
+            continue;
+          }
+        }
       }
 
       // We don't build clang's AST expressions for PHINodes nor for
diff --git a/lib/Decompiler/AddSCEVBarrierPass.cpp b/lib/Decompiler/AddSCEVBarrierPass.cpp
index a761a663d..1a5538f5a 100644
--- a/lib/Decompiler/AddSCEVBarrierPass.cpp
+++ b/lib/Decompiler/AddSCEVBarrierPass.cpp
@@ -22,7 +22,7 @@ struct AddSCEVBarrierPass : public llvm::FunctionPass {
 
   void getAnalysisUsage(llvm::AnalysisUsage &AU) const override {
     AU.addUsedIfAvailable<MarkForSerializationPass>();
-    AU.setPreservesCFG(); // Only the CFG is preserved, because we insert calls.
+    AU.setPreservesAll(); // Only the CFG is preserved, because we insert calls.
   }
 };
 
diff --git a/lib/Decompiler/MarkForSerialization.cpp b/lib/Decompiler/MarkForSerialization.cpp
index 3adced901..746609995 100644
--- a/lib/Decompiler/MarkForSerialization.cpp
+++ b/lib/Decompiler/MarkForSerialization.cpp
@@ -102,9 +102,20 @@ Analysis::InterruptType Analysis::transfer(const llvm::BasicBlock *BB) {
       // StoreInst and CallInst that are not pure always have side effects.
       ToSerialize[&I].set(HasSideEffects);
       revng_log(MarkLog, "Instr HasSideEffects");
+
+      // Also, force calls to revng_init_local_sp to behave like if they had
+      // many uses, so that they generate a local variable.
+      if (auto *Call = dyn_cast<CallInst>(&I)) {
+        llvm::StringRef CalleeName = Call->getCalledFunction()->getName();
+        if (CalleeName == "revng_init_local_sp") {
+          ToSerialize[&I].set(HasManyUses);
+          revng_log(MarkLog, "Instr HasManyUses");
+        }
+      }
     }
 
     switch (I.getNumUses()) {
+
     case 1: {
       User *U = I.uses().begin()->getUser();
       Instruction *UserI = cast<Instruction>(U);
@@ -113,8 +124,6 @@ Analysis::InterruptType Analysis::transfer(const llvm::BasicBlock *BB) {
       if (NBBDuplicates < UserNDuplicates) {
         ToSerialize[&I].set(HasDuplicatedUses);
         revng_log(MarkLog, "Instr HasDuplicatedUses");
-      } else {
-        Pending.insert(&I);
       }
     } break;