[llvm] 460efc1 - [Analysis] Be defensive when matching size_t in lib call signatures

[Bjorn Pettersson via llvm-commits]

Author: Bjorn Pettersson
Date: 2021-09-28T15:29:37+02:00
New Revision: 460efc1fb8354295ce1320b5dae62f06cdeda278

URL: https://github.com/llvm/llvm-project/commit/460efc1fb8354295ce1320b5dae62f06cdeda278
DIFF: https://github.com/llvm/llvm-project/commit/460efc1fb8354295ce1320b5dae62f06cdeda278.diff

LOG: [Analysis] Be defensive when matching size_t in lib call signatures

When TargetLibraryInfoImpl::isValidProtoForLibFunc is checking
function signatures to detect lib calls it may check that a parameter
or return value matches with the "size_t" type. For this to work it
has to derive the IR type matching with "size_t". Depending on if
a DataLayout is provided or not, this has been done in two different
way. Either a more strict check being based on IntPtrType (which is
given by the DataLayout) or a more relaxed check assuming that any
integer type matches with "size_t".

Given that the stricter approach exist it seems like we do not want
to trigger rewrites etc if we aren't sure that a function calls
actually match with the library function. Therefore it was questioned
why we actually have the more relaxed approach when not being able
to derive an IR type for "size_t". This patch will take a more
defensive approach, requiring that a DataLayout is passed to
isValidProtoForLibFunc.

Differential Revision: https://reviews.llvm.org/D110584

Added: 
    

Modified: 
    llvm/include/llvm/Analysis/TargetLibraryInfo.h
    llvm/lib/Analysis/TargetLibraryInfo.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Analysis/TargetLibraryInfo.h b/llvm/include/llvm/Analysis/TargetLibraryInfo.h
index c27e109e8687b..cc30e1322c2bd 100644
--- a/llvm/include/llvm/Analysis/TargetLibraryInfo.h
+++ b/llvm/include/llvm/Analysis/TargetLibraryInfo.h
@@ -76,7 +76,7 @@ class TargetLibraryInfoImpl {
   /// Return true if the function type FTy is valid for the library function
   /// F, regardless of whether the function is available.
   bool isValidProtoForLibFunc(const FunctionType &FTy, LibFunc F,
-                              const DataLayout *DL) const;
+                              const DataLayout &DL) const;
 
 public:
   /// List of known vector-functions libraries.
@@ -115,6 +115,8 @@ class TargetLibraryInfoImpl {
   ///
   /// If it is one of the known library functions, return true and set F to the
   /// corresponding value.
+  ///
+  /// FDecl is assumed to have a parent Module when using this function.
   bool getLibFunc(const Function &FDecl, LibFunc &F) const;
 
   /// Forces a function to be marked as unavailable.

diff  --git a/llvm/lib/Analysis/TargetLibraryInfo.cpp b/llvm/lib/Analysis/TargetLibraryInfo.cpp
index bc9aab31e36b8..a916ba6187fc0 100644
--- a/llvm/lib/Analysis/TargetLibraryInfo.cpp
+++ b/llvm/lib/Analysis/TargetLibraryInfo.cpp
@@ -725,20 +725,13 @@ bool TargetLibraryInfoImpl::getLibFunc(StringRef funcName, LibFunc &F) const {
 
 bool TargetLibraryInfoImpl::isValidProtoForLibFunc(const FunctionType &FTy,
                                                    LibFunc F,
-                                                   const DataLayout *DL) const {
+                                                   const DataLayout &DL) const {
   LLVMContext &Ctx = FTy.getContext();
   // FIXME: There is really no guarantee that sizeof(size_t) is equal to
   // sizeof(int*) for every target. So the assumption used here to derive the
   // SizeTTy based on DataLayout and getIntPtrType isn't always valid.
-  Type *SizeTTy = DL ? DL->getIntPtrType(Ctx, /*AddressSpace=*/0) : nullptr;
+  Type *SizeTTy = DL.getIntPtrType(Ctx, /*AddressSpace=*/0);
   auto IsSizeTTy = [SizeTTy](Type *Ty) {
-    // FIXME: For uknown historical reasons(?) we use a relaxed condition saying
-    // that any integer type may size_t, for example if we got no
-    // DataLayout. This seems like a potentially error prone relaxation (or why
-    // should we only be more strict and checking the exact type when we have a
-    // DataLayout?).
-    if (!SizeTTy)
-      return Ty->isIntegerTy();
     return Ty == SizeTTy;
   };
   unsigned NumParams = FTy.getNumParams();
@@ -1625,10 +1618,11 @@ bool TargetLibraryInfoImpl::getLibFunc(const Function &FDecl,
   // avoid string normalization and comparison.
   if (FDecl.isIntrinsic()) return false;
 
-  const DataLayout *DL =
-      FDecl.getParent() ? &FDecl.getParent()->getDataLayout() : nullptr;
+  const Module *M = FDecl.getParent();
+  assert(M && "Expecting FDecl to be connected to a Module.");
+
   return getLibFunc(FDecl.getName(), F) &&
-         isValidProtoForLibFunc(*FDecl.getFunctionType(), F, DL);
+         isValidProtoForLibFunc(*FDecl.getFunctionType(), F, M->getDataLayout());
 }
 
 void TargetLibraryInfoImpl::disableAllFunctions() {