[InstCombine] snprintf(NULL, 0, "%s", str) to strlen(str)

[xbolva00]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

2597

snprintf returns int, strlen returns size_t so... trunc should be fine?

[xbolva00]

Ping

[bjope]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

2594

Is there a specific use-case for doing this (I guess a user could use strlen instead if that is what they want). Is it to save cycles due to strlen probably being faster, or because it is more likely that there are subsequent transformations involving strlen?

2597

Isn't the transform only valid if the number of bits in CI is greater than the number of bits in V?
I.e. the unsigned value returned by strlen must fit into the non-negative part of the signed destination type. And given that constraint we always need the trunc.

[bjope]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

2597

Well, given that constraint we always need a zext, not trunc.

[xbolva00]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

2597

zext? We need change from i64 (from newly added strlen) to i32 (return type of snprintf)

[bjope]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

2597

If we consider the case when the value returned by strlen as size_t would be out-of-range for the non-negative range of int, then I guess snprintf should return a negative value? Or is the result undefined (I haven't seen any documentation saying anything about such cases for snprintf, while for strlen it says that the result in undefined if no null-termination is found)?

Either way it would be ok to do the transformation when sizeof(size_t)==sizeof(int), because that would give a negative value if the strlen result is out-of-range for the int.

But if sizeof(size_t)>sizeof(int), then maybe we need to saturate the value instead of doing a trunc, just to be safe?

[bjope]

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

2597

I was looking through old phabricator reviews still open, and tried to understand this conversation :-)

(And I think that I might have mixed up which function that returned int and size_t in some comments.)

What I've found out since last time is that, as described here https://www.austingroupbugs.net/view.php?id=761, the C standard (C11?) seems to have clarified that behavior is undefined if "The number of characters or wide characters transmitted by a formatted output function (or written to an array, or that would have been written to an array) is greater than INT_MAX (7.21.6.1, 7.29.2.1).".

And IIUC the snprintf would never return a negative value indicating conversion error for %s.

So it seems like we can assume that the snprintf return a value in the range [0, INT_MAX] or else it would be undefined.

Then I think we either need to check that sizeof(size_t) >= sizeof(int) (e.g. by comparing the type size for CI and V), to ensure that trunc is correct choice when the sizes aren't equal. If sizeof(size_t) < sizeof(int) (which probably is more unlikely) we could just skip doing the optimization (or we could make sure there will be a zext, e.g. by using CreateIntCast(V, CI->getType(), false)).

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

if (FormatStr[1] == 'c') {
       B.CreateStore(V, Ptr);
       Ptr = B.CreateGEP(B.getInt8Ty(), Ptr, B.getInt32(1), "nul");
       B.CreateStore(B.getInt8(0), Ptr);
 
       return ConstantInt::get(CI->getType(), 1);
     }
 
     if (FormatStr[1] == 's') {
+      // snprintf(NULL, 0, "%s", str) to strlen(str)
+      if (isa<ConstantPointerNull>(CI->getArgOperand(0)) && N == 0)
+        if (auto *V = emitStrLen(CI->getArgOperand(3), B, DL, TLI)) {
+          if (CI->getType() != V->getType())
+            V = B.CreateTrunc(V, CI->getType());
+          return V;
+        }
+
       // snprintf(dest, size, "%s", str) to llvm.memcpy(dest, str, len+1, 1)
       StringRef Str;
       if (!getConstantStringInfo(CI->getArgOperand(3), Str))
         return nullptr;
 
       if (N == 0)
         return ConstantInt::get(CI->getType(), Str.size());
       else if (N < Str.size() + 1)

llvm/test/Transforms/InstCombine/snprintf.ll

 ; CHECK-LABEL: @test_str_ok_size(
 ; CHECK-NEXT:    [[TMP1:%.*]] = bitcast i8* [[BUF:%.*]] to i32*
 ; CHECK-NEXT:    store i32 7500915, i32* [[TMP1]], align 1
 ; CHECK-NEXT:    ret i32 3
 ;
   %call = call i32 (i8*, i64, i8*, ...) @snprintf(i8* %buf, i64 32, i8* getelementptr inbounds ([3 x i8], [3 x i8]* @.str.3, i64 0, i64 0), i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str, i64 0, i64 0)) #2
   ret i32 %call
 }
+
+
+define i32 @snprint_unknown_str_len(i8* %fmt, i8* %s) {
+; CHECK-LABEL: @snprint_unknown_str_len(
+; CHECK-NEXT:    [[LEN:%.*]] = call i32 (i8*, i64, i8*, ...) @snprintf(i8* null, i64 0, i8* [[FMT:%.*]], i8* [[S:%.*]])
+; CHECK-NEXT:    ret i32 [[LEN]]
+;
+  %len = call i32 (i8*, i64, i8*, ...) @snprintf(i8* null, i64 0, i8* %fmt, i8* %s)
+  ret i32 %len
+}
+
+define i32 @snprint_to_strlen(i8* %s) {
+; CHECK-LABEL: @snprint_to_strlen(
+; CHECK-NEXT:    [[STRLEN:%.*]] = call i64 @strlen(i8* noundef nonnull dereferenceable(1) [[S:%.*]])
+; CHECK-NEXT:    [[TMP1:%.*]] = trunc i64 [[STRLEN]] to i32
+; CHECK-NEXT:    ret i32 [[TMP1]]
+;
+  %len = call i32 (i8*, i64, i8*, ...) @snprintf(i8* null, i64 0, i8* getelementptr inbounds ([3 x i8], [3 x i8]* @.str.3, i64 0, i64 0), i8* %s)
+  ret i32 %len
+}