From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id yQadAT5Ar2fBJzEAWB0awg (envelope-from ) for ; Fri, 14 Feb 2025 08:08:14 -0500 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=fqdZZAed; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id EA2C31E105; Fri, 14 Feb 2025 08:08:13 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-6.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham autolearn_force=no version=4.0.0 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id B42011E05C for ; Fri, 14 Feb 2025 08:08:12 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 48ECB3858430 for ; Fri, 14 Feb 2025 13:08:12 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 48ECB3858430 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=fqdZZAed Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id DB62A3858433 for ; Fri, 14 Feb 2025 13:05:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DB62A3858433 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 DB62A3858433 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=1739538345; cv=none; b=xwq5oYVyU37jm0+OoO2QB8trch0yojQf3ajkCsGphbY7Cm5C59ra/+zQHs4W3eqyIAaQBwWFVOGelvPw55zaTlXtPyyhOOzsl27A+K+laTlOGJBIj8ZD/oV9HUHS85HEdQKYt7kAnKCztdHitW4BG6+pBQk1asCJK3G6PwjUGr8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1739538345; c=relaxed/simple; bh=j85l96vNnPlx+u8LeE84I3qjyGPAjSxznncFJpfHBao=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=d3pH0j0a13FIWEjyhv68aG3KhaY7Oq9GUsiTfERdzzKgMdBUYS9X8adGk0WI37m55g3JVwh+ltUdsk58+mzwOpW/rDwk7DqrbW+W9gUejTjYK4lLRf3u4lfq+6Np+jlhu0LUC/7Kz9EYZOZKTiCoCseiLX4M+ng+0S5C8ZBJ5YI= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DB62A3858433 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1739538339; 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=4i63Xq4fVSmleEzlc3+c2KdsHF84EDGRF8vHZrs8WIU=; b=fqdZZAedL0hWroEmQEebSNQoDDlR9eapzEGik6A/m+jbb+qZcYPF9lNU4CnxJap3WMdo2r P5WHYMY8taFllw7RGgeekb4yGe7HheouAXkeTlng+IztafjbWM+qRT91tubndgICMbOVyY KATBUMoOUOFk876+0SiIKY7rMWsIQk4= Received: from mail-qt1-f198.google.com (mail-qt1-f198.google.com [209.85.160.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-538-ZuUN59o1MKW0zGUluvyztg-1; Fri, 14 Feb 2025 08:05:38 -0500 X-MC-Unique: ZuUN59o1MKW0zGUluvyztg-1 X-Mimecast-MFC-AGG-ID: ZuUN59o1MKW0zGUluvyztg_1739538338 Received: by mail-qt1-f198.google.com with SMTP id d75a77b69052e-471d66d2a89so4340881cf.2 for ; Fri, 14 Feb 2025 05:05:38 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1739538338; x=1740143138; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=4i63Xq4fVSmleEzlc3+c2KdsHF84EDGRF8vHZrs8WIU=; b=SkX0w+6cjHbJ0SArSOcEhSFDU5PNxsXiwl6w9qYARlDHydiqqDpQIq8LB5xaA2lYSK ASsUdczbNaaFYXV7f/Hcfk/+AGpJJDVjrG4P4aM9Yp2S+IpEwldhjLVaPgx81RWpov5i gPuedEz4/qLnWB9T+uKZkqUMJaVzJ9u0GDOw8xY2CQh82Gr4ltCkCtE5jf8fBNihbyu8 2G15MR/A+/+c7POPF5CVe/kOK4Z5fmxRJ1LLLd6JVGHWe53EkyNHgR81psOidHLhqDbA pQcKalYz/V6+UKRvo0SMTq6iAvTy2eukkijIBxpLpHA5wtsDTw+rNLuchRAgAf0CO5gJ s5Kg== X-Forwarded-Encrypted: i=1; AJvYcCVyt8STa9YXyoEe7yTxwnm23UQjPoyMX1GMD81WlkLOPnMzabkMGj14kwG4MpmFqdEn6sfklM1Xuzk1Pg==@sourceware.org X-Gm-Message-State: AOJu0YyORkg6zx0ct8XIoYD5AZPTKQ3OBsC4CK+CsoYgb9p5/0pRJQX3 ctVw1XYyHwiYgrJsigxEYas44xT40twCU+IMY3SJiapdY1lYXKr0QdBa9knqcgbbMIUbKsv2ubw vQqTQgnJ3YFaEGzvIebzD5QhG4ggJELiK9TLS5hZxoI5aC/St999lRrU2TkEZho1Zr6E= X-Gm-Gg: ASbGnctNQ3TdxpDFASKoiT1S5DrzjbyLmBgWSjzVjhA/GIiIh3/sNsKpP9dZi+asppn 0ATOVXtfS7x3gIENx67UgQgmLTqCPRwrPA/wrmwL3vOMYLg0/gyOHi0N31qPUNPSOgGZ7a42IeZ WyW67raJ3kdpGspHsB+nm6Yj0jlQQSftIQak1h6CQU/yLCBf9kOsic7+EZJHriVhlDyBUOcayyg qxuLIKLFUmkHwDLHttApWkB+M4V4HwupSk79etu/DJ2uWE4nGcWgczUjxkjN9z0JY0IV+WktFWr FEI+VlIW1DWm0O/3 X-Received: by 2002:a05:622a:20b:b0:471:cbef:4cbe with SMTP id d75a77b69052e-471cbef50c6mr69448081cf.11.1739538337795; Fri, 14 Feb 2025 05:05:37 -0800 (PST) X-Google-Smtp-Source: AGHT+IHfA+M/74ktkGf7BJyCzvIjiEaQohPqR62fQt2ooCW3VXd6o+yBfPtbu26h/A/l//hqwi/9qg== X-Received: by 2002:a05:622a:20b:b0:471:cbef:4cbe with SMTP id d75a77b69052e-471cbef50c6mr69447581cf.11.1739538337421; Fri, 14 Feb 2025 05:05:37 -0800 (PST) Received: from ?IPV6:2804:14d:8084:9a69::1002? ([2804:14d:8084:9a69::1002]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-471c2a11e77sm18017451cf.29.2025.02.14.05.05.36 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 14 Feb 2025 05:05:37 -0800 (PST) Message-ID: <262792bf-893e-4a09-a75a-0f4bee9ba1ee@redhat.com> Date: Fri, 14 Feb 2025 10:05:34 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] Fix leak with internal functions To: Tom Tromey , gdb-patches@sourceware.org References: <20240426155625.104269-1-tromey@adacore.com> From: Guinevere Larsen In-Reply-To: <20240426155625.104269-1-tromey@adacore.com> X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: RAsC8o6jS443Rse7PXHDCLBdJSzbbnynghUfZyWA-oI_1739538338 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org On 4/26/24 12:56 PM, Tom Tromey wrote: > Valgrind reported a memory leak on exit with internal functions. > > To fix this, I added a new case to clear_internalvar, and then > arranged to call clear_internalvar when an internalvar is deleted. > Also, because an internalvar can reference a value, it seemed prudent > to clear 'internalvars' from the final cleanup in value.c. > > Regression tested on x86-64 Fedora 38. I also did a bit of testing by > hand with valgrind and ASAN. > > This version includes the DISABLE_COPY_AND_ASSIGN requested in review. > The move constructor and assignment operator are needed by the > std::map. > --- Hi! I was looking at the "long standing unreviewed patches" in my backlog and noticed this. I just tried it and it applies cleanly to upstream, so I guess it was never fixed. This needs a tiny adjustement to compile, but other than that this patch looks good. > gdb/testsuite/gdb.base/gdbvars.exp | 8 ++++++++ > gdb/value.c | 31 ++++++++++++++++++++++++++++++ > 2 files changed, 39 insertions(+) > > diff --git a/gdb/testsuite/gdb.base/gdbvars.exp b/gdb/testsuite/gdb.base/gdbvars.exp > index 7ec7df36b9e..32fd4c4da4e 100644 > --- a/gdb/testsuite/gdb.base/gdbvars.exp > +++ b/gdb/testsuite/gdb.base/gdbvars.exp > @@ -102,6 +102,14 @@ proc test_convenience_functions {} { > > gdb_test "print \$_isvoid (foo_int ())" " = 0" \ > "Check whether non-void function is void" > + > + gdb_test "print \$isvoid_copy = \$_isvoid" \ > + " = " > + gdb_test "print \$isvoid_copy = 0" " = 0" > + > + # Can't reset the canonical name. > + gdb_test "print \$_isvoid = 0" \ > + "Cannot overwrite convenience function _isvoid" > } > > proc test_value_history {} { > diff --git a/gdb/value.c b/gdb/value.c > index e71f38b0ce4..47e7a616e48 100644 > --- a/gdb/value.c > +++ b/gdb/value.c > @@ -1883,6 +1883,28 @@ struct internalvar > : name (std::move (name)) > {} > > + ~internalvar () > + { > + clear_internalvar (this); > + } > + > + internalvar (internalvar &&other) > + : kind (other.kind), > + u (other.u) > + { > + other.kind = INTERNALVAR_VOID; > + } > + > + internalvar &operator= (internalvar &&other) > + { > + kind = other.kind; > + u = other.u; > + other.kind = INTERNALVAR_VOID; > + return *this; > + } > + > + DISABLE_COPY_AND_ASSIGN (internalvar); > + > std::string name; > > /* We support various different kinds of content of an internal variable. > @@ -2320,6 +2342,14 @@ clear_internalvar (struct internalvar *var) > xfree (var->u.string); > break; > > + case INTERNALVAR_FUNCTION: > + if (var->u.fn.canonical) > + { > + xfree (var->u.fn.function->name); > + xfree (var->u.fn.function); this needs to be changed to `delete var->u.fn.function;` with this change, it looks good Reviewed-By: Guinevere Larsen -- Cheers, Guinevere Larsen She/Her/Hers > + } > + break; > + > default: > break; > } > @@ -4514,5 +4544,6 @@ and exceeds this limit will cause an error."), > add_final_cleanup ([] () > { > all_values.clear (); > + internalvars.clear (); > }); > }