Changes to add itypes on undefined functions

clang/include/clang/3C/3CGlobalOptions.h

@@ -67,6 +67,8 @@ struct _3COptions {
   bool AllowRewriteFailures;
 
   bool ItypesForExtern;
+
+  bool InferTypesForUndefs;
 };
 
 // NOLINTNEXTLINE(readability-identifier-naming)

clang/include/clang/3C/ProgramInfo.h

@@ -269,6 +269,8 @@ class ProgramInfo : public ProgramVariableAdder {
   // constraint system for that pointer type.
   void addVariable(clang::DeclaratorDecl *D,
                    clang::ASTContext *AstContext) override;
+
+  void linkFunction(FunctionVariableConstraint *FV);
 };
 
 #endif

clang/include/clang/3C/Utils.h

@@ -181,6 +181,8 @@ bool isInSysHeader(clang::Decl *D);
 
 std::string getSourceText(const clang::SourceRange &SR,
                           const clang::ASTContext &C);
+std::string getSourceText(const clang::CharSourceRange &SR,
+                          const clang::ASTContext &C);
 
 // Find the longest common subsequence.
 unsigned longestCommonSubsequence(const char *Str1, const char *Str2,

clang/lib/3C/ConstraintVariables.cpp

@@ -280,7 +280,16 @@ PointerVariableConstraint::PointerVariableConstraint(
         SourceRange R = ITE->getSourceRange();
         if (R.isValid()) {
           ItypeStr = getSourceText(R, C);
-          assert(ItypeStr.size() > 0);
+        }
+
+        // ITE->isCompilerGenerated will be true when an itype expression is
+        // implied by a bounds expression on the declaration. When the itype
+        // expression is not generated by the compiler, but we failed to extract
+        // its string representation from the source, build the itype string
+        // from the AST.
+        if (!ITE->isCompilerGenerated() && ItypeStr.empty()) {
+          assert(!InteropType.getAsString().empty());
+          ItypeStr = "itype(" + InteropType.getAsString() + ")";
         }
       }
     }
@@ -547,12 +556,21 @@ std::string PointerVariableConstraint::tryExtractBaseType(DeclaratorDecl *D,
                                                           QualType QT,
                                                           const Type *Ty,
                                                           const ASTContext &C) {
-  bool FoundBaseTypeInSrc = false;
-  if (D && !TSI)
+  // Implicit parameters declarations from typedef function declarations will
+  // still have valid and non-empty source ranges, but implicit declarations
+  // aren't written in the source, so extracting the base type from this range
+  // gives incorrect type strings. For example, the base type for the implicit
+  // parameter for `foo_decl` in `typedef void foo(int*); foo foo_decl;` would
+  // be extracted as "foo_decl", when it should be "int".
+  if (!D || D->isImplicit())
+    return "";
+
+  if (!TSI)
     TSI = D->getTypeSourceInfo();
-  if (!QT->isOrContainsCheckedType() && !Ty->getAs<TypedefType>() && D && TSI) {
+  if (!QT->isOrContainsCheckedType() && !Ty->getAs<TypedefType>() && TSI) {
     // Try to extract the type from original source to preserve defines
     TypeLoc TL = TSI->getTypeLoc();
+    bool FoundBaseTypeInSrc = false;
     if (isa<FunctionDecl>(D)) {
       FoundBaseTypeInSrc = D->getAsFunction()->getReturnType() == QT;
       TL = getBaseTypeLoc(TL).getAs<FunctionTypeLoc>();
@@ -2211,8 +2229,30 @@ FVComponentVariable::FVComponentVariable(const QualType &QT,
   // parameters similarly to how we do it for pointers in regular function
   // declarations.
   if (D && D->getType() == QT) {
-    SourceRange SR = D->getSourceRange();
-    SourceDeclaration = SR.isValid() ? getSourceText(SR, C) : "";
+    SourceRange DRange = D->getSourceRange();
+    if (DRange.isValid()) {
+      const SourceManager &SM = C.getSourceManager();
+      SourceLocation DLoc = D->getLocation();
+      CharSourceRange CSR;
+      if (SM.isBeforeInTranslationUnit(DRange.getEnd(), DLoc)) {
+        // It's not clear to me why, but the end of the SourceRange for the
+        // declaration can come before the SourceLocation for the declaration.
+        // This can result in SourceDeclaration failing to capture the whole
+        // declaration string, causing rewriting errors. In this case it is also
+        // necessary to use CharSourceRange::getCharRange to work around some
+        // cases where CharSourceRange::getTokenRange (the default behavior of
+        // getSourceText when passed a SourceRange) would not get the full
+        // declaration. For example: a parameter declared without a name such as
+        // `_Ptr<char>` was captured as `_Ptr`.
+        CSR = CharSourceRange::getCharRange(DRange.getBegin(), DLoc);
+      } else {
+        CSR = CharSourceRange::getTokenRange(DRange);
+      }
+
+      SourceDeclaration = getSourceText(CSR , C);
+    } else {
+      SourceDeclaration = "";
+    }
   }
 }
 

clang/lib/3C/DeclRewriter.cpp

@@ -567,7 +567,14 @@ bool FunctionDeclBuilder::VisitFunctionDecl(FunctionDecl *FD) {
 
   // If this is an external function, there is no need to rewrite the
   // declaration. We cannot change the signature of external functions.
-  if (!FDConstraint->hasBody())
+  // Under the flag -infer-types-for-undef, however, undefined functions do need
+  // to be rewritten. If the rest of the 3c inference and rewriting code is
+  // correct, short-circuiting here shouldn't be necessary; the rest of the
+  // logic in this function should successfully not rewrite undefined functions
+  // when -infer-types-for-undef is not passed. This assumption could be
+  // transformed into an assertion if we're confident it won't fail in too many
+  // places.
+  if (!_3COpts.InferTypesForUndefs && !FDConstraint->hasBody())
     return true;
 
   // RewriteParams and RewriteReturn track if we will need to rewrite the

clang/lib/3C/ProgramInfo.cpp

@@ -417,78 +417,50 @@ bool ProgramInfo::link() {
 
   // For every global function that is an unresolved external, constrain
   // its parameter types to be wild. Unless it has a bounds-safe annotation.
-  for (const auto &U : ExternalFunctionFVCons) {
-    std::string FuncName = U.first;
-    FVConstraint *G = U.second;
-
-    // If there was a checked type on a variable in the input program, it
-    // should stay that way. Otherwise, we shouldn't be adding a checked type
-    // to an extern function.
-    std::string Rsn = (G->hasBody() ? ""
-                                    : "Unchecked pointer in parameter or "
-                                      "return of external function " +
-                                          FuncName);
-
-    // Handle the cases where itype parameters should not be treated as their
-    // unchecked type.
-    // TODO: Ditto re getting a PSL (in the case in which Rsn is non-empty and
-    // it is actually used).
-    G->equateWithItype(*this, Rsn, nullptr);
-
-    // If we've seen this symbol, but never seen a body for it, constrain
-    // everything about it.
-    // Some global symbols we don't need to constrain to wild, like
-    // malloc and free. Check those here and skip if we find them.
-    if (!G->hasBody()) {
-      const FVComponentVariable *Ret = G->getCombineReturn();
-      Ret->getInternal()->constrainToWild(CS, Rsn);
-      if (!Ret->getExternal()->srcHasItype() &&
-          !Ret->getExternal()->isGeneric())
-        Ret->getExternal()->constrainToWild(CS, Rsn);
-
-      for (unsigned I = 0; I < G->numParams(); I++) {
-        const FVComponentVariable *Param = G->getCombineParam(I);
-        Param->getInternal()->constrainToWild(CS, Rsn);
-        if (!Param->getExternal()->srcHasItype() &&
-            !Param->getExternal()->isGeneric())
-          Param->getExternal()->constrainToWild(CS, Rsn);
-      }
-    }
-  }
+  for (const auto &U : ExternalFunctionFVCons)
+    linkFunction(U.second);
 
   // Repeat for static functions.
   //
   // Static functions that don't have a body will always cause a linking
   // error during compilation. They may still be useful as code is developed,
   // so we treat them as if they are external, and constrain parameters
   // to wild as appropriate.
-  for (const auto &U : StaticFunctionFVCons) {
-    for (const auto &V : U.second) {
+  for (const auto &U : StaticFunctionFVCons)
+    for (const auto &V : U.second)
+      linkFunction(V.second);
 
-      std::string FileName = U.first;
-      std::string FuncName = V.first;
-      FVConstraint *G = V.second;
-
-      std::string Rsn = (G->hasBody() ? ""
-                                      : "Unchecked pointer in parameter or "
-                                        "return of static function " +
-                                            FuncName + " in " + FileName);
-
-      // TODO: Ditto re getting a PSL
-      G->equateWithItype(*this, Rsn, nullptr);
+  return true;
+}
 
-      if (!G->hasBody()) {
+void ProgramInfo::linkFunction(FunctionVariableConstraint *FV) {
+  // If there was a checked type on a variable in the input program, it
+  // should stay that way. Otherwise, we shouldn't be adding a checked type
+  // to an undefined function.
+  std::string Rsn = (FV->hasBody() ? "" : "Unchecked pointer in parameter or "
+                                          "return of undefined function " +
+                                          FV->getName());
+
+  // Handle the cases where itype parameters should not be treated as their
+  // unchecked type.
+  // TODO: Ditto re getting a PSL (in the case in which Rsn is non-empty and
+  // it is actually used).
+  FV->equateWithItype(*this, Rsn, nullptr);
+
+  auto LinkComponent = [this, Rsn](const FVComponentVariable *FVC) {
+    FVC->getInternal()->constrainToWild(CS, Rsn);
+    if (!_3COpts.InferTypesForUndefs &&
+        !FVC->getExternal()->srcHasItype() && !FVC->getExternal()->isGeneric())
+      FVC->getExternal()->constrainToWild(CS, Rsn);
+  };
 
-        if (!G->getExternalReturn()->isGeneric())
-          G->getExternalReturn()->constrainToWild(CS, Rsn);
-        for (unsigned I = 0; I < G->numParams(); I++)
-          if (!G->getExternalParam(I)->isGeneric())
-            G->getExternalParam(I)->constrainToWild(CS, Rsn);
-      }
-    }
+  // If we've seen this symbol, but never seen a body for it, constrain
+  // everything about it.
+  if (!FV->hasBody()) {
+    LinkComponent(FV->getCombineReturn());
+    for (unsigned I = 0; I < FV->numParams(); I++)
+      LinkComponent(FV->getCombineParam(I));
   }
-
-  return true;
 }
 
 // Populate Variables, VarDeclToStatement, RVariables, and DepthMap with

clang/lib/3C/Utils.cpp

@@ -387,11 +387,15 @@ bool isInSysHeader(clang::Decl *D) {
 
 std::string getSourceText(const clang::SourceRange &SR,
                           const clang::ASTContext &C) {
+  return getSourceText(CharSourceRange::getTokenRange(SR), C);
+}
+
+std::string getSourceText(const clang::CharSourceRange &SR,
+                          const clang::ASTContext &C) {
   assert(SR.isValid() && "Invalid Source Range requested.");
   auto &SM = C.getSourceManager();
   auto LO = C.getLangOpts();
-  llvm::StringRef Srctxt =
-      Lexer::getSourceText(CharSourceRange::getTokenRange(SR), SM, LO);
+  llvm::StringRef Srctxt = Lexer::getSourceText(SR, SM, LO);
   return Srctxt.str();
 }
 

clang/test/3C/implicit_casts_root_cause.c

@@ -11,7 +11,7 @@ void test_no_cause() {
   has_void(c);
 }
 
-void has_float(float* v); // expected-warning {{Unchecked pointer in parameter or return of external function has_float}}
+void has_float(float* v); // expected-warning {{Unchecked pointer in parameter or return of undefined function has_float}}
 void test_float_cause() {
   int *b, *c;
   has_float(b); // expected-warning {{1 unchecked pointer: Cast from int * to float *}}