[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

This revision was automatically updated to reflect the committed changes. Closed by commit rG3db6783d8a7d: Check result of emitStrLen before passing it to CreateGEP (authored by dim). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70143/new/ https://

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

jdoerfert added a comment. thx. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70143/new/ https://reviews.llvm.org/D70143 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70143/new/ https://reviews.llvm.org/D70143 ___

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

dim updated this revision to Diff 229172. dim added a comment. Now `opt` supports `-disable-builtin`, move the test to `llvm/test/Transforms/InstCombine`. Also use code style of nearest preceding constructs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

dim added a comment. I submitted D70193 for adding a `-disable-builtin` option to `opt`. Once that is committed, this review can continue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70143/new/ https://reviews

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

jdoerfert added inline comments. Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:370 +: nullptr; +} return nullptr; dim wrote: > jdoerfert wrote: > > Consistent style please: > > > > ``` > > if (Value *StrLen = emitStrLen(

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

dim marked an inline comment as done. dim added inline comments. Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:370 +: nullptr; +} return nullptr; jdoerfert wrote: > Consistent style please: > > ``` > if (Value *StrLen =

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

jdoerfert added a comment. One style comment, patch looks conceptually fine to me otherwise. I'll wait to accept on how we fall on the test issue: I'd opt for an `opt` test if possible. Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:370 +: nullp

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

efriedma added a comment. It wouldn't be that hard to add an equivalent flag to opt; just a matter of calling the API on TargetLibraryInfo. -disable-simplify-libcalls already exists, but I guess that's not quite what you want. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

dim added a comment. In D70143#1742885 , @lebedev.ri wrote: > This should have a llvm ir test in `llvm/test/transforms/instcombine` i > think, not a clang test. Yes, I thought so too, but I could not get it to work (i.e. crash) with LLVM IR. I just do

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

lebedev.ri added a comment. This should have a llvm ir test in `llvm/test/transforms/instcombine` i think, not a clang test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70143/new/ https://reviews.llvm.org/D70143 __

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

dim created this revision. dim added reviewers: xbolva00, spatel, jdoerfert, efriedma. Herald added subscribers: cfe-commits, hiraditya. Herald added projects: clang, LLVM. This fixes PR43081, where the transformation of `strchr(p, 0) -> p + strlen(p)` can cause a segfault, if `-fno-builtin-strlen