[SSAF] Let function parameters inherit linkage from their parent functions

[ziqingluo-90]

SSAF treats parameters as entities and may not always associate them back to their parent functions. Therefore, it needs to identify parameters of functions with external linkage across different TUs. Treating them as having no linkage (as in C++) causes the same parameter in different TUs to be assigned different EntityIDs. As a result, the behavior of the parameter across multiple TUs cannot be correlated.

rdar://178844032

[llvmorg-github-actions]

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang-ssaf

Author: Ziqing Luo (ziqingluo-90)

Changes

SSAF treats parameters as entities and may not always associate them back to their parent functions. Therefore, it needs to identify parameters of functions with external linkage across different TUs. Treating them as having no linkage (as in C++) causes the same parameter in different TUs to be assigned different EntityIDs. As a result, the behavior of the parameter across multiple TUs cannot be correlated.

rdar://178844032

---

Full diff: https://github.com/llvm/llvm-project/pull/201946.diff

4 Files Affected:

• (modified) clang/include/clang/ScalableStaticAnalysisFramework/Core/Model/EntityLinkage.h (+4-2)
• (modified) clang/lib/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummaryExtractor.cpp (+18)
• (added) clang/test/Analysis/Scalable/PointerFlow/external-inline-function-in-multi-tu.test (+63)
• (modified) clang/unittests/ScalableStaticAnalysisFramework/TUSummaryBuilderTest.cpp (+42)

diff --git a/clang/include/clang/ScalableStaticAnalysisFramework/Core/Model/EntityLinkage.h b/clang/include/clang/ScalableStaticAnalysisFramework/Core/Model/EntityLinkage.h
index cdae99ad0b6b2..b144d735a75a1 100644
--- a/clang/include/clang/ScalableStaticAnalysisFramework/Core/Model/EntityLinkage.h
+++ b/clang/include/clang/ScalableStaticAnalysisFramework/Core/Model/EntityLinkage.h
@@ -15,9 +15,11 @@
 namespace clang::ssaf {
 
 enum class EntityLinkageType {
-  None,     ///< local variables, function parameters
+  None,     ///< local variables, parameters of functions with no external
+            ///< linkage
   Internal, ///< static functions/variables, anonymous namespace
-  External  ///< globally visible across translation units
+  External  ///< globally visible across translation units (including parameters
+            ///< of functions with external linkage)
 };
 
 /// Represents the linkage properties of an entity in the program model.
diff --git a/clang/lib/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummaryExtractor.cpp b/clang/lib/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummaryExtractor.cpp
index 9594a6b179db5..88583f8fe73e2 100644
--- a/clang/lib/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummaryExtractor.cpp
+++ b/clang/lib/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummaryExtractor.cpp
@@ -8,10 +8,12 @@
 
 #include "clang/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummaryExtractor.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/ScalableStaticAnalysisFramework/Core/ASTEntityMapping.h"
 #include "clang/ScalableStaticAnalysisFramework/Core/Model/EntityId.h"
 #include "clang/ScalableStaticAnalysisFramework/Core/Model/EntityLinkage.h"
 #include "clang/ScalableStaticAnalysisFramework/Core/TUSummary/TUSummaryBuilder.h"
+#include "llvm/Support/Casting.h"
 #include <optional>
 
 using namespace clang;
@@ -22,6 +24,22 @@ static EntityLinkageType getLinkageForDecl(const Decl *D) {
   if (!ND)
     return EntityLinkageType::None;
 
+  // Parameters have no linkage in C++, but SSAF needs them to inherit
+  // the external linkage from their parent functions.
+  // Here is why:
+  //   SSAF treats parameters as entities and may not always associate them back
+  //   to their parent functions. Therefore, it needs to identify parameters of
+  //   functions with external linkage across different TUs. Treating them as
+  //   having no linkage (as in C++) causes the same parameter in different TUs
+  //   to be assigned different EntityIDs. As a result, the behavior of the
+  //   parameter across multiple TUs cannot be correlated.
+  if (const auto *PVD = dyn_cast<ParmVarDecl>(D)) {
+    if (const FunctionDecl *FD = llvm::dyn_cast_or_null<FunctionDecl>(
+            PVD->getParentFunctionOrMethod())) {
+      return getLinkageForDecl(FD);
+    }
+  }
+
   switch (ND->getFormalLinkage()) {
   case Linkage::Invalid: {
     llvm_unreachable("Shouldn't be invalid");
diff --git a/clang/test/Analysis/Scalable/PointerFlow/external-inline-function-in-multi-tu.test b/clang/test/Analysis/Scalable/PointerFlow/external-inline-function-in-multi-tu.test
new file mode 100644
index 0000000000000..dce0b28748b80
--- /dev/null
+++ b/clang/test/Analysis/Scalable/PointerFlow/external-inline-function-in-multi-tu.test
@@ -0,0 +1,63 @@
+// For an 'inline' function with multiple definitions in different
+// translation units (TUs), its call sites in different TUs and one of
+// its definitions, which is chosen by the linker, are properly connected
+// during bounds propagation.
+
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// Extract per-TU PointerFlow + UnsafeBufferUsage summaries.
+// RUN: %clang_cc1 -fsyntax-only -I %t %t/tu1.cpp \
+// RUN:   --ssaf-extract-summaries=PointerFlow \
+// RUN:   --ssaf-extract-summaries=UnsafeBufferUsage \
+// RUN:   --ssaf-tu-summary-file=%t/tu1.summary.json
+// RUN: %clang_cc1 -fsyntax-only -I %t %t/tu2.cpp \
+// RUN:   --ssaf-extract-summaries=PointerFlow \
+// RUN:   --ssaf-extract-summaries=UnsafeBufferUsage \
+// RUN:   --ssaf-tu-summary-file=%t/tu2.summary.json
+
+// Link the two TU summaries into a single LU summary.
+// RUN: clang-ssaf-linker %t/tu1.summary.json %t/tu2.summary.json \
+// RUN:   -o %t/lu.json
+
+// Run UnsafeBufferReachableAnalysis on the linked LU summary.
+// RUN: clang-ssaf-analyzer %t/lu.json -o %t/wpa.json \
+// RUN:   -a UnsafeBufferReachableAnalysisResult
+
+// RUN: FileCheck %s --input-file=%t/wpa.json
+
+//--- shared.h
+inline int shared_inline(int *p) {
+  int * l = p;      
+  return l[5];
+}
+
+//--- tu1.cpp
+#include "shared.h"
+int f(int *x) {
+  return shared_inline(x);
+}
+
+//--- tu2.cpp
+#include "shared.h"
+int g(int *y) {
+  return shared_inline(y);
+}
+
+
+// Check that these parameters are all reachable from the local variable 'l':
+// parameter 'y' of function 'g',
+// parameter 'x' of function 'f', and
+// parameter 'p' of function 'shared_inline'
+
+// CHECK-DAG: "suffix": "1",{{[[:space:]]+}}"usr": "c:@F@f#*I#"
+// CHECK-DAG: "suffix": "1",{{[[:space:]]+}}"usr": "c:@F@g#*I#"
+// CHECK-DAG: "suffix": "1",{{[[:space:]]+}}"usr": "c:@F@shared_inline#*I#"
+// CHECK-DAG: "usr": "c:shared.h@{{[0-9]+}}@F@shared_inline#{{.*}}@l"
+
+// Confirm UnsafeBufferReachableAnalysisResult is present in the output.
+// CHECK: "analysis_name": "UnsafeBufferReachableAnalysisResult"
+
+
diff --git a/clang/unittests/ScalableStaticAnalysisFramework/TUSummaryBuilderTest.cpp b/clang/unittests/ScalableStaticAnalysisFramework/TUSummaryBuilderTest.cpp
index 3fff10f08d38a..b29b8e874c0af 100644
--- a/clang/unittests/ScalableStaticAnalysisFramework/TUSummaryBuilderTest.cpp
+++ b/clang/unittests/ScalableStaticAnalysisFramework/TUSummaryBuilderTest.cpp
@@ -275,6 +275,7 @@ struct TUSummaryBuilderLinkageTest : TUSummaryBuilderTest {
   std::unique_ptr<ASTUnit> AST;
   static constexpr auto Internal = EntityLinkageType::Internal;
   static constexpr auto External = EntityLinkageType::External;
+  static constexpr auto None = EntityLinkageType::None;
 
   const FunctionDecl *findFnByName(StringRef Name) {
     return ssaf::findFnByName(Name, AST->getASTContext());
@@ -336,4 +337,45 @@ TEST_F(TUSummaryBuilderLinkageTest, ConstGlobalHasInternalLinkage) {
   EXPECT_EQ(getLinkageFor(Extractor.addEntity(VD)), Internal);
 }
 
+// See 'getLinkageForDecl' in
+// ScalableStaticAnalysisFramework/Core/TUSummary/TUSummaryExtractor.cpp
+
+TEST_F(TUSummaryBuilderLinkageTest, ParamOfExternalFunctionIsExternal) {
+  AST = tooling::buildASTFromCode("void target(int *ptr) {}");
+  const auto *PVD = findDeclByName<ParmVarDecl>("ptr", AST->getASTContext());
+  ASSERT_TRUE(PVD);
+  EXPECT_EQ(getLinkageFor(Extractor.addEntity(PVD)), External);
+}
+
+TEST_F(TUSummaryBuilderLinkageTest, ParamOfInlineFunctionIsExternal) {
+  AST = tooling::buildASTFromCode("inline void target(int *ptr) {}");
+  const auto *PVD = findDeclByName<ParmVarDecl>("ptr", AST->getASTContext());
+  ASSERT_TRUE(PVD);
+  EXPECT_EQ(getLinkageFor(Extractor.addEntity(PVD)), External);
+}
+
+TEST_F(TUSummaryBuilderLinkageTest, ParamOfStaticFunctionIsInternal) {
+  AST = tooling::buildASTFromCode("static void target(int *ptr) {}");
+  const auto *PVD = findDeclByName<ParmVarDecl>("ptr", AST->getASTContext());
+  ASSERT_TRUE(PVD);
+  EXPECT_EQ(getLinkageFor(Extractor.addEntity(PVD)), Internal);
+}
+
+TEST_F(TUSummaryBuilderLinkageTest, ParamOfAnonNamespaceFunctionIsInternal) {
+  AST = tooling::buildASTFromCode("namespace {\n"
+                                  "  void target(int *ptr) {}\n"
+                                  "}");
+  const auto *PVD = findDeclByName<ParmVarDecl>("ptr", AST->getASTContext());
+  ASSERT_TRUE(PVD);
+  EXPECT_EQ(getLinkageFor(Extractor.addEntity(PVD)), Internal);
+}
+
+TEST_F(TUSummaryBuilderLinkageTest, ParamOfClassMemberFunctionIsExternal) {
+  AST = tooling::buildASTFromCode(
+      "class C { public: void target(int *ptr) {} };");
+  const auto *PVD = findDeclByName<ParmVarDecl>("ptr", AST->getASTContext());
+  ASSERT_TRUE(PVD);
+  EXPECT_EQ(getLinkageFor(Extractor.addEntity(PVD)), External);
+}
+
 } // namespace

[github-actions]

🐧 Linux x64 Test Results

• 118267 tests passed
• 4815 tests skipped

✅ The build succeeded and all tests passed.