[llvm-dev] The semantics of nonnull attribute

Tue Feb 18 07:06:09 PST 2020

Hi Nuno,

(Disclaimer: This is before my first coffee)

On 02/18, Nuno Lopes wrote:
[ Moved from the end of Nuno's email]
> Maybe I misunderstood the semantics of used, but if not I hope the
> example above is sufficient to show it isn't sufficient.

I think we somehow talk past each other :(. It seems we actually agree
on the semantics of "used" but the meaning/use of attributes seems to be
the problem. I tried to explain where I am coming from below.

> Not sure the semantics of "used" you propose is sufficient. AFAIU the
> proposal, "used" could only be used in cases where the function will
> always trigger UB if poison is passed as argument.  The semantics of
> attributes is usually the other way around, since function calls need
> to have UB as strong as the worst behavior of the function. If a
> function may for some reason trigger UB with a given set of arguments,
> then the function call has to trigger UB as well.

"Used", as other attributes are "best effort". We can drop them any
time, so having "insufficient information" is not a correctness problem
but an optimization problem. I mean, even if the function always causes
UB if a `null` argument is given we do not need to know/annotate/exploit
that. In fact, we can, and do, throw away that information, e.g., by
removing the instruction known to cause UB which effectively defines
some semantic for the UB program (which is then "w/o UB"). Long story
short, I don't think "as strong as the worst behavior" is plausible (nor
possible TBH).

> Imagine something like:
> Call f(null, 1)
> 
> int f(nonnull %ptr, int c) {
>   if (c)
>     deref %ptr
> }
> 
> You can't use the "used" attribute, since UB is only conditional.

Agreed, for the callee (f) you cannot annotate "used". You can place
"used" on the call site though*. And you can place it on the callee (f)
if it was internal or you copy it to make it internal. So the
optimization potential is still there, even if it is a little trickier.

* FWIW, one-level of selective context sensitive propagaton is planned
  for the Attributor.

> we would say the return value of f(null, 1) is poison.  But if you
> inline the function you get UB. Inlining has to continue to be correct
> without exceptions.

The attributes will just never represent the possible UB exhaustively.
I mean, inlining is still correct and the UB that it will expose,
together with SCCP, was there all along. With one-level of context
sensitivity we could have even known, without we just didn't know but
that is fine too.

> ...
[ moved to the front ]

I hope this makes some sense.

Cheers,
  Johannes

P.S.
  I think I'll write "used" deduction later so we can take a look at how
  this would look like.

> Thanks,
> Nuno
> 
> 
> -----Original Message-----
> From: Johannes Doerfert <jdoerfert at anl.gov> 
> Sent: 18 de fevereiro de 2020 02:35
> Subject: Re: [llvm-dev] The semantics of nonnull attribute
> 
> Hi Juneyoung,
> 
> thank you for starting this discussion again, the last one [0] did not
> really end in a solution but the issue remains and will only become
> worse, e.g., enable the Attributor and you might see all of the problems
> below exposed in a single pass [1-4].
> 
> As I argued in [0], UB is problematic and I am strongly in favor of
> poison. In some sense I see UB as a very "strong" guarantee and poison
> as a fairly "week" one. We will have a hard time deriving the strong one
> and it might cause us to break things along the way, I mean your
> examples show that we already miscompile codes.
> 
> I agree that poison is on its own not "strong enough" for certain
> optimization, as you pointed out below, but we could reasonably augment
> it with another attribute, let's call it `used`, that basically bridges
> the gap:
> 
>   void @foo(nonnull);
>   void @bar(nonnull used);
> 
>   foo(NULL)  // <- argument is poison inside of @foo
>   bar(NULL)  // <- UB at the call site of @bar
> 
> A value is `used` if it would inevitably cause UB if it was poison at
> this program point. I hope this makes some sense if not I can explain in
> more detail.
> 
> Btw. I think this also integrates well with other attribute problems we
> had in the past, e.g., memcpy(NULL, NULL, 0), since the pointers in this
> example were "not used" (in some naive implementation of memcpy at
> least).
> 
> Maybe I have overlooked a problem with this solution so let me know what
> you think.
> 
> Cheers,
>   Johannes
> 
> 
> [0] https://reviews.llvm.org/D70749
> [1] 1. below: https://godbolt.org/z/J4wFVw
> [2] 2. below: https://godbolt.org/z/TAkSC6
> [3] 3. below: https://godbolt.org/z/UqYi6S (doesn't work yet, will though)
> [4] 4. below: https://godbolt.org/z/Qkkc_E
>               https://godbolt.org/z/uXAfp_
>               ...
> 
> On 02/18, Juneyoung Lee via llvm-dev wrote:
> > Hello all,
> > 
> > LangRef says it is undefined behavior to pass null to a nonnull argument
> > (`call f(nonnull null);`), but the semantics is too strong for a few
> > existing optimizations.
> > To support these, we can relax the semantics so `f(nonnull null)` is
> > equivalent to `f(poison)`, but (A) it again blocks another set of
> > optimizations, and (B) this makes the semantics of nonnull deviate from
> > other attributes like dereferenceable.
> > I found that there was a similar discussion about this issue in the past as
> > well, but seems it is not settled yet.
> > What should the semantics of nonnull be?
> > I listed a few optimizations that are relevant with this issue.
> > 
> > 
> > 1. Propagating nonnull attribute to callee's arg (
> > https://godbolt.org/z/-cVsVP )
> > 
> > g(i8* ptr) {
> > f(nonnull ptr);
> > }
> > =>
> > g(i8* nonnull ptr) {
> > f(nonnull ptr);
> > }
> > 
> > This is correct if f(nonnull null) is UB. If ptr == null, f(nonnull null)
> > should have raised UB, so ptr shouldn't be null.
> > However, this transformation is incorrect if f(nonnull null) is equivalent
> > to f(poison).
> > If f was an empty function, f(nonnull null) never raises UB regardless of
> > ptr. So we can't guarantee ptr != null at other uses of ptr.
> 
> For the given code snipped it is not even invalid to propagate nonnull
> in the poison case but your point is not wrong, if we have f(poison) we
> cannot derive anything from it if the argument might be unused.
> 
> 
> > 2. InstCombine (https://godbolt.org/z/HDQ7rD ):
> > 
> > %ptr_inb = gep inbounds %any_ptr, 1
> > f(%ptr_inb)
> > =>
> > %ptr_inb = .. (identical)
> > f(nonnull %ptr_inb)
> > 
> > This optimization is incorrect if `f(nonnull null)` is UB. The reason is as
> > follows.
> > If `gep inbounds %any_ptr, 1` yields poison, the source is `f(poison)`
> > whereas the optimized one is `f(nonnull poison)`.
> > `f(nonnull poison)` should be UB because `f(nonnull null)` is UB. So, the
> > transformation introduced UB.
> > This optimization is correct if both `f(nonnull null)` and `f(nonnull
> > poison)` are equivalent to `f(poison)`.
> > 
> > 
> > 3. https://reviews.llvm.org/D69477
> > 
> > f(nonnull ptr);
> > use(ptr);
> > =>
> > llvm.assume(ptr != null);
> > use(ptr);
> > f(nonnull ptr);
> > 
> > If f(nonnull null) is f(poison), this is incorrect. If ptr was null, the
> > added llvm.assume(ptr != null) raises UB whereas the source may not raise
> > UB at all. (e.g. assume that f() was an empty function)
> > If f(nonnull null) is UB, this is correct.
> > 
> > 
> > 4. Dead argument elimination (from https://reviews.llvm.org/D70749 )
> > 
> > f(nonnull ptr); // f’s argument is dead
> > =>
> > f(nonnull undef);
> > 
> > This is incorrect if f(nonnull null) is UB. To make this correct, nonnull
> > should be dropped. This becomes harder to fix if nonnull was attached at
> > the signature of a function (not at the callee site).
> > It is correct if f(nonnull null) is f(poison).
> > 
> > Actually the D70749's thread had an end-to-end miscompilation example due
> > to the interaction between this DAE and the optimization 3 (insertion of
> > llvm.assume).
> > 
> > Thanks,
> > Juneyoung Lee
> 

-- 

Johannes Doerfert
Researcher

Argonne National Laboratory
Lemont, IL 60439, USA

jdoerfert at anl.gov
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200218/077c89e3/attachment.sig>