[PATCH] D144682: Fix miscompilation from MergeFunctions-Inline-SROA-InstCombine optimization sequence

X via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 23 15:34:25 PST 2023


dingxiangfei2009 created this revision.
dingxiangfei2009 added reviewers: eeckstein, luqmana.
Herald added subscribers: JDevlieghere, hiraditya, kristof.beyls, arichardson.
Herald added a project: All.
dingxiangfei2009 requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Today we realize that we are able to hit a corner case that leads to mis-compilation. This is due to interaction of merge function passes with other optimization passes involving `!nonnull` assertions.

By applying MergeFunctions, Inline, SROA and InstCombine in this order to the following IR, the program semantics changes from calling the external function `@out` with arguments `(some pointer, null)` to calling `@out` with `(some pointer, some other non-null pointer)`.

  target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
  ; target triple = "arm64-apple-macosx11.0.0"
  
  %1 = type { { ptr, i64 }, ptr }
  %optstring = type { [1 x i64], ptr, [1 x i64] }
  
  @emptystr = external hidden unnamed_addr constant <{}>, align 8
  
  define hidden void @f1(ptr noalias nocapture noundef sret(%1) dereferenceable(24) %0, ptr noalias nocapture noundef dereferenceable(24) %1) unnamed_addr #0 {
    %3 = getelementptr inbounds { ptr, i64 }, ptr %1, i32 0, i32 0
    %4 = load ptr, ptr %3, align 8, !nonnull !2, !align !3, !noundef !2
    %5 = getelementptr inbounds { ptr, i64 }, ptr %1, i32 0, i32 1
    %6 = load i64, ptr %5, align 8
    %7 = getelementptr inbounds { { ptr, i64 }, ptr }, ptr %1, i32 0, i32 1
    %8 = load ptr, ptr %7, align 8, !nonnull !2, !align !6, !noundef !2
    %9 = getelementptr inbounds { ptr, i64 }, ptr %0, i32 0, i32 0
    store ptr %4, ptr %9, align 8
    %10 = getelementptr inbounds { ptr, i64 }, ptr %0, i32 0, i32 1
    store i64 %6, ptr %10, align 8
    %11 = getelementptr inbounds %1, ptr %0, i32 0, i32 1
    store ptr %8, ptr %11, align 8
    ret void
  }
  
  define hidden void @f2(ptr noalias nocapture noundef sret(%1) dereferenceable(24) %0, ptr noalias nocapture noundef dereferenceable(24) %1) unnamed_addr #0 {
    %3 = getelementptr inbounds { ptr, i64 }, ptr %1, i32 0, i32 0
    %4 = load ptr, ptr %3, align 8, !nonnull !2, !align !3, !noundef !2
    %5 = getelementptr inbounds { ptr, i64 }, ptr %1, i32 0, i32 1
    %6 = load i64, ptr %5, align 8
    %7 = getelementptr inbounds { { ptr, i64 }, ptr }, ptr %1, i32 0, i32 1
    %8 = load ptr, ptr %7, align 8, !align !6
    %9 = getelementptr inbounds { ptr, i64 }, ptr %0, i32 0, i32 0
    store ptr %4, ptr %9, align 8
    %10 = getelementptr inbounds { ptr, i64 }, ptr %0, i32 0, i32 1
    store i64 %6, ptr %10, align 8
    %11 = getelementptr inbounds %1, ptr %0, i32 0, i32 1
    store ptr %8, ptr %11, align 8
    ret void
  }
  
  define void @f(ptr noalias nocapture noundef dereferenceable(24) %string, ptr noalias nocapture noundef dereferenceable(4) %flag) personality ptr @rust_eh_personality {
    ; optstring: Option<String>
    %optstring = alloca %optstring, align 8
    ; kv1: (%str, &String)
    %kv1 = alloca { { ptr, i64 }, ptr }, align 8
    ; kv2: (%str, Option<&String>)
    %kv2 = alloca { { ptr, i64 }, ptr }, align 8
    ; singlekv1: SingleKV<&String>
    %singlekv1 = alloca { { ptr, i64 }, ptr }, align 8
    ; singlekv2: SingleKV<Option<&String>>
    %singlekv2 = alloca { { ptr, i64 }, ptr }, align 8
  
    %1 = getelementptr inbounds { { ptr, i64 }, ptr }, ptr %kv1, i32 0, i32 0, i32 0
    %2 = getelementptr inbounds { { ptr, i64 }, ptr }, ptr %kv1, i32 0, i32 0, i32 1
    %3 = getelementptr inbounds { { ptr, i64 }, ptr }, ptr %kv1, i32 0, i32 1
    store ptr @emptystr, ptr %kv1, align 8
    store i64 0, ptr %2, align 8
    store ptr %string, ptr %3, align 8
    ; kv1 constructed
  
    %cond = load i32, ptr %flag, align 8
    %4 = icmp eq i32 %cond, 0
    br i1 %4, label %cmpt, label %cmpf
  
  cmpt:
    %5 = getelementptr inbounds %optstring, ptr %optstring, i32 0, i32 1
    store ptr null, ptr %5, align 8
    br label %cmpd
  
  cmpf:
    store i64 0, ptr %optstring, align 8
    %6 = getelementptr inbounds i8, ptr %optstring, i64 8
    store ptr inttoptr (i64 1 to ptr), ptr %6, align 8
    %7 = getelementptr inbounds i8, ptr %optstring, i64 16
    store i64 0, ptr %7, align 8
    br label %cmpd
  
  cmpd:
    %8 = getelementptr inbounds %optstring, ptr %optstring, i64 0, i32 1
    %9 = load ptr, ptr %8, align 8
    %10 = icmp eq ptr %9, null
    %11 = select i1 %10, ptr null, ptr %optstring
  
    %12 = getelementptr inbounds { { ptr, i64 }, ptr }, ptr %kv1, i32 0, i32 0, i32 0
    %13 = getelementptr inbounds { { ptr, i64 }, ptr }, ptr %kv1, i32 0, i32 0, i32 1
    %14 = getelementptr inbounds { { ptr, i64 }, ptr }, ptr %kv1, i32 0, i32 1
    store ptr @emptystr, ptr %12, align 8
    store i64 0, ptr %13, align 8
    store ptr %11, ptr %14, align 8
    ; kv2 constructed
  
    invoke void @f1(ptr noalias nocapture noundef nonnull sret(%1) dereferenceable(24) %singlekv1, ptr noalias nocapture noundef nonnull dereferenceable(24) %kv1)
            to label %f1d unwind label %unwind
  
  f1d:
    invoke void @f2(ptr noalias nocapture noundef nonnull sret(%1) dereferenceable(24) %singlekv2, ptr noalias nocapture noundef nonnull dereferenceable(24) %kv2)
            to label %f2d unwind label %unwind
  
  f2d:
    invoke void @out(ptr %singlekv1, ptr %singlekv2) to label %outd unwind label %unwind
  
  outd:
    ret void
  
  unwind:
    %15 = landingpad { ptr, i32 }
            cleanup
    unreachable
  }
  
  declare void @out(ptr, ptr)
  
  declare noundef i32 @rust_eh_personality(i32, i32 noundef, i64, ptr, ptr) unnamed_addr #10
  
  !2 = !{}
  !3 = !{i64 1}
  !6 = !{i64 8}

What happens is that MergeFunctions is registering `@f2` and `@f1` as the same, even though `@f1` asserts that the value `%8` is not null. `@f1` and `@f2` operates on data types that shares the same layout but not the same semantics, thus the difference in the non-null assertion. By incorrectly replacing the call to `@f2` with a call to `@f2` and inlining the calls, SROA inserts an assertion that the value `%11` is not null, which is false since this can be controlled by `%flag`. Based on this incorrect assertion, InstCombine produces an IR that feeds a non-null pointer value to the second argument of the `@out` call unconditionally.

This differential proposes one of a possible remedy, by distinguishing non-null values in the function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144682

Files:
  llvm/lib/Transforms/Utils/FunctionComparator.cpp


Index: llvm/lib/Transforms/Utils/FunctionComparator.cpp
===================================================================
--- llvm/lib/Transforms/Utils/FunctionComparator.cpp
+++ llvm/lib/Transforms/Utils/FunctionComparator.cpp
@@ -787,6 +787,14 @@
   if (ConstR)
     return -1;
 
+  if (auto LIL = dyn_cast<LoadInst>(L), LIR = dyn_cast<LoadInst>(R);
+      LIL && LIR) {
+    auto NNL = bool(LIL->getMetadata(LLVMContext::MD_nonnull));
+    auto NNR = bool(LIR->getMetadata(LLVMContext::MD_nonnull));
+    if (NNL != NNR)
+      return NNL - NNR;
+  }
+
   const InlineAsm *InlineAsmL = dyn_cast<InlineAsm>(L);
   const InlineAsm *InlineAsmR = dyn_cast<InlineAsm>(R);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D144682.499996.patch
Type: text/x-patch
Size: 674 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230223/58c58931/attachment.bin>


More information about the llvm-commits mailing list