From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id sLX4I1b8F2cTMhcAWB0awg (envelope-from ) for ; Tue, 22 Oct 2024 15:26:14 -0400 Received: by simark.ca (Postfix, from userid 112) id 8D2A71E0BB; Tue, 22 Oct 2024 15:26:14 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-6.7 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, HTML_MESSAGE,MAILING_LIST_MULTI,RCVD_IN_DNSWL_BLOCKED, RCVD_IN_VALIDITY_CERTIFIED,RCVD_IN_VALIDITY_RPBL,RCVD_IN_VALIDITY_SAFE autolearn=unavailable autolearn_force=no version=4.0.0 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id CD92C1E05C for ; Tue, 22 Oct 2024 15:26:12 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 770293858C41 for ; Tue, 22 Oct 2024 19:26:12 +0000 (GMT) Received: from mail-oi1-f175.google.com (mail-oi1-f175.google.com [209.85.167.175]) by sourceware.org (Postfix) with ESMTPS id 6B53B3858D21 for ; Tue, 22 Oct 2024 19:25:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6B53B3858D21 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=andrewoates.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 6B53B3858D21 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.167.175 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1729625155; cv=none; b=CdrrXwJ1jH6wiNDKfDFIONQ63lDj5/rk8DGMdLzfKYZy0rS9Scu4YjIT7iUG3+imv2lq5n8foDR20AUY5gT7D03k4nZ+GKEwFNtGn47b63ldih36BBGkBdl5bzHcW9a3ih1pTsA9jByVY+1wa0s6TJFUhf5Ku9VhCnKY05kwGaw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1729625155; c=relaxed/simple; bh=rM/h7WnmXYId5FvtTuzENDnnvRVzA3Esd5ixdHrIj4k=; h=MIME-Version:From:Date:Message-ID:Subject:To; b=wkanuy60wFsAz6daowKYclr7AfK2VosDu0JKXT2MEy87IzHI+BvzFlP0HIBwp2Nt+dNiZ7v5FlMJxFGgrMiFuDqDqbOhkW4Q5ZKvk9lcCkwZOOD9yQbEtNtwRUVbcQfH0aO9ijZCv9ZsU5I9skOkopr67PgcWze9Q1VS8TI2Zqg= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-oi1-f175.google.com with SMTP id 5614622812f47-3e607556c83so2194169b6e.1 for ; Tue, 22 Oct 2024 12:25:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729625153; x=1730229953; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=lIqkXkmSVR8zCVK8pipedau6prbs2HsP0itc+4G4G1Q=; b=MMlv+wDRLM/dC3n4lIMFB4zTSoBEpvtB1Y8rohwBrRkWBRzF6KubXTQ6sT9nCvzYas n6zJIZmIfFCpLVWUJ04SLw7JmHpBLTJvtXfwdv+k7fCyFxVFGVEhutMS6G8txRX125bb pexpfMfZ79Mgou619N8KyDO5a7+CxOOaP265hHYTEU1jOSluMQ5DPSx5eYKgLhHUnPgi J5sSNGyH0M+GpkR0Q/ZUBQbxlGXW1fT9cy50QOQXBRIKHE0LoqFqOt3B4y3GnYnZhOC6 SCX3y4J7OBmWWQJlqP6h+AGv9CxWmCim4J278boGwzOsDOE1rAIMGfa9a7KwXIzRNV0q qweQ== X-Forwarded-Encrypted: i=1; AJvYcCWhE9mEJrFAcgkAts+LGC7fquqKLfr/duDO9XZ6nyfHMXaJ5LpOZjqUTgJcz9billPG76rIsXztuKjkGw==@sourceware.org X-Gm-Message-State: AOJu0YxhGrOTds048+xdemiZFvYj98ARo+YzxAsHd0BGN+z/egkM+gfo d4a5BVI10aRHw0xmcgEI4bv17pJA7+gtUXK7H1odRZCbNBYW5HncCfssNC7HA+aeQTuqZ2AwhoN wtP7+jYQWf76W2wByaF1gHwsKzUQ= X-Google-Smtp-Source: AGHT+IFDCH/QVXHOys8U6Do09703akxZwvLs/qGfufVCa1fst63ujWuYPDiw2xZeY3+dkad/6oOcGHg9Chas9Tg1NtU= X-Received: by 2002:a05:6808:2dc9:b0:3e5:f9d1:4f24 with SMTP id 5614622812f47-3e6245d1e7emr197054b6e.47.1729625152675; Tue, 22 Oct 2024 12:25:52 -0700 (PDT) MIME-Version: 1.0 References: <20241020180001.67425-1-andrew@andrewoates.com> In-Reply-To: From: Andrew Oates Date: Tue, 22 Oct 2024 15:25:42 -0400 Message-ID: Subject: Re: [PATCH] Fix compile error due to [[noreturn]] with clang To: Guinevere Larsen Cc: Simon Marchi , gdb-patches@sourceware.org Content-Type: multipart/alternative; boundary="000000000000d1e898062515bc85" X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org --000000000000d1e898062515bc85 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Oct 22, 2024 at 3:10=E2=80=AFPM Guinevere Larsen wrote: > On 10/22/24 3:50 PM, Andrew Oates wrote: > > > > On Tue, Oct 22, 2024 at 2:29=E2=80=AFPM Guinevere Larsen > wrote: > >> On 10/20/24 3:00 PM, andrew@andrewoates.com wrote: >> > From: Andrew Oates >> > >> > 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 =3D 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 d9deb60b2e9e94b532f43a7d3ddddf5ddf6dbd= d3 > (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 t= he > code and isn't driven by an actual need, so I would think walking back th= is > specific change should be ok, but I'll defer to Simon on this. > > > >> >> 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]]. > > Yeah, the way I'm reading the error messages, it seems to me that clang i= s > ignoring [[noreturn]] completely, and gcc engineers agreed this should > work, so I am confident the bug is at least worth filing. Worst case > scenario we'll learn the logic behind clang and we can document in the co= de. > Makes sense! I'll file the bug against clang and CC you and we'll see where it goes :) > > >> >> -- >> Cheers, >> Guinevere Larsen >> She/Her/Hers >> >> > >> > Tested by compiling on macos both with the system clang, as well as wi= th >> > 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 */ >> > >> >> > -- > Cheers, > Guinevere Larsen > She/Her/Hers > > --000000000000d1e898062515bc85 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Tue, Oct 22, 2024 at 3:10=E2=80=AF= PM Guinevere Larsen <guinevere@r= edhat.com> wrote:
=20 =20 =20
On 10/22/24 3:50 PM, Andrew Oates wrote:
=20


On Tue, Oct 22, 2024 at 2:29=E2=80=AFPM 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:
>
>=C2=A0 =C2=A0CXX=C2=A0 =C2=A0 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 'voi= d (host_callback
> *, const char *, ...)' (aka 'void (host_callback_s= truct *, const char *, ...)')
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 gdb_callback.error =3D gdb_os_e= rror;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0^~~~~~~~~~~~
> 1 error generated.
>
> This appears to be due to the mismatch between ATTRIBUTE_NORETURN and
> [[noreturn]] on gdb_os_error.=C2=A0 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=C2=A0d9deb60b2e9e94b532f43a7d3ddddf5ddf6dbdd3 (which see= ms 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 i= t can be assigned to a [[noreturn]] function, that would be better.=C2=A0 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, s= o I would think walking back this specific change should be ok, but I'll defer to Simon on this.

=C2=A0

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.=C2=A0 That didn't occur to me, but you could= be right, if clang should be recognizing __attribute__((noreturn)) as equivalent to [[noreturn]].
Yeah, the way I'm reading the error messages, it seems to me that clang is ignoring [[noreturn]] completely, and gcc engineers agreed this should work, so I am confident the bug is at least worth filing. Worst case scenario we'll learn the logic behind clang and we can document in the code.

= Makes sense!=C2=A0 I'll file the bug against clang and CC you and we= 9;ll see where it goes :)
=C2=A0
=C2=A0

--
Cheers,
Guinevere Larsen
She/Her/Hers

>
> Tested by compiling on macos both with the system clang, as well as with
> GCC 14.=C2=A0 With clang, remote-sim.c does not compile (p= er above) without
> this patch.=C2=A0 With GCC, it compiles with and without t= he patch (it
> doesn't link, but AFAICT that is unrelated).
> ---
>=C2=A0 =C2=A0include/sim/callback.h | 2 +-
>=C2=A0 =C2=A01 file changed, 1 insertion(+), 1 deletion(-)<= br> >
> 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
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 In the case of gdb "exitin= g" means doing a longjmp back to the main
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 command loop.=C2=A0 */
>=C2=A0 =C2=A0 =C2=A0void (*error) (host_callback *, const c= har *, ...)
> -=C2=A0 =C2=A0 ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF_2;
> +=C2=A0 =C2=A0 ATTRIBUTE_PRINTF_2;
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0 =C2=A0int last_errno;=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0/* host format */
>=C2=A0 =C2=A0


--=20
Cheers,
Guinevere Larsen
She/Her/Hers
--000000000000d1e898062515bc85--