Re: [PATCH] Fix compile error due to [[noreturn]] with clang

[Andrew Oates <andrew@andrewoates.com>]

[-- Attachment #1: Type: text/plain, Size: 3079 bytes --]

On Tue, Oct 22, 2024 at 2:29 PM Guinevere Larsen <guinevere@redhat.com>
wrote:

> On 10/20/24 3:00 PM, andrew@andrewoates.com wrote:
> > From: Andrew Oates <andrew@andrewoates.com>
> >
> > Since commit d9deb60b2e9e94b532f43a7d3ddddf5ddf6dbdd3, I get the
> > following compiler error when building binutils (cross-compiling) on
> > macos:
> >
> >   CXX    remote-sim.o
> > ../../gdb/remote-sim.c:334:28: error: assigning to 'void
> (*)(host_callback *, const char *, ...) __attribute__((noreturn))' (aka
> 'void (*)(host_callback_struct *, const char *, ...)
> __attribute__((noreturn))') from incompatible type 'void (host_callback
> > *, const char *, ...)' (aka 'void (host_callback_struct *, const char *,
> ...)')
> >        gdb_callback.error = gdb_os_error;
> >                             ^~~~~~~~~~~~
> > 1 error generated.
> >
> > This appears to be due to the mismatch between ATTRIBUTE_NORETURN and
> > [[noreturn]] on gdb_os_error.  Removing ATTTRIBUTE_NORETURN on the
> > declaration of host_callback::error resolves the issue.
>
> Have you tried using ATTRIBUTE_NORETURN for gdb_os_error instead? If the
> problem is the mismatch, I would prefer that we made them match over
> removing information for the compiler.
>

gdb_os_error used to have ATTRIBUTE_NORETURN on it, but that was removed in
favor of [[noreturn]] in commit d9deb60b2e9e94b532f43a7d3ddddf5ddf6dbdd3
(which seems to have done ATTRIBUTE_NORETURN -> [[noreturn]] through most
of the codebase).

I'm definitely not an expert here, but I agree that if there's a way to
annotate the function pointer instead so it can be assigned to a
[[noreturn]] function, that would be better.  I tried doing that but
couldn't make it work.


>
> Also, from my little knowledge in this area, this sounds like a clang
> bug. I encourage you to report it to upstream clang, or I can do it
> myself if you'd prefer.
>

Ah...maybe.  That didn't occur to me, but you could be right, if clang
should be recognizing __attribute__((noreturn)) as equivalent to
[[noreturn]].


>
> --
> Cheers,
> Guinevere Larsen
> She/Her/Hers
>
> >
> > Tested by compiling on macos both with the system clang, as well as with
> > GCC 14.  With clang, remote-sim.c does not compile (per above) without
> > this patch.  With GCC, it compiles with and without the patch (it
> > doesn't link, but AFAICT that is unrelated).
> > ---
> >   include/sim/callback.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/sim/callback.h b/include/sim/callback.h
> > index f69f783abac..045ac3411af 100644
> > --- a/include/sim/callback.h
> > +++ b/include/sim/callback.h
> > @@ -129,7 +129,7 @@ struct host_callback_struct
> >        In the case of gdb "exiting" means doing a longjmp back to the
> main
> >        command loop.  */
> >     void (*error) (host_callback *, const char *, ...)
> > -    ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF_2;
> > +    ATTRIBUTE_PRINTF_2;
> >
> >     int last_errno;           /* host format */
> >
>
>

[-- Attachment #2: Type: text/html, Size: 4198 bytes --]