Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Miscompile in code with indirect gotos #102351

Closed
hiraditya opened this issue Aug 7, 2024 · 2 comments · Fixed by #103688 or #117778
Closed

Miscompile in code with indirect gotos #102351

hiraditya opened this issue Aug 7, 2024 · 2 comments · Fixed by #103688 or #117778

Comments

@hiraditya
Copy link
Collaborator

The simplifycfg pass (incorrectly?) optimizes one of the case statements.

target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

@.str = private unnamed_addr constant [23 x i8] c"OP_1:(instruction=%d)\0A\00", align 1
@.str.1 = private unnamed_addr constant [28 x i8] c"TERMINATE:(instruction=%d)\0A\00", align 1

; Function Attrs: mustprogress norecurse uwtable
define dso_local noundef i32 @main() #0 {
entry:
  %bytecode = alloca [2 x ptr], align 16
  store ptr blockaddress(@main, %VM__OP_1), ptr %bytecode, align 16, !tbaa !5
  %arrayidx1 = getelementptr inbounds [2 x ptr], ptr %bytecode, i64 0, i64 1
  store ptr blockaddress(@main, %VM__TERMINATE), ptr %arrayidx1, align 8, !tbaa !5
  br label %while.body

while.body:                                       ; preds = %entry, %sw.epilog
  %state.0 = phi i32 [ 0, %entry ], [ %state.1, %sw.epilog ]
  %index.0 = phi i32 [ 0, %entry ], [ %index.2, %sw.epilog ]
  switch i32 %state.0, label %sw.epilog [
    i32 0, label %sw.bb
    i32 1, label %VM__OP_1
    i32 2, label %sw.bb4
  ]

sw.bb:                                            ; preds = %while.body
  br label %indirectgoto

VM__OP_1:                                         ; preds = %while.body, %indirectgoto
  %index.1 = phi i32 [ %index.3, %indirectgoto ], [ %index.0, %while.body ]
  br label %sw.epilog

sw.bb4:                                           ; preds = %while.body
  %call = call i32 (ptr, ...) @printf(ptr noundef nonnull dereferenceable(1) @.str, i32 noundef %index.0)
  %inc = add nsw i32 %index.0, 1
  br label %indirectgoto

sw.epilog:                                        ; preds = %while.body, %VM__OP_1
  %state.1 = phi i32 [ %state.0, %while.body ], [ 2, %VM__OP_1 ]
  %index.2 = phi i32 [ %index.0, %while.body ], [ %index.1, %VM__OP_1 ]
  br label %while.body, !llvm.loop !9

VM__TERMINATE:                                    ; preds = %indirectgoto
  %call7 = call i32 (ptr, ...) @printf(ptr noundef nonnull dereferenceable(1) @.str.1, i32 noundef %index.3)
  ret i32 0

indirectgoto:                                     ; preds = %sw.bb4, %sw.bb
  %index.3 = phi i32 [ %inc, %sw.bb4 ], [ %index.0, %sw.bb ]
  %idxprom.pn = sext i32 %index.3 to i64
  %indirect.goto.dest.in = getelementptr inbounds [2 x ptr], ptr %bytecode, i64 0, i64 %idxprom.pn
  %indirect.goto.dest = load ptr, ptr %indirect.goto.dest.in, align 8, !tbaa !5
  indirectbr ptr %indirect.goto.dest, [label %VM__OP_1, label %VM__TERMINATE]
}
declare i32 @printf(ptr noundef, ...) #1

attributes #0 = { mustprogress norecurse uwtable "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
attributes #1 = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }

!llvm.module.flags = !{!0, !1, !2, !3}
!llvm.ident = !{!4}

!0 = !{i32 1, !"wchar_size", i32 4}
!1 = !{i32 8, !"PIC Level", i32 2}
!2 = !{i32 7, !"PIE Level", i32 2}
!3 = !{i32 7, !"uwtable", i32 2}
!4 = !{!"clang version 18.0.0git (https://github.com/llvm/llvm-project.git 67782d2de5ea9c8653b8f0110237a3c355291c0e)"}
!5 = !{!6, !6, i64 0}
!6 = !{!"any pointer", !7, i64 0}
!7 = !{!"omnipotent char", !8, i64 0}
!8 = !{!"Simple C++ TBAA"}
!9 = !{!10, !10, i64 0}
!10 = !{!"int", !7, i64 0}
!11 = distinct !{!11, !12, !13}
!12 = !{!"llvm.loop.mustprogress"}
!13 = !{!"llvm.loop.unroll.disable"}

$ opt -passes=simplifycfg orig.ll -S -o out.ll

; ModuleID = 'orig.ll'
source_filename = "orig.ll"
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

@.str = private unnamed_addr constant [23 x i8] c"OP_1:(instruction=%d)\0A\00", align 1
@.str.1 = private unnamed_addr constant [28 x i8] c"TERMINATE:(instruction=%d)\0A\00", align 1

; Function Attrs: mustprogress norecurse uwtable
define dso_local noundef i32 @main() #0 {
entry:
  %bytecode = alloca [2 x ptr], align 16
  store ptr blockaddress(@main, %VM__OP_1), ptr %bytecode, align 16, !tbaa !5
  %arrayidx1 = getelementptr inbounds [2 x ptr], ptr %bytecode, i64 0, i64 1
  store ptr blockaddress(@main, %VM__TERMINATE), ptr %arrayidx1, align 8, !tbaa !5
  br label %while.body

while.body:                                       ; preds = %sw.epilog, %entry
  %state.0 = phi i32 [ 0, %entry ], [ %state.1, %sw.epilog ]
  %index.0 = phi i32 [ 0, %entry ], [ %index.0, %sw.epilog ]
  switch i32 %state.0, label %sw.epilog [
    i32 0, label %VM__TERMINATE
    i32 1, label %VM__OP_1
    i32 2, label %sw.bb4
  ]

VM__OP_1:                                         ; preds = %while.body
  br label %sw.epilog

sw.bb4:                                           ; preds = %while.body
  %call = call i32 (ptr, ...) @printf(ptr noundef nonnull dereferenceable(1) @.str, i32 noundef %index.0)
  %inc = add nsw i32 %index.0, 1
  br label %VM__TERMINATE

sw.epilog:                                        ; preds = %VM__OP_1, %while.body
  %state.1 = phi i32 [ %state.0, %while.body ], [ 2, %VM__OP_1 ]
  br label %while.body, !llvm.loop !9

VM__TERMINATE:                                    ; preds = %sw.bb4, %while.body
  %index.3 = phi i32 [ %inc, %sw.bb4 ], [ %index.0, %while.body ]
  %call7 = call i32 (ptr, ...) @printf(ptr noundef nonnull dereferenceable(1) @.str.1, i32 noundef %index.3)
  ret i32 0
}

declare i32 @printf(ptr noundef, ...) #1

attributes #0 = { mustprogress norecurse uwtable "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
attributes #1 = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }

!llvm.module.flags = !{!0, !1, !2, !3}
!llvm.ident = !{!4}

!0 = !{i32 1, !"wchar_size", i32 4}
!1 = !{i32 8, !"PIC Level", i32 2}
!2 = !{i32 7, !"PIE Level", i32 2}
!3 = !{i32 7, !"uwtable", i32 2}
!4 = !{!"clang version 18.0.0git (https://github.com/llvm/llvm-project.git 67782d2de5ea9c8653b8f0110237a3c355291c0e)"}
!5 = !{!6, !6, i64 0}
!6 = !{!"any pointer", !7, i64 0}
!7 = !{!"omnipotent char", !8, i64 0}
!8 = !{!"Simple C++ TBAA"}
!9 = !{!10, !10, i64 0}
!10 = !{!"int", !7, i64 0}
@hiraditya
Copy link
Collaborator Author

Original C code

extern int printf(const char *fmt, ...);

int main() {
  void* bytecode[2];
  bytecode[0] = &&VM__OP_1;
  bytecode[1] = &&VM__TERMINATE;

  int state = 0;
  int index = 0;

  while (1) {
    switch (state) {
    case 0:
      goto *bytecode[index];
    case 1:
      // NOTE: THIS IS ONLY REACHABLE VIA INDIRECT GOTOS
      VM__OP_1:
      state = 2;
      break;
    case 2:
      printf("OP_1:(instruction=%d)\n", index);
      index++;
      goto *bytecode[index];
    }
  }

VM__TERMINATE:
  printf("TERMINATE:(instruction=%d)\n", index);
  return 0;
}

#76295 makes the miscompile go away(https://godbolt.org/z/d9bGnhhv6) but I think simplifycfg still has the bug.

@efriedma-quic
Copy link
Collaborator

Looks like TryToSimplifyUncondBranchFromEmptyBlock is not handling indirectbr predecessors correctly: the indirectbr's successor list is rewritten so it has a successor that isn't address-taken. That edge is then proven impossible, so the indirectbr is completely eliminated.

@hiraditya hiraditya changed the title Miscompile in code with inderect gotos Miscompile in code with indirect gotos Aug 15, 2024
hiraditya added a commit that referenced this issue Sep 11, 2024
hiraditya added a commit that referenced this issue Nov 26, 2024
Remove check for PHI in pred as pointed out in #103688 
Reduced the testcase to remove redundant phi in pred

Fixes: #102351
tru pushed a commit to llvmbot/llvm-project that referenced this issue Dec 2, 2024
The bug was introduced by
llvm#68473

Fixes: llvm#102351
(cherry picked from commit 3c9022c)
tru pushed a commit to llvmbot/llvm-project that referenced this issue Dec 2, 2024
Remove check for PHI in pred as pointed out in llvm#103688
Reduced the testcase to remove redundant phi in pred

Fixes: llvm#102351
(cherry picked from commit 39601a6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants