[PATCH] Fix memory-region overlapping checking

[PATCH] Fix memory-region overlapping checking

[Wei-cheng Wang]

Hi,

  After adding a memory-region with <hi addr> as max CORE_ADDR+1,
  no more memory-region can be added. It always complains about "overlapping"

  For example,

(gdb) mem 0x50 0x80 ro
(gdb) mem 0xffffff00 0x100000000 ro
(gdb) info mem
Using user-defined memory regions.
Num Enb Low Addr   High Addr  Attrs
1   y   0x00000050 0x00000080 ro nocache
2   y   0xffffff00 0x100000000 ro nocache
(gdb) mem 0x100 0x200 ro
overlapping memory region

   When checking whether the new memory-region includes a previous one,
   it should take care special case, where hi == 0 means max CORE_ADDR+1.

Wei-cheng

2012-08-02  Wei-cheng Wang  <cole945@gmail.com>

	* memattr.c (create_mem_region): Fix memory-region overlapping checking
	in special case.

diff --git a/gdb/memattr.c b/gdb/memattr.c
--- 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;

Re: [PATCH] Fix memory-region overlapping checking

[Yao Qi]

On Thursday, August 02, 2012 02:10:40 PM Wei-cheng Wang wrote:
> Hi,
> 
>   After adding a memory-region with <hi addr> as max CORE_ADDR+1,
>   no more memory-region can be added. It always complains about
> "overlapping"

Thanks for reporting this problem, and drafting a patch to fix it.

> 
>   For example,
> 
> (gdb) mem 0x50 0x80 ro
> (gdb) mem 0xffffff00 0x100000000 ro

However, the real problem is that we don't do range checking for inputs in 
command 'mem'.  The expected behavior is, on a 32-bit target, when user types 
0x100000000, gdb should emit an error saying that it is out of the range of 
CORE_ADDR, or something like that.

> (gdb) info mem
> Using user-defined memory regions.
> Num Enb Low Addr   High Addr  Attrs
> 1   y   0x00000050 0x00000080 ro nocache
> 2   y   0xffffff00 0x100000000 ro nocache
> (gdb) mem 0x100 0x200 ro
> overlapping memory region
> 
>    When checking whether the new memory-region includes a previous one,
>    it should take care special case, where hi == 0 means max CORE_ADDR+1.

The case you gave works well on my x86_64-linux.

-- 
Yao (齐尧)

Re: [PATCH] Fix memory-region overlapping checking

[Wei-cheng Wang]

Hi Yao,

Let me correct my previous example. It should be
(gdb) mem 0x50 0x80 ro
(gdb) mem 0xffffff00 0 ro
(gdb) mem 0x100 0x200 ro
overlapping memory region

(instread of 0x100000000)
As documented in gdb manual (10.16),
``Note that upper == 0 is a special case: it is treated as the
target's maximum memory address.''

Anyway, it doesn't matter the real issue.
In my humble opinion, in order for that to work,
it should fixed as such. Otherwise, it doesn't work even on x86_64-linux.

thanks a lot,
Wei-cheng

Re: [PATCH] Fix memory-region overlapping checking

[Yao Qi]

Given the example you posted,

> Let me correct my previous example. It should be
> (gdb) mem 0x50 0x80 ro
> (gdb) mem 0xffffff00 0 ro
> (gdb) mem 0x100 0x200 ro
> overlapping memory region
> 

It is a bug.


On Thursday, August 02, 2012 02:10:40 PM Wei-cheng Wang wrote:
> Wei-cheng
> 
> 2012-08-02  Wei-cheng Wang  <cole945@gmail.com>
> 
>         * memattr.c (create_mem_region): Fix memory-region overlapping
> checking in special case.
> 
> diff --git a/gdb/memattr.c b/gdb/memattr.c
> --- 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;

I read your patch, and draw some charts on paper to show the relationship of 
these four variables here for overlapping.

This line is to check the overlapping like,

  lo  n->lo  n->hi  hi

However, without your fix, the following case is treated as overlapping by 
mistake,

  lo  hi  n->lo  n->hi (0)

Your patch looks correct to me, however I am not the people to approve it.

This line of code was written in 2002, so a 10-year-old bug is fixed! :)

-- 
Yao (齐尧)

Re: [PATCH] Fix memory-region overlapping checking

[Wei-cheng Wang]

Hi Yao,

On Fri, Aug 3, 2012 at 6:53 PM, Yao Qi <yao@codesourcery.com> wrote:
> Your patch looks correct to me, however I am not the people to approve it.
> This line of code was written in 2002, so a 10-year-old bug is fixed! :)

Thanks for your review and confirming.
So how can I get it approved? Shall I report it in bug-gdb?

I found this bug simply because I am using this feature,
so I think it would be great if it could get fixed :)

Wei-cheng

Re: [PATCH] Fix memory-region overlapping checking

[Yao Qi]

On Tuesday, August 07, 2012 10:35:08 AM Wei-cheng Wang wrote:
> Thanks for your review and confirming.
> So how can I get it approved? Shall I report it in bug-gdb?

Wait to see if global maintainers have a chance to look at your patch.  If 
there is no response in one week, send a ping.

b.t.w, it is create if you can write some test cases in 
testsuite/gdb.base/memattr.exp to cover these corner cases of memory 
attributes.

-- 
Yao (齐尧)

Re: [PATCH] Fix memory-region overlapping checking

[Wei-cheng Wang]

Hi,

Here are the fix with test cases.


2012-08-08  Wei-cheng Wang  <cole945@gmail.com>

	* memattr.c: Fix memory region overlapping checking
---
diff --git a/gdb/memattr.c b/gdb/memattr.c
index ec7deb5..5396138 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;
--


2012-08-08  Wei-cheng Wang  <cole945@gmail.com>

	* gdb.base/memattr.exp: Add cases for overlapping checking.
---
diff --git a/gdb/testsuite/gdb.base/memattr.exp
b/gdb/testsuite/gdb.base/memattr.exp
index 4065808..f491b60 100644
--- a/gdb/testsuite/gdb.base/memattr.exp
+++ b/gdb/testsuite/gdb.base/memattr.exp
@@ -448,3 +448,46 @@ 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
+#
+#       lo'        hi'
+#        |---------|
+#  10 20 30 40 50 60 70 80
+#      |----|             FAIL
+#           |---|         FAIL
+#               |----|    FAIL
+#      |-------------|    FAIL
+#   |----|                PASS
+#                  |----| PASS
+
+proc delete_memory {} {
+    global gdb_prompt
+
+    send_gdb "delete mem\n"
+    gdb_expect 100 {
+	-re "Delete all memory regions.*y or n.*$" {
+	    send_gdb "y\n";
+	    exp_continue
+	}
+	-re "$gdb_prompt $" { }
+    }
+}
+
+# Test normal case (upper != 0)
+delete_memory
+send_gdb "mem 0x30 0x60 ro\n"; gdb_expect -re "$gdb_prompt $"
+gdb_test "mem 0x20 0x40 ro" "overlapping memory region"
+gdb_test "mem 0x40 0x50 ro" "overlapping memory region"
+gdb_test "mem 0x50 0x70 ro" "overlapping memory region"
+gdb_test_no_output "mem 0x10 0x30 ro"
+gdb_test_no_output "mem 0x60 0x80 ro"
+
+# Test sepcial case (upper == 0)
+delete_memory
+send_gdb "mem 0x30 0x0 ro\n"; gdb_expect -re "$gdb_prompt $"
+gdb_test "mem 0x20 0x40 ro" "overlapping memory region"
+gdb_test "mem 0x40 0x50 ro" "overlapping memory region"
+gdb_test "mem 0x50 0x70 ro" "overlapping memory region"
+gdb_test_no_output "mem 0x10 0x30 ro"
--

Re: [PATCH] Fix memory-region overlapping checking

[Yao Qi]

Wei-cheng,
Thanks for writing a test.  Some comments below,

On Tuesday, August 07, 2012 02:01:59 PM Wei-cheng Wang wrote:
> +
> +#
> +# Test overlapping checking
> +#
> +#       lo'        hi'
> +#        |---------|
> +#  10 20 30 40 50 60 70 80
> +#      |----|             FAIL
> +#           |---|         FAIL
> +#               |----|    FAIL
> +#      |-------------|    FAIL
> +#   |----|                PASS
> +#                  |----| PASS
> +
> +proc delete_memory {} {
> +    global gdb_prompt
> +
> +    send_gdb "delete mem\n"
> +    gdb_expect 100 {
> +       -re "Delete all memory regions.*y or n.*$" {
> +           send_gdb "y\n";
> +           exp_continue
> +       }
> +       -re "$gdb_prompt $" { }
> +    }

We can use gdb_test_multiple here.

> +}
> +
> +# Test normal case (upper != 0)
> +delete_memory
> +send_gdb "mem 0x30 0x60 ro\n"; gdb_expect -re "$gdb_prompt $"

We can use gdb_test_no_output "mem 0x30 0x60 ro"

> +gdb_test "mem 0x20 0x40 ro" "overlapping memory region"
> +gdb_test "mem 0x40 0x50 ro" "overlapping memory region"
> +gdb_test "mem 0x50 0x70 ro" "overlapping memory region"
> +gdb_test_no_output "mem 0x10 0x30 ro"
> +gdb_test_no_output "mem 0x60 0x80 ro"
> +
> +# Test sepcial case (upper == 0)
          ^^^^^^^ typo "special"

> +delete_memory
> +send_gdb "mem 0x30 0x0 ro\n"; gdb_expect -re "$gdb_prompt $"

gdb_test_no_output

> +gdb_test "mem 0x20 0x40 ro" "overlapping memory region"
> +gdb_test "mem 0x40 0x50 ro" "overlapping memory region"
> +gdb_test "mem 0x50 0x70 ro" "overlapping memory region"
> +gdb_test_no_output "mem 0x10 0x30 ro"

the test result in summary is *not* unique,
$ cat testsuite/gdb.sum  | grep "PASS" | sort | uniq -c | sort -n
....
      2 PASS: gdb.base/memattr.exp: mem 0x20 0x40 ro
      2 PASS: gdb.base/memattr.exp: mem 0x40 0x50 ro
      2 PASS: gdb.base/memattr.exp: mem 0x50 0x70 ro

See more about this rule here 
http://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique

Below is a new one fixed all the issues in your original patch.  What do you 
think?

Without your fix, the new test exposes a fail,

  FAIL: gdb.base/memattr.exp: 0x10 0x30 2

and fail is fixed with the patch applied.

-- 
Yao (齐尧)

2012-08-08  Wei-cheng Wang  <cole945@gmail.com>
		 Yao Qi <yao@codesourcery.com>

        * gdb.base/memattr.exp: Add cases for overlapping checking.

diff --git INDEX:/gdb/testsuite/gdb.base/memattr.exp 
--- INDEX:/gdb/testsuite/gdb.base/memattr.exp
+++ WORKDIR:/gdb/testsuite/gdb.base/memattr.exp
@@ -448,3 +448,45 @@ 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
+#
+#       lo'        hi'
+#        |---------|
+#  10 20 30 40 50 60 70 80
+#      |----|             FAIL
+#           |---|         FAIL
+#               |----|    FAIL
+#      |-------------|    FAIL
+#   |----|                PASS
+#                  |----| PASS
+
+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" "0x20 0x40 1"
+gdb_test "mem 0x40 0x50 ro" "overlapping memory region" "0x40 0x50 1"
+gdb_test "mem 0x50 0x70 ro" "overlapping memory region" "0x50 0x70 1"
+gdb_test_no_output "mem 0x10 0x30 ro" "0x10 0x30 1"
+gdb_test_no_output "mem 0x60 0x80 ro"
+
+# Test special case (upper == 0)
+delete_memory
+gdb_test_no_output "mem 0x30 0x0 ro"
+gdb_test "mem 0x20 0x40 ro" "overlapping memory region" "0x20 0x40 2"
+gdb_test "mem 0x40 0x50 ro" "overlapping memory region" "0x40 0x50 2"
+gdb_test "mem 0x50 0x70 ro" "overlapping memory region" "0x50 0x70 2"
+gdb_test_no_output "mem 0x10 0x30 ro" "0x10 0x30 2"

Re: [PATCH] Fix memory-region overlapping checking

[Wei-cheng Wang]

Hi Yao,

  Thanks for your correction.
  I wasn't aware of that rule and wondered what the message is used for.

  After I reviewed the cases, I think the cases should be altered a little.
  When testing against 0x30-0x0, 0x50-0x70 is no different than 0x40-0x50,
  so I remove it and add various boundary cases.

  Below is the altered one based on your fix.

--
Wei-cheng


2012-08-08  Wei-cheng Wang  <cole945@gmail.com>
	    Yao Qi <yao@codesourcery.com>

	* gdb.base/memattr.exp: Add cases for overlapping checking.

diff --git INDEX:/gdb/testsuite/gdb.base/memattr.exp
--- INDEX:/gdb/testsuite/gdb.base/memattr.exp
+++ WORKDIR:/gdb/testsuite/gdb.base/memattr.exp
@@ -448,3 +448,45 @@ 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
+#
+#       lo'        hi'
+#        |---------|
+#  10 20 30 40 50 60 70 80
+#      |----|             FAIL
+#        |--|             FAIL
+#           |---|         FAIL
+#               |--|      FAIL
+#               |----|    FAIL
+#        |---------|      FAIL
+#      |-------------|    FAIL
+#      |----------------- FAIL
+#           |------------ FAIL
+#                    |--- FAIL
+#   |----|                PASS
+#                  |----| PASS
+
+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
+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"

Re: [PATCH] Fix memory-region overlapping checking

[Pedro Alves]

On 08/16/2012 11:02 AM, Wei-cheng Wang wrote:
> Hi Yao,
> 
>   Thanks for your correction.
>   I wasn't aware of that rule and wondered what the message is used for.
> 
>   After I reviewed the cases, I think the cases should be altered a little.
>   When testing against 0x30-0x0, 0x50-0x70 is no different than 0x40-0x50,
>   so I remove it and add various boundary cases.
> 
>   Below is the altered one based on your fix.

I just tried this test out, and noticed that it doesn't produce any
fail without the gdb fix, while Yao's does.  Could you sort that
out, please?  Thanks!

-- 
Pedro Alves

Re: [PATCH] Fix memory-region overlapping checking

[Wei-cheng Wang]

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

Wei-cheng

---

2012-08-08  Wei-cheng Wang  <cole945@gmail.com>
            Yao Qi <yao@codesourcery.com>

        * gdb.base/memattr.exp: Add cases for overlapping checking.

		
diff --git a/gdb/testsuite/gdb.base/memattr.exp
b/gdb/testsuite/gdb.base/memattr.exp
index 4065808..024204d 100644
--- a/gdb/testsuite/gdb.base/memattr.exp
+++ b/gdb/testsuite/gdb.base/memattr.exp
@@ -448,3 +448,58 @@ 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
+#
+#       lo'        hi'
+#        |---------|
+#  10 20 30 40 50 60 70 80
+#      |----|             FAIL
+#        |--|             FAIL
+#           |---|         FAIL
+#               |--|      FAIL
+#               |----|    FAIL
+#        |---------|      FAIL
+#      |-------------|    FAIL
+#      |----------------- FAIL
+#           |------------ FAIL
+#                    |--- FAIL
+#   |----|                PASS
+#                  |----| PASS
+
+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
+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"

Re: [PATCH] Fix memory-region overlapping checking

[Pedro Alves]

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  <cole945@gmail.com>
>
> 	* 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  <cole945@gmail.com>
>             Yao Qi <yao@codesourcery.com>
> 
>         * 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  <cole945@gmail.com>

	* memattr.c (create_mem_region): Fix memory region overlapping
	checking.

2012-08-24  Wei-cheng Wang  <cole945@gmail.com>
            Yao Qi <yao@codesourcery.com>
            Pedro Alves <palves@redhat.com>

        * 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"
+}

Re: [PATCH] Fix memory-region overlapping checking

[Wei-cheng Wang]

On Fri, Aug 24, 2012 at 11:11 PM, Pedro Alves <palves@redhat.com> wrote:
> WDYT?
I think it's great!
Thank you and Yao.


Wei-cheng

Re: [PATCH] Fix memory-region overlapping checking

[Pedro Alves]

I've checked it in now.

-- 
Pedro Alves