subject:"\[Lldb\-commits\] \[PATCH\] D133036\: \[InstCombine\] Treat passing undef to noundef params as UB"

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2023-02-21 Thread Tim Neumann via Phabricator via lldb-commits

TimNN added a comment.
Herald added a subscriber: StephenFan.

The ThinLTO related breakage I mentioned above should be fixed as of 
https://github.com/llvm/llvm-project/commit/451799bb8261bde52bbfef226d019caf1d82aa42.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133036/new/

https://reviews.llvm.org/D133036

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-09-22 Thread Arthur Eubanks via Phabricator via lldb-commits

aeubanks updated this revision to Diff 462245.
aeubanks added a comment.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

"fix" lldb test (hopefully)
skip when msan
add flag to disable (to be removed in the future)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133036/new/

https://reviews.llvm.org/D133036

Files:
  clang/test/CodeGenOpenCL/overload.cl
  lldb/test/API/functionalities/optimized_code/main.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/test/Transforms/InstCombine/call-undef.ll
  llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll

Index: llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
===
--- llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
+++ llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
@@ -6,7 +6,7 @@
 ; CHECK-LABEL: @test_out_of_bounds(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:[[AND1:%.*]] = and i32 [[A:%.*]], 3
-; CHECK-NEXT:tail call void @llvm.assume(i1 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret i32 [[AND1]]
 ;
 entry:
@@ -20,7 +20,7 @@
 define i128 @test_non64bit(i128 %a) {
 ; CHECK-LABEL: @test_non64bit(
 ; CHECK-NEXT:[[AND1:%.*]] = and i128 [[A:%.*]], 3
-; CHECK-NEXT:tail call void @llvm.assume(i1 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret i128 [[AND1]]
 ;
   %and1 = and i128 %a, 3
Index: llvm/test/Transforms/InstCombine/call-undef.ll
===
--- llvm/test/Transforms/InstCombine/call-undef.ll
+++ llvm/test/Transforms/InstCombine/call-undef.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+; RUN: opt < %s -passes=instcombine -S -instcombine-disable-optimize-passing-undef-ub | FileCheck %s --check-prefix=DISABLE
 
 declare void @c(i32 noundef)
 declare void @d(ptr dereferenceable(1))
@@ -8,8 +9,12 @@
 
 define void @test1() {
 ; CHECK-LABEL: @test1(
-; CHECK-NEXT:call void @c(i32 undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test1(
+; DISABLE-NEXT:call void @c(i32 undef)
+; DISABLE-NEXT:ret void
 ;
   call void @c(i32 undef)
   ret void
@@ -17,8 +22,12 @@
 
 define void @test2() {
 ; CHECK-LABEL: @test2(
-; CHECK-NEXT:call void @c(i32 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test2(
+; DISABLE-NEXT:call void @c(i32 poison)
+; DISABLE-NEXT:ret void
 ;
   call void @c(i32 poison)
   ret void
@@ -26,8 +35,12 @@
 
 define void @test3() {
 ; CHECK-LABEL: @test3(
-; CHECK-NEXT:call void @e(i32 noundef undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test3(
+; DISABLE-NEXT:call void @e(i32 noundef undef)
+; DISABLE-NEXT:ret void
 ;
   call void @e(i32 noundef undef)
   ret void
@@ -35,8 +48,12 @@
 
 define void @test4() {
 ; CHECK-LABEL: @test4(
-; CHECK-NEXT:call void @e(i32 noundef poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test4(
+; DISABLE-NEXT:call void @e(i32 noundef poison)
+; DISABLE-NEXT:ret void
 ;
   call void @e(i32 noundef poison)
   ret void
@@ -44,8 +61,12 @@
 
 define void @test5() {
 ; CHECK-LABEL: @test5(
-; CHECK-NEXT:call void @d(ptr undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test5(
+; DISABLE-NEXT:call void @d(ptr undef)
+; DISABLE-NEXT:ret void
 ;
   call void @d(ptr undef)
   ret void
@@ -53,8 +74,12 @@
 
 define void @test6() {
 ; CHECK-LABEL: @test6(
-; CHECK-NEXT:call void @d(ptr poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test6(
+; DISABLE-NEXT:call void @d(ptr poison)
+; DISABLE-NEXT:ret void
 ;
   call void @d(ptr poison)
   ret void
@@ -62,8 +87,12 @@
 
 define void @test7() {
 ; CHECK-LABEL: @test7(
-; CHECK-NEXT:call void @f(ptr dereferenceable(1) undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test7(
+; DISABLE-NEXT:call void @f(ptr dereferenceable(1) undef)
+; DISABLE-NEXT:ret void
 ;
   call void @f(ptr dereferenceable(1) undef)
   ret void
@@ -71,17 +100,38 @@
 
 define void @test8() {
 ; CHECK-LABEL: @test8(
-; CHECK-NEXT:call void @f(ptr dereferenceable(1) poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test8(
+; DISABLE-NEXT:call void @f(ptr dereferenceable(1) poison)
+; DISABLE-NEXT:ret void
 ;
   call void @f(ptr dereferenceable(1) poison)
   ret void
 }
 
+define void @test9() sanitize_memory {
+; CHECK-LABEL: @test9(

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-09-22 Thread Arthur Eubanks via Phabricator via lldb-commits

aeubanks updated this revision to Diff 462247.
aeubanks added a comment.

add flag to disable (to be removed in the future)
"fix" lldb test
skip on msan instrumented functions


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133036/new/

https://reviews.llvm.org/D133036

Files:
  clang/test/CodeGenOpenCL/overload.cl
  lldb/test/API/functionalities/optimized_code/main.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/test/Transforms/InstCombine/call-undef.ll
  llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll

Index: llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
===
--- llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
+++ llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
@@ -6,7 +6,7 @@
 ; CHECK-LABEL: @test_out_of_bounds(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:[[AND1:%.*]] = and i32 [[A:%.*]], 3
-; CHECK-NEXT:tail call void @llvm.assume(i1 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret i32 [[AND1]]
 ;
 entry:
@@ -20,7 +20,7 @@
 define i128 @test_non64bit(i128 %a) {
 ; CHECK-LABEL: @test_non64bit(
 ; CHECK-NEXT:[[AND1:%.*]] = and i128 [[A:%.*]], 3
-; CHECK-NEXT:tail call void @llvm.assume(i1 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret i128 [[AND1]]
 ;
   %and1 = and i128 %a, 3
Index: llvm/test/Transforms/InstCombine/call-undef.ll
===
--- llvm/test/Transforms/InstCombine/call-undef.ll
+++ llvm/test/Transforms/InstCombine/call-undef.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+; RUN: opt < %s -passes=instcombine -S -instcombine-disable-optimize-passing-undef-ub | FileCheck %s --check-prefix=DISABLE
 
 declare void @c(i32 noundef)
 declare void @d(ptr dereferenceable(1))
@@ -8,8 +9,12 @@
 
 define void @test1() {
 ; CHECK-LABEL: @test1(
-; CHECK-NEXT:call void @c(i32 undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test1(
+; DISABLE-NEXT:call void @c(i32 undef)
+; DISABLE-NEXT:ret void
 ;
   call void @c(i32 undef)
   ret void
@@ -17,8 +22,12 @@
 
 define void @test2() {
 ; CHECK-LABEL: @test2(
-; CHECK-NEXT:call void @c(i32 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test2(
+; DISABLE-NEXT:call void @c(i32 poison)
+; DISABLE-NEXT:ret void
 ;
   call void @c(i32 poison)
   ret void
@@ -26,8 +35,12 @@
 
 define void @test3() {
 ; CHECK-LABEL: @test3(
-; CHECK-NEXT:call void @e(i32 noundef undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test3(
+; DISABLE-NEXT:call void @e(i32 noundef undef)
+; DISABLE-NEXT:ret void
 ;
   call void @e(i32 noundef undef)
   ret void
@@ -35,8 +48,12 @@
 
 define void @test4() {
 ; CHECK-LABEL: @test4(
-; CHECK-NEXT:call void @e(i32 noundef poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test4(
+; DISABLE-NEXT:call void @e(i32 noundef poison)
+; DISABLE-NEXT:ret void
 ;
   call void @e(i32 noundef poison)
   ret void
@@ -44,8 +61,12 @@
 
 define void @test5() {
 ; CHECK-LABEL: @test5(
-; CHECK-NEXT:call void @d(ptr undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test5(
+; DISABLE-NEXT:call void @d(ptr undef)
+; DISABLE-NEXT:ret void
 ;
   call void @d(ptr undef)
   ret void
@@ -53,8 +74,12 @@
 
 define void @test6() {
 ; CHECK-LABEL: @test6(
-; CHECK-NEXT:call void @d(ptr poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test6(
+; DISABLE-NEXT:call void @d(ptr poison)
+; DISABLE-NEXT:ret void
 ;
   call void @d(ptr poison)
   ret void
@@ -62,8 +87,12 @@
 
 define void @test7() {
 ; CHECK-LABEL: @test7(
-; CHECK-NEXT:call void @f(ptr dereferenceable(1) undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test7(
+; DISABLE-NEXT:call void @f(ptr dereferenceable(1) undef)
+; DISABLE-NEXT:ret void
 ;
   call void @f(ptr dereferenceable(1) undef)
   ret void
@@ -71,17 +100,38 @@
 
 define void @test8() {
 ; CHECK-LABEL: @test8(
-; CHECK-NEXT:call void @f(ptr dereferenceable(1) poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test8(
+; DISABLE-NEXT:call void @f(ptr dereferenceable(1) poison)
+; DISABLE-NEXT:ret void
 ;
   call void @f(ptr dereferenceable(1) poison)
   ret void
 }
 
+define void @test9() sanitize_memory {
+; CHECK-LABEL: @test9(
+; CHECK-NEXT:call void @c(i32 undef)
+; CHECK-NEXT:r

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-09-22 Thread Arthur Eubanks via Phabricator via lldb-commits

aeubanks updated this revision to Diff 462249.
aeubanks added a comment.

extra parens


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133036/new/

https://reviews.llvm.org/D133036

Files:
  clang/test/CodeGenOpenCL/overload.cl
  lldb/test/API/functionalities/optimized_code/main.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/test/Transforms/InstCombine/call-undef.ll
  llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll

Index: llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
===
--- llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
+++ llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
@@ -6,7 +6,7 @@
 ; CHECK-LABEL: @test_out_of_bounds(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:[[AND1:%.*]] = and i32 [[A:%.*]], 3
-; CHECK-NEXT:tail call void @llvm.assume(i1 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret i32 [[AND1]]
 ;
 entry:
@@ -20,7 +20,7 @@
 define i128 @test_non64bit(i128 %a) {
 ; CHECK-LABEL: @test_non64bit(
 ; CHECK-NEXT:[[AND1:%.*]] = and i128 [[A:%.*]], 3
-; CHECK-NEXT:tail call void @llvm.assume(i1 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret i128 [[AND1]]
 ;
   %and1 = and i128 %a, 3
Index: llvm/test/Transforms/InstCombine/call-undef.ll
===
--- llvm/test/Transforms/InstCombine/call-undef.ll
+++ llvm/test/Transforms/InstCombine/call-undef.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+; RUN: opt < %s -passes=instcombine -S -instcombine-disable-optimize-passing-undef-ub | FileCheck %s --check-prefix=DISABLE
 
 declare void @c(i32 noundef)
 declare void @d(ptr dereferenceable(1))
@@ -8,8 +9,12 @@
 
 define void @test1() {
 ; CHECK-LABEL: @test1(
-; CHECK-NEXT:call void @c(i32 undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test1(
+; DISABLE-NEXT:call void @c(i32 undef)
+; DISABLE-NEXT:ret void
 ;
   call void @c(i32 undef)
   ret void
@@ -17,8 +22,12 @@
 
 define void @test2() {
 ; CHECK-LABEL: @test2(
-; CHECK-NEXT:call void @c(i32 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test2(
+; DISABLE-NEXT:call void @c(i32 poison)
+; DISABLE-NEXT:ret void
 ;
   call void @c(i32 poison)
   ret void
@@ -26,8 +35,12 @@
 
 define void @test3() {
 ; CHECK-LABEL: @test3(
-; CHECK-NEXT:call void @e(i32 noundef undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test3(
+; DISABLE-NEXT:call void @e(i32 noundef undef)
+; DISABLE-NEXT:ret void
 ;
   call void @e(i32 noundef undef)
   ret void
@@ -35,8 +48,12 @@
 
 define void @test4() {
 ; CHECK-LABEL: @test4(
-; CHECK-NEXT:call void @e(i32 noundef poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test4(
+; DISABLE-NEXT:call void @e(i32 noundef poison)
+; DISABLE-NEXT:ret void
 ;
   call void @e(i32 noundef poison)
   ret void
@@ -44,8 +61,12 @@
 
 define void @test5() {
 ; CHECK-LABEL: @test5(
-; CHECK-NEXT:call void @d(ptr undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test5(
+; DISABLE-NEXT:call void @d(ptr undef)
+; DISABLE-NEXT:ret void
 ;
   call void @d(ptr undef)
   ret void
@@ -53,8 +74,12 @@
 
 define void @test6() {
 ; CHECK-LABEL: @test6(
-; CHECK-NEXT:call void @d(ptr poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test6(
+; DISABLE-NEXT:call void @d(ptr poison)
+; DISABLE-NEXT:ret void
 ;
   call void @d(ptr poison)
   ret void
@@ -62,8 +87,12 @@
 
 define void @test7() {
 ; CHECK-LABEL: @test7(
-; CHECK-NEXT:call void @f(ptr dereferenceable(1) undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test7(
+; DISABLE-NEXT:call void @f(ptr dereferenceable(1) undef)
+; DISABLE-NEXT:ret void
 ;
   call void @f(ptr dereferenceable(1) undef)
   ret void
@@ -71,17 +100,38 @@
 
 define void @test8() {
 ; CHECK-LABEL: @test8(
-; CHECK-NEXT:call void @f(ptr dereferenceable(1) poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test8(
+; DISABLE-NEXT:call void @f(ptr dereferenceable(1) poison)
+; DISABLE-NEXT:ret void
 ;
   call void @f(ptr dereferenceable(1) poison)
   ret void
 }
 
+define void @test9() sanitize_memory {
+; CHECK-LABEL: @test9(
+; CHECK-NEXT:call void @c(i32 undef)
+; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test9(
+; DISABLE-NEXT:call void @c(i32 undef)
+; DISAB

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-09-22 Thread Nikita Popov via Phabricator via lldb-commits

nikic requested changes to this revision.
nikic added a comment.
This revision now requires changes to proceed.

I'd like to block this on `-fsanitize-memory-param-retval` (aka msan eager 
checks) being enabled by default. It seems pretty clear that there is a *lot* 
of UB due to uninitialized parameters floating around, so we should at least 
make sure that it is detected in default sanitizer configurations before we 
start exploiting it this aggressively.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133036/new/

https://reviews.llvm.org/D133036

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-09-26 Thread Arthur Eubanks via Phabricator via lldb-commits

aeubanks added a comment.

@vitalybuka can we turn on -fsanitize-memory-param-retval by default upstream?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133036/new/

https://reviews.llvm.org/D133036

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-09-26 Thread Arthur Eubanks via Phabricator via lldb-commits

aeubanks added a comment.

In D133036#3816002 , @aeubanks wrote:

> @vitalybuka can we turn on -fsanitize-memory-param-retval by default upstream?

attempt in https://reviews.llvm.org/D134669


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133036/new/

https://reviews.llvm.org/D133036

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-10-15 Thread Nikita Popov via Phabricator via lldb-commits

nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.

The default switch has happened, so unblocking this.




Comment at: clang/test/CodeGenOpenCL/overload.cl:23
   generic int *local *genloc;
   generic int *global *genglob;
+  // CHECK-DAG: call spir_func void @_Z3fooPU3AS1iS0_(i32 addrspace(1)* 
noundef {{.*}}, i32 addrspace(1)* noundef {{.*}})

Maybe initialize the relevant variables instead? I'd assume just NULL would 
work here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133036/new/

https://reviews.llvm.org/D133036

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-11-12 Thread Tim Neumann via Phabricator via lldb-commits

TimNN added a comment.

I'm still trying to properly minimize this, but this definitely interacts badly 
with other optimizations (which triggers the Rust CI failure I mentioned above):

- We start with two functions, `outer` and `inner`. `outer` calls `inner`. 
`outer` has a `noundef` argument that it forwards to `inner` (where the 
argument is also `noundef`).
- `PostOrderFunctionAttrsPass` decides, for both functions, that the argument 
is `readnone`. The `readnone` attribute is //only// added to the function 
definitions. The `readnone` attribute is //not// added at the `call` to `inner` 
from `outer`.
- The `DeadArgumentEliminationPass` decides that when `outer` calls `inner` the 
argument should be `poison`.
- This patch sees the `poison` being passed to a `noundef` argument and kills 
the call.

I don't know which component should be considered "at fault" here (or if the 
`readnone` bit is even relevant).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133036/new/

https://reviews.llvm.org/D133036

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-11-12 Thread Nikita Popov via Phabricator via lldb-commits

nikic added a comment.

@TimNN Do you have an IR sample for this? DAE is supposed to strip UB-implying 
attributes when converting arguments to poison here: 
https://github.com/llvm/llvm-project/blob/cbe5b2dd914b7ee47bb4d0f67af154a40be4d208/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp#LL291C17-L291C37


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133036/new/

https://reviews.llvm.org/D133036

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-11-12 Thread Tim Neumann via Phabricator via lldb-commits

TimNN added a comment.

I've included excerpts from the IR below. It will take me a bit to provide 
something compilable. Though you are right, the `noundef` did indeed get 
removed from the `call`.

  *** IR Dump Before DeadArgumentEliminationPass on [module] ***
  ; Function Attrs: nonlazybind uwtable
  define void 
@_RNvXs2_NtNtCs840rfDNPFol_10proc_macro6bridge3rpcbINtB5_6EncodeuE6encodeB9_(i1 
noundef zeroext %0, ptr noalias noundef align 8 dereferenceable(40) %1, ptr 
noalias nocapture noundef nonnull readnone align 1 %2) unnamed_addr #0 !dbg 
!18135 {
%4 = zext i1 %0 to i8, !dbg !18137
call void 
@_RNvXs0_NtNtCs840rfDNPFol_10proc_macro6bridge3rpchINtB5_6EncodeuE6encodeB9_(i8 
%4, ptr noalias noundef nonnull align 8 dereferenceable(40) %1, ptr noalias 
noundef nonnull align 1 %2), !dbg !18137
ret void, !dbg !18138
  }
  *** IR Dump After DeadArgumentEliminationPass on [module] ***
  ; Function Attrs: nonlazybind uwtable
  define void 
@_RNvXs2_NtNtCs840rfDNPFol_10proc_macro6bridge3rpcbINtB5_6EncodeuE6encodeB9_(i1 
noundef zeroext %0, ptr noalias noundef align 8 dereferenceable(40) %1, ptr 
noalias nocapture noundef nonnull readnone align 1 %2) unnamed_addr #0 !dbg 
!18136 {
%4 = zext i1 %0 to i8, !dbg !18138
call void 
@_RNvXs0_NtNtCs840rfDNPFol_10proc_macro6bridge3rpchINtB5_6EncodeuE6encodeB9_(i8 
%4, ptr noalias noundef nonnull align 8 dereferenceable(40) %1, ptr noalias 
nonnull align 1 poison), !dbg !18138
ret void, !dbg !18139
  }
  *** IR Dump Before InstCombinePass on 
_RNvXs2_NtNtCs840rfDNPFol_10proc_macro6bridge3rpcbINtB5_6EncodeuE6encodeB9_ ***
  ; Function Attrs: nonlazybind uwtable
  define available_externally void 
@_RNvXs2_NtNtCs840rfDNPFol_10proc_macro6bridge3rpcbINtB5_6EncodeuE6encodeB9_(i1 
noundef zeroext %0, ptr noalias noundef align 8 dereferenceable(40) %1, ptr 
noalias nocapture noundef nonnull readnone align 1 %2) unnamed_addr #2 !dbg 
!20759 {
%4 = zext i1 %0 to i8, !dbg !20762
call void 
@_RNvXs0_NtNtCs840rfDNPFol_10proc_macro6bridge3rpchINtB5_6EncodeuE6encodeB9_(i8 
%4, ptr noalias noundef nonnull align 8 dereferenceable(40) %1, ptr noalias 
nonnull align 1 poison), !dbg !20762
ret void, !dbg !20763
  }
  *** IR Dump After InstCombinePass on 
_RNvXs2_NtNtCs840rfDNPFol_10proc_macro6bridge3rpcbINtB5_6EncodeuE6encodeB9_ ***
  ; Function Attrs: nonlazybind uwtable
  define available_externally void 
@_RNvXs2_NtNtCs840rfDNPFol_10proc_macro6bridge3rpcbINtB5_6EncodeuE6encodeB9_(i1 
noundef zeroext %0, ptr noalias noundef align 8 dereferenceable(40) %1, ptr 
noalias nocapture noundef nonnull readnone align 1 %2) unnamed_addr #2 !dbg 
!20759 {
store i1 true, ptr poison, align 1
ret void, !dbg !20762
  }


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133036/new/

https://reviews.llvm.org/D133036

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-11-12 Thread Tim Neumann via Phabricator via lldb-commits

TimNN added a comment.

I didn't manage to repro with `opt`, so still no compilable IR. I did some more 
debugging, though:

- Inside `removeDeadArgumentsFromCallers`, `CB->getCalledFunction()->dump()` 
(after the modification) is `define void 
@_RNvXs0_NtNtCs840rfDNPFol_10proc_macro6bridge3rpchINtB5_6EncodeuE6encodeB9_(i8 
%0, ptr noalias noundef align 8 dereferenceable(40) %1, ptr noalias nocapture 
nonnull readnone align 1 %2) unnamed_addr #0 personality ptr 
@rust_eh_personality !dbg !18034 { ... }`. **Definition, no `noundef`.**
- Inside `callPassesUndefToPassingUndefUBParam`, 
`Call.getCalledFunction()->dump()` is `declare void 
@_RNvXs0_NtNtCs840rfDNPFol_10proc_macro6bridge3rpchINtB5_6EncodeuE6encodeB9_(i8 
%0, ptr noalias noundef align 8 dereferenceable(40) %1, ptr noalias noundef 
nonnull align 1 %2) unnamed_addr #2`. **Declaration, `noundef` is back.**


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133036/new/

https://reviews.llvm.org/D133036

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-11-13 Thread Tim Neumann via Phabricator via lldb-commits

TimNN added a comment.

I'm sorry for the noise. Further investigation has shown that this happens when 
Rust is doing (thin) LTO, and I don't think this patch can be considered in any 
way "at fault" here, so this is the last you'll hear from me on the topic here.

I don't know whether the fault is with how Rust implements (thin) LTO or 
something on the LLVM side. Basically, after running the `FunctionImporter` on 
a module, we end up with a `define available_externally void outer` and a 
`declare void inner`, presumably coming from different modules. `outer` 
contains the call to `inner` with `poison`, but the parameter is `noundef` on 
the declaration on `inner`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133036/new/

https://reviews.llvm.org/D133036

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-11-13 Thread Arthur Eubanks via Phabricator via lldb-commits

aeubanks updated this revision to Diff 475017.
aeubanks added a comment.

I somehow missed the previous comments

rebased on top of fixing UB in tests changes


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133036/new/

https://reviews.llvm.org/D133036

Files:
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/test/Transforms/InstCombine/call-undef.ll
  llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll

Index: llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
===
--- llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
+++ llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
@@ -6,7 +6,7 @@
 ; CHECK-LABEL: @test_out_of_bounds(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:[[AND1:%.*]] = and i32 [[A:%.*]], 3
-; CHECK-NEXT:tail call void @llvm.assume(i1 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret i32 [[AND1]]
 ;
 entry:
@@ -20,7 +20,7 @@
 define i128 @test_non64bit(i128 %a) {
 ; CHECK-LABEL: @test_non64bit(
 ; CHECK-NEXT:[[AND1:%.*]] = and i128 [[A:%.*]], 3
-; CHECK-NEXT:tail call void @llvm.assume(i1 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret i128 [[AND1]]
 ;
   %and1 = and i128 %a, 3
Index: llvm/test/Transforms/InstCombine/call-undef.ll
===
--- llvm/test/Transforms/InstCombine/call-undef.ll
+++ llvm/test/Transforms/InstCombine/call-undef.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+; RUN: opt < %s -passes=instcombine -S -instcombine-disable-optimize-passing-undef-ub | FileCheck %s --check-prefix=DISABLE
 
 declare void @c(i32 noundef)
 declare void @d(ptr dereferenceable(1))
@@ -8,8 +9,12 @@
 
 define void @test1() {
 ; CHECK-LABEL: @test1(
-; CHECK-NEXT:call void @c(i32 undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test1(
+; DISABLE-NEXT:call void @c(i32 undef)
+; DISABLE-NEXT:ret void
 ;
   call void @c(i32 undef)
   ret void
@@ -17,8 +22,12 @@
 
 define void @test2() {
 ; CHECK-LABEL: @test2(
-; CHECK-NEXT:call void @c(i32 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test2(
+; DISABLE-NEXT:call void @c(i32 poison)
+; DISABLE-NEXT:ret void
 ;
   call void @c(i32 poison)
   ret void
@@ -26,8 +35,12 @@
 
 define void @test3() {
 ; CHECK-LABEL: @test3(
-; CHECK-NEXT:call void @e(i32 noundef undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test3(
+; DISABLE-NEXT:call void @e(i32 noundef undef)
+; DISABLE-NEXT:ret void
 ;
   call void @e(i32 noundef undef)
   ret void
@@ -35,8 +48,12 @@
 
 define void @test4() {
 ; CHECK-LABEL: @test4(
-; CHECK-NEXT:call void @e(i32 noundef poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test4(
+; DISABLE-NEXT:call void @e(i32 noundef poison)
+; DISABLE-NEXT:ret void
 ;
   call void @e(i32 noundef poison)
   ret void
@@ -44,8 +61,12 @@
 
 define void @test5() {
 ; CHECK-LABEL: @test5(
-; CHECK-NEXT:call void @d(ptr undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test5(
+; DISABLE-NEXT:call void @d(ptr undef)
+; DISABLE-NEXT:ret void
 ;
   call void @d(ptr undef)
   ret void
@@ -53,8 +74,12 @@
 
 define void @test6() {
 ; CHECK-LABEL: @test6(
-; CHECK-NEXT:call void @d(ptr poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test6(
+; DISABLE-NEXT:call void @d(ptr poison)
+; DISABLE-NEXT:ret void
 ;
   call void @d(ptr poison)
   ret void
@@ -62,8 +87,12 @@
 
 define void @test7() {
 ; CHECK-LABEL: @test7(
-; CHECK-NEXT:call void @f(ptr dereferenceable(1) undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test7(
+; DISABLE-NEXT:call void @f(ptr dereferenceable(1) undef)
+; DISABLE-NEXT:ret void
 ;
   call void @f(ptr dereferenceable(1) undef)
   ret void
@@ -71,17 +100,38 @@
 
 define void @test8() {
 ; CHECK-LABEL: @test8(
-; CHECK-NEXT:call void @f(ptr dereferenceable(1) poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test8(
+; DISABLE-NEXT:call void @f(ptr dereferenceable(1) poison)
+; DISABLE-NEXT:ret void
 ;
   call void @f(ptr dereferenceable(1) poison)
   ret void
 }
 
+define void @test9() sanitize_memory {
+; CHECK-LABEL: @test9(
+; CHECK-NEXT:call void @c(i32 undef)
+; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test9(
+; DISABLE-NEXT:call void @c(i32 undef)
+; DISABLE-NEXT:ret void
+;

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-11-14 Thread Arthur Eubanks via Phabricator via lldb-commits

aeubanks added a comment.

I'll hold off on submitting this until that bug is figured out


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133036/new/

https://reviews.llvm.org/D133036

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

15 matches

Site Navigation

Mail list logo

Footer information