From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x42b.google.com (mail-wr1-x42b.google.com [IPv6:2a00:1450:4864:20::42b]) by sourceware.org (Postfix) with ESMTPS id 4B870386480B for ; Fri, 10 Feb 2023 14:18:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4B870386480B Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=embecosm.com Received: by mail-wr1-x42b.google.com with SMTP id m14so5184369wrg.13 for ; Fri, 10 Feb 2023 06:18:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=mime-version:user-agent:references:message-id:in-reply-to:subject :cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=qNXNZKacmyudI/B2EurcSiGSiskRHOzR1+MeHzYkAig=; b=EgFJl9ukE9reou+LuYgW6WSIqliVehDd65yLlJ+X2ceUNLdy+HF8e0f4wAK+iyoms3 8QNIe7tkfbHO33kJn/q8jb8GJmDEcNbJHPL5vLqB31GNlg0KGjGppTfy6bKAOQMH8y/Y jGW2HR5rnS9II3AzYd2YjrnBX/i85iK/heMpU04aiD9n7F6TU1lNivLZIhsH5QDYSlcl i6ZUb1FIowDRZhbrKbvp66JtruggUO3dRBhFMsZy9vRNGYnq/wULbd0Ql8R1vk4pAMKw BImCla358hbhGwhiTgFs72Q63Lw26R9c2oLGG+Vv0cfzrOMzK6Ap1CYxQ9zJVqMgFKIm /FIw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:user-agent:references:message-id:in-reply-to:subject :cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=qNXNZKacmyudI/B2EurcSiGSiskRHOzR1+MeHzYkAig=; b=qqprhj07w/gBHTAhrvM/lZ78naLW/WISOMiGhS3px5ItyhLfZfk5lNfaALOdZqelyf IaAoYuquuwu+rDdAMNpTT4OoATO7hW0S9S9/M81ukBKd1DbwUX0AzFlIMzcGkBpr/eaG M3+mEPCqYZfv0aj9DHQEAqEkclUrND1w0LuaVBc4xgOWbFZi1bHmn+zq9/5QIoIdqGsk bRaH2XzyjXYqMX/YtbZXsZSvBnN7cH/sdIHzhrMVusMWaHXcMz9hSNX1Ji580+GejaSc xOqC3xweJmhAIciNFNPUiGa21IjcjwxfnWvp3VVetFH6rYUeD4sqSadI45dcxMVcVAaU 8QNQ== X-Gm-Message-State: AO0yUKV0VW2/Y8UOrrr99Q/6O4Ji88CRwPBZarDW4vvgv04Ka5sWKVY1 iiowlZnrtwBso3b7TCgAQ+NqOQ== X-Google-Smtp-Source: AK7set/bTnfABk4PR7n44jVG9VBDg1pv0n7bcsuZ4XtNrhuz68EoWDJXL5aV7rk+8pft/4FYm8N4SQ== X-Received: by 2002:a05:6000:1811:b0:2c5:4b17:166 with SMTP id m17-20020a056000181100b002c54b170166mr1533835wrh.48.1676038697119; Fri, 10 Feb 2023 06:18:17 -0800 (PST) Received: from tpp.orcam.me.uk (tpp.orcam.me.uk. [2001:8b0:154:0:ea6a:64ff:fe24:f2fc]) by smtp.gmail.com with ESMTPSA id f5-20020adff445000000b002c53f5b13f9sm3200013wrp.0.2023.02.10.06.18.15 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 10 Feb 2023 06:18:15 -0800 (PST) Date: Fri, 10 Feb 2023 14:18:14 +0000 (GMT) From: "Maciej W. Rozycki" To: Tom Tromey cc: gdb-patches@sourceware.org, Andrew Burgess , Richard Bunt Subject: Re: [PATCH v3 3/5] GDB: Only make data actually retrieved into value history available In-Reply-To: <87fsbqlewl.fsf@tromey.com> Message-ID: References: <87fsbqlewl.fsf@tromey.com> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP 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 Tue, 31 Jan 2023, Tom Tromey wrote: > Maciej> +#if !defined (LONG_MIN) /* 0x80000000 for 32-bits */ > Maciej> +#define LONG_MIN ((long) (~(long) 0 ^ LONG_MAX)) > Maciej> +#endif > > I wasn't aware of this code being here. > > For LONG_*, I tend to think we don't need it. should provide > it, and gnulib seems to think there aren't any systems without this: > > https://www.gnu.org/software/gnulib/manual/html_node/limits_002eh.html I guess it's still C thinking kicking around here at least on my side. Since we've switched to C++11 it's that language standard that we have been driven by, and it mandates the presence of these macros. Except we need to switch to . I don't like to maintain a mess, so I'll repost this patch series with a suitable update included. > Maciej> +#if !defined (LONGEST_MIN) /* 0x8000000000000000 for 64-bits */ > Maciej> +#define LONGEST_MIN ((LONGEST) (~(LONGEST) 0 ^ LONGEST_MAX)) > Maciej> +#endif > > This one probably belongs in gdbsupport/common-types.h. > > We should clean up this part of defs.h but I can do that. Already done with the upcoming update. > Maciej> @@ -1950,6 +1955,15 @@ record_latest_value (struct value *val) > Maciej> a value on the value history never changes. */ > Maciej> if (value_lazy (val)) > Maciej> value_fetch_lazy (val); > Maciej> + > Maciej> + /* Don't pretend we have anything available there in the history beyond > Maciej> + the boundaries of the value recorded. It's not like inferior memory > Maciej> + where there is actual stuff underneath. */ > Maciej> + ULONGEST length = value_enclosing_type (val)->length (); > Maciej> + mark_value_bits_unavailable (val, LONGEST_MIN, 0 ^ LONGEST_MIN); > Maciej> + mark_value_bits_unavailable (val, length * TARGET_CHAR_BIT, > Maciej> + LONGEST_MAX - length * TARGET_CHAR_BIT); > > I'm struggling to understand why this is needed. > Like it seems to me that a value should always report that bytes outside > of its content range are unavailable. I have investigated whether a check based solely on the length of the data type associated with the value recorded would do, but that triggered Python regressions: FAIL: gdb.python/flexible-array-member.exp: python print(ns['items'][0]) FAIL: gdb.python/flexible-array-member.exp: python print(ns['items'][1]) FAIL: gdb.python/flexible-array-member.exp: python print(ns['items'][2]) FAIL: gdb.python/flexible-array-member.exp: python print(zs['items'][0]) FAIL: gdb.python/flexible-array-member.exp: python print(zs['items'][1]) FAIL: gdb.python/flexible-array-member.exp: python print(zs['items'][2]) FAIL: gdb.python/flexible-array-member.exp: python print(zso['items'][0]) FAIL: gdb.python/flexible-array-member.exp: python print(zso['items'][1]) FAIL: gdb.python/flexible-array-member.exp: python print(zso['items'][2]) where flexible array member elements are accessed like: (gdb) python ns = gdb.parse_and_eval('ns').dereference() (gdb) python print(ns['items'][0]) that is using a value associated with a Python variable created on the fly rather than a value history entry. These now showed as and it broke the test cases, which expect actual values, i.e.: python print(ns['items'][0]) (gdb) FAIL: gdb.python/flexible-array-member.exp: python print(ns['items'][0]) compared to: python print(ns['items'][0]) 101 (gdb) PASS: gdb.python/flexible-array-member.exp: python print(ns['items'][0]) So instead I looked into making value history entries a distinct lvalue type (it would have a nice side effect of showing "no such vector element" when accessing out-of-bounds array elements) replacing `lval_memory', `lval_register', etc. It might also cure the history mutability issue I have observed (for which I have filed PR exp/30107 to track BTW). But while possibly a nice clean-up I realised it would be a major task, so it's something perhaps for another occasion. So finally I came up with just a boolean flag set for entries recorded in the value history. It has passed my regression testing now and I'll be posting it shortly along with the rest of the patches. Unless I hear to the contrary, I'm going to follow your earlier approval for the changes already handled, but this gives you or someone else a chance to chime in just in case an issue gets spotted. Thank you for your review. Maciej