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.129.124]) by sourceware.org (Postfix) with ESMTPS id 618493857C71 for ; Mon, 12 Dec 2022 13:33:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 618493857C71 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1670852016; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=NS2yrO8/j7J/7rGb+O/uiLfuqW59MotaGQq8Dp2xLD4=; b=HWTCPUSFXQ9GLQ257RkP7OZhvTvKaGTvp8bXhDkCPE7IiyAM4Y+faSJZEzwZogZ4s9EtTI 94L6m/1j7mwGxkXmlgzOL3e5IW/aC+gJ7gv7Y2MikyRGkkpHwZmgv+AJAlhvoqyy7nhp43 YGFKgY5uQNMXtydhQgaNFQugbBbS4hU= Received: from mail-ed1-f69.google.com (mail-ed1-f69.google.com [209.85.208.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-573-ev39u9p5PDyWGUvZ4MejAw-1; Mon, 12 Dec 2022 08:33:35 -0500 X-MC-Unique: ev39u9p5PDyWGUvZ4MejAw-1 Received: by mail-ed1-f69.google.com with SMTP id h8-20020a056402280800b0046af59e0986so5164842ede.22 for ; Mon, 12 Dec 2022 05:33:34 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=NS2yrO8/j7J/7rGb+O/uiLfuqW59MotaGQq8Dp2xLD4=; b=dJQexZTKOpmKCUfhITIW2IsWDNdsPUsiu9djmO4x6Kw9lIcLGSuNfnYu0Tv52izBkb /0HAnoDqGlpMd0Qz+Ix57i6e//vNou7fyCnmixhFYusJ2f/rNHbKzbik9id/B9Pq+g3M XhUnzy34iGqPcZJ9Y9aEutDoFZJ9sSqwAvE+dXhHmUqO8yMry74XX6eVpFJqBQogMnLI E+qZhZTREcpSN98conczzHY4tD/VzRv4xUQ3XPRVO4Q7ddIIBUi7FznpzQMi/G/GDlMD kQXdww17bPaXyRCMHhShtnZvi6pF9YE7thkm56NbGrqM0/deuYfeRTQ6xqFFQvwZzfyB MFHQ== X-Gm-Message-State: ANoB5pmQij2Qcczmt1eEfA4QI4LQ+Dcs47Sb0deXTLq7W1l4ezMb18Ji fjtwT5O0lBqI/A1cdcsNGIIOSzJm/dRU1bTYn1gZFGMY1BVeiXFqmC32tkrmz++VS9ljtT52/jU 0LvbjErpYe8dNPnj1FoXt+A== X-Received: by 2002:a17:907:8c0c:b0:7c0:8e62:56d3 with SMTP id ta12-20020a1709078c0c00b007c08e6256d3mr15206672ejc.50.1670852013897; Mon, 12 Dec 2022 05:33:33 -0800 (PST) X-Google-Smtp-Source: AA0mqf5rcZyNDMvLiDd+7qo4mGVqGLui9H/tzOj396PXzQb/6qQf8hMLtBIup3gUqVhRYuCmp+Pkrg== X-Received: by 2002:a17:907:8c0c:b0:7c0:8e62:56d3 with SMTP id ta12-20020a1709078c0c00b007c08e6256d3mr15206655ejc.50.1670852013687; Mon, 12 Dec 2022 05:33:33 -0800 (PST) Received: from [10.43.2.105] (nat-pool-brq-t.redhat.com. [213.175.37.10]) by smtp.gmail.com with ESMTPSA id b27-20020a1709062b5b00b0078db18d7972sm3273834ejg.117.2022.12.12.05.33.33 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 12 Dec 2022 05:33:33 -0800 (PST) Message-ID: Date: Mon, 12 Dec 2022 14:33:32 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.1 Subject: Re: [PATCH 3/6] gdb: make user-created frames reinflatable To: Simon Marchi , gdb-patches@sourceware.org References: <20221202180052.212745-1-simon.marchi@polymtl.ca> <20221202180052.212745-4-simon.marchi@polymtl.ca> From: Bruno Larsen In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,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 12/12/2022 14:17, Simon Marchi wrote: > >>> diff --git a/gdb/frame-info.c b/gdb/frame-info.c >>> index 40a872ea152d..d61fb7ed0e95 100644 >>> --- a/gdb/frame-info.c >>> +++ b/gdb/frame-info.c >>> @@ -21,6 +21,9 @@ >>> #include "frame-info.h" >>> #include "frame.h" >>> +#include "gdbsupport/selftest.h" >>> +#include "scoped-mock-context.h" >>> +#include "test-target.h" >>> /* See frame-info-ptr.h. */ >>> @@ -33,7 +36,8 @@ frame_info_ptr::prepare_reinflate () >>> { >>> m_cached_level = frame_relative_level (*this); >>> - if (m_cached_level != 0) >>> + if (m_cached_level != 0 >>> + || (m_ptr != nullptr && frame_is_user_created (m_ptr))) >>> m_cached_id = get_frame_id (*this); >>> } >>> @@ -54,7 +58,13 @@ frame_info_ptr::reinflate () >>> /* Frame #0 needs special handling, see comment in select_frame. */ >>> if (m_cached_level == 0) >>> - m_ptr = get_current_frame ().get (); >>> + { >>> + if (!frame_id_p (m_cached_id)) >> You seem to be using the stack being valid to check if the frame is > stack -> frame id? No, I meant stack because frame_id_p checks stack_status == FID_STACK_INVALID. > >> user created, but in the commit message you mention that you use null >> frame id. Wouldn't it be more reliable to check if m_cached_id == >> null_frame_id ? > Comparing anything against null_frame_id (even a null frame id) yields > false: > > if (stack_status == FID_STACK_INVALID > || r.stack_status == FID_STACK_INVALID) > /* Like a NaN, if either ID is invalid, the result is false. > Note that a frame ID is invalid iff it is the null frame ID. */ > eq = false; > > From: https://gitlab.com/gnutools/binutils-gdb/-/blob/a28fedbc3f582ce7c8bad2eb017b1dc072bb1da7/gdb/frame.c#L759-763 > > I don't really understand the point, I think it would be useful to be > able to compare a frame id to null_frame_id, like we compare pointers to > nullptr. But currently, the correct (and only?) way of checking if we > have a frame id or not in a frame_id is frame_id_p. Ah right, that explains it. I think we could add a comment to frame_id_p to make it more clear for newer contributors like me. However, it isn't the only way to check if a frame_id is null, we can use the fact that null_frame_id != null_frame_id and check for: if (m_cached_id != m_cached_id)   /* get current frame */ else   /* recreate user frame */ This is (apparently) very common for folks using Javascript to check if something is NaN. I personally find it hideous and would much prefer checking against null_frame_id directly. I just figured I might as well throw this cursed knowledge out there :) > > When the commit message says: > > for user-created frames, m_cached_id is a non-null frame id, whereas > for the current target frame, m_cached_id is left null. > > I could say "is valid" and "is invalid" instead. No, I think the message is fine, I was mostly getting confused about the use of frame_id_p. > > Simon > -- Cheers, Bruno