From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x342.google.com (mail-wm1-x342.google.com [IPv6:2a00:1450:4864:20::342]) by sourceware.org (Postfix) with ESMTPS id 580E03858D34 for ; Tue, 2 Jun 2020 15:52:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 580E03858D34 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wm1-x342.google.com with SMTP id u13so3438140wml.1 for ; Tue, 02 Jun 2020 08:52:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Gn/avqPmIJQwQ8sZsF+9V4bpGzvQC2kC1GTzX2+WgJw=; b=K5dEzx677eyeU8JR5Rpd43VluAN/DdKKRV2ldTCEhWcmM0aMVfEXHktJ0htXoZxCpq Nfh9NfbBWY0QrNOV6+YWij12XanZ/t1mo9eTiTp1ixRhINwbSRYlduIHGsW0LU9Ke8jb XDttEEeVyRPsVfU9nkUKOHOUpg0z2OE60LilvAYcTFeZySQ3mF2ES6sXwum1LZ6I5Q+P oCQvb8vx9oqya+Gw2y4DLJg1MynKw031xmXlVRIzkxgOFzihZF28IswCK0ayFSyRkZNW Ts66NczqoZQ4nbm9WFF2FkqugaNgVDDmDfP+8dXmUl3xIC/D0oTWx8OjGNUSVGUmfAqR ho2w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=Gn/avqPmIJQwQ8sZsF+9V4bpGzvQC2kC1GTzX2+WgJw=; b=FcpSWxINM/ROjj6SqeojBvAqrr0zGOTkhQV0n+BfDoUUjyhophXYT71lsQTcsjvV8F YSTpM2b2OHBBPbh3pGArpT4F6Cq01hGZXeLjv4KkmOxlgvAgMuBgB+ryz4BHg4rjUlsY E1pRvwojAvkCQ9mkJ/PCr3l3qoDUyti0ENilrjPiZ9dNHdVhfVimqUHw1B/+BtbCcu4d QxOLJVKnWqsempjSGsZKNz5UKRV+dugcyptXKaSTWoAcJCx4GLIBGa1hxIU8ucjZvQYB OaZLJ6FybAcMmW7d3w5Xz2F66kOIJfvRninlU2nF12fyemLFMyTa5ruEZIU2PSI1BFin 7dvw== X-Gm-Message-State: AOAM533xMNXB0DnEzdbbkks0kCwVD+VtbLTT6EHiVQImwvr0atuyDReO aEmL7qNM1HtBQxxkm+qMumqKxw== X-Google-Smtp-Source: ABdhPJxBVp2+P6HLoAegeZOMpt8QlXANUqtKwH9NxyNKxKvt8qYpaUnjp0wdfJykOOlDfCJQa+emNw== X-Received: by 2002:a1c:e908:: with SMTP id q8mr4615804wmc.116.1591113133378; Tue, 02 Jun 2020 08:52:13 -0700 (PDT) Received: from localhost (host86-128-12-16.range86-128.btcentralplus.com. [86.128.12.16]) by smtp.gmail.com with ESMTPSA id a3sm4353179wrp.91.2020.06.02.08.52.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Jun 2020 08:52:12 -0700 (PDT) Date: Tue, 2 Jun 2020 16:52:11 +0100 From: Andrew Burgess To: Tom de Vries Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 3/3][gdb/testsuite] Warn about leaked global array Message-ID: <20200602155211.GA3522@embecosm.com> References: <20200519163004.GA9045@delia> <20200602153830.GZ3522@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200602153830.GZ3522@embecosm.com> X-Operating-System: Linux/5.5.17-200.fc31.x86_64 (x86_64) X-Uptime: 16:49:57 up 43 days, 6:24, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Jun 2020 15:52:16 -0000 * Andrew Burgess [2020-06-02 16:38:30 +0100]: > * Tom de Vries [2020-05-19 18:30:06 +0200]: > > > Hi, > > > > A variable name cannot be used both as scalar and array without an > > intermediate unset. Trying to do so will result in tcl errors, for > > example, for: > > ... > > set var "bla" > > set var(1) "bla" > > ... > > we get: > > ... > > can't set "var(1)": variable isn't array > > ... > > and for the reverse statement order we get: > > ... > > can't set "var": variable is array > > ... > > > > So, since a global name in one test-case can leak to another > > test-case, setting a global name in one test-case can result in > > a tcl error in another test-case that reuses the name in a different > > way. > > > > Warn about leaking a global array from a test-case. > > > > Also, add a possibility to skip the warning in a given test-case using > > variable gdb_skip_check_global_vars, and use it in gdb.mi/mi2-var-child.exp > > and gdb.mi/mi-var-cp.exp. > > > > Tested on x86_64-linux. > > > > Any comments? > > If we're going to add code to loop over all globals anyway, then why > not, instead of warning about bad cases, and then wrapping tests in a > namespace, just have this code "fix" the leaked globals by deleting > them? > > My thinking is that any global that exists when we start a test should > continue to exist at the end of the test. Any other global should > just be unset when the test script finishes. > > If there really is some global state that is lazily created by a > particular test script then (a) this seems like a bug anyway, and (b) > this is easy to fix by giving it an earlier creation / initialisation. > > In this way, folk can just write test scripts, dump their junk all > over the global namespace as they like, and we'll just clean up for > them. > > Thoughts? Here's a really quick patch implementing the idea above. It needs cleaning up and commenting, etc, but it passes the testsuite with no regressions, and taking a look at its debug output, I can see it deleting some of the problem global arrays that are causing issues. What do you think of this approach? Thanks, Andrew ---- diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index 444cea01c36..c983c9cc172 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -5094,7 +5094,11 @@ set banned_procedures { strace } # if the banned variables and procedures are already traced. set banned_traced 0 +array set known_globals {} + proc gdb_init { test_file_name } { + global known_globals + # Reset the timeout value to the default. This way, any testcase # that changes the timeout value without resetting it cannot affect # the timeout used in subsequent testcases. @@ -5196,13 +5200,27 @@ proc gdb_init { test_file_name } { global gdb_instances set gdb_instances 0 - return [default_gdb_init $test_file_name] + set result [default_gdb_init $test_file_name] + + foreach varname [info globals] { + set known_globals($varname) 1 + } + + return $result } proc gdb_finish { } { global gdbserver_reconnect_p global gdb_prompt global cleanfiles + global known_globals + + foreach varname [info globals] { + if {![info exists known_globals($varname)]} { + verbose -log "APB: Deleting '$varname'" + upvar 0 unset $varname + } + } # Exit first, so that the files are no longer in use. gdb_exit