Re: [lmi] Puzzled by this clang '-Winvalid-noreturn' warning

[Greg Chicares]

On 6/25/22 12:34, Vadim Zeitlin wrote:
> On Sat, 25 Jun 2022 10:44:29 +0000 Greg Chicares <gchicares@sbcglobal.net> 
> wrote:
> 
> GC> This declaration:
> GC>   void __cxa_throw(void* thrown_exception, struct std::type_info * tinfo, 
> void (*dest)(void*));
> GC> specified here:
> GC>   https://libcxxabi.llvm.org/spec.html
> GC> has no evident 'noreturn' attribute. However:
> GC> 
> GC> /opt/lmi/src/lmi[0]$make $coefficiency unit_tests 
> unit_test_targets=sandbox_test
> GC> clang++ [...] /opt/lmi/src/lmi/unwind.cpp -ounwind.o
> GC> /opt/lmi/src/lmi/unwind.cpp:200:1: warning: function declared 'noreturn' 
> should not return [-Winvalid-noreturn]
> GC> }
> GC> ^
> GC> 1 warning generated.
> GC> 
> GC> I can suppress the warning with the patch below, but is there a better 
> way?
> 
>  I didn't have time to test it yet (and probably won't until Monday, which
> is why I'm writing this hasty reply right now), but my idea would be to use
> noreturn with cxa_throw_t to let the compiler know that original_cxa_throw
> doesn't return.

I had tried adding
  __attribute__ ((noreturn))
after the right parenthesis of the function's parameter list.
The error message still occurred. I concluded that I had
misunderstood: the message says that the function is 'noreturn',
so making it more explicitly 'noreturn' would have no effect.

>  Otherwise, I'm almost certain that putting __builtin_unreachable() after
> the call to original_cxa_throw() should suppress the compiler warning as
> well.

I committed my original suggestion instead:

     original_cxa_throw(thrown_exception, tinfo, dest);
+#if defined LMI_CLANG
+    throw "Unreachable--silences a compiler diagnostic.";
+#endif // defined LMI_CLANG
 }

AFAICS, that tells the compiler: I'm pretty sure this
is unreachable, but if execution ever does reach this
point, then throw an exception.

OTOH, inserting "__builtin_unreachable()" says something
stronger and more dangerous, AFAICT: I'm absolutely sure
this is unreachable, and if execution ever does reach this
point, then you have my permission to do whatever it is
you do in such a case, which could have disastrous
consequences beyond my power to imagine.

IOW, I think clang is wrong in this case--it seems to
find a 'noreturn' attribute that cannot be present
according to the clang documentation, AFAICS--so I'd
rather do something that I think I understand (throw)
than invoke a builtin that I know I don't understand.

Of course, I could be completely wrong: maybe it's
catastrophic to throw a C++ exception while unwinding
the exception stack. Maybe
  std::puts("oops!"); exit(13);
would be better. But for now what I committed does
seem to work.