[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://rev

[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 mailin

[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

[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 i

[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_macro6bridge3rpchINt

[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 uw

[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#LL291C

[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 tha

[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; + // C

[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 ACTIO

[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 ___ lld

[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 para

[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/function

[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

[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 A