[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:
> > 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).


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

https://reviews.llvm.org/D71960