[PATCH] Fixing a corner-case bug in lib call optimization

Yunzhong Gao Yunzhong_Gao at playstation.sony.com
Tue Aug 6 10:06:00 PDT 2013


ygao added you to the CC list for the revision "Fixing a corner-case bug in lib call optimization".

Hi,
In the following test case:
```
/* test.c */
#include <string.h>
#include <stdio.h>
#include <stdint.h>
#include <assert.h>

int main(void)
{
  int  content = 0xFF00;  // 0x00 after converting to char
  char *string = "";      // 0x00 as the terminating char
  char *result_ptr;

  result_ptr = strchr(string, content);
  // should get a valid address in result_ptr

  printf("%016lx\n", (uint64_t)result_ptr);

  return 0;
}
/* end of test.c */
```

With -fno-builtin, we can get some address as output:

  $ clang -O2 -fno-builtin test.c -o a.out
  $ ./a.out
  00000000004006c2

But without it, a null pointer is returned, which is incorrect:

  $ clang -O2 test.c -o a.out
  $ ./a.out
  0000000000000000

The problem is in the lib call optimization where the input character is not converted to char before comparing with zero. The same problem exists for both strchr and strrchr.

Please review and let me know whether the proposed patch is good to go in.

Many thanks,
- Gao.

http://llvm-reviews.chandlerc.com/D1279

Files:
  llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
  llvm/test/Transforms/InstCombine/strchr-1.ll
  llvm/test/Transforms/InstCombine/strrchr-1.ll

Index: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
===================================================================
--- llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -477,7 +477,7 @@
 
     // Compute the offset, make sure to handle the case when we're searching for
     // zero (a weird way to spell strlen).
-    size_t I = CharC->getSExtValue() == 0 ?
+    size_t I = (char)(CharC->getSExtValue()) == 0 ?
         Str.size() : Str.find(CharC->getSExtValue());
     if (I == StringRef::npos) // Didn't find the char.  strchr returns null.
       return Constant::getNullValue(CI->getType());
@@ -513,7 +513,7 @@
     }
 
     // Compute the offset.
-    size_t I = CharC->getSExtValue() == 0 ?
+    size_t I = (char)(CharC->getSExtValue()) == 0 ?
         Str.size() : Str.rfind(CharC->getSExtValue());
     if (I == StringRef::npos) // Didn't find the char. Return null.
       return Constant::getNullValue(CI->getType());
Index: llvm/test/Transforms/InstCombine/strchr-1.ll
===================================================================
--- llvm/test/Transforms/InstCombine/strchr-1.ll
+++ llvm/test/Transforms/InstCombine/strchr-1.ll
@@ -52,3 +52,14 @@
   store i8* %dst, i8** @chp
   ret void
 }
+
+define void @test_simplify5() {
+; CHECK: store i8* getelementptr inbounds ([14 x i8]* @hello, i32 0, i32 13)
+; CHECK-NOT: call i8* @strchr
+; CHECK: ret void
+
+  %src = getelementptr [14 x i8]* @hello, i32 0, i32 0
+  %dst = call i8* @strchr(i8* %src, i32 65280)
+  store i8* %dst, i8** @chp
+  ret void
+}
Index: llvm/test/Transforms/InstCombine/strrchr-1.ll
===================================================================
--- llvm/test/Transforms/InstCombine/strrchr-1.ll
+++ llvm/test/Transforms/InstCombine/strrchr-1.ll
@@ -42,6 +42,17 @@
   ret void
 }
 
+define void @test_simplify4() {
+; CHECK: store i8* getelementptr inbounds ([14 x i8]* @hello, i32 0, i32 13)
+; CHECK-NOT: call i8* @strrchr
+; CHECK: ret void
+
+  %src = getelementptr [14 x i8]* @hello, i32 0, i32 0
+  %dst = call i8* @strrchr(i8* %src, i32 65280)
+  store i8* %dst, i8** @chp
+  ret void
+}
+
 define void @test_nosimplify1(i32 %chr) {
 ; CHECK-LABEL: @test_nosimplify1(
 ; CHECK: call i8* @strrchr
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1279.1.patch
Type: text/x-patch
Size: 2261 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130806/78acb3e7/attachment.bin>


More information about the llvm-commits mailing list