Skip to content

[GVN] Use after free during load PRE #69301

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

Closed
nikic opened this issue Oct 17, 2023 · 8 comments · Fixed by #69314 or llvm/llvm-project-release-prs#758
Closed

[GVN] Use after free during load PRE #69301

nikic opened this issue Oct 17, 2023 · 8 comments · Fixed by #69314 or llvm/llvm-project-release-prs#758
Assignees
Labels
llvm:crash llvm:GVN GVN and NewGVN stages (Global value numbering) release:backport release:merged

Comments

@nikic
Copy link
Contributor

nikic commented Oct 17, 2023

; RUN: opt -S -passes=gvn < %s
define i64 @test(i1 %c, ptr %p) {
entry:
  br label %loop

loop:
  %iv = phi i64 [ 0, %entry ], [ %add, %loop.latch ]
  %ptr.iv = phi ptr [ %p, %entry ], [ %select, %loop.latch ]
  %icmp = icmp eq i64 %iv, 0
  br i1 %icmp, label %exit, label %loop.cont

loop.cont:
  %add = add i64 %iv, -1
  br i1 %c, label %exit, label %loop.latch

loop.latch:
  %load = load i64, ptr %ptr.iv, align 8
  %load6 = load i64, ptr null, align 8
  %icmp7 = icmp ugt i64 %load, %load6
  %select = select i1 %icmp7, ptr %ptr.iv, ptr null
  br label %loop

exit:
  %res = load i64, ptr %ptr.iv, align 8
  ret i64 %res
}

Produces a use after free.

@nikic
Copy link
Contributor Author

nikic commented Oct 17, 2023

I believe the problem is that replaceValuesPerBlockEntry() doesn't handle select values.

nikic added a commit to nikic/llvm-project that referenced this issue Oct 17, 2023
replaceValuesPerBlockEntry() only handled simple and coerced load
values, however the load may also be referenced by a select value.

Additionally, I suspect that the previous code might have been
incorrect if a load had an offset, as it always constructed the
AvailableValue from scratch.

Fixes llvm#69301.
@EugeneZelenko EugeneZelenko added the llvm:GVN GVN and NewGVN stages (Global value numbering) label Oct 17, 2023
@nikic nikic reopened this Oct 19, 2023
@nikic
Copy link
Contributor Author

nikic commented Oct 19, 2023

/cherry-pick 7f1733a

@llvmbot
Copy link
Member

llvmbot commented Oct 19, 2023

/branch llvm/llvm-project-release-prs/issue69301

llvmbot pushed a commit to llvm/llvm-project-release-prs that referenced this issue Oct 19, 2023
…314)

replaceValuesPerBlockEntry() only handled simple and coerced load
values, however the load may also be referenced by a select value.

Additionally, I suspect that the previous code might have been incorrect
if a load had an offset, as it always constructed the AvailableValue
from scratch.

Fixes llvm/llvm-project#69301.

(cherry picked from commit 7f1733a252cbbad74445bd54dc95aeec52bb3199)
@llvmbot
Copy link
Member

llvmbot commented Oct 19, 2023

/pull-request llvm/llvm-project-release-prs#742

@nikic nikic moved this from Needs Triage to Needs Review in LLVM Release Status Oct 19, 2023
@owenca owenca closed this as completed Oct 19, 2023
@nikic nikic reopened this Oct 19, 2023
@nikic
Copy link
Contributor Author

nikic commented Oct 19, 2023

@owenca Please, can you fix whatever is causing your account to spuriously close issues?

@owenca
Copy link
Contributor

owenca commented Oct 19, 2023

Sorry about that! I really don't understand why your issue got closed when I pushed to another repo. I'll stop pushing to it until I can figure it out.

nikic added a commit to nikic/llvm-project that referenced this issue Oct 30, 2023
…m#69314)

replaceValuesPerBlockEntry() only handled simple and coerced load
values, however the load may also be referenced by a select value.

Additionally, I suspect that the previous code might have been incorrect
if a load had an offset, as it always constructed the AvailableValue
from scratch.

Fixes llvm#69301.

(cherry picked from commit 7f1733a)
@nikic
Copy link
Contributor Author

nikic commented Oct 30, 2023

/branch nikic/llvm-project/gvn-backport

@llvmbot
Copy link
Member

llvmbot commented Oct 30, 2023

/pull-request llvm/llvm-project-release-prs#758

tru pushed a commit to llvm/llvm-project-release-prs that referenced this issue Oct 31, 2023
…314)

replaceValuesPerBlockEntry() only handled simple and coerced load
values, however the load may also be referenced by a select value.

Additionally, I suspect that the previous code might have been incorrect
if a load had an offset, as it always constructed the AvailableValue
from scratch.

Fixes llvm/llvm-project#69301.

(cherry picked from commit 7f1733a252cbbad74445bd54dc95aeec52bb3199)
@tru tru moved this from Needs Review to Done in LLVM Release Status Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment