From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2193 invoked by alias); 24 Aug 2012 15:12:20 -0000 Received: (qmail 2176 invoked by uid 22791); 24 Aug 2012 15:12:18 -0000 X-SWARE-Spam-Status: No, hits=-7.6 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 24 Aug 2012 15:11:47 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q7OFBhqQ026093 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 24 Aug 2012 11:11:43 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q7OFBfXp004915; Fri, 24 Aug 2012 11:11:42 -0400 Message-ID: <503799AD.3080905@redhat.com> Date: Fri, 24 Aug 2012 15:12:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20120717 Thunderbird/14.0 MIME-Version: 1.0 To: Wei-cheng Wang CC: Pedro Alves , Yao Qi , gdb-patches@sourceware.org Subject: Re: [PATCH] Fix memory-region overlapping checking References: <1591920.sFon8Oz8ME@qiyao.dyndns.org> <1450628.xUgDUToN8c@qiyao.dyndns.org> <503762F6.3080804@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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 X-SW-Source: 2012-08/txt/msg00725.txt.bz2 On 08/24/2012 12:47 PM, Wei-cheng Wang wrote: > Hi Pedro Alves, > > The line number in my previous mail was wrong (s/45/58/), > so it cannot be fully applied. Please try this again. > Sorry for the inconvenience Ah, thanks. The previous patch had applied, but with fuzz. The fix is okay. I caught a few nits, and when trying to grok the test, ended up extending it some more. See below. If you don't have further comments, I'll put it in. Thanks for this, and thanks to Yao too. > + || (lo <= n->lo && ((hi >= n->hi && n->hi != 0)|| hi == 0))) Missing space before second '||'. > 2012-08-08 Wei-cheng Wang > > * memattr.c: Fix memory region overlapping checking Missing function that changed in parens, and period at end of sentence. > 2012-08-08 Wei-cheng Wang > Yao Qi > > * gdb.base/memattr.exp: Add cases for overlapping checking. Ditto. > +# Test overlapping checking > +# > +# lo' hi' > +# |---------| > +# 10 20 30 40 50 60 70 80 > +# |----| FAIL > +# |--| FAIL > +# |---| FAIL > +# |--| FAIL > +# |----| FAIL > +# |---------| FAIL > +# |-------------| FAIL > +# |----------------- FAIL > +# |------------ FAIL > +# |--- FAIL > +# |----| PASS > +# |----| PASS This table doesn't match 1 on 1 with the tests below. And I think we can be even more thorough. We should test all the '==' cases for the '>=' conditions in the code. IOW, when lo == n->lo, etc. > + > +proc delete_memory {} { > + global gdb_prompt > + > + gdb_test_multiple "delete mem" "delete mem" { > + -re "Delete all memory regions.*y or n.*$" { > + send_gdb "y\n"; > + exp_continue > + } > + -re "$gdb_prompt $" { } > + } > +} > + > +# Test normal case (upper != 0) > +delete_memory > +gdb_test_no_output "mem 0x30 0x60 ro" > +gdb_test "mem 0x20 0x40 ro" "overlapping memory region" "mem 0x20 0x40 1" > +gdb_test "mem 0x30 0x40 ro" "overlapping memory region" "mem 0x30 0x40 1" > +gdb_test "mem 0x40 0x50 ro" "overlapping memory region" "mem 0x40 0x50 1" > +gdb_test "mem 0x50 0x60 ro" "overlapping memory region" "mem 0x50 0x60 1" > +gdb_test "mem 0x30 0x60 ro" "overlapping memory region" "mem 0x30 0x60 1" > +gdb_test "mem 0x50 0x70 ro" "overlapping memory region" "mem 0x50 0x70 1" > +gdb_test "mem 0x20 0x0 ro" "overlapping memory region" "mem 0x20 0x0 1" > +gdb_test "mem 0x30 0x0 ro" "overlapping memory region" "mem 0x30 0x0 1" > +gdb_test_no_output "mem 0x10 0x30 ro" "mem 0x10 0x30 1" > +gdb_test_no_output "mem 0x60 0x80 ro" "mem 0x60 0x80 1" > +gdb_test_no_output "mem 0x80 0x0 ro" "mem 0x80 0x0 1" > + > +# Test special case (upper == 0) > +delete_memory Since I was looking, I thought these could use some neat ascii art too. > +gdb_test_no_output "mem 0x30 0x0 ro" > +gdb_test "mem 0x20 0x40 ro" "overlapping memory region" "mem 0x20 0x40 2" > +gdb_test "mem 0x30 0x40 ro" "overlapping memory region" "mem 0x30 0x40 2" > +gdb_test "mem 0x20 0x0 ro" "overlapping memory region" "mem 0x20 0x0 2" > +gdb_test "mem 0x30 0x0 ro" "overlapping memory region" "mem 0x30 0x0 2" > +gdb_test_no_output "mem 0x10 0x30 ro" "mem 0x10 0x30 2" I also took the liberty of adding a couple of convenience functions that make writing these lines, and eyeballing against the tables easier (I got confused easily). WDYT? 2012-08-24 Wei-cheng Wang * memattr.c (create_mem_region): Fix memory region overlapping checking. 2012-08-24 Wei-cheng Wang Yao Qi Pedro Alves * gdb.base/memattr.exp (delete_memory, region_pass, region_fail): New procedures. (top level): Add overlap checking tests. --- gdb/memattr.c | 2 - gdb/testsuite/gdb.base/memattr.exp | 94 ++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/gdb/memattr.c b/gdb/memattr.c index ec7deb5..bd92f1d 100644 --- a/gdb/memattr.c +++ b/gdb/memattr.c @@ -207,7 +207,7 @@ create_mem_region (CORE_ADDR lo, CORE_ADDR hi, if ((lo >= n->lo && (lo < n->hi || n->hi == 0)) || (hi > n->lo && (hi <= n->hi || n->hi == 0)) - || (lo <= n->lo && (hi >= n->hi || hi == 0))) + || (lo <= n->lo && ((hi >= n->hi && n->hi != 0) || hi == 0))) { printf_unfiltered (_("overlapping memory region\n")); return; diff --git a/gdb/testsuite/gdb.base/memattr.exp b/gdb/testsuite/gdb.base/memattr.exp index 4065808..a83ca73 100644 --- a/gdb/testsuite/gdb.base/memattr.exp +++ b/gdb/testsuite/gdb.base/memattr.exp @@ -448,3 +448,97 @@ gdb_test_multiple "info mem" "mem 2-4 were deleted" { gdb_test "delete mem 8" "No memory region number 8." \ "delete non-existant region" + +# +# Test overlapping checking +# + +proc delete_memory {} { + global gdb_prompt + + gdb_test_multiple "delete mem" "delete mem" { + -re "Delete all memory regions.*y or n.*$" { + send_gdb "y\n"; + exp_continue + } + -re "$gdb_prompt $" { } + } +} + +# Create a region that doesn't overlap (a PASS in the table). + +proc region_pass { region } { + gdb_test_no_output "mem $region ro" "$region: no-overlap" +} + +# Try to create a region that overlaps (a FAIL in the table). + +proc region_fail { region } { + gdb_test "mem $region ro" \ + "overlapping memory region" \ + "$region: overlap" +} + +# Test normal case (upper != 0) +# +# lo' hi' +# |--------| +# 10 20 30 40 50 60 70 80 90 +# |-----| FAIL +# |--| FAIL +# |--| FAIL +# |--| FAIL +# |-----| FAIL +# |--------| FAIL +# |--------------| FAIL +# |----------------- FAIL +# |-------------- FAIL +# |----------- FAIL +# |--| PASS +# |--| PASS +# |--- PASS + +delete_memory +gdb_test_no_output "mem 0x30 0x60 ro" +with_test_prefix "0x30 0x60" { + region_fail "0x20 0x40" + region_fail "0x30 0x40" + region_fail "0x40 0x50" + region_fail "0x50 0x60" + region_fail "0x50 0x70" + region_fail "0x30 0x60" + region_fail "0x20 0x70" + region_fail "0x20 0x0" + region_fail "0x30 0x0" + region_fail "0x40 0x0" + region_pass "0x20 0x30" + region_pass "0x60 0x70" + region_pass "0x80 0x0" +} + +# Test special case (upper == 0) +# +# lo' hi' +# |--------------- +# 00 10 20 30 40 50 60 70 80 +# |--------| FAIL +# |-----| FAIL +# |--| FAIL +# |------------------ FAIL +# |--------------- FAIL +# |------------ FAIL +# |--| PASS +# |--| PASS + +delete_memory +gdb_test_no_output "mem 0x30 0x0 ro" +with_test_prefix "0x30 0x0" { + region_fail "0x20 0x50" + region_fail "0x30 0x50" + region_fail "0x40 0x50" + region_fail "0x20 0x0" + region_fail "0x30 0x0" + region_fail "0x40 0x0" + region_pass "0x20 0x30" + region_pass "0x00 0x10" +}