From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 141DB3858D34 for ; Wed, 6 Mar 2024 17:40:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 141DB3858D34 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 141DB3858D34 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709746804; cv=none; b=ORkrm9R0jD+8i4/0WC4o3078v9US57n3TTKMUsc7LkMGeguIfXucXpAy534g+H8MrlihvNrPVtBcGrf5C5szr5sweHd3c5EVrtVXYFOw77dGY7TldmBeayJmzkL0cEmpSSvYRPzoULV2ZmQ3vG7KcvxhaqOF3jGZIzHiI/R/c0A= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709746804; c=relaxed/simple; bh=SRpJ7gqa92aldhFouMvswMAtGyesasetjxvrmruiMhA=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=lf8eqUMOL5GPnA45kZ2JfQOFbXv0b+0D1ZMKK9cTuldjVnR/4okGFKNyej4xy3O7X3Rpy3O1SSxyZaSTc4DA9sCJgYzQrNHhwZETykZSGRn+es7pjbuHjhYYS8Pp8OtXKjcX7ZGi6bEdKwIcRe1iEmepQPSCDqoxZbFVCkWIJ8w= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1709746801; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=c2RG+qj8vheggJauyhgo2VMt2D9crIVCXqFdnyxvoNk=; b=TeVHJQvh8YMQlV9uJvmiTh+bv0W5yJNVo45yYqUgh73FRQQjha2gtMchbYWH09eE0k6ccD 9uXE0ANX6KeQZpqDj7hnA+/4AKur/kKy7dZYo8rZ0x/+6ewjw4seRALVbBgKcrau+kF6Q9 wQ9ovaZT6cyFIZ7QXqQdWwnluH8iS4Q= Received: from mail-lj1-f197.google.com (mail-lj1-f197.google.com [209.85.208.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-390-6TrEn0NTNVStIII7YjzepA-1; Wed, 06 Mar 2024 12:40:00 -0500 X-MC-Unique: 6TrEn0NTNVStIII7YjzepA-1 Received: by mail-lj1-f197.google.com with SMTP id 38308e7fff4ca-2d401e3e388so6448411fa.1 for ; Wed, 06 Mar 2024 09:40:00 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709746798; x=1710351598; h=content-transfer-encoding:mime-version:message-id:date:references :in-reply-to:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=JPO1heykqKa4iBpOashFzg8BBiC9Ym18blorL31gE/Y=; b=ZA2eUeilBJQkF+9uE7Lyrga4vYtjTOlPECRKt9SAW8klwjSGE9ZAOD4w0/aNG+Z8oN ndX0pQuiyu4juxmHXzwI8hRpP1C/+5vGeIRWXZZvhKVe/nGvl+W27I+O35nWy34free4 C+2mCjnPVJQUX4jrquL+7K3Vc5wADsKVL4l09KQRMn/NyDvQxv/22lZD8mkt6pf8HLkp FQvwSq2dCdzQ1btmHXQrY6e/X6HO2j2pLGbwfRMZdM1tUAXWdjx0bVrL55Xw7P7EZmli gy1E8xfJor0reovVdEx4Q+t3gMc/VlXBIbL28Q8iplcrDUXDrQ4KcUEq82k1MKBG2wKE Ceaw== X-Forwarded-Encrypted: i=1; AJvYcCWob0v4qM3GqqbPH3AoUOd03oeBspvuU450e7FIWbEgw8lpE4a9H8GrH28w00mchFO/a/kJkcYlGi/uxz92Qg4KBvP8rHHgy9b9bA== X-Gm-Message-State: AOJu0YwlNuPPDpjtSR4i/6OlkRAVnBdfM7GV6rf5zsw1ZjNSUUuSHbo9 fMGqf3b9bgekiNRAIcGr6L9LSB4RM54XjJKtCvcXck+Exku5xATmDRKGAnI6PG4cdv/8vfTwCs8 lsP1Rk3bb8i3TSgQ1LGHeR0FQ5h1/jcS2KYTcnzXHl3lXLPI+nc0KtwdUaEY= X-Received: by 2002:a2e:920b:0:b0:2d2:abcf:1fd6 with SMTP id k11-20020a2e920b000000b002d2abcf1fd6mr3983961ljg.4.1709746797937; Wed, 06 Mar 2024 09:39:57 -0800 (PST) X-Google-Smtp-Source: AGHT+IGcIUXNXdYi9vsu46H+8Xyew0g1CbWvfE+uikKvfBz+w8F97NDk+AvQLgDbEsXi/x37FCLnOA== X-Received: by 2002:a2e:920b:0:b0:2d2:abcf:1fd6 with SMTP id k11-20020a2e920b000000b002d2abcf1fd6mr3983949ljg.4.1709746797281; Wed, 06 Mar 2024 09:39:57 -0800 (PST) Received: from localhost (185.223.159.143.dyn.plus.net. [143.159.223.185]) by smtp.gmail.com with ESMTPSA id h6-20020a0564020e0600b00566d87734c0sm6044583edh.16.2024.03.06.09.39.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Mar 2024 09:39:56 -0800 (PST) From: Andrew Burgess To: Tom de Vries , gdb-patches@sourceware.org Cc: Simon Marchi Subject: Re: [PATCH] [gdb/build] Fix static cast of virtual base In-Reply-To: <20240222161804.1134-1-tdevries@suse.de> References: <20240222161804.1134-1-tdevries@suse.de> Date: Wed, 06 Mar 2024 17:39:56 +0000 Message-ID: <87frx39s43.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Tom de Vries writes: > With this change in bfd/development.sh: > ... > -development=3Dtrue > +development=3Dfalse > ... > we run into: > ... > In file included from tui-data.h:28:0, > from tui-command.c:24: > gdb-checked-static-cast.h: In instantiation of \ > =E2=80=98T gdb::checked_static_cast(V*) [with T =3D tui_cmd_window*; V = =3D tui_win_info]=E2=80=99: > tui-command.c:65:15: required from here > gdb-checked-static-cast.h:63:14: error: cannot convert from pointer to ba= se \ > class =E2=80=98tui_win_info=E2=80=99 to pointer to derived class =E2=80= =98tui_cmd_window=E2=80=99 because \ > the base is virtual > T result =3D static_cast (v); > ^~~~~~~~~~~~~~~~~~ Issues like there were supposed to be spotted by the static_asserts in gdb::checked_static_cast. Clearly the existing asserts are not good enough. I'd like to propose the patch below which would have prevented this issue sneaking in unseen. Thoughts? Thanks, Andrew --- commit 9d7cb56af0361a2276621203e0e508b6fda780f9 Author: Andrew Burgess Date: Wed Mar 6 17:28:48 2024 +0000 gdb: use static_cast in gdb::checked_static_cast =20 This commit: =20 commit 6fe4779ac4b1874c995345e3eabd89cb1a05fbdf Date: Sat Feb 24 11:00:20 2024 +0100 =20 [gdb/build] Fix static cast of virtual base =20 addressed an issue where GDB would not compile in production mode due to a use of gdb::checked_static_cast. The problem was that we were asking GDB to cast from a virtual base class to a sub-class, this works fine when using dynamic_cast, but does not work with static_cast. =20 The gdb::checked_static_cast actually uses dynamic_cast under the hood in development mode in order to ensure that the cast is valid, while in a production build we use static_cast as this is more efficient. =20 What this meant however, was that when gdb::checked_static_cast was used to cast from a virtual base class, the dynamic_cast of a non-production build worked fine, while the production build's static_cast caused a build failure. =20 However, the gdb::checked_static_cast function already contains some static_assert calls that are intended to catch any issues with invalid type casting, the goal of these asserts was to prevent issues like this: the build only failing in production mode. Clearly the current asserts are not enough. =20 I don't think there is a std::is_virtual_base type trait check, so what I propose instead is that in non-production mode we also make use of static_cast. This will ensure that any errors that crop up in production mode should also be revealed in non-production mode, and should catch issues like this in the future. =20 There should be no user visible changes after this commit. =20 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=3D31399 diff --git a/gdbsupport/gdb-checked-static-cast.h b/gdbsupport/gdb-checked-= static-cast.h index 227010e46ea..33cab48cd9b 100644 --- a/gdbsupport/gdb-checked-static-cast.h +++ b/gdbsupport/gdb-checked-static-cast.h @@ -57,8 +57,14 @@ checked_static_cast (V *v) if (v =3D=3D nullptr) return nullptr; =20 - T result =3D dynamic_cast (v); - gdb_assert (result !=3D nullptr); + gdb_assert (dynamic_cast (v) !=3D nullptr); + + /* If a base class of V is virtual then the dynamic_cast will succeed, + but the production mode static_cast will fail. So having checked wit= h + the dynamic_cast that we didn't get nullptr, now use static_cast to + catch the virtual base case. This has the side effect of guaranteein= g + that GDB will compile in production mode. */ + T result =3D static_cast (v); #else T result =3D static_cast (v); #endif