[PATCH] D71960: [Attributor] AAUndefinedBehavior: AAValueSimplify in memory accessing instructions.

[Stefanos Baziotis via Phabricator via llvm-commits]

baziotis marked an inline comment as done.
baziotis added inline comments.


================
Comment at: llvm/test/Transforms/Attributor/ArgumentPromotion/2008-07-02-array-indexing.ll:12
-; CHECK-NEXT:    [[A_0:%.*]] = load i32, i32* null
-; CHECK-NEXT:    br i1 false, label [[T:%.*]], label [[F:%.*]]
 ; CHECK:       T:
----------------
uenoku wrote:
> baziotis wrote:
> > uenoku wrote:
> > > baziotis wrote:
> > > > jdoerfert wrote:
> > > > > We should also repair this test. pass a valid pointer into `callee`, e.g., an argument. 
> > > > Ok,yes. Btw, the `nonnull` and `dereferenceable` attributes for `%A` are wrong right? I guess these are the kind of checks for violations you have proposed that AAUB could do.
> > > > Btw, the nonnull and dereferenceable attributes for %A are wrong right? 
> > > I think to mark nonnull and dereferenceable attributes for %A itself is not wrong. 
> > > (but anyway %A will be deleted with D68765 or additional patches)
> > > I mean **passing** null to this function is UB too. So `AAUB` can prove 
> > > ```
> > > %X = call i32 @callee(i1 false, i32* null) 
> > > ```
> > > is UB. 
> > So, generally for attributes, I'm not sure when is the caller responsible for it to hold or the "marker" (or both).
> > What I mean is that the way I see it, in this example, there are 2 cases (in the general case, replace "parameter" with "value that can be attributed"):
> > 1) If the caller is responsible, then I can mark any attributes in the parameter (e.g. `nonnull`) and whoever calls this function is responsible for the attributes to hold (e.g. not pass a `null`). Otherwise, it is UB.
> > 2) If the marker is responsible, then whoever marks a parameter (e.g. `nonnull`)  is responsible to validate that anyone who
> > calls this function does not pass a `null` parameter.
> > 
> > The 1) makes more sense since 2) is practically impossible if the function is external. But 2) isn't really what the Attributor does?
> > (e.g. the Attributor tries to be sure that an argument is `dereferenceable` to mark it as such).
> > If the caller is responsible, then I can mark any attributes in the parameter (e.g. nonnull) and whoever calls this function is responsible for the attributes to hold (e.g. not pass a null). Otherwise, it is UB.
> > If the marker is responsible, then whoever marks a parameter (e.g. nonnull) is responsible to validate that anyone who calls this function does not pass a null parameter.
> > The 1) makes more sense since 2) is practically impossible if the function is external. But 2) isn't really what the Attributor does?
> Yes, almost all the attributes are using only 2) but some (ex. nonnull, align, dereferenceable) are doing deduction in both ways.  In this case, `%A` is known as `nonnull` because `%A` is loaded at an entry point. 
> 
> If the function is an external function, Attributor gives up 2).  (please look at `checkForAllCallsites` implementation)
Aha, got it,  thanks!


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

https://reviews.llvm.org/D71960