[PATCH] D138958: [clang] Better UX for Clang’s unwind-affecting attributes

[Eli Friedman via Phabricator via cfe-commits]

efriedma added subscribers: nikic, efriedma.
efriedma added reviewers: nikic, jdoerfert.
efriedma added a comment.

>From an IR semantics standpoint, I'm not sure the `memory(none)` marking is right if the function throws an exception.  LangRef doesn't explicitly exclude the possibility, but take the following:

  void f() { throw 1; }

It gets lowered to:

  define dso_local void @_Z1fv() local_unnamed_addr #0 {
  entry:
    %exception = tail call ptr @__cxa_allocate_exception(i64 4) #1
    store i32 1, ptr %exception, align 16, !tbaa !5
    tail call void @__cxa_throw(ptr nonnull %exception, ptr nonnull @_ZTIi, ptr null) #2
    unreachable
  }

So we have a function that essentially makes a heap allocation, store a value to it, then throws it.  And throwing the exception then makes other changes to the exception handling state which are visible to the caller.  Given these details are exposed in LLVM IR, saying that this is `memory(none)` seems a bit dubious.  (I don't have a testcase that breaks off the top of my head, but I suspect it's possible to write such a testcase.)  Given that, if you're going to drop the nounwind attribute, I suspect it also makes sense to also drop the `memory(none)` attribute.

The AddPotentialArgAccess() helper exists to handle a similar sort of situation: a function is marked `pure` in C, but the return value is returned indirectly, so it's not actually pure from the perspective of LLVM IR.  To fix that, we adjust the IR attributes to something more appropriate.

(Adding @nikic and @jdoerfert for additional perspective on the IR attributes.)

-----

See also https://github.com/llvm/llvm-project/issues/36098 , which is also about a mismatch between the meaning of the C `pure` attribute and the LLVM IR `memory(none)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138958