[PATCH] D95122: [Libcalls, Attrs] Annotate libcalls with noundef

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D95122#2512575 <https://reviews.llvm.org/D95122#2512575>, @aqjune wrote:

> In D95122#2512404 <https://reviews.llvm.org/D95122#2512404>, @jdoerfert wrote:
>
>> LGTM. FWIW, `nonnull` should imply `noundef` because if it was `undef` we could pick `null`.
>
> I did not look into the changes in this patch yet,  but does it mean that from
>
>   f(i8* nonnull %p)
>
> it is allowed to attach noundef?
>
>   f(i8* nonnull noundef %p)
>
> I think this shouldn't be allowed, because the former one isn't UB when %p was null but the latter one raises UB.

My comment should be read in the context of this patch. We only add `nonnull` when we know there is an access, so the poison of passing null would be accessed causing UB.

In D95122#2512515 <https://reviews.llvm.org/D95122#2512515>, @nikic wrote:

> I think this goes against the rules established in D87994 <https://reviews.llvm.org/D87994>, which allows memory accesses based on partially undefined values. Now, I think the change in D87994 <https://reviews.llvm.org/D87994> was a very bad one and we should undo it, but until then I don't think we can annotate with noundef here.

Agreed. It seems to make `noundef` unusable.


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

https://reviews.llvm.org/D95122