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

Roman Lebedev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Jan 15 16:59:48 PST 2023


lebedev.ri added subscribers: nlopes, regehr.
lebedev.ri added a comment.

In D138958#4054903 <https://reviews.llvm.org/D138958#4054903>, @jdoerfert wrote:

> In D138958#4045567 <https://reviews.llvm.org/D138958#4045567>, @efriedma wrote:
>
>> 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
>>   }
>
> If you mark this one as `readonly/none` we might, at some point, see the mismatch in the body and declare it UB. 
> From the outside, it should almost be fine since it's not-`nounwind` and the memory that is accessed is freshly allocated.
> However, it still would be a problem waiting to happen to have escaping memory being written in a `readonly/none` function.
>
> That said, I think we anyway want a `memory` category to express all accesses are to freshly allocated memory that may escape the function. This is a common pattern.
> So `memory(allocated, inaccessiblemem)` would express that the allocation does access some inaccesible memory and the function will also access the newly allocated memory.
> Its arguably not as good as `readnone` but I'm not sure how to get there. 
> The best idea I have off the top of my head is a dedicated `exception` category for `memory` such that it won't interfere with anything but other exceptions, which it already does due to `unwind`.
> Thus, `pure/const` -> `memory(exception).

Thank you for commenting! Is there a feeling that just dropping the attributes,
until they can be reasonably represented, hinders optimizations too much,
and we must retain the ongoing miscompile?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138958



More information about the cfe-commits mailing list