[LangRef] Clarify undefined behavior for function attributes.

[efriedma]

Summary

Violating the invariants specified by attributes is undefined behavior. Maybe we could use poison instead for some of the parameter attributes, but I don't think it's worthwhile.

[nlopes]

docs/LangRef.rst

1540 ↗

(On Diff #154450)

uhm, in the way it's written, functions marked with readnone, readonly, writeonly, argmemonly cannot e.g. divide by 0. So either all divisions are guarded to ensure that they never trigger UB, or functions with potentially unsafe divisions cannot be marked as *only.
Is this intended?

For readnone/readonly I can see the motivation: this allows hoisting these functions safely (modulo the non-returning functions problem).
For write-only and argmemonly, what's the motivation to block any other UB?

Should the has-no-UB bit be split into a separate flag? Are there use cases for the clients of readnone/readonly? I guess at least ModRef could benefit of a readonly without the no-UB bit?

[sanjoy]

docs/LangRef.rst

1540 ↗

(On Diff #154450)

We should also state that these properties need to hold for all inputs to the function.

I too think the no-UB property should have a different bit, otherwise we will have a harder time inferring readnone -- we'd have to do some control flow analysis to ensure that we don't branch on inputs that could possibly be poison and that we don't have (UB via) infloops.

[hfinkel]

docs/LangRef.rst

1540 ↗

(On Diff #154450)

I also agree. We need a separate attribute for the no-UB part. I'd like to have it, but it will be a breaking change to add to the semantics for readnone.

[efriedma]

docs/LangRef.rst

1540 ↗

(On Diff #154450)

The intent here is that a function can have undefined behavior even if it's "readnone"; undefined behavior isn't a side-effect. (The "other side-effects" bit is supposed to cover external state changes, like a syscall, which might not modify memory visible to the process.) Do you have a suggested rewrite to make that more clear?

We already have a "speculatable" attribute to mark a function as never-UB; some intrinsics have this marking.

[hfinkel]

docs/LangRef.rst

1540 ↗

(On Diff #154450)

We already have a "speculatable" attribute to mark a function as never-UB; some intrinsics have this marking.

Sadly, I'd completely forgotten that we had finally added this attribute. We aren't inferring it, however. A patch to do that: D49144.

[efriedma]

Ping

[hfinkel]

LGTM

llvm/trunk/docs/LangRef.rst

``inalloca``
     used in conjunction with other attributes that affect argument
     storage, like ``inreg``, ``nest``, ``sret``, or ``byval``. The
     ``inalloca`` attribute also disables LLVM's implicit lowering of
     large aggregate return values, which means that frontend authors
     must lower them with ``sret`` pointers.
 
     When the call site is reached, the argument allocation must have
     been the most recent stack allocation that is still live, or the
-    results are undefined. It is possible to allocate additional stack
+    behavior is undefined. It is possible to allocate additional stack
     space after an argument allocation and before its call site, but it
     must be cleared off with :ref:`llvm.stackrestore
     <int_stackrestore>`.
 
     See :doc:`InAlloca` for more information on how to use this
     attribute.
 
 ``sret``
``returned``
     checked or enforced when generating the callee. The parameter and the
     function return type must be valid operands for the
     :ref:`bitcast instruction <i_bitcast>`. This is not a valid attribute for
     return values and can only be applied to one parameter.
 
 ``nonnull``
     This indicates that the parameter or return pointer is not null. This
     attribute may only be applied to pointer typed parameters. This is not
-    checked or enforced by LLVM, the caller must ensure that the pointer
-    passed in is non-null, or the callee must ensure that the returned pointer
-    is non-null.
+    checked or enforced by LLVM; if the parameter or return pointer is null,
+    the behavior is undefined.
 
 ``dereferenceable(<n>)``
     This indicates that the parameter or return pointer is dereferenceable. This
     attribute may only be applied to pointer typed parameters. A pointer that
     is dereferenceable can be loaded from speculatively without a risk of
     trapping. The number of bytes known to be dereferenceable must be provided
     in parentheses. It is legal for the number of bytes to be less than the
     size of the pointee type. The ``nonnull`` attribute does not imply
``convergent``
 
     The optimizer may remove the ``convergent`` attribute on functions when it
     can prove that the function does not execute any convergent operations.
     Similarly, the optimizer may remove ``convergent`` on calls/invokes when it
     can prove that the call/invoke cannot call a convergent function.
 ``inaccessiblememonly``
     This attribute indicates that the function may only access memory that
     is not accessible by the module being compiled. This is a weaker form
-    of ``readnone``.
+    of ``readnone``. If the function reads or writes other memory, the
+    behavior is undefined.
 ``inaccessiblemem_or_argmemonly``
     This attribute indicates that the function may only access memory that is
     either not accessible by the module being compiled, or is pointed to
-    by its pointer arguments. This is a weaker form of  ``argmemonly``
+    by its pointer arguments. This is a weaker form of  ``argmemonly``. If the
+    function reads or writes other memory, the behavior is undefined.
 ``inlinehint``
     This attribute indicates that the source code contained a hint that
     inlining this function is desirable (such as the "inline" keyword in
     C/C++). It is just a hint; it imposes no requirements on the
     inliner.
 ``jumptable``
     This attribute indicates that the function should be added to a
     jump-instruction table at code-generation time, and that all address-taken
``readnone``
     to callers. This means while it cannot unwind exceptions by calling
     the ``C++`` exception throwing methods (since they write to memory), there may
     be non-``C++`` mechanisms that throw exceptions without writing to LLVM
     visible memory.
 
     On an argument, this attribute indicates that the function does not
     dereference that pointer argument, even though it may read or write the
     memory that the pointer points to if accessed through other pointers.
+
+    If a readnone function reads or writes memory visible to the program, or
+    has other side-effects, the behavior is undefined. If a function reads from
+    or writes to a readnone pointer argument, the behavior is undefined.
 ``readonly``
     On a function, this attribute indicates that the function does not write
     through any pointer arguments (including ``byval`` arguments) or otherwise
     modify any state (e.g. memory, control registers, etc) visible to
     caller functions. It may dereference pointer arguments and read
     state that may be set in the caller. A readonly function always
     returns the same value (or unwinds an exception identically) when
     called with the same set of arguments and global state.  This means while it
     cannot unwind exceptions by calling the ``C++`` exception throwing methods
     (since they write to memory), there may be non-``C++`` mechanisms that throw
     exceptions without writing to LLVM visible memory.
 
     On an argument, this attribute indicates that the function does not write
     through this pointer argument, even though it may write to the memory that
     the pointer points to.
+
+    If a readonly function writes memory visible to the program, or
+    has other side-effects, the behavior is undefined. If a function writes to
+    a readonly pointer argument, the behavior is undefined.
 ``"stack-probe-size"``
     This attribute controls the behavior of stack probes: either
     the ``"probe-stack"`` attribute, or ABI-required stack probes, if any.
     It defines the size of the guard region. It ensures that if the function
     may use more stack space than the size of the guard region, stack probing
     sequence will be emitted. It takes one required integer value, which
     is 4096 by default.
 
     If a function that has a ``"stack-probe-size"`` attribute is inlined into
     a function with another ``"stack-probe-size"`` attribute, the resulting
     function has the ``"stack-probe-size"`` attribute that has the lower
     numeric value. If a function that has a ``"stack-probe-size"`` attribute is
     inlined into a function that has no ``"stack-probe-size"`` attribute
     at all, the resulting function has the ``"stack-probe-size"`` attribute
     of the callee.
 ``"no-stack-arg-probe"``
     This attribute disables ABI-required stack probes, if any.
 ``writeonly``
     On a function, this attribute indicates that the function may write to but
     does not read from memory.
 
     On an argument, this attribute indicates that the function may write to but
     does not read through this pointer argument (even though it may read from
     the memory that the pointer points to).
+
+    If a writeonly function reads memory visible to the program, or
+    has other side-effects, the behavior is undefined. If a function reads
+    from a writeonly pointer argument, the behavior is undefined.
 ``argmemonly``
     This attribute indicates that the only memory accesses inside function are
     loads and stores from objects pointed to by its pointer-typed arguments,
     with arbitrary offsets. Or in other words, all memory operations in the
     function can refer to memory only using pointers based on its function
     arguments.
+
     Note that ``argmemonly`` can be used together with ``readonly`` attribute
     in order to specify that function reads only from its arguments.
+
+    If an argmemonly function reads or writes memory other than the pointer
+    arguments, or has other side-effects, the behavior is undefined.
 ``returns_twice``
     This attribute indicates that this function can return twice. The C
     ``setjmp`` is an example of such a function. The compiler disables
     some optimizations (like tail calls) in the caller of these
     functions.
 ``safestack``
     This attribute indicates that
     `SafeStack <http://clang.llvm.org/docs/SafeStack.html>`_