further IDEIIAA fixes

secure-software-engineering · Oct 31, 2022 · 11aab80 · 11aab80
1 parent 7b7c8de
commit 11aab80
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 36 deletions.
diff --git a/include/phasar/Config/Configuration.h b/include/phasar/Config/Configuration.h
@@ -17,14 +17,14 @@
 #ifndef PHASAR_CONFIG_CONFIGURATION_H_
 #define PHASAR_CONFIG_CONFIGURATION_H_
 
-#include <filesystem>
-#include <string>
-
-#include "boost/program_options.hpp"
+#include "phasar/Config/Version.h"
 
 #include "llvm/ADT/iterator_range.h"
 
-#include "phasar/Config/Version.h"
+#include "boost/program_options.hpp"
+
+#include <filesystem>
+#include <string>
 
 #define XSTR(S) STR(S)
 #define STR(S) #S

diff --git a/include/phasar/PhasarLLVM/DataFlowSolver/IfdsIde/Problems/IDEInstInteractionAnalysis.h b/include/phasar/PhasarLLVM/DataFlowSolver/IfdsIde/Problems/IDEInstInteractionAnalysis.h
@@ -833,23 +833,43 @@ class IDEInstInteractionAnalysisT
     struct MapFactsAlongsideCallSite : public FlowFunction<IDEIIAFlowFact> {
       bool OnlyDecls;
       const llvm::CallBase *CallSite;
+      d_t ZeroValue;
+      std::set<f_t> Callees;
 
-      MapFactsAlongsideCallSite(bool OnlyDecls, const llvm::CallBase *CallSite)
-          : OnlyDecls(OnlyDecls), CallSite(CallSite) {}
+      MapFactsAlongsideCallSite(bool OnlyDecls, const llvm::CallBase *CallSite,
+                                d_t ZeroValue, std::set<f_t> Callees)
+          : OnlyDecls(OnlyDecls), CallSite(CallSite), ZeroValue(ZeroValue),
+            Callees(Callees) {}
 
       std::set<IDEIIAFlowFact> computeTargets(IDEIIAFlowFact Source) override {
-        // Pass ZeroValue as is
-        if (LLVMZeroValue::getInstance()->isLLVMZeroValue(Source.getBase())) {
-          return {Source};
-        }
-        // Pass variables as identity, if no function definition is available
-        // as they would be otherwise killed.
+        // There are a few things to consider, in case only declarations of
+        // callee targets are available.
         if (OnlyDecls) {
-          return {Source};
+          auto AllVoidRetTys = [](const std::set<f_t> &Callees) {
+            // Check if one of the callee targets returns a value.
+            bool AllVoidRetTys = true;
+            for (const auto *Callee : Callees) {
+              if (!Callee->getReturnType()->isVoidTy()) {
+                AllVoidRetTys = false;
+              }
+            }
+            return AllVoidRetTys;
+          };
+          if (!AllVoidRetTys(Callees)) {
+            // If one or more of the declaration-only targets return a value, it
+            // must be generated from zero!
+            if (Source == ZeroValue) {
+              return {Source, CallSite};
+            }
+          } else {
+            // If all declaration-only callee targets return void, just pass
+            // everything as identity.
+            return {Source};
+          }
         }
-        // Do not global variables (if definitions of the callee function(s) are
-        // available), since the effect of the callee on these values will be
-        // modelled using combined getCallFlowFunction and
+        // Do not pass global variables if definitions of the callee
+        // function(s) are available, since the effect of the callee on these
+        // values will be modelled using combined getCallFlowFunction and
         // getReturnFlowFunction.
         if (llvm::isa<llvm::Constant>(Source.getBase())) {
           return {};
@@ -861,7 +881,8 @@ class IDEInstInteractionAnalysisT
       }
     };
     return std::make_shared<MapFactsAlongsideCallSite>(
-        OnlyDecls, llvm::dyn_cast<llvm::CallBase>(CallSite));
+        OnlyDecls, llvm::dyn_cast<llvm::CallBase>(CallSite),
+        this->getZeroValue(), Callees);
   }
 
   inline FlowFunctionPtrType
@@ -1715,7 +1736,7 @@ class IDEInstInteractionAnalysisT
   }
 
 private:
-  /// Filters out all variables that had a non empty set during edge functions
+  /// Filters out all variables that had a non-empty set during edge functions
   /// computations.
   inline std::unordered_set<d_t> removeVariablesWithoutEmptySetValue(
       const SolverResults<n_t, d_t, l_t> &Solution,
@@ -1729,7 +1750,7 @@ class IDEInstInteractionAnalysisT
       // at some point. Therefore, we only care for the variables and their
       // associated values and ignore at which point a variable may holds as a
       // data-flow fact.
-      const auto *Variable = Result.getColumnKey();
+      const auto Variable = Result.getColumnKey();
       const auto &Value = Result.getValue();
       // skip result entry if variable is not in the set of all variables
       if (Variables.find(Variable) == Variables.end()) {

diff --git a/unittests/PhasarLLVM/DataFlowSolver/IfdsIde/Problems/IDEInstInteractionAnalysisTest.cpp b/unittests/PhasarLLVM/DataFlowSolver/IfdsIde/Problems/IDEInstInteractionAnalysisTest.cpp
@@ -126,15 +126,8 @@ class IDEInstInteractionAnalysisTest : public ::testing::Test {
         llvm::raw_string_ostream RSO(FactStr);
         RSO << *Fact.getBase();
         llvm::StringRef FactRef(FactStr);
-        // llvm::outs() << "Iterating result map entry: " << Fact << ", " <<
-        // Value
-        //              << " with FactRef: " << FactRef
-        //              << " and VarName: " << VarName << '\n';
         if (FactRef.ltrim().startswith("%" + VarName + " ") ||
             FactRef.ltrim().startswith("@" + VarName + " ")) {
-          //   llvm::outs() << "Found fact: " << FactRef.ltrim() << '\n';
-          //   llvm::outs() << "Compare Value: " << Value
-          //                << " and LatticeVal: " << LatticeVal << '\n';
           PHASAR_LOG_LEVEL(DFADEBUG, "Checking variable: " << FactStr);
           EXPECT_EQ(LatticeVal, Value);
           FactFound = true;
@@ -144,8 +137,6 @@ class IDEInstInteractionAnalysisTest : public ::testing::Test {
         PHASAR_LOG_LEVEL(DFADEBUG, "Variable '" << VarName << "' missing at '"
                                                 << llvmIRToString(IRLine)
                                                 << "'.");
-        // llvm::outs() << "Variable '" << VarName << "' missing at '"
-        //              << llvmIRToString(IRLine) << "'.";
       }
       EXPECT_TRUE(FactFound);
     }
@@ -503,16 +494,16 @@ PHASAR_SKIP_TEST(TEST_F(IDEInstInteractionAnalysisTest, HandleBasicTest_04) {
           "main", 23, "argc.addr", {"14", "21"}));
   GroundTruth.emplace(
       std::tuple<std::string, size_t, std::string, BitVectorSet<std::string>>(
-          "main", 24, "argv.addr", {"9", "15"}));
+          "main", 23, "argv.addr", {"15"}));
   GroundTruth.emplace(
       std::tuple<std::string, size_t, std::string, BitVectorSet<std::string>>(
-          "main", 24, "i", {"10", "16", "17"}));
+          "main", 23, "i", {"16", "17"}));
   GroundTruth.emplace(
       std::tuple<std::string, size_t, std::string, BitVectorSet<std::string>>(
-          "main", 24, "j", {"10", "11", "16", "19", "24"}));
+          "main", 23, "j", {"16", "17", "19", "18", "24"}));
   GroundTruth.emplace(
       std::tuple<std::string, size_t, std::string, BitVectorSet<std::string>>(
-          "main", 24, "k", {"10", "11", "12", "16", "19", "20", "25", "27"}));
+          "main", 23, "k", {"16", "17", "18", "19", "20", "24", "25", "27"}));
   doAnalysisAndCompareResults("basic_04_cpp.ll", {"main"}, GroundTruth, false);
 })
 
@@ -871,7 +862,7 @@ PHASAR_SKIP_TEST(TEST_F(IDEInstInteractionAnalysisTest, HandleRVOTest_01) {
   std::set<IIACompactResult_t> GroundTruth;
   GroundTruth.emplace(
       std::tuple<std::string, size_t, std::string, BitVectorSet<std::string>>(
-          "main", 16, "retval", {"23", "35", "37"}));
+          "main", 16, "retval", {"75", "76", "78"}));
   GroundTruth.emplace(
       std::tuple<std::string, size_t, std::string, BitVectorSet<std::string>>(
           "main", 16, "str", {"70", "65", "72", "74", "77"}));

diff --git a/unittests/TestUtils/TestConfig.h b/unittests/TestUtils/TestConfig.h
@@ -1,10 +1,12 @@
 #ifndef UNITTEST_TESTUTILS_TESTCONFIG_H_
 #define UNITTEST_TESTUTILS_TESTCONFIG_H_
 
-#include <string>
-
 #include "phasar/Config/Configuration.h"
 
+#include "gtest/gtest.h"
+
+#include <string>
+
 namespace psr::unittest {
 
 inline const std::string PathToLLTestFiles(PhasarConfig::PhasarDirectory() +