From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id Fl9uEwTQvGc2aTwAWB0awg (envelope-from ) for ; Mon, 24 Feb 2025 15:01:08 -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=gdCiqviV; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 3E10C1E105; Mon, 24 Feb 2025 15:01:08 -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 6423C1E05C for ; Mon, 24 Feb 2025 15:01:07 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E87BB3858D33 for ; Mon, 24 Feb 2025 20:01:06 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E87BB3858D33 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=gdCiqviV 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 A5A0C3858C2C for ; Mon, 24 Feb 2025 20:00:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A5A0C3858C2C 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 A5A0C3858C2C 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=1740427229; cv=none; b=h6gafnuCd0DSmITtvVIb8Ibb2/j3OnjE5FSwRcfEoBj4zjVzKkh3b8ElcD1MRluZvnUFoBzfY05Gu3rfPnOAp+c8sMwNOelJDl/eitLz0TzChh9hadceY6xkM0ZaGXinCvMlia4POHBM7sd7uNWw1PP5L9gXVHcuIG5+V6URIWg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1740427229; c=relaxed/simple; bh=ilmFAihEDxhqR97PXcyulNAvaRGWLlDJ1+HtoEr97oM=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=LwXNI7sqo5NvF4kB+RhEr06wXK52J2O7SL4AkKBA83ZEV4Uzp5i5DP849QgxscbQn8bkPs9ne0AkIzKo0ZpBr26ZUWQYjsV/Olg+JVkCJDLsfg63uEENvusPIrHps2hY9ERc99UB8ip8PZDCYv7zL15z8lNHJuxHp+uDe5ZJKXU= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A5A0C3858C2C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1740427229; 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=jRj7+z0+0JMVH16TIYiSETMC5Q+ww15EnQyf5QOen9c=; b=gdCiqviVumWa/m4tKQ40Ixkdqz5QgdJVCYCvbQkQ6acb4WxBx2GpFuZRirFzr4H6d+7Uga jpGDBOeJ125kC37VBuACljUuzs3No4oaS6GX7ow8zaSTwYVq8biza9QTSaHbT2CHB59dxD UM94rg19y/xaJe68weoscN2RHiLEuyY= Received: from mail-pj1-f71.google.com (mail-pj1-f71.google.com [209.85.216.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-118-C43fVkJSN7-vzWwqN-3w8Q-1; Mon, 24 Feb 2025 15:00:26 -0500 X-MC-Unique: C43fVkJSN7-vzWwqN-3w8Q-1 X-Mimecast-MFC-AGG-ID: C43fVkJSN7-vzWwqN-3w8Q_1740427226 Received: by mail-pj1-f71.google.com with SMTP id 98e67ed59e1d1-2fc5888c192so9254437a91.0 for ; Mon, 24 Feb 2025 12:00:26 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740427225; x=1741032025; 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=jRj7+z0+0JMVH16TIYiSETMC5Q+ww15EnQyf5QOen9c=; b=czbaFZbL/gKh8s4nfLYEPczBlOaFdNlRqeIeYxIgOOMSrbXgJxjd+1nBe8z9hz0lXZ /lfKt+A5HvikTF+u7QmRF8d/U1vbfUZw4ANf/oVOvEokznLVqRlJjN+8W5bb2jcg33S3 tt1tUZsrsF4wZq+kn+LpGoTklQ2hRSk1ebiCiYI1+punUFSQJwCT+mE77g19uXZAMUzZ dyDRhPSDx/jXQkdpA4g/jnW5W9JVygO2r6YDbG+0CnVmRlojpQkM4uQKqdEQWF1O7efD Vy+HQ1intHIG1N3oYBEZ1srAAbI3ElR39ZD85gRijJSyGgRPziW2gvdfo7YwPCBWYB2z gg0Q== X-Forwarded-Encrypted: i=1; AJvYcCVeJpu1gr1KHFVA3d/154FVq8NgTEBtGCHrE6G36n4oW13az73HzwSj+euPh0dOfa6RNNBeXLjVvHXi/w==@sourceware.org X-Gm-Message-State: AOJu0YwmqVH8pTKS4EPBx7DpzogJDw3iq6SZZ6gymF1NtjE+xzuw4IGq PLrgGZmZkXyVq8AEWAXsLrwGVN9b3CbTtjoj1Acnyg1JkDFmIAdiKUm9WgLffbMqdkHPGC/HYo/ SRc2lq72h3ZswH4eOaEzc9YodxzcYwVwbzIfT4c8TTaS/fprP66OEgzHpHlYaYjEBrt0= X-Gm-Gg: ASbGncsibz0C7QhCRVUQ1W5hSfTtqx0Ihz4fi6jftJ3zyeMunqadTQ170AppV2EfKfe /boGSHBaHy9WWOHMgUQmbLVA+NPGs88RA8lGCL0+2WxDhGQYq4LrxNXK5+MCVQ5yp65/p7XbxbB 5JAir0GK4H0XU/owYl+iOzmDZwjjLJnA0rpqRlM/eBFtOvPh78nFsBPzGoj5PxdRUSEdxOQ/4QY X3HhP3jBAdM74m+AtZwGr7qvW3MHFamZQiuKvoHAB955RdjM1f+ps4oVJdC/2v/cxzYUA7Gwior 6F4iu8HNxq4ncIb71CgjUpc/RAw= X-Received: by 2002:a17:90b:3d84:b0:2fa:15ab:4dff with SMTP id 98e67ed59e1d1-2fce7b35d13mr22902649a91.31.1740427224831; Mon, 24 Feb 2025 12:00:24 -0800 (PST) X-Google-Smtp-Source: AGHT+IHBB4Akkyjr7YjA2DyFRxykW/ZUUzrh4w0Ukih6JrKVVb2KWZm0U2DAuIBbBkyiOhzmfOl9uQ== X-Received: by 2002:a17:90b:3d84:b0:2fa:15ab:4dff with SMTP id 98e67ed59e1d1-2fce7b35d13mr22902611a91.31.1740427224326; Mon, 24 Feb 2025 12:00:24 -0800 (PST) Received: from ?IPV6:2804:14d:8084:9a69::1002? ([2804:14d:8084:9a69::1002]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2fceb05f8e1sm7037409a91.27.2025.02.24.12.00.22 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 24 Feb 2025 12:00:23 -0800 (PST) Message-ID: Date: Mon, 24 Feb 2025 17:00:21 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] gdb/testsuite: add test for memory requirements of gcore To: Andrew Burgess , gdb-patches@sourceware.org References: <20250224140407.252987-1-guinevere@redhat.com> <87tt8jmcfm.fsf@redhat.com> From: Guinevere Larsen In-Reply-To: <87tt8jmcfm.fsf@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: opg8sDomDWSJqkMjV0pPNMjIKJ-b5-nO0kYI5LkcG5U_1740427226 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 2/24/25 3:21 PM, Andrew Burgess wrote: > 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. oops, sorry, habits... > >> +{ >> + /* 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. Oh, I see. This makes sense, I'll go with it then > >> + >> + /* 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? I think I forgot about the existence of binfile. This was copied from the upstream test... > >> + 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? If I found the correct documentation page, I think it shouldn't be hard to extend spawn_wait_for_attach to handle arguments. And using optional arguments, it should be a  very minor patch, so I'll see what I can cook up. Thanks for the pointer, this definitely sounds like a better solution. > >> + 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. Right, I see. This makes sense if the inferior can spin, so I'll switch to this. -- Cheers, Guinevere Larsen She/Her/Hers > > 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