From patchwork Tue Sep 12 15:24:14 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Preud'homme X-Patchwork-Id: 22847 Received: (qmail 101637 invoked by alias); 12 Sep 2017 15:24:38 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 87902 invoked by uid 89); 12 Sep 2017 15:24:28 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=treating, H*M:a007, H*M:d17a X-HELO: foss.arm.com Received: from usa-sjc-mx-foss1.foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 12 Sep 2017 15:24:25 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 5DF871596; Tue, 12 Sep 2017 08:24:16 -0700 (PDT) Received: from [10.2.206.52] (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D61733F58C; Tue, 12 Sep 2017 08:24:15 -0700 (PDT) Subject: Re: [PATCH, gdb/testsuite] Fix compare-sections.exp FAIL To: Pedro Alves , GDB Patches References: <33d0f2eb-4802-8b06-44bf-3e4299c5fdb4@foss.arm.com> From: Thomas Preudhomme Message-ID: Date: Tue, 12 Sep 2017 16:24:14 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: X-IsSubscribed: yes Hi Pedro, Thanks for the review. Please find an updated patch attached. Updated ChangeLog entry and description are as follow: compare-sections.exp has two cases that are not handled appropriately: 1) value read from read-only section is negative 2) error while patching that section This patch adapts the regular expression to allow a minus sign to deal with 1) and test for the error message to not set written if read-only section cannot be written to so as to solve 2). ChangeLog entry is as follows: *** gdb/testsuite/ChangeLog *** 2017-09-11 Thomas Preud'homme Pedro Alves * gdb.base/compare-sections.exp (get value of read-only section): Read as unsigned value. (corrupt read-only section): Don't set written if patching failed. Without this patch I get a Tcl error on the first test while it now PASSes and the second one is marked UNSUPPORTED. Is this ok for master? Best regards, Thomas On 06/09/17 18:13, Pedro Alves wrote: > On 09/06/2017 05:58 PM, Thomas Preudhomme wrote: > >> >> diff --git a/gdb/testsuite/gdb.base/compare-sections.exp b/gdb/testsuite/gdb.base/compare-sections.exp >> index e4b99216408f86745a890f80175e2f26c39303b5..8a0cf89a4c7c51299c59f59175865b5eb211f213 100644 >> --- a/gdb/testsuite/gdb.base/compare-sections.exp >> +++ b/gdb/testsuite/gdb.base/compare-sections.exp >> @@ -118,7 +118,7 @@ with_test_prefix "read-only" { >> >> set test "get value of read-only section" >> gdb_test_multiple "print /d *(unsigned char *) $ro_address" "$test" { >> - -re " = (\[0-9\]*).*$gdb_prompt $" { >> + -re " = (-?\[0-9\]+).*$gdb_prompt $" { > > This is reading bytes off of memory, as we can infer from > use of "unsigned char". The test is most probably using > explicit /d to avoid printing the number as a character. > I think it'd be clearer to change the "/d" to "/u", here > and below too. > > Also, I think treating as negative integer would actually > be buggy because if the value happens to be 0xff, then you > get back -1, which would incorrectly match here: > > if {$orig == -1} { > untested "couldn't read address of read-only section" > return -1 > } >> set orig $expect_out(1,string) >> pass "$test" >> } >> @@ -136,6 +136,8 @@ with_test_prefix "read-only" { >> set written -1 >> set test "corrupt read-only section" >> gdb_test_multiple "print /d *(unsigned char *) $ro_address = $patch" "$test" { >> + -re " = .*Cannot access memory at address $ro_address.*$gdb_prompt $" { > > Add: > pass "$test (cannot write)" > >> + } >> -re " = (\[0-9\]*).*$gdb_prompt $" { >> set written $expect_out(1,string) >> pass "$test" >> > > Thanks, > Pedro Alves > diff --git a/gdb/testsuite/gdb.base/compare-sections.exp b/gdb/testsuite/gdb.base/compare-sections.exp index e4b99216408f86745a890f80175e2f26c39303b5..90676b5b9872984af90e2b6b5e602a8287f8356f 100644 --- a/gdb/testsuite/gdb.base/compare-sections.exp +++ b/gdb/testsuite/gdb.base/compare-sections.exp @@ -117,7 +117,7 @@ with_test_prefix "read-only" { set orig -1 set test "get value of read-only section" - gdb_test_multiple "print /d *(unsigned char *) $ro_address" "$test" { + gdb_test_multiple "print /u *(unsigned char *) $ro_address" "$test" { -re " = (\[0-9\]*).*$gdb_prompt $" { set orig $expect_out(1,string) pass "$test" @@ -136,6 +136,9 @@ with_test_prefix "read-only" { set written -1 set test "corrupt read-only section" gdb_test_multiple "print /d *(unsigned char *) $ro_address = $patch" "$test" { + -re " = .*Cannot access memory at address $ro_address.*$gdb_prompt $" { + pass "$test (cannot write)" + } -re " = (\[0-9\]*).*$gdb_prompt $" { set written $expect_out(1,string) pass "$test"