From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-2.mimecast.com [205.139.110.61]) by sourceware.org (Postfix) with ESMTP id 41B2A3851C27 for ; Wed, 3 Jun 2020 09:49:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 41B2A3851C27 Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-348-E7KfJkvDMPuykalnnl6IQg-1; Wed, 03 Jun 2020 05:49:40 -0400 X-MC-Unique: E7KfJkvDMPuykalnnl6IQg-1 Received: by mail-wr1-f71.google.com with SMTP id p9so879736wrx.10 for ; Wed, 03 Jun 2020 02:49:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=gLoin4bIkAj5VBPGqksd0KtUx4w1YYFgutAwTI4IBtI=; b=boAiYYav9rB9qYAzUzjdXKhvpHzFuLAgwBVcZWv/3f8ZZRfX5i7az4U42PCVGI+ZmF GrppSTJdL6TO/hRotxnxhCbPrXpLIuMcxp+xtmX4eiANrYrBsVyZTG7n2HnJ75zP0MVW Jvzh/ZlGPeskV9E/WmD/mghbsFsb7W+3mMcm7ZfGRcMB6D7kz1icpMTm7lsg4Bs/azEQ 53b4e89RqJ5yo/vTs2N8Wp5HEu15L1HdKkqHMjw8MYh+3R8XY76h+JHiy4b7G0XyusIa gn00EWs+Xi+VyuVdtOkJZM5GJZCAo7V0IamwJGREUktDZBnGeop5RNDwuJG0eOul2tt5 PSsQ== X-Gm-Message-State: AOAM532zf8VzU+kgrwZe38T098LIJazbjlq6MPoQB97DH4iPncuH6uHd HiW/ljNo7fxdgSaiHNxbu5LcHREXD6nc5Ot3x3WGzC9TTIv2JatRXX1W8MQZ45XnvLCGeyKq0KD uYiLR6m4PXSXeeHGnR1T4PQ== X-Received: by 2002:adf:e2c9:: with SMTP id d9mr30144998wrj.227.1591177779461; Wed, 03 Jun 2020 02:49:39 -0700 (PDT) X-Google-Smtp-Source: ABdhPJytEfcU3x/ONDaeGJL/xFDyHXqSd+aDdam8y9A6aYIQKiUq0shuFLVZuEO37viIuztGTJzWRQ== X-Received: by 2002:adf:e2c9:: with SMTP id d9mr30144985wrj.227.1591177779244; Wed, 03 Jun 2020 02:49:39 -0700 (PDT) Received: from ?IPv6:2001:8a0:f922:c400:56ee:75ff:fe8d:232b? ([2001:8a0:f922:c400:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id u3sm2469459wrw.89.2020.06.03.02.49.38 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 03 Jun 2020 02:49:38 -0700 (PDT) Subject: Re: [PATCH 3/3][gdb/testsuite] Warn about leaked global array To: Andrew Burgess , Tom de Vries References: <20200519163004.GA9045@delia> <20200602153830.GZ3522@embecosm.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <72b3c10c-d316-9a0e-13e9-56dee7c765a7@redhat.com> Date: Wed, 3 Jun 2020 10:49:37 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20200602153830.GZ3522@embecosm.com> Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, 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: Wed, 03 Jun 2020 09:49:45 -0000 On 6/2/20 4:38 PM, Andrew Burgess wrote: > 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? Hmm, I thought of this when I saw Tom's detection patch the first time, but dismissed it, perhaps too quickly, thinking that it wouldn't be safe. You mention global state lazily created by test scripts, but I was thinking of lazy initialized globals within Dejagnu itself, or whatever Tcl library we might end up using the future, which might need to be carried over from testcase to testcase. Imagine, this in a Dejagnu function: proc fail {} { global number_of_fails # Hadn't been set before. (valid since Tcl 8.5; otherwise, imagine it wrapped in 'info exits') incr number_of_fails } This is obviously not the real "fail", it is just to show the point, or the risk. I imagined there might be similar functions in random Dejagnu boards. ( Now that I look, it seems like Dejagnu wants to be sure to initialize all global variables it uses in runtest.exp: # Initialize a few global variables used by all tests. # `reset_vars' resets several of these, we define them here to document their # existence. In fact, it would be nice if all globals used by some interface # of dejagnu proper were documented here. And I guess we can always initialize some ourselves, if we find that missing. ) So if people think the benefits outweigh the risks, then I guess we should give it a go. BTW, global variables alone aren't the full scope of the bleeding between testcases. There's also the case of testcases overriding procedures, like gdb.base/dbx.exp, but those are perhaps rare enough that we can continue handling it "manually" as before. Thanks, Pedro Alves