[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

Nuno Lopes via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 18 14:34:24 PST 2022


nlopes added inline comments.


================
Comment at: clang/lib/CodeGen/CGCall.cpp:2535
         ArgAttrs[FirstIRArg + i] =
             llvm::AttributeSet::get(getLLVMContext(), Attrs);
     }
----------------
ab wrote:
> Hmm, if I'm reading this right, this overwrites the `nonnull dereferenceable align` attributes separately computed for `this` around l2335, right? (or `inalloca` and `sret` before that)
> 
> It sounds like an ancient bug, that's only exposed because `noundef` ends up triggering this logic much more often?
> 
> Many of our downstream tests hit this. The hacked up patch seems to do the job; ideally we'd feed the previously-computed attrs when constructing the AttrBuilder (which would also fix the padding argument), but we'd need to match up the IR args first.  Maybe that's fine as a special-case for arg 0 though
nice catch! It's an ancient bug where arg 0 is overwritten.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169



More information about the cfe-commits mailing list