From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id y5I0ELm4vGfpUzwAWB0awg (envelope-from ) for ; Mon, 24 Feb 2025 13:21:45 -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=XV9r/AM4; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 364421E105; Mon, 24 Feb 2025 13:21:45 -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 83C9E1E05C for ; Mon, 24 Feb 2025 13:21:44 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2C5283858C51 for ; Mon, 24 Feb 2025 18:21:44 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2C5283858C51 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=XV9r/AM4 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id 2BBC13858D29 for ; Mon, 24 Feb 2025 18:21:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2BBC13858D29 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 2BBC13858D29 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1740421266; cv=none; b=oc0Pa8p85EvDxin7KvmnvBaOQFkHLxzIT6CWoqLtlhHRq3qKiMCDPOVWYu/0HRICLDunYLGujm7ArlPnOzJp1NItfvIwHz+0rsSmJx45X7s8m0ttvIJhpeOnC/rhX6+vuQcFybeL8zyWJlLZZ4ihJVDyDQ8Az6/B3mYqO9R19VE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1740421266; c=relaxed/simple; bh=LaFlLMQmpARCrEoqjraRxnKNXFUTv6r1D5H1IIIibrs=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=XgjxF7oxvwALKRTVXgHlXHVJtL9p23hjOJ595CCC12WjTsBq9Ftk2pt2Jd6S6XQ3fLASNgnOb9paYToaAbruDsH3TeZcSWxzKriClRdcLtTQixJz8IqTdJFtCvcWBhTSqM9iBZEiFDzRvMH4xLSxpIxRC6sgasOGUMqdwNYSOJc= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2BBC13858D29 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1740421265; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=LTp6PiuvjS1Ypj6SoT7NgIboZwXx0/a1wMgbTxuhDoI=; b=XV9r/AM4n2JkRl7DuZg8YGptve3d8vOUe8PEe5Yixc6eqoBba2Tip9aUK5Iw2V8hO64rEg Lau43SF6b5hr28WgGZzJ0FZC7rUR1HqxiGmUjZwpm6Le/vrYL/6rClCjD3gRzGfLQyWMd1 209BMqrWsfwoAU4YTX+XnejSyzjLPEk= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-75-rooVMip3NjGmzr4_663xeQ-1; Mon, 24 Feb 2025 13:21:04 -0500 X-MC-Unique: rooVMip3NjGmzr4_663xeQ-1 X-Mimecast-MFC-AGG-ID: rooVMip3NjGmzr4_663xeQ_1740421263 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-4393e873962so24019645e9.3 for ; Mon, 24 Feb 2025 10:21:04 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740421263; x=1741026063; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=LTp6PiuvjS1Ypj6SoT7NgIboZwXx0/a1wMgbTxuhDoI=; b=uNkuIV02iDjZCdDI3VdC+a0oqeEw7mY864hIQELn/bS/S2FslAA5D0cn1yQrywCw8t NxnVd0mRk51ZhEAwWG/qILu6NmZ85J3KZm8XagAe6rBeuD41X1a9GxRLgOrEQSj2j4v+ 29BH6CtA6vjqwvWIkyfR/nVkuIeNkf4IKGEK8AmWnnrHR1P3DeyHUIER2+i5oQp/V4P+ n3s1oJCsQlYPpcxClaHAvv5QlVfzkPQItt6clMzfrFsv2p93ftPcAfpv943zv0dlvA1h MtQfQjqhem7XVKQG8ejx2rymVs/gSOQoyAgl+sqmh63U9xLKgr5WuoK7nlchVAzT+H/N UIAA== X-Forwarded-Encrypted: i=1; AJvYcCUCWk5+EuB62PmANd+bonflK/WvbPyHgewADSjXcUCs4EqsGvr4b8jNrLeYwu1fEbO29ZMxrmnAzQyYcA==@sourceware.org X-Gm-Message-State: AOJu0Yzr3si3yp22n2S1HD1u2opdMogRwDorzEALfwQWrxwGJ50XGCH3 zib+z8ZvXxTxKP2EyXELohjYw9/Os0WnfMGz46inyQSbUM3bMFdM9h4QDEM7siK+eWD1rMIuBTC hf8GJzB/lppcUckibkt7nIj+qrJbxTp/Q5IXJR92OUsYNaszxC/CV6whlOsY4ZXYo3Pk= X-Gm-Gg: ASbGncsQCzhn7ynHZLquxOR1z1T+DpRhAAq1z4Lxv+YutuIiZfq03of3t813Wuv8v7D FK5GkIQiLkqUD9cLK0HZ1D69JvBk+3R2m4FVfYw+DFmDjtpkPYq+dbIy52vMezwG0e11RmS7eHh zg478jkJPuIJ9SZQRovPJBqp7RfQMwjjPMhEoCliVad6jrFKuAfHEWKIt0H0L7ibD6XKQ2O56az M/V96ap8J40m3L8PQ8USCZN8be4oevIFJwj/7FRnXdo/u/YY9tRLpns/5K0xF5kLwHTIj394EzC vCxONkG4tx5Z2VN55XGaUfvNlgeTRQhAKLOSCv4d X-Received: by 2002:a05:600c:1c1c:b0:439:a1ad:6851 with SMTP id 5b1f17b1804b1-43ab0f65f0fmr2743975e9.23.1740421262974; Mon, 24 Feb 2025 10:21:02 -0800 (PST) X-Google-Smtp-Source: AGHT+IE+rRxs5pI/tVM9y7BW8sDxSIkZsdd/ObRij28oTQhneGnHfJhGIr3n+qvSeCQprBhEtFYiqQ== X-Received: by 2002:a05:600c:1c1c:b0:439:a1ad:6851 with SMTP id 5b1f17b1804b1-43ab0f65f0fmr2743745e9.23.1740421262491; Mon, 24 Feb 2025 10:21:02 -0800 (PST) Received: from localhost (44.226.159.143.dyn.plus.net. [143.159.226.44]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-38f258f8ddbsm32322760f8f.47.2025.02.24.10.21.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 24 Feb 2025 10:21:02 -0800 (PST) From: Andrew Burgess To: Guinevere Larsen , gdb-patches@sourceware.org Cc: Guinevere Larsen Subject: Re: [PATCH] gdb/testsuite: add test for memory requirements of gcore In-Reply-To: <20250224140407.252987-1-guinevere@redhat.com> References: <20250224140407.252987-1-guinevere@redhat.com> Date: Mon, 24 Feb 2025 18:21:01 +0000 Message-ID: <87tt8jmcfm.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: FHRts3q7t2-HGc41iopfOlxYx7ZO9UPYT7K4Kj1YLco_1740421263 X-Mimecast-Originator: redhat.com Content-Type: text/plain 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 Guinevere Larsen writes: > For a long time, Fedora has been carrying an out-of-tree patch with a > similar test to the one proposed in this patch, that ensures that the > memory requirements don't grow with the inferior's memory. It's been > so long that the context for why this test exists has been lost, but > it looked like it could be interesting for upstream. > > The test runs twice, once with the inferior allocating 4Mb of memory, > and the other allocating 64Mb. My plan was to find the rate at which > things increase based on inferior size, and have that tested to ensure > we're not growing that requirement accidentally, but my testing > actually showed memory requirements going down as the inferior increases, > so instead I just hardcoded that we need less than 2Mb for the command, > and it can be tweaked later if necessary. > --- > gdb/testsuite/gdb.base/gcore-memory-usage.c | 37 +++++++ > gdb/testsuite/gdb.base/gcore-memory-usage.exp | 97 +++++++++++++++++++ > 2 files changed, 134 insertions(+) > create mode 100644 gdb/testsuite/gdb.base/gcore-memory-usage.c > create mode 100644 gdb/testsuite/gdb.base/gcore-memory-usage.exp > > diff --git a/gdb/testsuite/gdb.base/gcore-memory-usage.c b/gdb/testsuite/gdb.base/gcore-memory-usage.c > new file mode 100644 > index 00000000000..514fdb905cf > --- /dev/null > +++ b/gdb/testsuite/gdb.base/gcore-memory-usage.c > @@ -0,0 +1,37 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2025 Free Software Foundation, Inc. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see . */ > + > + > +#include > +#include > +#include > + > +int main(int argc, char **argv) This should use GNU style. So 'main' on a new line. > +{ > + /* Use argv to calculate how many megabytes to allocate. > + Do this to see how much gcore memory usage increases > + based on inferior dynamic memory. */ > + int megs = atoi (argv[1]); > + char *p = malloc (megs * 1024 * 1024); > + > + /* Wait a long time so GDB can attach to this. */ > + sleep (10); Ideally we'd not use a hard coded sleep like this. Instead, you could have the test set an alarm with some large delay (common for tests that can spin), then have the test enter a spin loop: while (some_flag_that_is_true) sleep (1); Then GDB can terminate the test by setting some_flag_that_is_true to false if it wants, or you can just stick with the existing approach of sending SIGKILL, which is fine too. Adding an alarm call ensures the test will self terminate eventually, if DejaGNU doesn't clean it up correctly for some reason. > + > + /* Let's be nice citizens :). */ > + free (p); > + return 0; > +} > diff --git a/gdb/testsuite/gdb.base/gcore-memory-usage.exp b/gdb/testsuite/gdb.base/gcore-memory-usage.exp > new file mode 100644 > index 00000000000..cdb6d73c11b > --- /dev/null > +++ b/gdb/testsuite/gdb.base/gcore-memory-usage.exp > @@ -0,0 +1,97 @@ > +# Copyright (C) 2025 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +# Test that gcore doesn't end up using excessive memory. > + > +require {istarget "*-*-linux*"} > + > +standard_testfile > + > +if {[build_executable "failed to prepare" $testfile $srcfile {debug}] == -1} { > + return -1 > +} > + > +# Read the proc_pid_status page, to find how much memory the given > +# PID is using. This is meant to be used to find the > +# memory usage for the GDB in this test. > +# Returns memory usage in Kb, or -1 if it couldn't be found. > +proc get_mem_usage {pid prefix} { > + set fd [open "/proc/$pid/status"] > + set memory -1 > + while {[gets $fd line] != -1} { > + if {[regexp {VmSize:\s*([0-9]+) kB} $line full mem]} { > + set memory $mem > + break > + } > + } > + close $fd > + > + gdb_assert {$memory != -1} "$prefix: Managed to read the memory usage" > + > + return $memory > +} > + > +# This proc restarts GDB, runs the inferior with the desired > +# amount of memory, then checks how much memory is necessary > +# to run the gcore command. It will return -1 if the gcore > +# command fails, 0 otherwise. > +proc run_test {megs} { > + with_test_prefix "$megs Mb" { > + clean_restart $::testfile > + > + set bin [standard_output_file $::testfile] Isn't bin the same as binfile, which is setup by standard_testfile? Or am I missing something? > + set corefile [standard_output_file "${::testfile}.core"] > + set inferior_pid [eval exec $bin $megs &] If there was no argument passing here then I'd say you should be using spawn_wait_for_attach. But that doesn't support argument passing... ... however, if you read that proc (and its helper proc) you'll see some comments that suggest using eval/exec like you do are not the right choice. So maybe we should either extend (somehow) spawn_wait_for_attach to allow argument passing, or write something like spawn_wait_for_attach that handles arguments? > + set gdb_pid [exp_pid -i [board_info host fileid]] > + > + # wait for memory allocation to finish. Capitalise comments please. > + sleep 1 Ideally we'd not rely on sleeps like this. Better would be to attach, and then ensure the inferior has reached some known point before sending the gcore command. Thanks, Andrew > + > + # Get the important info. > + gdb_test "attach $inferior_pid" "Attaching to.*" > + set mem_before [get_mem_usage $gdb_pid before] > + if {![gdb_gcore_cmd $corefile "create the corefile"]} { > + return -1 > + } > + set mem_after [get_mem_usage $gdb_pid after] > + > + # Do the main part of the test: How much is the memory > + # usage of GDB going to grow after using the gcore command. > + set diff_k [expr $mem_after - $mem_before] > + set diff [expr $diff_k/1024] > + verbose -log "The gcore command used $diff Mb ($diff_k Kb)" > + # The original plan was to compare to a multiple of MEGS > + # but since the requirements don't seem to go up as the > + # inferior allocated more memory, we instead just hardcode > + # 2 megs, since sometimes 1 is used. > + gdb_assert {$diff < 2} \ > + "gdb did not use as much memory as the inferior" > + > + # Kill the inferior so we don't have to wait until the process > + # finishes. > + gdb_test "signal SIGKILL" ".*The program no longer exists." > + } > + return 0 > +} > + > +# If we couldn't create the first corefile, there's no point > +# in running the second part of the test. > +if {[run_test 4] != 0} { > + return > +} > +# Surprisingly enough, the larger inferior doesn't seem to use > +# any extra memory, it usually uses less memory. Which is good, > +# it means our memory requirements aren't growing with the inferior. > +run_test 64 > -- > 2.48.1