[PATCH] [PR corefiles/32441] Fix segfault if target_fileio_read_alloc fails

[PATCH] [PR corefiles/32441] Fix segfault if target_fileio_read_alloc fails

[brandon.belew]

Check for target_fileio_read_alloc failure in linux_fill_prpsinfo
before dereferencing buffer. This fixes a segfault in the 'gcore'
command when attached to certain remote targets.
---
This is my first contribution to GDB, and my first use of
git-send-email, so please let me know if this is formatted
incorrectly! I initially submitted the bug and a v1 patch at
https://sourceware.org/bugzilla/show_bug.cgi?id=32441 and received the
following from Thiago Bauermann:

> Thank you for the patch. In general it looks good to me, just a couple of minor
> comments:
>
> 1. Since target_fileio_read_alloc () returns LONGEST, I think it's better if
> the buf_len variable also has that type.

I decided to stick with ssize_t for the variable, as this matches the
usage elsewhere in linux-tdep.c in linux_info_proc (which already was
correctly checking the length).

> 2. GDB is (very) slowly transitioning from C to C++. We currently prefer to use
> nullptr rather than NULL, so I suggest using this patch as an opportunity to
> change NULL to nullptr in lines 1876, 1877 and 1879.

I made the requested NULL -> nullptr changes.

Let me know if this is good or if I need to make any changes in my
workflow to adhere to GNU or gdb project conventions.

 gdb/linux-tdep.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index d3452059ce2..c10c4c76451 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -1867,17 +1867,17 @@ linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
   /* The number of fields read by `sscanf'.  */
   int n_fields = 0;

-  gdb_assert (p != NULL);
+  gdb_assert (p != nullptr);

   /* Obtaining PID and filename.  */
   pid = inferior_ptid.pid ();
   xsnprintf (filename, sizeof (filename), "/proc/%d/cmdline", (int) pid);
   /* The full name of the program which generated the corefile.  */
-  gdb_byte *buf = NULL;
-  size_t buf_len = target_fileio_read_alloc (NULL, filename, &buf);
+  gdb_byte *buf = nullptr;
+  ssize_t buf_len = target_fileio_read_alloc (nullptr, filename, &buf);
   gdb::unique_xmalloc_ptr<char> fname ((char *)buf);

-  if (buf_len < 1 || fname.get ()[0] == '\0')
+  if (buf_len < 1 || fname.get () == nullptr || fname.get ()[0] == '\0')
     {
       /* No program name was read, so we won't be able to retrieve more
 	 information about the process.  */
--
2.46.0

Re: [PATCH] [PR corefiles/32441] Fix segfault if target_fileio_read_alloc fails

[Luis Machado]

Hi Brandon,

Sorry for the late reply. Again, feel free to ping it if it takes over a week without
a reply.

On 12/20/24 22:17, brandon.belew wrote:
> Check for target_fileio_read_alloc failure in linux_fill_prpsinfo
> before dereferencing buffer. This fixes a segfault in the 'gcore'
> command when attached to certain remote targets.
> ---
> This is my first contribution to GDB, and my first use of
> git-send-email, so please let me know if this is formatted
> incorrectly! I initially submitted the bug and a v1 patch at
> https://sourceware.org/bugzilla/show_bug.cgi?id=32441 and received the
> following from Thiago Bauermann:


Formatting-wise, The commit message goes into the patch itself. See other examples
on the list.

Also, since you've opened a bug, we have hooks to refer to the bug. For instance,
for commit ca263aec20adfffe6f9dab3a18f8a7b24667f99c.

PR testsuite/32489
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32489

> 
>> Thank you for the patch. In general it looks good to me, just a couple of minor
>> comments:
>>
>> 1. Since target_fileio_read_alloc () returns LONGEST, I think it's better if
>> the buf_len variable also has that type.
> 
> I decided to stick with ssize_t for the variable, as this matches the
> usage elsewhere in linux-tdep.c in linux_info_proc (which already was
> correctly checking the length).
> 
>> 2. GDB is (very) slowly transitioning from C to C++. We currently prefer to use
>> nullptr rather than NULL, so I suggest using this patch as an opportunity to
>> change NULL to nullptr in lines 1876, 1877 and 1879.
> 
> I made the requested NULL -> nullptr changes.
> 
> Let me know if this is good or if I need to make any changes in my
> workflow to adhere to GNU or gdb project conventions.
> 
>  gdb/linux-tdep.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index d3452059ce2..c10c4c76451 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -1867,17 +1867,17 @@ linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
>    /* The number of fields read by `sscanf'.  */
>    int n_fields = 0;
> 
> -  gdb_assert (p != NULL);
> +  gdb_assert (p != nullptr);
> 
>    /* Obtaining PID and filename.  */
>    pid = inferior_ptid.pid ();
>    xsnprintf (filename, sizeof (filename), "/proc/%d/cmdline", (int) pid);
>    /* The full name of the program which generated the corefile.  */
> -  gdb_byte *buf = NULL;
> -  size_t buf_len = target_fileio_read_alloc (NULL, filename, &buf);
> +  gdb_byte *buf = nullptr;
> +  ssize_t buf_len = target_fileio_read_alloc (nullptr, filename, &buf);
>    gdb::unique_xmalloc_ptr<char> fname ((char *)buf);
> 
> -  if (buf_len < 1 || fname.get ()[0] == '\0')
> +  if (buf_len < 1 || fname.get () == nullptr || fname.get ()[0] == '\0')
>      {
>        /* No program name was read, so we won't be able to retrieve more
>  	 information about the process.  */
> --
> 2.46.0

The change itself looks OK to me. I'd like another pair of eyes to look at
it before we approve it. I think this can go in as a trivial change when it
gets approved.

Reviewed-By: Luis Machado <luis.machado@arm.com>

Re: [PATCH] [PR corefiles/32441] Fix segfault if target_fileio_read_alloc fails

[Brandon Belew]

Thank you Luis for reviewing and responding to my comments in IRC! Just a few
more questions: 

Luis Machado <luis.machado@arm.com> writes:
> On 12/20/24 22:17, brandon.belew wrote:
>> Check for target_fileio_read_alloc failure in linux_fill_prpsinfo
>> before dereferencing buffer. This fixes a segfault in the 'gcore'
>> command when attached to certain remote targets.
>> ---
>> This is my first contribution to GDB, and my first use of
>> git-send-email, so please let me know if this is formatted
>> incorrectly! I initially submitted the bug and a v1 patch at
>> https://sourceware.org/bugzilla/show_bug.cgi?id=32441 and received the
>> following from Thiago Bauermann:
>
> Formatting-wise, The commit message goes into the patch itself. See other examples
> on the list.

It was my understanding of git-format-patch / git-send-email (and in
reverse, git-am) that the commit message would be taken from everything
up to the '---'. Then everything between the '---' and the actual diff
was considered "timely commentary" and would not be present in the
commit message. This typically contains the diffstat output but also is
used for commentary on the patch that shouldn't go into the commit
itself.

As described in https://git-scm.com/docs/git-format-patch:
> Typically it will be placed in a MUA’s drafts folder, edited to add
> timely commentary that should not go in the changelog ***after the
> three dashes*** [emphasis added] 

And also as described at
https://spacekookie.de/blog/collaborating-with-git-send-email/:
> Another often overlooked feature here is "timely commentary", are
> comments in the patch e-mail that won't be part of the patch or the
> commit message itself. They can be made after the --- marker in a
> patch mail, but before the actual patch starts. This section is
> usually used for the diff-stat of that particular patch.

I tried sending the patch email to myself before I sent it to the list,
piping it to `git-am`, and it correctly applied the commit without my
"timely commentary", so I was pretty convinced I had formatted it
correctly.

Do you get something different when you `git am` my message? If so, can
you be more specific about how I need to reformat the email?

> Also, since you've opened a bug, we have hooks to refer to the bug. For instance,
> for commit ca263aec20adfffe6f9dab3a18f8a7b24667f99c.
>
> PR testsuite/32489
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32489

I'm confused about this - are you asking or suggesting that I format my
message in a different way to take advantage of these hooks? Where are
the hooks applied? 

> The change itself looks OK to me. I'd like another pair of eyes to look at
> it before we approve it. I think this can go in as a trivial change when it
> gets approved.
>
> Reviewed-By: Luis Machado <luis.machado@arm.com>

Thank again for your review and help! 

   ~Brandon 

Re: [PATCH] [PR corefiles/32441] Fix segfault if target_fileio_read_alloc fails

[Andrew Burgess]

"brandon.belew" <brandon.belew@zetier.com> writes:

> Check for target_fileio_read_alloc failure in linux_fill_prpsinfo
> before dereferencing buffer. This fixes a segfault in the 'gcore'
> command when attached to certain remote targets.
> ---
> This is my first contribution to GDB, and my first use of
> git-send-email, so please let me know if this is formatted
> incorrectly! I initially submitted the bug and a v1 patch at
> https://sourceware.org/bugzilla/show_bug.cgi?id=32441 and received the
> following from Thiago Bauermann:
>
>> Thank you for the patch. In general it looks good to me, just a couple of minor
>> comments:
>>
>> 1. Since target_fileio_read_alloc () returns LONGEST, I think it's better if
>> the buf_len variable also has that type.
>
> I decided to stick with ssize_t for the variable, as this matches the
> usage elsewhere in linux-tdep.c in linux_info_proc (which already was
> correctly checking the length).

I think you should reconsider here.  The function returns LONGEST, so
that's what should be used.  GDB's general policy is to fix little
bugs like this as the code gets touched for other reasons.

Otherwise, I agree with Luis, this looks great.  If you repost with the
description in the commit message we can get this merged.

Thanks,
Andrew

>
>> 2. GDB is (very) slowly transitioning from C to C++. We currently prefer to use
>> nullptr rather than NULL, so I suggest using this patch as an opportunity to
>> change NULL to nullptr in lines 1876, 1877 and 1879.
>
> I made the requested NULL -> nullptr changes.
>
> Let me know if this is good or if I need to make any changes in my
> workflow to adhere to GNU or gdb project conventions.
>
>  gdb/linux-tdep.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index d3452059ce2..c10c4c76451 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -1867,17 +1867,17 @@ linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
>    /* The number of fields read by `sscanf'.  */
>    int n_fields = 0;
>
> -  gdb_assert (p != NULL);
> +  gdb_assert (p != nullptr);
>
>    /* Obtaining PID and filename.  */
>    pid = inferior_ptid.pid ();
>    xsnprintf (filename, sizeof (filename), "/proc/%d/cmdline", (int) pid);
>    /* The full name of the program which generated the corefile.  */
> -  gdb_byte *buf = NULL;
> -  size_t buf_len = target_fileio_read_alloc (NULL, filename, &buf);
> +  gdb_byte *buf = nullptr;
> +  ssize_t buf_len = target_fileio_read_alloc (nullptr, filename, &buf);
>    gdb::unique_xmalloc_ptr<char> fname ((char *)buf);
>
> -  if (buf_len < 1 || fname.get ()[0] == '\0')
> +  if (buf_len < 1 || fname.get () == nullptr || fname.get ()[0] == '\0')
>      {
>        /* No program name was read, so we won't be able to retrieve more
>  	 information about the process.  */
> --
> 2.46.0

Re: [PATCH] [PR corefiles/32441] Fix segfault if target_fileio_read_alloc fails

[Brandon Belew]

Andrew Burgess <aburgess@redhat.com> writes:
> I think you should reconsider here.  The function returns LONGEST, so
> that's what should be used.  GDB's general policy is to fix little
> bugs like this as the code gets touched for other reasons.

Thanks Andrew! I'll go back and look at the LONGEST return value and get
back to you on that with an updated patch. To be clear, you want the
other function that was already using ssize_t for the return fixed as
well? 

> Otherwise, I agree with Luis, this looks great.  If you repost with the
> description in the commit message we can get this merged.

I replied to Luis' message with some further questions about
formatting. For reference, my original reply has a Message-ID of
hwjgya7c6ulo46.fsf@brandonb.zetier.com. I'll redo those questions here:

It was my understanding of git-format-patch / git-send-email (and in
reverse, git-am) that the commit message would be taken from everything
up to the '---'. Then everything between the '---' and the actual diff
was considered "timely commentary" and would not be present in the
commit message. This typically contains the diffstat output but also is
used for commentary on the patch that shouldn't go into the commit
itself.

As described in https://git-scm.com/docs/git-format-patch:
> Typically it will be placed in a MUA’s drafts folder, edited to add
> timely commentary that should not go in the changelog ***after the
> three dashes*** [emphasis added] 

And also as described at
https://spacekookie.de/blog/collaborating-with-git-send-email/:
> Another often overlooked feature here is "timely commentary", are
> comments in the patch e-mail that won't be part of the patch or the
> commit message itself. They can be made after the --- marker in a
> patch mail, but before the actual patch starts. This section is
> usually used for the diff-stat of that particular patch.

I tried sending the patch email to myself before I sent it to the list,
piping it to `git-am`, and it correctly applied the commit without my
"timely commentary", so I was pretty convinced I had formatted it
correctly.

Do you get something different when you `git am` my message? If so, can
you be more specific about how I need to reformat the email?

Thanks!
    ~Brandon 

Re: [PATCH] [PR corefiles/32441] Fix segfault if target_fileio_read_alloc fails

[Andrew Burgess]

Brandon Belew <brandon.belew@zetier.com> writes:

> Andrew Burgess <aburgess@redhat.com> writes:
>> I think you should reconsider here.  The function returns LONGEST, so
>> that's what should be used.  GDB's general policy is to fix little
>> bugs like this as the code gets touched for other reasons.
>
> Thanks Andrew! I'll go back and look at the LONGEST return value and get
> back to you on that with an updated patch. To be clear, you want the
> other function that was already using ssize_t for the return fixed as
> well? 
>
>> Otherwise, I agree with Luis, this looks great.  If you repost with the
>> description in the commit message we can get this merged.
>
> I replied to Luis' message with some further questions about
> formatting. For reference, my original reply has a Message-ID of
> hwjgya7c6ulo46.fsf@brandonb.zetier.com. I'll redo those questions here:
>
> It was my understanding of git-format-patch / git-send-email (and in
> reverse, git-am) that the commit message would be taken from everything
> up to the '---'. Then everything between the '---' and the actual diff
> was considered "timely commentary" and would not be present in the
> commit message. This typically contains the diffstat output but also is
> used for commentary on the patch that shouldn't go into the commit
> itself.
>
> As described in https://git-scm.com/docs/git-format-patch:
>> Typically it will be placed in a MUA’s drafts folder, edited to add
>> timely commentary that should not go in the changelog ***after the
>> three dashes*** [emphasis added] 
>
> And also as described at
> https://spacekookie.de/blog/collaborating-with-git-send-email/:
>> Another often overlooked feature here is "timely commentary", are
>> comments in the patch e-mail that won't be part of the patch or the
>> commit message itself. They can be made after the --- marker in a
>> patch mail, but before the actual patch starts. This section is
>> usually used for the diff-stat of that particular patch.
>
> I tried sending the patch email to myself before I sent it to the list,
> piping it to `git-am`, and it correctly applied the commit without my
> "timely commentary", so I was pretty convinced I had formatted it
> correctly.
>
> Do you get something different when you `git am` my message? If so, can
> you be more specific about how I need to reformat the email?

I guess if that works, then it's fine.  It's not a layout I've seen
before, and it doesn't make much sense to me.  That is, placing the
email reply between the commit message and the patch seems an odd
ordering to me, I'd usually reply to the various points, and then just
place the patch, including commit message, and the tail of the email.

But thanks for referencing the actual docs.

Thanks,
Andrew

[PATCH v2] [PR corefiles/32441] Fix segfault if target_fileio_read_alloc fails

[Brandon Belew]

Check for target_fileio_read_alloc failure in linux_fill_prpsinfo
before dereferencing buffer. This fixes a segfault in the 'gcore'
command when attached to certain remote targets.
---
 gdb/linux-tdep.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index d3ab02d03e0..735d20dc050 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -854,7 +854,7 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
     {
       xsnprintf (filename, sizeof filename, "/proc/%ld/cmdline", pid);
       gdb_byte *buffer;
-      ssize_t len = target_fileio_read_alloc (NULL, filename, &buffer);
+      LONGEST len = target_fileio_read_alloc (nullptr, filename, &buffer);

       if (len > 0)
 	{
@@ -2180,17 +2180,17 @@ linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
   /* The number of fields read by `sscanf'.  */
   int n_fields = 0;

-  gdb_assert (p != NULL);
+  gdb_assert (p != nullptr);

   /* Obtaining PID and filename.  */
   pid = inferior_ptid.pid ();
   xsnprintf (filename, sizeof (filename), "/proc/%d/cmdline", (int) pid);
   /* The full name of the program which generated the corefile.  */
-  gdb_byte *buf = NULL;
-  size_t buf_len = target_fileio_read_alloc (NULL, filename, &buf);
+  gdb_byte *buf = nullptr;
+  LONGEST buf_len = target_fileio_read_alloc (nullptr, filename, &buf);
   gdb::unique_xmalloc_ptr<char> fname ((char *)buf);

-  if (buf_len < 1 || fname.get ()[0] == '\0')
+  if (buf_len < 1 || fname.get () == nullptr || fname.get ()[0] == '\0')
     {
       /* No program name was read, so we won't be able to retrieve more
 	 information about the process.  */
--
2.47.1

Re: [PATCH v2] [PR corefiles/32441] Fix segfault if target_fileio_read_alloc fails

[Brandon Belew]

Ping, does anyonee have time to review this patch? 

> Check for target_fileio_read_alloc failure in linux_fill_prpsinfo
> before dereferencing buffer. This fixes a segfault in the 'gcore'
> command when attached to certain remote targets.
> ---
>  gdb/linux-tdep.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index d3ab02d03e0..735d20dc050 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -854,7 +854,7 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
>      {
>        xsnprintf (filename, sizeof filename, "/proc/%ld/cmdline", pid);
>        gdb_byte *buffer;
> -      ssize_t len = target_fileio_read_alloc (NULL, filename, &buffer);
> +      LONGEST len = target_fileio_read_alloc (nullptr, filename, &buffer);
>
>        if (len > 0)
>  	{
> @@ -2180,17 +2180,17 @@ linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
>    /* The number of fields read by `sscanf'.  */
>    int n_fields = 0;
>
> -  gdb_assert (p != NULL);
> +  gdb_assert (p != nullptr);
>
>    /* Obtaining PID and filename.  */
>    pid = inferior_ptid.pid ();
>    xsnprintf (filename, sizeof (filename), "/proc/%d/cmdline", (int) pid);
>    /* The full name of the program which generated the corefile.  */
> -  gdb_byte *buf = NULL;
> -  size_t buf_len = target_fileio_read_alloc (NULL, filename, &buf);
> +  gdb_byte *buf = nullptr;
> +  LONGEST buf_len = target_fileio_read_alloc (nullptr, filename, &buf);
>    gdb::unique_xmalloc_ptr<char> fname ((char *)buf);
>
> -  if (buf_len < 1 || fname.get ()[0] == '\0')
> +  if (buf_len < 1 || fname.get () == nullptr || fname.get ()[0] == '\0')
>      {
>        /* No program name was read, so we won't be able to retrieve more
>  	 information about the process.  */
> --
> 2.47.1

Re: [PATCH v2] [PR corefiles/32441] Fix segfault if target_fileio_read_alloc fails

[Andrew Burgess]

Brandon Belew <brandon.belew@zetier.com> writes:

> Check for target_fileio_read_alloc failure in linux_fill_prpsinfo
> before dereferencing buffer. This fixes a segfault in the 'gcore'
> command when attached to certain remote targets.

LGTM.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew


> ---
>  gdb/linux-tdep.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index d3ab02d03e0..735d20dc050 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -854,7 +854,7 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
>      {
>        xsnprintf (filename, sizeof filename, "/proc/%ld/cmdline", pid);
>        gdb_byte *buffer;
> -      ssize_t len = target_fileio_read_alloc (NULL, filename, &buffer);
> +      LONGEST len = target_fileio_read_alloc (nullptr, filename, &buffer);
>
>        if (len > 0)
>  	{
> @@ -2180,17 +2180,17 @@ linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
>    /* The number of fields read by `sscanf'.  */
>    int n_fields = 0;
>
> -  gdb_assert (p != NULL);
> +  gdb_assert (p != nullptr);
>
>    /* Obtaining PID and filename.  */
>    pid = inferior_ptid.pid ();
>    xsnprintf (filename, sizeof (filename), "/proc/%d/cmdline", (int) pid);
>    /* The full name of the program which generated the corefile.  */
> -  gdb_byte *buf = NULL;
> -  size_t buf_len = target_fileio_read_alloc (NULL, filename, &buf);
> +  gdb_byte *buf = nullptr;
> +  LONGEST buf_len = target_fileio_read_alloc (nullptr, filename, &buf);
>    gdb::unique_xmalloc_ptr<char> fname ((char *)buf);
>
> -  if (buf_len < 1 || fname.get ()[0] == '\0')
> +  if (buf_len < 1 || fname.get () == nullptr || fname.get ()[0] == '\0')
>      {
>        /* No program name was read, so we won't be able to retrieve more
>  	 information about the process.  */
> --
> 2.47.1

GDB 16.3 release - 2025-03-01 update

[Joel Brobecker]

Hi team,

Quick update on the GDB 16.3 release, normally scheduled around 3 months
after the corresponding .1 which was released Jan 18, therefore:

  - Target Release Date: Apr 19-20.

So far, we have two fixes in the gdb-16-branch since we release 16.2
(see below). No additional issues currently listed in bugzilla as
targeting the 16.3 release.

If you know of issues that we should aim to fix before we publish
the 16.3 release, please create a PR, set the "Target Milestone"
to 16.3, with a note explaining the rationale.

Thank you!

Meanwhile, here is a quick summary of the 16.3 release:

Fixed Since the Previous Update:
--------------------------------

  * [TomDV] corefiles/32634
    [gdb/corefiles] segfault in gdb.arch/i386-biarch-core.exp
    https://sourceware.org/bugzilla/show_bug.cgi?id=32634

  * [AndrewB] tui/32623
    TUI console window doesn't update while inferior is running
    https://sourceware.org/bugzilla/show_bug.cgi?id=32623


Added Since the Last Update:
----------------------------

  < none :) >

Other Ongoing Items:
--------------------

   < none :) >

Not Blocking, But Keep An Eye On:
---------------------------------

  < none :) >

-- 
Joel

Re: GDB 16.3 release - 2025-03-01 update

[Luis Machado]

Hi Joel,

On 3/1/25 10:23, Joel Brobecker wrote:
> Hi team,
> 
> Quick update on the GDB 16.3 release, normally scheduled around 3 months
> after the corresponding .1 which was released Jan 18, therefore:
> 
>   - Target Release Date: Apr 19-20.
> 
> So far, we have two fixes in the gdb-16-branch since we release 16.2
> (see below). No additional issues currently listed in bugzilla as
> targeting the 16.3 release.
> 
> If you know of issues that we should aim to fix before we publish
> the 16.3 release, please create a PR, set the "Target Milestone"
> to 16.3, with a note explaining the rationale.
> 
> Thank you!
> 
> Meanwhile, here is a quick summary of the 16.3 release:
> 
> Fixed Since the Previous Update:
> --------------------------------
> 
>   * [TomDV] corefiles/32634
>     [gdb/corefiles] segfault in gdb.arch/i386-biarch-core.exp
>     https://sourceware.org/bugzilla/show_bug.cgi?id=32634
> 
>   * [AndrewB] tui/32623
>     TUI console window doesn't update while inferior is running
>     https://sourceware.org/bugzilla/show_bug.cgi?id=32623
> 
> 
> Added Since the Last Update:
> ----------------------------
> 
>   < none :) >
> 
> Other Ongoing Items:
> --------------------
> 
>    < none :) >
> 
> Not Blocking, But Keep An Eye On:
> ---------------------------------
> 
>   < none :) >
> 

I'd like to include a fix, for which I'll open a bug soon.

https://inbox.sourceware.org/gdb-patches/20250228110513.2449558-1-luis.machado@arm.com/T/#u

Re: GDB 16.3 release - 2025-03-01 update

[Brandon Belew]

brobecker@adacore.com (Joel Brobecker) writes:

> Quick update on the GDB 16.3 release, normally scheduled around 3 months
> after the corresponding .1 which was released Jan 18, therefore:
>
>   - Target Release Date: Apr 19-20.
>
> So far, we have two fixes in the gdb-16-branch since we release 16.2
> (see below). No additional issues currently listed in bugzilla as
> targeting the 16.3 release.
>
> If you know of issues that we should aim to fix before we publish
> the 16.3 release, please create a PR, set the "Target Milestone"
> to 16.3, with a note explaining the rationale.

Hi, I'm a new contributor and I don't have permissions to modify the
Target Milestone on the bug I reported in bugzilla, but bug 32441 [1]
has an attached patch that was just approved by Andrew Burgess; see
Message ID <87ikouz5ys.fsf@redhat.com> [2]. This fixes a segfault in
core file generation, much like the other corefiles/32634 fix already
being included in the 16.3 release. I would love to see this fix
included!

Thanks!
    ~Brandon


[1] https://sourceware.org/bugzilla/show_bug.cgi?id=32441
[2] https://sourceware.org/pipermail/gdb-patches/2025-February/215909.html

Re: GDB 16.3 release - 2025-03-01 update

[Joel Brobecker]

> I'd like to include a fix, for which I'll open a bug soon.
> 
> https://inbox.sourceware.org/gdb-patches/20250228110513.2449558-1-luis.machado@arm.com/T/#u

Thanks for the heads up, Luis. I'll keep that one in my radar.

-- 
Joel

Re: GDB 16.3 release - 2025-03-01 update

[Joel Brobecker]

Hello Brandon,

> Hi, I'm a new contributor and I don't have permissions to modify the
> Target Milestone on the bug I reported in bugzilla, but bug 32441 [1]
> has an attached patch that was just approved by Andrew Burgess; see
> Message ID <87ikouz5ys.fsf@redhat.com> [2]. This fixes a segfault in
> core file generation, much like the other corefiles/32634 fix already
> being included in the 16.3 release. I would love to see this fix
> included!
> 
> Thanks!
>     ~Brandon
> 
> 
> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=32441
> [2] https://sourceware.org/pipermail/gdb-patches/2025-February/215909.html

Thanks for the heads up and the references.

I think the request to backport looks reasonable to me, but you will
need one of the Global Maintainers to also approve the backport.

If you haven't pushed the patch to master yet, can you amend
the commit message to include the URL to the GDB PR so that
bugzill automatically gets notified that this was pushed, and
logs its in the activity log?

If you have already pushed to master, no problem, but can you
at least make sure you do this for the version that gets pushed
to the gdb-16-branch (after it gets approved for backporting)?

Thank you!
-- 
Joel

[PATCH v3] [PR corefiles/32441] Fix segfault if target_fileio_read_alloc fails

[Brandon Belew]

Was: Re: GDB 16.3 release - 2025-03-01 update. This is a v3 of my patch
as requested by Joel Brobecker with a link to the bugzilla PR included
in the commit message.

Joel Brobecker <brobecker@adacore.com> writes:
> Hello Brandon,
> 
>> Hi, I'm a new contributor and I don't have permissions to modify the
>> Target Milestone on the bug I reported in bugzilla, but bug 32441 [1]
>> has an attached patch that was just approved by Andrew Burgess; see
>> Message ID <87ikouz5ys.fsf@redhat.com> [2]. This fixes a segfault in
>> core file generation, much like the other corefiles/32634 fix already
>> being included in the 16.3 release. I would love to see this fix
>> included!
>> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=32441
>> [2] https://sourceware.org/pipermail/gdb-patches/2025-February/215909.html
>
> Thanks for the heads up and the references.
>
> I think the request to backport looks reasonable to me, but you will
> need one of the Global Maintainers to also approve the backport.
>
> If you haven't pushed the patch to master yet, can you amend
> the commit message to include the URL to the GDB PR so that
> bugzill automatically gets notified that this was pushed, and
> logs its in the activity log?
>
> If you have already pushed to master, no problem, but can you
> at least make sure you do this for the version that gets pushed
> to the gdb-16-branch (after it gets approved for backporting)?
>
> Thank you!

Thanks for the updates, Joel. I do not have any commit rights, so my
patch hasn't been comitted anywhere. I'm not sure what that process
looks like as this is my first contribution. I've been trying to get
this merged since December and Andrew gave it a "LGTM" on Feb 28; does
someone else need to approve it before it can be merged? Is there
someone else I need to ping to request the patch be merged now that it
has been approved? Given that it hasn't been merged yet, I can update
the commit message to include the link to bugzilla, although I did
include "[PR corefiles/32441]" in the subject line of my patch which I
thought was sufficient. At any rate, here's an updated patch that
includes the URL within the commit message.


---
Check for target_fileio_read_alloc failure in linux_fill_prpsinfo
before dereferencing buffer. This fixes a segfault in the 'gcore'
command when attached to certain remote targets.

Fixes https://sourceware.org/bugzilla/show_bug.cgi?id=32441
---
 gdb/linux-tdep.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index d3ab02d03e0..735d20dc050 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -854,7 +854,7 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
     {
       xsnprintf (filename, sizeof filename, "/proc/%ld/cmdline", pid);
       gdb_byte *buffer;
-      ssize_t len = target_fileio_read_alloc (NULL, filename, &buffer);
+      LONGEST len = target_fileio_read_alloc (nullptr, filename, &buffer);

       if (len > 0)
 	{
@@ -2180,17 +2180,17 @@ linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
   /* The number of fields read by `sscanf'.  */
   int n_fields = 0;

-  gdb_assert (p != NULL);
+  gdb_assert (p != nullptr);

   /* Obtaining PID and filename.  */
   pid = inferior_ptid.pid ();
   xsnprintf (filename, sizeof (filename), "/proc/%d/cmdline", (int) pid);
   /* The full name of the program which generated the corefile.  */
-  gdb_byte *buf = NULL;
-  size_t buf_len = target_fileio_read_alloc (NULL, filename, &buf);
+  gdb_byte *buf = nullptr;
+  LONGEST buf_len = target_fileio_read_alloc (nullptr, filename, &buf);
   gdb::unique_xmalloc_ptr<char> fname ((char *)buf);

-  if (buf_len < 1 || fname.get ()[0] == '\0')
+  if (buf_len < 1 || fname.get () == nullptr || fname.get ()[0] == '\0')
     {
       /* No program name was read, so we won't be able to retrieve more
 	 information about the process.  */
--
2.47.1

Re: GDB 16.3 release - 2025-03-01 update

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> I think the request to backport looks reasonable to me, but you will
Joel> need one of the Global Maintainers to also approve the backport.

It's fine by me.

thanks,
Tom

Re: [PATCH v2] [PR corefiles/32441] Fix segfault if target_fileio_read_alloc fails

[Joel Brobecker]

> > Check for target_fileio_read_alloc failure in linux_fill_prpsinfo
> > before dereferencing buffer. This fixes a segfault in the 'gcore'
> > command when attached to certain remote targets.
> 
> LGTM.
> 
> Approved-By: Andrew Burgess <aburgess@redhat.com>

Thank you Andrew. Brandon reached out privately saying he does not have
push access, and so I pushed the patch for him.

Tom also approved in a different thread (about the gdb 16.3 release
status) the backport to the gdb-16-branch, so I will do that next.

-- 
Joel