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 */
>