[InstCombine] Look through more casts when folding memchr and memcmp

Enhance getConstantDataArrayInfo to let the memchr and memcmp library
call folders look through arbitrarily long sequences of bitcast and
GEP instructions.

Reviewed By: nikic

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

llvm/lib/Analysis/ValueTracking.cpp

@@ -4187,60 +4187,45 @@ bool llvm::getConstantDataArrayInfo(const Value *V,
                                     unsigned ElementSize, uint64_t Offset) {
   assert(V);
 
-  // Look through bitcast instructions and geps.
-  V = V->stripPointerCasts();
-
-  // If the value is a GEP instruction or constant expression, treat it as an
-  // offset.
-  if (const GEPOperator *GEP = dyn_cast<GEPOperator>(V)) {
-    // Fail if the first GEP operand is not a constant zero and we're
-    // not indexing into the initializer.
-    const ConstantInt *FirstIdx = dyn_cast<ConstantInt>(GEP->getOperand(1));
-    if (!FirstIdx || !FirstIdx->isZero())
-      return false;
+  // Drill down into the pointer expression V, ignoring any intervening
+  // casts, and determine the identity of the object it references along
+  // with the cumulative byte offset into it.
+  const GlobalVariable *GV =
+    dyn_cast<GlobalVariable>(getUnderlyingObject(V));
+  if (!GV || !GV->isConstant() || !GV->hasDefinitiveInitializer())
+    // Fail if V is not based on constant global object.
+    return false;
 
-    Value *Op0 = GEP->getOperand(0);
-    const GlobalVariable *GV = dyn_cast<GlobalVariable>(Op0);
-    if (!GV)
-      return false;
+  const DataLayout &DL = GV->getParent()->getDataLayout();
+  APInt Off(DL.getIndexTypeSizeInBits(V->getType()), 0);
 
-    // Fail if the offset into the initializer is not constant.
-    const DataLayout &DL = GV->getParent()->getDataLayout();
-    APInt Off(DL.getIndexSizeInBits(GEP->getPointerAddressSpace()), 0);
-    if (!GEP->accumulateConstantOffset(DL, Off))
-      return false;
+  if (GV != V->stripAndAccumulateConstantOffsets(DL, Off,
+                                                 /*AllowNonInbounds*/ true))
+    // Fail if a constant offset could not be determined.
+    return false;
 
+  uint64_t StartIdx = Off.getLimitedValue();
+  if (StartIdx == UINT64_MAX)
     // Fail if the constant offset is excessive.
-    uint64_t StartIdx = Off.getLimitedValue();
-    if (StartIdx == UINT64_MAX)
-      return false;
-
-    return getConstantDataArrayInfo(Op0, Slice, ElementSize, StartIdx + Offset);
-  }
-
-  // The GEP instruction, constant or instruction, must reference a global
-  // variable that is a constant and is initialized. The referenced constant
-  // initializer is the array that we'll use for optimization.
-  const GlobalVariable *GV = dyn_cast<GlobalVariable>(V);
-  if (!GV || !GV->isConstant() || !GV->hasDefinitiveInitializer())
     return false;
 
-  const DataLayout &DL = GV->getParent()->getDataLayout();
+  Offset += StartIdx;
+
   ConstantDataArray *Array = nullptr;
   ArrayType *ArrayTy = nullptr;
 
   if (GV->getInitializer()->isNullValue()) {
     Type *GVTy = GV->getValueType();
     uint64_t SizeInBytes = DL.getTypeStoreSize(GVTy).getFixedSize();
     uint64_t Length = SizeInBytes / (ElementSize / 8);
-    if (Length <= Offset)
-      // Bail on undersized constants to let sanitizers detect library
-      // calls with them as arguments.
-      return false;
 
     Slice.Array = nullptr;
     Slice.Offset = 0;
-    Slice.Length = Length - Offset;
+    // Return an empty Slice for undersized constants to let callers
+    // transform even undefined library calls into simpler, well-defined
+    // expressions.  This is preferable to making the calls although it
+    // prevents sanitizers from detecting such calls.
+    Slice.Length = Length < Offset ? 0 : Length - Offset;
     return true;
   }
 
@@ -4292,6 +4277,12 @@ bool llvm::getConstantStringInfo(const Value *V, StringRef &Str,
 
   if (Slice.Array == nullptr) {
     if (TrimAtNul) {
+      // Return a nul-terminated string even for an empty Slice.  This is
+      // safe because all existing SimplifyLibcalls callers require string
+      // arguments and the behavior of the functions they fold is undefined
+      // otherwise.  Folding the calls this way is preferable to making
+      // the undefined library calls, even though it prevents sanitizers
+      // from reporting such calls.
       Str = StringRef();
       return true;
     }
@@ -4371,9 +4362,13 @@ static uint64_t GetStringLengthH(const Value *V,
     return 0;
 
   if (Slice.Array == nullptr)
+    // Zeroinitializer (including an empty one).
     return 1;
 
-  // Search for nul characters
+  // Search for the first nul character.  Return a conservative result even
+  // when there is no nul.  This is safe since otherwise the string function
+  // being folded such as strlen is undefined, and can be preferable to
+  // making the undefined library call.
   unsigned NullIndex = 0;
   for (unsigned E = Slice.Length; NullIndex < E; ++NullIndex) {
     if (Slice.Array->getElementAsInteger(Slice.Offset + NullIndex) == 0)

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

@@ -945,6 +945,12 @@ Value *LibCallSimplifier::optimizeMemRChr(CallInst *CI, IRBuilderBase &B) {
   if (!getConstantStringInfo(SrcStr, Str, 0, /*TrimAtNul=*/false))
     return nullptr;
 
+  if (Str.size() == 0)
+    // If the array is empty fold memrchr(A, C, N) to null for any value
+    // of C and N on the basis that the only valid value of N is zero
+    // (otherwise the call is undefined).
+    return NullPtr;
+
   uint64_t EndOff = UINT64_MAX;
   if (LenC) {
     EndOff = LenC->getZExtValue();
@@ -1046,6 +1052,12 @@ Value *LibCallSimplifier::optimizeMemChr(CallInst *CI, IRBuilderBase &B) {
     return B.CreateSelect(Cmp, NullPtr, SrcPlus);
   }
 
+  if (Str.size() == 0)
+    // If the array is empty fold memchr(A, C, N) to null for any value
+    // of C and N on the basis that the only valid value of N is zero
+    // (otherwise the call is undefined).
+    return NullPtr;
+
   if (LenC)
     Str = substr(Str, LenC->getZExtValue());
 

llvm/test/Transforms/InstCombine/memchr-10.ll

@@ -0,0 +1,84 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+;
+; Verify that the result of memchr calls with past-the-end pointers used
+; in equality expressions don't cause trouble and either are folded when
+; they might be valid or not when they're provably undefined.
+
+declare i8* @memchr(i8*, i32, i64)
+
+
+@a5 = constant [5 x i8] c"12345"
+
+
+; Fold memchr(a5 + 5, c, 1) == a5 + 5 to an arbitrary constrant.
+; The call is transformed to a5[5] == c by the memchr simplifier, with
+; a5[5] being indeterminate.  The equality then is the folded with
+; an undefined/arbitrary result.
+
+define i1 @call_memchr_ap5_c_1_eq_a(i32 %c, i64 %n) {
+; CHECK-LABEL: @call_memchr_ap5_c_1_eq_a(
+; CHECK-NEXT:    ret i1
+;
+  %pap5 = getelementptr [5 x i8], [5 x i8]* @a5, i32 0, i32 5
+  %qap5 = getelementptr [5 x i8], [5 x i8]* @a5, i32 1, i32 0
+  %q = call i8* @memchr(i8* %pap5, i32 %c, i64 1)
+  %cmp = icmp eq i8* %q, %qap5
+  ret i1 %cmp
+}
+
+
+; Fold memchr(a5 + 5, c, 5) == a5 + 5 to an arbitrary constant.
+
+define i1 @call_memchr_ap5_c_5_eq_a(i32 %c, i64 %n) {
+; CHECK-LABEL: @call_memchr_ap5_c_5_eq_a(
+; CHECK-NEXT:    ret i1
+;
+  %pap5 = getelementptr [5 x i8], [5 x i8]* @a5, i32 0, i32 5
+  %qap5 = getelementptr [5 x i8], [5 x i8]* @a5, i32 1, i32 0
+  %q = call i8* @memchr(i8* %pap5, i32 %c, i64 5)
+  %cmp = icmp eq i8* %q, %qap5
+  ret i1 %cmp
+}
+
+
+; Fold memchr(a5 + 5, c, n) == a5 to false.
+
+define i1 @fold_memchr_ap5_c_n_eq_a(i32 %c, i64 %n) {
+; CHECK-LABEL: @fold_memchr_ap5_c_n_eq_a(
+; CHECK-NEXT:    ret i1 false
+;
+  %pa = getelementptr [5 x i8], [5 x i8]* @a5, i32 0, i32 0
+  %pap5 = getelementptr [5 x i8], [5 x i8]* @a5, i32 0, i32 5
+  %q = call i8* @memchr(i8* %pap5, i32 %c, i64 %n)
+  %cmp = icmp eq i8* %q, %pa
+  ret i1 %cmp
+}
+
+
+; Fold memchr(a5 + 5, c, n) == null to true on the basis that n must
+; be zero in order for the call to be valid.
+
+define i1 @fold_memchr_ap5_c_n_eqz(i32 %c, i64 %n) {
+; CHECK-LABEL: @fold_memchr_ap5_c_n_eqz(
+; CHECK-NEXT:    ret i1 true
+;
+  %p = getelementptr [5 x i8], [5 x i8]* @a5, i32 0, i32 5
+  %q = call i8* @memchr(i8* %p, i32 %c, i64 %n)
+  %cmp = icmp eq i8* %q, null
+  ret i1 %cmp
+}
+
+
+; Fold memchr(a5 + 5, '\0', n) == null to true on the basis that n must
+; be zero in order for the call to be valid.
+
+define i1 @fold_memchr_a_nul_n_eqz(i64 %n) {
+; CHECK-LABEL: @fold_memchr_a_nul_n_eqz(
+; CHECK-NEXT:    ret i1 true
+;
+  %p = getelementptr [5 x i8], [5 x i8]* @a5, i32 0, i32 5
+  %q = call i8* @memchr(i8* %p, i32 0, i64 %n)
+  %cmp = icmp eq i8* %q, null
+  ret i1 %cmp
+}