From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 58275 invoked by alias); 26 Jan 2017 17:37:44 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 58247 invoked by uid 89); 26 Jan 2017 17:37:43 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS,URIBL_RED autolearn=ham version=3.3.2 spammy=H*i:sk:830534b, H*f:sk:830534b, instinct, H*MI:sk:830534b X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 26 Jan 2017 17:37:33 +0000 Received: from svr-orw-mbx-03.mgc.mentorg.com ([147.34.90.203]) by relay1.mentorg.com with esmtp id 1cWnzH-0002yY-JJ from Luis_Gustavo@mentor.com ; Thu, 26 Jan 2017 09:37:31 -0800 Received: from [172.30.8.148] (147.34.91.1) by svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Thu, 26 Jan 2017 09:37:28 -0800 Reply-To: Luis Machado Subject: Re: [PATCH] Harden tests that deal with memory regions References: <1485206680-4402-1-git-send-email-lgustavo@codesourcery.com> <830534b5-d820-d6dd-3d80-3644fcf50f5e@redhat.com> To: Pedro Alves , From: Luis Machado Message-ID: <91ac79a4-986c-38f5-8ee1-16c764b8c563@codesourcery.com> Date: Thu, 26 Jan 2017 17:37:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <830534b5-d820-d6dd-3d80-3644fcf50f5e@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) To svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) X-IsSubscribed: yes X-SW-Source: 2017-01/txt/msg00594.txt.bz2 On 01/26/2017 07:17 AM, Pedro Alves wrote: > On 01/23/2017 09:24 PM, Luis Machado wrote: > >> 2017-01-23 Luis Machado >> >> * lib/gdb-memory.exp: New file. > > Do we need "gdb-" in the file name? > > What other procedures to you envision being placed here? Should > this have "regions" in the file name, like "memory-regions.exp"? > The file's intro comment talks about memory regions. > I guess we don't really need the gdb prefix. I originally envisioned this particular file storing all proc's dealing with memory checks and manipulation (though i ended up describing it in a different way). I wanted to avoid having to add more helper functions to lib/gdb.exp. But maybe it wouldn't be a big problem? My instinct is to modularize it. Either way is fine with me though, lib/gdb.exp or lib/memory.exp. >> * lib/gdb.exp: Load gdb-memory.exp > > Missing period. > Thanks. >> --- a/gdb/testsuite/gdb.base/breakpoint-in-ro-region.exp >> +++ b/gdb/testsuite/gdb.base/breakpoint-in-ro-region.exp >> @@ -137,6 +137,9 @@ if ![get_function_bounds "main" main_lo main_hi] { >> return -1 >> } >> >> +# Delete all memory regions. >> +delete_memory_regions >> + > > The comment as-is practically just reads the function name > in English. The important detail missing here > is "target-supplied". So: > > # Delete all target-supplied memory regions. > delete_memory_regions > > Likewise in the other spot. > On second thought, i've pulled these comments from the test files. The updated proc documentation should be enough. What do you think? >> gdb_test_no_output "mem 0x30 0x0 ro" >> with_test_prefix "0x30 0x0" { >> region_fail "0x20 0x50" >> diff --git a/gdb/testsuite/lib/gdb-memory.exp b/gdb/testsuite/lib/gdb-memory.exp >> new file mode 100644 >> index 0000000..3377011 >> --- /dev/null >> +++ b/gdb/testsuite/lib/gdb-memory.exp >> @@ -0,0 +1,38 @@ >> +# Copyright 2017 Free Software Foundation, Inc. > > The file's non-boilerplate code is copyright 2012, so > preserve that. (git show 1591a1e8) > Done. >> + >> +# 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 . >> + >> +# This file was written by Fred Fish. (fnf@cygnus.com) > > No it wasn't. > >> + >> +# Generic gdb subroutines that should work for any target. If these >> +# need to be modified for any target, it can be done with a variable >> +# or by passing arguments. > > Stale comment. > Thanks copy/paste. Fixed. >> + >> +# This file holds functions and data dealing with memory regions manipulation. >> + >> +# Deletes all the memory regions GDB currently knows about. >> + >> +proc delete_memory_regions {} { I've added the target-supplied bit to this as well.