From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id GKOdOL/4F2dNLxcAWB0awg (envelope-from ) for ; Tue, 22 Oct 2024 15:10:55 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=UOqBhw7n; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id DA28B1E0BB; Tue, 22 Oct 2024 15:10:55 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-7.8 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,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 A78391E05C for ; Tue, 22 Oct 2024 15:10:54 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 307E53858D33 for ; Tue, 22 Oct 2024 19:10:54 +0000 (GMT) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id A75693858D21 for ; Tue, 22 Oct 2024 19:10:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A75693858D21 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A75693858D21 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1729624232; cv=none; b=VZg86nhBokeXmgNnkQRtK851sc81JzrEPNXjLWbGCFq2HDfOyW8GO620JlQKW+WxF2Q2Q6sbJefJsYS3qXacgP9CYQWZGtJs3s+mAjiCF5/6iuZOMu68xsRILZEshl+oZ45OYGTgY12iDCM4iFoOU7v340zclzWm0AF9EZCD5/M= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1729624232; c=relaxed/simple; bh=2AbtPoiMTXOOM/PTWL5hclONlr7dNY2mqhBQ8fL5iAY=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=pYaVcOcBFWlTNeNF3VHPq2MZyn/RZYsqklcK09tAAEHg2556D8QcGTjCjqO+tcvZXRvneyEQ9fXcn4Z1DcwPZbYe3+MyjOp5gkcipAZ5Glk/ADXfT0/EoTGPrHAjflcGLdHv6i75GEMWtZEJAVAbLs0N6AVDYVclrn+eAxcq36c= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1729624229; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=c0QIfLT3JdVQYsezpriWW6b9fySq7rDpse37NraKyGk=; b=UOqBhw7nmirZBtARKgdkoaA664doUMoNd7x+fRRMEUcIcluRW+tJgLcWLoQhtX6/0dNZVL UoTsp8uoaenGpVaJ6rBCzSTJtbTtuSIus0iqasyYW+erxX1lBS/A1OX7c3rwLAugIXkcRW /xZl3EKlmGKIw0x4xrWEpiFOz6GzYFw= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-235-khA-zLrtNr6600XuikHMdg-1; Tue, 22 Oct 2024 15:10:28 -0400 X-MC-Unique: khA-zLrtNr6600XuikHMdg-1 Received: by mail-qk1-f198.google.com with SMTP id af79cd13be357-7b161d4c422so303580085a.0 for ; Tue, 22 Oct 2024 12:10:28 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729624227; x=1730229027; h=in-reply-to:from:content-language:references:cc:to:subject :user-agent:mime-version:date:message-id:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=c0QIfLT3JdVQYsezpriWW6b9fySq7rDpse37NraKyGk=; b=q4g7Cwcef7BjlgfgBZQRVgLX108e6e6WsRfWN0DksVpEaBeyVOslw7IdTmiRnrIuCj P5Tm96kPg4SKLzVyP97nHCNkUDS/qv14P7qLlePUH0p8CC1Ybeyj1fzfzlNiTScUPnbF uv897SxSo3f77HKIjLl+gX1GMqRKo93KCYU6qrP+CoTr1cf067sH+MV+OFfHmVPIQ8c1 Vbz9h7DOzrtqlBvlGHafSHuYwlEEi89AluZnO/mtwav0Po/UYbrkfkAYTsNlJcMLhGdA CNULnkw2+AshXd8ijV3kKO3IJpbmCpERzMkWAtx7kdWSfftSC2GBH96phP1DFb3dYXqD qDXw== X-Gm-Message-State: AOJu0YwSIqeBbIpo70L/1px/vn8emPMj1vH+BKnnFFjEI8GmlcBGnUk0 ZkPFCPVqP475dLR6YUQTA271Sgf6HXeH2US10XXCZrVJ4uFBKbLJa3Pmq0MLTdriSsbdEH3mzU/ Vb2CU0UrJZnxAvbqv/k5WY8SuqHEuAWbrUNk1VGZflRKvoBK24h0BQ/ilKW1Kb1fgZnY= X-Received: by 2002:a05:620a:40c4:b0:7b1:3bf5:2da9 with SMTP id af79cd13be357-7b17e57cd44mr8361085a.31.1729624227443; Tue, 22 Oct 2024 12:10:27 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFWBHxa6kKbl74SJ/OT/JfTfFBXN5MkNboo+1fZzShVPFBUolJwShrlgmk3ddxREcA+XDGwPA== X-Received: by 2002:a05:620a:40c4:b0:7b1:3bf5:2da9 with SMTP id af79cd13be357-7b17e57cd44mr8358885a.31.1729624227106; Tue, 22 Oct 2024 12:10:27 -0700 (PDT) Received: from ?IPV6:2804:14d:8084:92c5::1001? ([2804:14d:8084:92c5::1001]) by smtp.gmail.com with ESMTPSA id af79cd13be357-7b165a01ab5sm308735985a.63.2024.10.22.12.10.25 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 22 Oct 2024 12:10:26 -0700 (PDT) Message-ID: Date: Tue, 22 Oct 2024 16:10:24 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] Fix compile error due to [[noreturn]] with clang To: Andrew Oates , Simon Marchi Cc: gdb-patches@sourceware.org References: <20241020180001.67425-1-andrew@andrewoates.com> From: Guinevere Larsen In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/alternative; boundary="------------aDiT1uPLFt7NoFV0ZDdZnnV0" Content-Language: en-US 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 This is a multi-part message in MIME format. --------------aDiT1uPLFt7NoFV0ZDdZnnV0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 10/22/24 3:50 PM, Andrew Oates wrote: > > > On Tue, Oct 22, 2024 at 2:29 PM 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 = 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. > > 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 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. > > > -- > 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 */ > > > -- Cheers, Guinevere Larsen She/Her/Hers --------------aDiT1uPLFt7NoFV0ZDdZnnV0 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit
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.

 

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 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.
 

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


-- 
Cheers,
Guinevere Larsen
She/Her/Hers
--------------aDiT1uPLFt7NoFV0ZDdZnnV0--