[clang][Interp] Fix handling integral function pointers

As expected, we need to be a little more careful when the Function* is created from an integer.
llvm · Apr 11, 2024 · b2ea38f · b2ea38f
1 parent 9c4aca2
commit b2ea38f
Show file tree

Hide file tree

Showing 2 changed files with 88 additions and 18 deletions.
diff --git a/clang/lib/AST/Interp/FunctionPointer.h b/clang/lib/AST/Interp/FunctionPointer.h
@@ -20,14 +20,15 @@ namespace interp {
 class FunctionPointer final {
 private:
   const Function *Func;
+  bool Valid;
 
 public:
-  // FIXME: We might want to track the fact that the Function pointer
-  // has been created from an integer and is most likely garbage anyway.
-  FunctionPointer(uintptr_t IntVal = 0, const Descriptor *Desc = nullptr)
-      : Func(reinterpret_cast<const Function *>(IntVal)) {}
+  FunctionPointer(const Function *Func) : Func(Func), Valid(true) {
+    assert(Func);
+  }
 
-  FunctionPointer(const Function *Func) : Func(Func) { assert(Func); }
+  FunctionPointer(uintptr_t IntVal = 0, const Descriptor *Desc = nullptr)
+      : Func(reinterpret_cast<const Function *>(IntVal)), Valid(false) {}
 
   const Function *getFunction() const { return Func; }
   bool isZero() const { return !Func; }
@@ -37,14 +38,21 @@ class FunctionPointer final {
       return APValue(static_cast<Expr *>(nullptr), CharUnits::Zero(), {},
                      /*OnePastTheEnd=*/false, /*IsNull=*/true);
 
+    if (!Valid)
+      return APValue(static_cast<Expr *>(nullptr),
+                     CharUnits::fromQuantity(getIntegerRepresentation()), {},
+                     /*OnePastTheEnd=*/false, /*IsNull=*/false);
+
     return APValue(Func->getDecl(), CharUnits::Zero(), {},
                    /*OnePastTheEnd=*/false, /*IsNull=*/false);
   }
 
   void print(llvm::raw_ostream &OS) const {
     OS << "FnPtr(";
-    if (Func)
+    if (Func && Valid)
       OS << Func->getName();
+    else if (Func)
+      OS << reinterpret_cast<uintptr_t>(Func);
     else
       OS << "nullptr";
     OS << ")";

diff --git a/clang/unittests/AST/Interp/toAPValue.cpp b/clang/unittests/AST/Interp/toAPValue.cpp
@@ -90,7 +90,8 @@ TEST(ToAPValue, Pointers) {
 
 TEST(ToAPValue, FunctionPointers) {
   constexpr char Code[] = " constexpr bool foo() { return true; }\n"
-                          " constexpr bool (*func)() = foo;\n";
+                          " constexpr bool (*func)() = foo;\n"
+                          " constexpr bool (*nullp)() = nullptr;\n";
 
   auto AST = tooling::buildASTFromCodeWithArgs(
       Code, {"-fexperimental-new-constant-interpreter"});
@@ -112,15 +113,76 @@ TEST(ToAPValue, FunctionPointers) {
     return Prog.getPtrGlobal(*Prog.getGlobal(D));
   };
 
-  const Pointer &GP = getGlobalPtr("func");
-  const FunctionPointer &FP = GP.deref<FunctionPointer>();
-  ASSERT_FALSE(FP.isZero());
-  APValue A = FP.toAPValue();
-  ASSERT_TRUE(A.hasValue());
-  ASSERT_TRUE(A.isLValue());
-  ASSERT_TRUE(A.hasLValuePath());
-  const auto &Path = A.getLValuePath();
-  ASSERT_EQ(Path.size(), 0u);
-  ASSERT_FALSE(A.getLValueBase().isNull());
-  ASSERT_EQ(A.getLValueBase().dyn_cast<const ValueDecl *>(), getDecl("foo"));
+  {
+    const Pointer &GP = getGlobalPtr("func");
+    const FunctionPointer &FP = GP.deref<FunctionPointer>();
+    ASSERT_FALSE(FP.isZero());
+    APValue A = FP.toAPValue();
+    ASSERT_TRUE(A.hasValue());
+    ASSERT_TRUE(A.isLValue());
+    ASSERT_TRUE(A.hasLValuePath());
+    const auto &Path = A.getLValuePath();
+    ASSERT_EQ(Path.size(), 0u);
+    ASSERT_FALSE(A.getLValueBase().isNull());
+    ASSERT_EQ(A.getLValueBase().dyn_cast<const ValueDecl *>(), getDecl("foo"));
+  }
+
+  {
+    const ValueDecl *D = getDecl("nullp");
+    ASSERT_NE(D, nullptr);
+    const Pointer &GP = getGlobalPtr("nullp");
+    const auto &P = GP.deref<FunctionPointer>();
+    APValue A = P.toAPValue();
+    ASSERT_TRUE(A.isLValue());
+    ASSERT_TRUE(A.getLValueBase().isNull());
+    ASSERT_TRUE(A.isNullPointer());
+    APSInt I;
+    bool Success = A.toIntegralConstant(I, D->getType(), AST->getASTContext());
+    ASSERT_TRUE(Success);
+    ASSERT_EQ(I, 0);
+  }
+}
+
+TEST(ToAPValue, FunctionPointersC) {
+  // NB: The declaration of func2 is useless, but it makes us register a global
+  // variable for func.
+  constexpr char Code[] = "const int (* const func)(int *) = (void*)17;\n"
+                          "const int (*func2)(int *) = func;\n";
+  auto AST = tooling::buildASTFromCodeWithArgs(
+      Code, {"-x", "c", "-fexperimental-new-constant-interpreter"});
+
+  auto &Ctx = AST->getASTContext().getInterpContext();
+  Program &Prog = Ctx.getProgram();
+
+  auto getDecl = [&](const char *Name) -> const ValueDecl * {
+    auto Nodes =
+        match(valueDecl(hasName(Name)).bind("var"), AST->getASTContext());
+    assert(Nodes.size() == 1);
+    const auto *D = Nodes[0].getNodeAs<ValueDecl>("var");
+    assert(D);
+    return D;
+  };
+
+  auto getGlobalPtr = [&](const char *Name) -> Pointer {
+    const VarDecl *D = cast<VarDecl>(getDecl(Name));
+    return Prog.getPtrGlobal(*Prog.getGlobal(D));
+  };
+
+  {
+    const ValueDecl *D = getDecl("func");
+    const Pointer &GP = getGlobalPtr("func");
+    ASSERT_TRUE(GP.isLive());
+    const FunctionPointer &FP = GP.deref<FunctionPointer>();
+    ASSERT_FALSE(FP.isZero());
+    APValue A = FP.toAPValue();
+    ASSERT_TRUE(A.hasValue());
+    ASSERT_TRUE(A.isLValue());
+    const auto &Path = A.getLValuePath();
+    ASSERT_EQ(Path.size(), 0u);
+    ASSERT_TRUE(A.getLValueBase().isNull());
+    APSInt I;
+    bool Success = A.toIntegralConstant(I, D->getType(), AST->getASTContext());
+    ASSERT_TRUE(Success);
+    ASSERT_EQ(I, 17);
+  }
 }