[PATCH] D108485: [DSE] Check post-dominance for malloc+memset->calloc transform.

Sat Aug 21 02:56:42 PDT 2021

yurai007 added inline comments.

================
Comment at: llvm/test/Transforms/DeadStoreElimination/noop-stores.ll:415
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i8* [[CALL]], null
 ; CHECK-NEXT:    br i1 [[CMP]], label [[CLEANUP:%.*]], label [[IF_END:%.*]]
 ; CHECK:       if.end:
----------------
xbolva00 wrote:
> asbirlea wrote:
> > xbolva00 wrote:
> > > I would rather see better fix, so this motivating case still works (allow 'ptr compared with NULL'). Othewise I see no value from this optimization at all and it should be just removed from DSE.
> > > 
> > > I am somewhat scared that people talking about this pattern
> > > ```
> > > memset(malloc(size), 0, size)
> > > ```
> > > 
> > > not suprised that why there are so many nasty bugs/exploits in C codebases.
> > > I would rather see better fix, so this motivating case still works (allow 'ptr compared with NULL'). Othewise I see no value from this optimization at all and it should be just removed from DSE.
> > > 
> > Sure, revert is a viable option while looking to add the 'ptr compared with NULL' check, as that's not so straightforward.
> But probably we should just take your patch, to avoid regression as SLC's transformation was removed as DSE-based one was implemented.
> 'ptr compared with NULL' check, as that's not so straightforward.
 Yes, it requires some dance with matching like here (old patch doing transformation in SLC): https://reviews.llvm.org/D101176

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108485