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

[Simon Marchi <simark@simark.ca>]

On 2024-10-22 15:10, Guinevere Larsen wrote:
> On 10/22/24 3:50 PM, Andrew Oates wrote:
>>
>>
>> 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.
> 
> Yeah, [[noreturn]] only works for functions, not meant to be used by members or variables, so this wouldn't work. We have to walk back the [[noreturn]] change for this function if we want to continue adding this information for the compiler.
> 
> I added Simon on CC since he's the one who made the original commit you pointed to. My reading seems to be that the patch is meant to modernize the code and isn't driven by an actual need, so I would think walking back this specific change should be ok, but I'll defer to Simon on this.

So, I can't reproduce the build failure when configuring with
`--enable-targets=all --enable-sim`.  It looks like remote-sim.o doesn't
get built, which is odd given that the simulator is enabled.  Sounds
like a bug?

When building GDB for the ARM target specifically, remote-sim.o does get
built and I see the failure.

Reverting to using the old attribute is fine with me, I don't see any
other way around.

Simon