From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.223.131]) by sourceware.org (Postfix) with ESMTPS id 04A3F3858C53 for ; Wed, 21 Feb 2024 14:04:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 04A3F3858C53 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 04A3F3858C53 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=195.135.223.131 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708524306; cv=none; b=LQc4i5z7jKzGr9nlXdRnATSnwk6LTkNqS0RXKks4hNJTob8UEgGgtVZ6pgYWS/dDx1qvXyUUyulJc7UQAVGgaUW/PFcsxC4FVZseIpIwcLeYDpZtTdPF4yS9g8Vt+nrbiHUe/z1T+bm22q7TxKzXO2t9PLehdoSTbZls5cWuHGg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708524306; c=relaxed/simple; bh=88asklV8tbSGuR43hA1+VceWGsB763W3papBZH9EvMg=; h=DKIM-Signature:DKIM-Signature:DKIM-Signature:DKIM-Signature: Message-ID:Date:MIME-Version:Subject:To:From; b=GMCEcvPAv3deakgqJhuDEl9XEaoASTOpKoFKQ+4wc/hl5lwOumK89AdSM89hyDS5WFwiU8tbcWrQKjABz+j8Onf8zdzG3LVaqwuqhfAFHnJbY6HbUWZL5E2nfERC/25xBBVsSBjaupZnOXF171iDQul8pniO1u/z+KcLRQN6dzE= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104:10:150:64:97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id D44011FB5E; Wed, 21 Feb 2024 14:04:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1708524295; h=from:from:reply-to: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=+ESKXPWQyHGrY7zEaZXAO/qyNEmhSzyp1YeBSaTBWdw=; b=ozX5o80xxWSlF0dMakmPs3PakZnNcnVcNf0o78IodTbNd0GlxUdxIPVeiDaMt2ZNX6RZGN E3X4Be5N3AH5xU/BmiqsAcFNbl8Za8e9l5wh1f1cVz+Q21Bl4vXZeadWHt9A9j1Gp6jSIt +dzkivnUXGLQ8ZVnB9jHcaDoJDYGx7Q= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1708524295; h=from:from:reply-to: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=+ESKXPWQyHGrY7zEaZXAO/qyNEmhSzyp1YeBSaTBWdw=; b=a8bInkEm3hASjt1PPnFBIniWggeARWSmZNc9f7YRg2g9jtJF2Li+6FtPHwzNYMhn8ec6gn 2qrSpGbtEg3y8WCw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1708524294; h=from:from:reply-to: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=+ESKXPWQyHGrY7zEaZXAO/qyNEmhSzyp1YeBSaTBWdw=; b=M+voPSV5N/g2MnsAd9YaISQI3jTH263fs4wbhbLWqJiKlD/1VBFuNE886Kw22iEY/+EgC0 Nz+KYlgYafAHpMolcHD0K+CbcZEZlC0I+4KYFjO5oKNIpY1oJnwVa+kqane8OC81VXYodr 2w2/ruRN0cEPrnLWN7sZU+VOaUU+pCI= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1708524294; h=from:from:reply-to: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=+ESKXPWQyHGrY7zEaZXAO/qyNEmhSzyp1YeBSaTBWdw=; b=M9Fb3ufvcLxt3h93rqwesagQ0drg6T61pjR67WE1cLCc4NQkkyXOpEQQV0fV6FQZx4GKJ5 VjFKUJfTQK5e7SDA== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id B785813A69; Wed, 21 Feb 2024 14:04:54 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id fnxqKwYD1mWYfAAAD6G6ig (envelope-from ); Wed, 21 Feb 2024 14:04:54 +0000 Message-ID: <6f8a8a30-7a7e-4d33-a965-129c78bf3d57@suse.de> Date: Wed, 21 Feb 2024 15:04:50 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] [gdb/exp] Fix printing of out of bounds struct members Content-Language: en-US To: Simon Marchi , gdb-patches@sourceware.org Cc: Tom Tromey References: <20240124161018.8844-1-tdevries@suse.de> From: Tom de Vries In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Authentication-Results: smtp-out2.suse.de; dkim=pass header.d=suse.de header.s=susede2_rsa header.b=M+voPSV5; dkim=pass header.d=suse.de header.s=susede2_ed25519 header.b=M9Fb3ufv X-Spamd-Result: default: False [-3.30 / 50.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; R_DKIM_ALLOW(-0.20)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; XM_UA_NO_VERSION(0.01)[]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[3]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; RECEIVED_SPAMHAUS_BLOCKED_OPENRESOLVER(0.00)[2a07:de40:b281:106:10:150:64:167:received]; BAYES_HAM(-3.00)[100.00%]; RCVD_COUNT_THREE(0.00)[3]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; DKIM_TRACE(0.00)[suse.de:+]; MX_GOOD(-0.01)[]; DBL_BLOCKED_OPENRESOLVER(0.00)[suse.de:dkim]; FUZZY_BLOCKED(0.00)[rspamd.com]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_TLS_ALL(0.00)[]; MID_RHS_MATCH_FROM(0.00)[] X-Rspamd-Server: rspamd1.dmz-prg2.suse.org X-Rspamd-Queue-Id: D44011FB5E X-Spam-Level: X-Spam-Score: -3.30 X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,SPF_HELO_NONE,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 2/19/24 17:47, Simon Marchi wrote: > > > 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`. > Hi Simon, thanks for the post-commit review. Moving up the check one level means not checking the problem when value::contents_copy_raw is called from somewhere else than value::primitive_field. To me that looks like the opposite of an improvement. Thanks, - Tom > 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