From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 8241A3858D37 for ; Mon, 19 Feb 2024 16:47:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8241A3858D37 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 8241A3858D37 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708361236; cv=none; b=XZevqULFvmkZths/IeB1gyIvg1mBYUu7XFA16JFJ1KaevVZDtXuVE4YSF5bxBkniBZIgiPeOojeggPzRXlEU62qp5O4NdlPY9vN/PKOLipzEdnOKifREehvc0+UnrputJTvN2ZMK/rtlOpzzkrefh8Ens5kARGQn0gLWHXLoOt0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708361236; c=relaxed/simple; bh=6VbToSQIhfPtDKolf38jmKMaZKtUBj0o89LP9PMo3Lg=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=mKa3OGxJOPSFiDqONpF6/+qXUICFX5nzjm/OEUe7SiJwVtFWEWn60sn2UwKzQacgn+ZcgzGNhCjcBM00Z3OgeBR2O3/CaW3XyMP2MWgp+3zvgoAmj3fh5Cqt1UchK8Hl/Cd0xup6y5BswwmrqEHIIb1okaAowJY+wEvmldZX6AE= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1708361234; bh=6VbToSQIhfPtDKolf38jmKMaZKtUBj0o89LP9PMo3Lg=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=wfSF/aJX52/8WBn5id6MWQjaG5uXlkEFNfC10LQLo98s2kUsi5CrFkZF7izvV3MxM LCsBzr6j/AecY0EG/cgTwVBJWPZxrqxNtyXqe29F1EHCI+KwfdZjkI1juJNCIDeVz9 qahj15DYPkvNCFngtENa+Tv5asp/l8h3X7sVptbA= Received: from [10.0.0.11] (modemcable238.237-201-24.mc.videotron.ca [24.201.237.238]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id B09231E030; Mon, 19 Feb 2024 11:47:13 -0500 (EST) Message-ID: Date: Mon, 19 Feb 2024 11:47:13 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] [gdb/exp] Fix printing of out of bounds struct members To: Tom de Vries , gdb-patches@sourceware.org Cc: Tom Tromey References: <20240124161018.8844-1-tdevries@suse.de> Content-Language: en-US From: Simon Marchi In-Reply-To: <20240124161018.8844-1-tdevries@suse.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,SPF_HELO_PASS,SPF_PASS,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: On 2024-01-24 11:10, Tom de Vries wrote: > diff --git a/gdb/value.c b/gdb/value.c > index 4ec9babcce8..373b3d929da 100644 > --- a/gdb/value.c > +++ b/gdb/value.c > @@ -1229,6 +1229,9 @@ value::contents_copy_raw (struct value *dst, LONGEST dst_offset, > gdb_assert (!dst->bits_any_optimized_out (TARGET_CHAR_BIT * dst_offset, > TARGET_CHAR_BIT * length)); > > + if ((src_offset + copy_length) * unit_size > enclosing_type ()-> length ()) > + error (_("access outside bounds of object")); I stumbled on this randomly because I'm looking at conflict resolution downstream involving this (due to an unrelated change downstream). I'm not convinced that a low-level function like this is the right place to throw an error. I think that if we get here trying to do an out of bounds copy, it's a symptom of something that went wrong earlier, and there's usually a better way to address it. In this case, my understanding is that: - the expression for DW_AT_byte_size, given the uninitialized state of the variable, yields 8 - as if `A` was true. - the description of the variant, given the uninitialized state of the variable, tells us that fields C and D are active - as if `B` was false. When we sum the size of all the active fields of the type, it gives us 12. So, it seems to me like the debug information contradicts itself in this case. It can't tell us that the type is 8 bytes long but then tell us that 12 bytes worth of fields are active. Or, it should be able to tells us what is the PC range in which the variable can be considered initialized (I think this is a longstanding issue with DWARF). I think that a first step that could be taken by the compiler today to make the debug info more robust would be to not use a default variant case. When seeing an invalid discriminant value, GDB would avoid printing the variant fields altogether. As for how to handle the current situation in GDB today, I would perhaps suggest to at least move the bounds check / error call up one level, to `value::primitive_field`. Another option could be to ignore the DW_AT_byte_size for the size of resolved types (rely on the sum of the sizes of the active fields). But I don't know the implications of that, the code that overrides the resolved type's size here [1] perhaps exists for a good reason. Simon [1] https://gitlab.com/gnutools/binutils-gdb/-/blob/b47cef7ca8a2fa55acdab367c87c1ea076aec4b2/gdb/gdbtypes.c#L2849-2853