[PATCH] Fix various procfs.c compilation errors

[PATCH] Fix various procfs.c compilation errors

[Rainer Orth]

[-- Attachment #1: Type: text/plain, Size: 3779 bytes --]

procfs.c has accumulated several compilation errors lately (some of them
new with GCC 12), which are fixed by this patch:

* auxv_parse gets:

/vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:144:7: error: ‘int procfs_target::auxv_parse(gdb_byte**, gdb_byte*, CORE_ADDR*, CORE_ADDR*)’ marked ‘override’, but does not override
  144 |   int auxv_parse (gdb_byte **readptr,
      |       ^~~~~~~~~~

  Obviouly, procfs.c was missed in the auxv_parse constification.

* dead_procinfo has:

/vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c: In function ‘void dead_procinfo(procinfo*, const char*, int)’:
/vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:563:11: warning: the address of ‘procinfo::pathname’ will never be NULL [-Waddress]
  563 |   if (pi->pathname)
      |       ~~~~^~~~~~~~
/vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:238:8: note: ‘procinfo::pathname’ declared here
  238 |   char pathname[MAX_PROC_NAME_SIZE];    /* Pathname to /proc entry */
      |        ^~~~~~~~

  The warning is correct, so the code can lose support for the NULL
  pathname case.

* create_inferior has this ugly warning:

/vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c: In member function ‘virtual void procfs_target::create_inferior(const char*, const std::string&, char**, int)’:
/vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:2815:19: warning: ‘char* std::strncpy(char*, const char*, size_t)’ output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
 2815 |           strncpy (tryname, p, len);
      |           ~~~~~~~~^~~~~~~~~~~~~~~~~
/vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:2814:26: note: length computed here
 2814 |             len = strlen (p);
      |                   ~~~~~~~^~~

  It seems that this is another case of GCC PR middle-end/88059, which
  Martin Sebor refuses to fix.  So I'm using the hack suggested in the
  PR to use memcpy instead of strncpy.

* find_memory_regions_callback fails with

/vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c: In function ‘int find_memory_regions_callback(prmap*, find_memory_region_ftype, void*)’:
/vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:3167:18: error: too few arguments to function
 3167 |   return (*func) ((CORE_ADDR) map->pr_vaddr,
      |          ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
 3168 |                   map->pr_size,
      |                   ~~~~~~~~~~~~~
 3169 |                   (map->pr_mflags & MA_READ) != 0,
      |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 3170 |                   (map->pr_mflags & MA_WRITE) != 0,
      |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 3171 |                   (map->pr_mflags & MA_EXEC) != 0,
      |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 3172 |                   1, /* MODIFIED is unknown, pass it as true.  */
      |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 3173 |                   data);
      |                   ~~~~~

  Again, procfs.c was overlooked when adding the new memory_tagged arg.
  Unfortunately, it wasn't even documented in gdb/defs.h when it was
  added in

commit 68cffbbd4406b4efe1aa6e18460b1d7ca02549f1
Author: Luis Machado <luis.machado@arm.com>
Date:   Thu Mar 31 11:42:35 2022 +0100

    [AArch64] MTE corefile support

With those changes, procfs.c compiles again.  Together with the hack
from the Solaris gdbsupport breakage reported in PR build/29791, I was
able to build and test gdb on both amd64-pc-solaris2.11 and
sparcv9-sun-solaris2.11.

Will commit the patch soon.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sol2-procfs-various.patch --]
[-- Type: text/x-patch, Size: 1945 bytes --]

diff --git a/gdb/procfs.c b/gdb/procfs.c
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -141,8 +141,8 @@ public:
   bool info_proc (const char *, enum info_proc_what) override;
 
 #if PR_MODEL_NATIVE == PR_MODEL_LP64
-  int auxv_parse (gdb_byte **readptr,
-		  gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
+  int auxv_parse (const gdb_byte **readptr,
+		  const gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
     override;
 #endif
 
@@ -169,11 +169,12 @@ static procfs_target the_procfs_target;
    is presented in 64-bit format.  We need to provide a custom parser
    to handle that.  */
 int
-procfs_target::auxv_parse (gdb_byte **readptr,
-			   gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
+procfs_target::auxv_parse (const gdb_byte **readptr,
+			   const gdb_byte *endptr, CORE_ADDR *typep,
+			   CORE_ADDR *valp)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
-  gdb_byte *ptr = *readptr;
+  const gdb_byte *ptr = *readptr;
 
   if (endptr == ptr)
     return 0;
@@ -559,15 +560,7 @@ enum { NOKILL, KILL };
 static void
 dead_procinfo (procinfo *pi, const char *msg, int kill_p)
 {
-  char procfile[80];
-
-  if (pi->pathname)
-    print_sys_errmsg (pi->pathname, errno);
-  else
-    {
-      xsnprintf (procfile, sizeof (procfile), "process %d", pi->pid);
-      print_sys_errmsg (procfile, errno);
-    }
+  print_sys_errmsg (pi->pathname, errno);
   if (kill_p == KILL)
     kill (pi->pid, SIGKILL);
 
@@ -2813,7 +2806,7 @@ procfs_target::create_inferior (const ch
 	    len = p1 - p;
 	  else
 	    len = strlen (p);
-	  strncpy (tryname, p, len);
+	  memcpy (tryname, p, len);
 	  tryname[len] = '\0';
 	  strcat (tryname, "/");
 	  strcat (tryname, shell_file);
@@ -3170,6 +3163,7 @@ find_memory_regions_callback (struct prm
 		  (map->pr_mflags & MA_WRITE) != 0,
 		  (map->pr_mflags & MA_EXEC) != 0,
 		  1, /* MODIFIED is unknown, pass it as true.  */
+		  false,
 		  data);
 }
 

Re: [PATCH] Fix various procfs.c compilation errors

[Simon Marchi]

On 11/16/22 10:02, Rainer Orth wrote:
> procfs.c has accumulated several compilation errors lately (some of them
> new with GCC 12), which are fixed by this patch:
> 
> * auxv_parse gets:
> 
> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:144:7: error: ‘int procfs_target::auxv_parse(gdb_byte**, gdb_byte*, CORE_ADDR*, CORE_ADDR*)’ marked ‘override’, but does not override
>   144 |   int auxv_parse (gdb_byte **readptr,
>       |       ^~~~~~~~~~
> 
>   Obviouly, procfs.c was missed in the auxv_parse constification.

Sorry, my bad for this one.

> 
> * dead_procinfo has:
> 
> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c: In function ‘void dead_procinfo(procinfo*, const char*, int)’:
> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:563:11: warning: the address of ‘procinfo::pathname’ will never be NULL [-Waddress]
>   563 |   if (pi->pathname)
>       |       ~~~~^~~~~~~~
> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:238:8: note: ‘procinfo::pathname’ declared here
>   238 |   char pathname[MAX_PROC_NAME_SIZE];    /* Pathname to /proc entry */
>       |        ^~~~~~~~
> 
>   The warning is correct, so the code can lose support for the NULL
>   pathname case.
> 
> * create_inferior has this ugly warning:
> 
> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c: In member function ‘virtual void procfs_target::create_inferior(const char*, const std::string&, char**, int)’:
> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:2815:19: warning: ‘char* std::strncpy(char*, const char*, size_t)’ output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
>  2815 |           strncpy (tryname, p, len);
>       |           ~~~~~~~~^~~~~~~~~~~~~~~~~
> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:2814:26: note: length computed here
>  2814 |             len = strlen (p);
>       |                   ~~~~~~~^~~
> 
>   It seems that this is another case of GCC PR middle-end/88059, which
>   Martin Sebor refuses to fix.  So I'm using the hack suggested in the
>   PR to use memcpy instead of strncpy.

You could also make the code use std::string to build tryname, it would
be simpler and less error-prone.  But I'm fine with what you have too.

> 
> * find_memory_regions_callback fails with
> 
> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c: In function ‘int find_memory_regions_callback(prmap*, find_memory_region_ftype, void*)’:
> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:3167:18: error: too few arguments to function
>  3167 |   return (*func) ((CORE_ADDR) map->pr_vaddr,
>       |          ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
>  3168 |                   map->pr_size,
>       |                   ~~~~~~~~~~~~~
>  3169 |                   (map->pr_mflags & MA_READ) != 0,
>       |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  3170 |                   (map->pr_mflags & MA_WRITE) != 0,
>       |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  3171 |                   (map->pr_mflags & MA_EXEC) != 0,
>       |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  3172 |                   1, /* MODIFIED is unknown, pass it as true.  */
>       |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  3173 |                   data);
>       |                   ~~~~~
> 
>   Again, procfs.c was overlooked when adding the new memory_tagged arg.
>   Unfortunately, it wasn't even documented in gdb/defs.h when it was
>   added in
> 
> commit 68cffbbd4406b4efe1aa6e18460b1d7ca02549f1
> Author: Luis Machado <luis.machado@arm.com>
> Date:   Thu Mar 31 11:42:35 2022 +0100
> 
>     [AArch64] MTE corefile support
> 
> With those changes, procfs.c compiles again.  Together with the hack
> from the Solaris gdbsupport breakage reported in PR build/29791, I was
> able to build and test gdb on both amd64-pc-solaris2.11 and
> sparcv9-sun-solaris2.11.
> 
> Will commit the patch soon.
> 
> 	Rainer

Thanks, you can add my:

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon

Re: [PATCH] Fix various procfs.c compilation errors

[Rainer Orth]

Hi Simon,

> On 11/16/22 10:02, Rainer Orth wrote:
>> procfs.c has accumulated several compilation errors lately (some of them
>> new with GCC 12), which are fixed by this patch:
>> 
>> * auxv_parse gets:
>> 
>> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:144:7: error: ‘int
>> procfs_target::auxv_parse(gdb_byte**, gdb_byte*, CORE_ADDR*, CORE_ADDR*)’
>> marked ‘override’, but does not override
>>   144 |   int auxv_parse (gdb_byte **readptr,
>>       |       ^~~~~~~~~~
>> 
>>   Obviouly, procfs.c was missed in the auxv_parse constification.
>
> Sorry, my bad for this one.

no worries while the fixes are as easy as this one.  I thought about
reviving the old Solaris buildbots with the new buildmaster to catch
stuff like this early, but am uncertain: the old buildbods had been
compile-only for various reasons, but ISTM that the new ones need to be
compile-and-test, and zero-FAIL actually, which is completely
unattainable for Solaris right now.

Thanks.
	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] Fix various procfs.c compilation errors

[Tom Tromey]

>>>>> "Rainer" == Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

Rainer> procfs.c has accumulated several compilation errors lately (some of them
Rainer> new with GCC 12), which are fixed by this patch:

Thank you.

Rainer>   It seems that this is another case of GCC PR middle-end/88059, which
Rainer>   Martin Sebor refuses to fix.  So I'm using the hack suggested in the
Rainer>   PR to use memcpy instead of strncpy.

FWIW it's also fine to change this code to use std::string, which is
probably both simpler to understand and will avoid warnings.  Up to you
though.

Tom

Re: [PATCH] Fix various procfs.c compilation errors

[Rainer Orth]

Hi Tom,

>>>>>> "Rainer" == Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:
>
> Rainer> procfs.c has accumulated several compilation errors lately (some of them
> Rainer> new with GCC 12), which are fixed by this patch:
>
> Thank you.
>
> Rainer>   It seems that this is another case of GCC PR middle-end/88059, which
> Rainer>   Martin Sebor refuses to fix.  So I'm using the hack suggested in the
> Rainer>   PR to use memcpy instead of strncpy.
>
> FWIW it's also fine to change this code to use std::string, which is
> probably both simpler to understand and will avoid warnings.  Up to you
> though.

I'll keep it as-is for now, to have a minimal patch ATM.

Thanks.
        Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] Fix various procfs.c compilation errors

[Luis Machado]

On 11/16/22 15:02, Rainer Orth wrote:
> procfs.c has accumulated several compilation errors lately (some of them
> new with GCC 12), which are fixed by this patch:
> 
> * auxv_parse gets:
> 
> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:144:7: error: ‘int procfs_target::auxv_parse(gdb_byte**, gdb_byte*, CORE_ADDR*, CORE_ADDR*)’ marked ‘override’, but does not override
>    144 |   int auxv_parse (gdb_byte **readptr,
>        |       ^~~~~~~~~~
> 
>    Obviouly, procfs.c was missed in the auxv_parse constification.
> 
> * dead_procinfo has:
> 
> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c: In function ‘void dead_procinfo(procinfo*, const char*, int)’:
> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:563:11: warning: the address of ‘procinfo::pathname’ will never be NULL [-Waddress]
>    563 |   if (pi->pathname)
>        |       ~~~~^~~~~~~~
> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:238:8: note: ‘procinfo::pathname’ declared here
>    238 |   char pathname[MAX_PROC_NAME_SIZE];    /* Pathname to /proc entry */
>        |        ^~~~~~~~
> 
>    The warning is correct, so the code can lose support for the NULL
>    pathname case.
> 
> * create_inferior has this ugly warning:
> 
> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c: In member function ‘virtual void procfs_target::create_inferior(const char*, const std::string&, char**, int)’:
> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:2815:19: warning: ‘char* std::strncpy(char*, const char*, size_t)’ output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
>   2815 |           strncpy (tryname, p, len);
>        |           ~~~~~~~~^~~~~~~~~~~~~~~~~
> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:2814:26: note: length computed here
>   2814 |             len = strlen (p);
>        |                   ~~~~~~~^~~
> 
>    It seems that this is another case of GCC PR middle-end/88059, which
>    Martin Sebor refuses to fix.  So I'm using the hack suggested in the
>    PR to use memcpy instead of strncpy.
> 
> * find_memory_regions_callback fails with
> 
> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c: In function ‘int find_memory_regions_callback(prmap*, find_memory_region_ftype, void*)’:
> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:3167:18: error: too few arguments to function
>   3167 |   return (*func) ((CORE_ADDR) map->pr_vaddr,
>        |          ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
>   3168 |                   map->pr_size,
>        |                   ~~~~~~~~~~~~~
>   3169 |                   (map->pr_mflags & MA_READ) != 0,
>        |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   3170 |                   (map->pr_mflags & MA_WRITE) != 0,
>        |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   3171 |                   (map->pr_mflags & MA_EXEC) != 0,
>        |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   3172 |                   1, /* MODIFIED is unknown, pass it as true.  */
>        |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   3173 |                   data);
>        |                   ~~~~~
> 
>    Again, procfs.c was overlooked when adding the new memory_tagged arg.
>    Unfortunately, it wasn't even documented in gdb/defs.h when it was
>    added in
> 

Sorry, that was an oversight. I failed to updated all the hooks. I think there was a BSD one that got fixed after the following commit was pushed.

> commit 68cffbbd4406b4efe1aa6e18460b1d7ca02549f1
> Author: Luis Machado <luis.machado@arm.com>
> Date:   Thu Mar 31 11:42:35 2022 +0100
> 
>      [AArch64] MTE corefile support
> 
> With those changes, procfs.c compiles again.  Together with the hack
> from the Solaris gdbsupport breakage reported in PR build/29791, I was
> able to build and test gdb on both amd64-pc-solaris2.11 and
> sparcv9-sun-solaris2.11.
> 
> Will commit the patch soon.
> 
> 	Rainer
> 

Re: [PATCH] Fix various procfs.c compilation errors

[Rainer Orth]

Hi Luis,

>> * find_memory_regions_callback fails with
>> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c: In function ‘int
>> find_memory_regions_callback(prmap*, find_memory_region_ftype, void*)’:
>> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:3167:18: error: too few arguments to function
>>   3167 |   return (*func) ((CORE_ADDR) map->pr_vaddr,
>>        |          ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
>>   3168 |                   map->pr_size,
>>        |                   ~~~~~~~~~~~~~
>>   3169 |                   (map->pr_mflags & MA_READ) != 0,
>>        |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>   3170 |                   (map->pr_mflags & MA_WRITE) != 0,
>>        |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>   3171 |                   (map->pr_mflags & MA_EXEC) != 0,
>>        |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>   3172 |                   1, /* MODIFIED is unknown, pass it as true.  */
>>        |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>   3173 |                   data);
>>        |                   ~~~~~
>>    Again, procfs.c was overlooked when adding the new memory_tagged arg.
>>    Unfortunately, it wasn't even documented in gdb/defs.h when it was
>>    added in
>> 
>
> Sorry, that was an oversight. I failed to updated all the hooks. I think
> there was a BSD one that got fixed after the following commit was pushed.

no problem: the primary issue was finding/understanding what the new arg
meant/needs to be here.

Could you take care of updating the defs.h comment to avoid such
confusion in the future?

Thanks.
	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] Fix various procfs.c compilation errors

[Luis Machado]

On 11/17/22 09:50, Rainer Orth wrote:
> Hi Luis,
> 
>>> * find_memory_regions_callback fails with
>>> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c: In function ‘int
>>> find_memory_regions_callback(prmap*, find_memory_region_ftype, void*)’:
>>> /vol/src/gnu/gdb/hg/master/dist/gdb/procfs.c:3167:18: error: too few arguments to function
>>>    3167 |   return (*func) ((CORE_ADDR) map->pr_vaddr,
>>>         |          ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>    3168 |                   map->pr_size,
>>>         |                   ~~~~~~~~~~~~~
>>>    3169 |                   (map->pr_mflags & MA_READ) != 0,
>>>         |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>    3170 |                   (map->pr_mflags & MA_WRITE) != 0,
>>>         |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>    3171 |                   (map->pr_mflags & MA_EXEC) != 0,
>>>         |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>    3172 |                   1, /* MODIFIED is unknown, pass it as true.  */
>>>         |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>    3173 |                   data);
>>>         |                   ~~~~~
>>>     Again, procfs.c was overlooked when adding the new memory_tagged arg.
>>>     Unfortunately, it wasn't even documented in gdb/defs.h when it was
>>>     added in
>>>
>>
>> Sorry, that was an oversight. I failed to updated all the hooks. I think
>> there was a BSD one that got fixed after the following commit was pushed.
> 
> no problem: the primary issue was finding/understanding what the new arg
> meant/needs to be here.
> 
> Could you take care of updating the defs.h comment to avoid such
> confusion in the future?

Surely.

> 
> Thanks.
> 	Rainer
>