[PATCH 0/2] Fix gdb.base/coremaker.c problems

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/2] Fix gdb.base/coremaker.c problems

Sourceware - gdb-patches mailing list
Two patches which fix gdb.base/coremaker.c problems encountered
while running gdb.base/corefile2.exp on ppc64le and x86_64/-m32.

Both patches have been tested together on x86_64, x86_64/-m32, ppc64le,
s390x, and aarch64, all running either Fedora 32 or rawhide.

Kevin Buettner (2):
  Fix gdb.base/corefile2.exp test case for ppc64le
  gdb.base/coremaker2.c: Fix compilation problems for x86_64 -m32
    multilib

 gdb/testsuite/gdb.base/coremaker2.c | 42 ++++++++++++++++-------------
 1 file changed, 23 insertions(+), 19 deletions(-)

--
2.26.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] Fix gdb.base/corefile2.exp test case for ppc64le

Sourceware - gdb-patches mailing list
It turns out that the recently added gdb.base/corefile2.exp test won't
run on ppc64le linux.  The test case fails the internal checks which
ensure that a mmap'd region can be placed within the statically
allocated regions buf_rw[] and buf_ro[].

ppc64le linux apparently has 64k pages, which is much larger than
the 24k regions originally allocated for buf_rw[] and buf_ro[].

This patch increases the size of each region to 256 KiB.

Tested on either rawhide or Fedora 32 for these architectures: x86_64,
x86_64/-m32, ppc64le, aarch64, and s390x.

gdb/testsuite/ChangeLog:

        * gdb.base/coremaker2.c (buf_rw): Increase size to 256 KiB.
        (C5_24k): Delete.
        (C5_8k, C5_64k, C5_256k): New macros.
        (buf_ro): Allocate 256 KiB of initialized data.
---
 gdb/testsuite/gdb.base/coremaker2.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/gdb/testsuite/gdb.base/coremaker2.c b/gdb/testsuite/gdb.base/coremaker2.c
index ecba247f50..3c89bd790b 100644
--- a/gdb/testsuite/gdb.base/coremaker2.c
+++ b/gdb/testsuite/gdb.base/coremaker2.c
@@ -47,12 +47,8 @@ unsigned long long addr;
 char *mbuf_ro;
 char *mbuf_rw;
 
-/* 24 KiB buffer.  */
-char buf_rw[24 * 1024];
-
-/* 24 KiB worth of data.  For this test case, we can't allocate a
-   buffer and then fill it; we want GDB to have to read this data
-   from the executable; it should NOT find it in the core file.  */
+/* 256 KiB buffer.  */
+char buf_rw[256 * 1024];
 
 #define C5_16 \
   0xc5, 0xc5, 0xc5, 0xc5, \
@@ -69,15 +65,22 @@ char buf_rw[24 * 1024];
 #define C5_1k \
   C5_256, C5_256, C5_256, C5_256
 
-#define C5_24k \
-  C5_1k, C5_1k, C5_1k, C5_1k, \
-  C5_1k, C5_1k, C5_1k, C5_1k, \
-  C5_1k, C5_1k, C5_1k, C5_1k, \
-  C5_1k, C5_1k, C5_1k, C5_1k, \
+#define C5_8k \
   C5_1k, C5_1k, C5_1k, C5_1k, \
   C5_1k, C5_1k, C5_1k, C5_1k
 
-const char buf_ro[] = { C5_24k };
+#define C5_64k \
+  C5_8k, C5_8k, C5_8k, C5_8k, \
+  C5_8k, C5_8k, C5_8k, C5_8k
+
+#define C5_256k \
+  C5_64k, C5_64k, C5_64k, C5_64k
+
+/* 256 KiB worth of data.  For this test case, we can't allocate a
+   buffer and then fill it; we want GDB to have to read this data
+   from the executable; it should NOT find it in the core file.  */
+
+const char buf_ro[] = { C5_256k };
 
 int
 main (int argc, char **argv)
--
2.26.2

Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] gdb.base/coremaker2.c: Fix compilation problems for x86_64 -m32 multilib

Sourceware - gdb-patches mailing list
In reply to this post by Sourceware - gdb-patches mailing list
There are compilation warnings / errors when compiling coremaker2.c
for the gdb.base/corefile2.exp tests.  Here's the command to use
on x86_64 linux:

make check RUNTESTFLAGS="--target_board unix/-m32" \
           TESTS="gdb.base/corefile2.exp"

These are the warnings / errors - I've shortened the paths somewhat:

gdb compile failed, gdb/testsuite/gdb.base/coremaker2.c: In function 'main':
gdb/testsuite/gdb.base/coremaker2.c:106:11: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
  106 |   addr = ((unsigned long long) buf_ro + pagesize) & ~(pagesize - 1);
      |           ^
gdb/testsuite/gdb.base/coremaker2.c:108:15: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
  108 |   if (addr <= (unsigned long long) buf_ro
      |               ^
gdb/testsuite/gdb.base/coremaker2.c:109:18: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
  109 |       || addr >= (unsigned long long) buf_ro + sizeof (buf_ro))
      |                  ^
gdb/testsuite/gdb.base/coremaker2.c:115:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
  115 |   mbuf_ro = mmap ((void *) addr, pagesize, PROT_READ,
      |                   ^
gdb/testsuite/gdb.base/coremaker2.c:130:11: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
  130 |   addr = ((unsigned long long) buf_rw + pagesize) & ~(pagesize - 1);
      |           ^
gdb/testsuite/gdb.base/coremaker2.c:132:15: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
  132 |   if (addr <= (unsigned long long) buf_rw
      |               ^
gdb/testsuite/gdb.base/coremaker2.c:133:18: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
  133 |       || addr >= (unsigned long long) buf_rw + sizeof (buf_rw))
      |                  ^
gdb/testsuite/gdb.base/coremaker2.c:139:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
  139 |   mbuf_rw = mmap ((void *) addr, pagesize, PROT_READ,
      |                   ^

These were fixed by changing unsigned long long to size_t.

Tested on Fedora 32 with architectures: x86_64, x86_64/-m32, aarch64,
s390x, and ppc64le.

gdb/testsuite/ChangeLog:

        * gdb.base/coremaker2.c: Change all uses of 'unsigned long long'
        to 'size_t'
        (stddef.h): Include.
---
 gdb/testsuite/gdb.base/coremaker2.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/gdb.base/coremaker2.c b/gdb/testsuite/gdb.base/coremaker2.c
index 3c89bd790b..29649b84e9 100644
--- a/gdb/testsuite/gdb.base/coremaker2.c
+++ b/gdb/testsuite/gdb.base/coremaker2.c
@@ -39,11 +39,12 @@
 #include <unistd.h>
 #include <string.h>
 #include <errno.h>
+#include <stddef.h>
 
 /* These are globals so that we can find them easily when debugging
    the core file.  */
 long pagesize;
-unsigned long long addr;
+size_t addr;
 char *mbuf_ro;
 char *mbuf_rw;
 
@@ -106,10 +107,10 @@ main (int argc, char **argv)
     }
 
   /* Compute an address that should be within buf_ro.  Complain if not.  */
-  addr = ((unsigned long long) buf_ro + pagesize) & ~(pagesize - 1);
+  addr = ((size_t) buf_ro + pagesize) & ~(pagesize - 1);
 
-  if (addr <= (unsigned long long) buf_ro
-      || addr >= (unsigned long long) buf_ro + sizeof (buf_ro))
+  if (addr <= (size_t) buf_ro
+      || addr >= (size_t) buf_ro + sizeof (buf_ro))
     {
       fprintf (stderr, "Unable to compute a suitable address within buf_ro.\n");
       exit (1);
@@ -130,10 +131,10 @@ main (int argc, char **argv)
 
   /* Compute an mmap address within buf_rw.  Complain if it's somewhere
      else.  */
-  addr = ((unsigned long long) buf_rw + pagesize) & ~(pagesize - 1);
+  addr = ((size_t) buf_rw + pagesize) & ~(pagesize - 1);
 
-  if (addr <= (unsigned long long) buf_rw
-      || addr >= (unsigned long long) buf_rw + sizeof (buf_rw))
+  if (addr <= (size_t) buf_rw
+      || addr >= (size_t) buf_rw + sizeof (buf_rw))
     {
       fprintf (stderr, "Unable to compute a suitable address within buf_rw.\n");
       exit (1);
--
2.26.2

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Fix gdb.base/coremaker.c problems

Sourceware - gdb-patches mailing list
In reply to this post by Sourceware - gdb-patches mailing list
On Thu, 30 Jul 2020 21:21:55 -0700
Kevin Buettner <[hidden email]> wrote:

> Two patches which fix gdb.base/coremaker.c problems encountered

Ahem, that should have been coremaker2.c in both the subject and
the description.

Kevin

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] gdb.base/coremaker2.c: Fix compilation problems for x86_64 -m32 multilib

Simon Marchi-4
In reply to this post by Sourceware - gdb-patches mailing list
On 2020-07-31 12:21 a.m., Kevin Buettner via Gdb-patches wrote:

> There are compilation warnings / errors when compiling coremaker2.c
> for the gdb.base/corefile2.exp tests.  Here's the command to use
> on x86_64 linux:
>
> make check RUNTESTFLAGS="--target_board unix/-m32" \
>            TESTS="gdb.base/corefile2.exp"
>
> These are the warnings / errors - I've shortened the paths somewhat:
>
> gdb compile failed, gdb/testsuite/gdb.base/coremaker2.c: In function 'main':
> gdb/testsuite/gdb.base/coremaker2.c:106:11: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>   106 |   addr = ((unsigned long long) buf_ro + pagesize) & ~(pagesize - 1);
>       |           ^
> gdb/testsuite/gdb.base/coremaker2.c:108:15: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>   108 |   if (addr <= (unsigned long long) buf_ro
>       |               ^
> gdb/testsuite/gdb.base/coremaker2.c:109:18: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>   109 |       || addr >= (unsigned long long) buf_ro + sizeof (buf_ro))
>       |                  ^
> gdb/testsuite/gdb.base/coremaker2.c:115:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
>   115 |   mbuf_ro = mmap ((void *) addr, pagesize, PROT_READ,
>       |                   ^
> gdb/testsuite/gdb.base/coremaker2.c:130:11: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>   130 |   addr = ((unsigned long long) buf_rw + pagesize) & ~(pagesize - 1);
>       |           ^
> gdb/testsuite/gdb.base/coremaker2.c:132:15: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>   132 |   if (addr <= (unsigned long long) buf_rw
>       |               ^
> gdb/testsuite/gdb.base/coremaker2.c:133:18: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>   133 |       || addr >= (unsigned long long) buf_rw + sizeof (buf_rw))
>       |                  ^
> gdb/testsuite/gdb.base/coremaker2.c:139:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
>   139 |   mbuf_rw = mmap ((void *) addr, pagesize, PROT_READ,
>       |                   ^
>
> These were fixed by changing unsigned long long to size_t.
>
> Tested on Fedora 32 with architectures: x86_64, x86_64/-m32, aarch64,
> s390x, and ppc64le.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.base/coremaker2.c: Change all uses of 'unsigned long long'
> to 'size_t'
> (stddef.h): Include.
> ---
>  gdb/testsuite/gdb.base/coremaker2.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/coremaker2.c b/gdb/testsuite/gdb.base/coremaker2.c
> index 3c89bd790b..29649b84e9 100644
> --- a/gdb/testsuite/gdb.base/coremaker2.c
> +++ b/gdb/testsuite/gdb.base/coremaker2.c
> @@ -39,11 +39,12 @@
>  #include <unistd.h>
>  #include <string.h>
>  #include <errno.h>
> +#include <stddef.h>
>  
>  /* These are globals so that we can find them easily when debugging
>     the core file.  */
>  long pagesize;
> -unsigned long long addr;
> +size_t addr;
>  char *mbuf_ro;
>  char *mbuf_rw;
>  
> @@ -106,10 +107,10 @@ main (int argc, char **argv)
>      }
>  
>    /* Compute an address that should be within buf_ro.  Complain if not.  */
> -  addr = ((unsigned long long) buf_ro + pagesize) & ~(pagesize - 1);
> +  addr = ((size_t) buf_ro + pagesize) & ~(pagesize - 1);
>  
> -  if (addr <= (unsigned long long) buf_ro
> -      || addr >= (unsigned long long) buf_ro + sizeof (buf_ro))
> +  if (addr <= (size_t) buf_ro
> +      || addr >= (size_t) buf_ro + sizeof (buf_ro))
>      {
>        fprintf (stderr, "Unable to compute a suitable address within buf_ro.\n");
>        exit (1);
> @@ -130,10 +131,10 @@ main (int argc, char **argv)
>  
>    /* Compute an mmap address within buf_rw.  Complain if it's somewhere
>       else.  */
> -  addr = ((unsigned long long) buf_rw + pagesize) & ~(pagesize - 1);
> +  addr = ((size_t) buf_rw + pagesize) & ~(pagesize - 1);
>  
> -  if (addr <= (unsigned long long) buf_rw
> -      || addr >= (unsigned long long) buf_rw + sizeof (buf_rw))
> +  if (addr <= (size_t) buf_rw
> +      || addr >= (size_t) buf_rw + sizeof (buf_rw))
>      {
>        fprintf (stderr, "Unable to compute a suitable address within buf_rw.\n");
>        exit (1);
> --
> 2.26.2
>

Not sure if it will make a difference in practice, but I think uintptr_t would be more
appropriate than size_t, since you are converting an address (pointer) to an integer.

https://stackoverflow.com/questions/1464174/size-t-vs-uintptr-t

Otherwise, both of your patches LGTM.

Simon
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] gdb.base/coremaker2.c: Fix compilation problems for x86_64 -m32 multilib

Sourceware - gdb-patches mailing list
On Fri, 31 Jul 2020 11:31:33 -0400
Simon Marchi <[hidden email]> wrote:
 
> Not sure if it will make a difference in practice, but I think uintptr_t would be more
> appropriate than size_t, since you are converting an address (pointer) to an integer.
>
> https://stackoverflow.com/questions/1464174/size-t-vs-uintptr-t
>
> Otherwise, both of your patches LGTM.

I made the uintptr_t change and have pushed those commits.

Kevin