[PATCH] D46752: [InstCombine] Validate the assumption about return type in optimizeSnPrintFString

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo created this revision.
mstorsjo added reviewers: rja, bkramer, spatel, xbolva00, efriedma, majnemer.
Herald added a subscriber: mehdi_amini.

If the sprintf function is static, earlier optimization passes could optimize out the return value altogether, and make it void.

Just bail out instead of going on with the assumption about the return type.

Alternatively, each of the function's

  return ConstantInt::get(...)

could be made into

  return CI->getType()->isVoidType() ? B.CreateRetVoid() ? ConstantInt::get(...)

to keep this optimization enabled for this corner case as well.

I chose this version since it should be the safest with least risk of running into some seldom tested codepath assuming the return type. Otherwise the return handling could maybe be made into a helper function to avoid the large expression for each return.


Repository:
  rL LLVM

https://reviews.llvm.org/D46752

Files:
  lib/Transforms/Utils/SimplifyLibCalls.cpp
  test/Transforms/InstCombine/sprintf-void.ll


Index: test/Transforms/InstCombine/sprintf-void.ll
===================================================================
--- /dev/null
+++ test/Transforms/InstCombine/sprintf-void.ll
@@ -0,0 +1,19 @@
+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:128:128"
+
+ at hello_world = constant [13 x i8] c"hello world\0A\00"
+
+declare void @sprintf(i8*, i8*, ...)
+
+; Check that a sprintf call, that would otherwise be optimized, but with
+; optimized out return type, doesn't crash the optimizer.
+
+define void @test_simplify1(i8* %dst) {
+; CHECK-LABEL: @test_simplify1(
+  %fmt = getelementptr [13 x i8], [13 x i8]* @hello_world, i32 0, i32 0
+  call void (i8*, i8*, ...) @sprintf(i8* %dst, i8* %fmt)
+; CHECK-NEXT: call void (i8*, i8*, ...) @sprintf(i8* %dst, i8* getelementptr inbounds ([13 x i8], [13 x i8]* @hello_world, i32 0, i32 0))
+  ret void
+; CHECK-NEXT: ret void
+}
Index: lib/Transforms/Utils/SimplifyLibCalls.cpp
===================================================================
--- lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -1837,6 +1837,11 @@
   StringRef FormatStr;
   if (!getConstantStringInfo(CI->getArgOperand(1), FormatStr))
     return nullptr;
+  // The rest of the function assumes the return value is an integer type.
+  // If the sprintf function is static, an earlier optimization pass could
+  // remove the return value and make it void.
+  if (!isa<IntegerType>(CI->getType()))
+    return nullptr;
 
   // If we just have a format string (nothing else crazy) transform it.
   if (CI->getNumArgOperands() == 2) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46752.146312.patch
Type: text/x-patch
Size: 1726 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180511/3b7096e3/attachment.bin>