[FuncSpec] Conditional jump or move depends on uninitialised value(s).

[labrinea]

Summary

I found this bug when performing a two-stage build of clang with Function Specialization enabled and tuned aggressively. The crash appears only on release builds. Fixes https://github.com/llvm/llvm-project/issues/55000.

Before accessing the contents of the Argument iterator inside SCCPInstVisitor::markArgInFuncSpecialization, we should be checking that the iterator is valid.

[ChuanqiXu]

The test is not good. It looks it comes from the actual failure cases. We should try to find a reduced example.

[fhahn]

Also please elaborate in the patch description what the issue is and *why* using TrackingVH is the best fix.

[labrinea]

Thank you both for the feedback. I'll investigate further as I think there's more to it. I've added some additional info in https://github.com/llvm/llvm-project/issues/55000#issuecomment-1105169299. Cheers.

[labrinea]

Changes to prior revision:

• added a validity check before accessing the contents of iterator
• reduced the testcase

[ChuanqiXu]

Now it looks good to me.

llvm/lib/Transforms/Utils/SCCPSolver.cpp

void SCCPInstVisitor::markArgInFuncSpecialization(
   auto Iter = Args.begin();
   Argument *NewArg = F->arg_begin();
   Argument *OldArg = Args[0].Formal->getParent()->arg_begin();
   for (auto End = F->arg_end(); NewArg != End; ++NewArg, ++OldArg) {
 
     LLVM_DEBUG(dbgs() << "SCCP: Marking argument "
                       << NewArg->getNameOrAsOperand() << "\n");
 
-    if (OldArg == Iter->Formal) {
+    if (Iter != Args.end() && OldArg == Iter->Formal) {
       // Mark the argument constants in the new function.
       markConstant(NewArg, Iter->Actual);
       ++Iter;
     } else if (ValueState.count(OldArg)) {
       // For the remaining arguments in the new function, copy the lattice state
       // over from the old function.
       //
       // Note: This previously looked like this:

llvm/test/Transforms/FunctionSpecialization/bug55000-read-uninitialized-value.ll

+; RUN: opt -function-specialization -func-specialization-max-iters=2 -func-specialization-size-threshold=20 -func-specialization-avg-iters-cost=20 -function-specialization-for-literal-constant=true -S < %s | FileCheck %s
+
+declare hidden i1 @compare(ptr) align 2
+declare hidden { i8, ptr } @getType(ptr) align 2
+
+; CHECK-LABEL: @foo
+; CHECK-LABEL: @foo.1
+; CHECK-LABEL: @foo.2
+
+define internal void @foo(ptr %TLI, ptr %DL, ptr %Ty, ptr %ValueVTs, ptr %Offsets, i64 %StartingOffset) {
+entry:
+  %VT = alloca i64, align 8
+  br i1 undef, label %if.then, label %if.end4
+
+if.then:                                          ; preds = %entry
+  ret void
+
+if.end4:                                          ; preds = %entry
+  %cmp = call zeroext i1 @compare(ptr undef)
+  br i1 %cmp, label %for.body, label %for.cond16
+
+for.body:                                         ; preds = %if.end4
+  %add13 = add i64 %StartingOffset, undef
+  call void @foo(ptr %TLI, ptr %DL, ptr undef, ptr %ValueVTs, ptr %Offsets, i64 %add13)
+  unreachable
+
+for.cond16:                                       ; preds = %for.cond34, %if.end4
+  %call27 = call { i8, ptr } @getType(ptr %VT)
+  br label %for.cond34
+
+for.cond34:                                       ; preds = %for.body37, %for.cond16
+  br i1 undef, label %for.body37, label %for.cond16
+
+for.body37:                                       ; preds = %for.cond34
+  %tobool39 = icmp ne ptr %Offsets, null
+  br label %for.cond34
+}
+
+define hidden { ptr, i32 } @bar(ptr %this) {
+entry:
+  %Offsets = alloca i64, align 8
+  %cmp26 = call zeroext i1 @compare(ptr undef)
+  br i1 %cmp26, label %for.body28, label %for.cond.cleanup27
+
+for.cond.cleanup27:                               ; preds = %entry
+  ret { ptr, i32 } undef
+
+for.body28:                                       ; preds = %entry
+  %call33 = call zeroext i1 @compare(ptr undef)
+  br i1 %call33, label %if.then34, label %if.end106
+
+if.then34:                                        ; preds = %for.body28
+  call void @foo(ptr %this, ptr undef, ptr undef, ptr undef, ptr null, i64 0)
+  unreachable
+
+if.end106:                                        ; preds = %for.body28
+  call void @foo(ptr %this, ptr undef, ptr undef, ptr undef, ptr %Offsets, i64 0)
+  unreachable
+}
+