gdb: Check for nullptr when computing srcpath

Message ID CAJDtP-Q8E=Kyf4n8-o3OJ=gKjR4e+sHKTA0FbL0h8uTqF-NmnQ@mail.gmail.com
State New
Headers show
Series
  • gdb: Check for nullptr when computing srcpath
Related show

Commit Message

Aaron Merey Feb. 27, 2020, 6 p.m.

Comments

Simon Marchi Feb. 27, 2020, 7:18 p.m. | #1
On 2020-02-27 1:00 p.m., Aaron Merey wrote:
> 


Hi Aaron,

Please provide in the commit message an explanation of what this fixes, including
how to reproduce the crash.  Since this fixes the execution of an existing test
case, you can include the "make check" command line used to run it, like:

  make check TESTS="gdb.dwarf2/dw2-ranges-base"

Simon
Simon Marchi Feb. 27, 2020, 7:20 p.m. | #2
On 2020-02-27 2:18 p.m., Simon Marchi wrote:
> On 2020-02-27 1:00 p.m., Aaron Merey wrote:

>>

> 

> Hi Aaron,

> 

> Please provide in the commit message an explanation of what this fixes, including

> how to reproduce the crash.  Since this fixes the execution of an existing test

> case, you can include the "make check" command line used to run it, like:

> 

>   make check TESTS="gdb.dwarf2/dw2-ranges-base"

> 

> Simon

> 


Also, do you think you'd be able to send your patches using git-send-email?  That makes
it much easier to read and comment on.

Thanks,

Simon
Pedro Alves via Gdb-patches Feb. 27, 2020, 7:33 p.m. | #3
+   if (build_id != nullptr && srcpath.size () > 0)

I usually prefer !srcpath.empty ()

Also, I would like to second simark's suggestion for git-send-email!

Christian

On Thu, Feb 27, 2020 at 12:00 PM Aaron Merey <amerey@redhat.com> wrote:
>

>
Aaron Merey Feb. 27, 2020, 10:11 p.m. | #4
On Thu, Feb 27, 2020 at 2:18 PM Simon Marchi <simark@simark.ca> wrote:
> Please provide in the commit message an explanation of what this fixes, including

> how to reproduce the crash.  Since this fixes the execution of an existing test

> case, you can include the "make check" command line used to run it, like:

>

>   make check TESTS="gdb.dwarf2/dw2-ranges-base"


Fixed.

> Also, do you think you'd be able to send your patches using git-send-email?  That makes

> it much easier to read and comment on.


Sure.

On Thu, Feb 27, 2020 at 2:33 PM Christian Biesinger <cbiesinger@google.com> wrote:
> +   if (build_id != nullptr && srcpath.size () > 0)

>

> I usually prefer !srcpath.empty ()


Fixed.

Aaron


From d048f84f027006782cd96dc66a10477ed5a78243 Mon Sep 17 00:00:00 2001
From: Aaron Merey <amerey@redhat.com>

Date: Thu, 27 Feb 2020 15:51:11 -0500
Subject: [PATCH] gdb: Check for nullptr when computing srcpath

This fixes a regression caused by commit 0d79cdc494d5:

  $ make check TESTS="gdb.dwarf2/dw2-ranges-base.exp"
  [...]
  ERROR: GDB process no longer exists

This error is caused by an abort during the computation of srcpath
when SYMTAB_DIRNAME (s) == NULL.

Computing srcpath only when SYMTAB_DIRNAME (s) is not NULL fixes this
error. Also change the condition for calling debuginfod_source_query
to include whether srcpath could be computed.

gdb/ChangeLog:

2020-02-27  Aaron Merey  <amerey@redhat.com>

        * source.c (open_source_file): Check for nullptr when computing
        srcpath.
---
 gdb/ChangeLog | 4 ++++
 gdb/source.c  | 4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4376161673..371ef91421 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2020-02-27  Aaron Merey  <amerey@redhat.com>
+	* source.c (open_source_file): Check for nullptr when computing
+	srcpath.
+
 2020-02-27  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* gdbtypes.c (create_array_type_with_stride): Use std::abs not
diff --git a/gdb/source.c b/gdb/source.c
index 051caf5c57..50de93952b 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1160,7 +1160,7 @@ open_source_file (struct symtab *s)
 	  std::string srcpath;
 	  if (IS_ABSOLUTE_PATH (s->filename))
 	    srcpath = s->filename;
-	  else
+	  else if (SYMTAB_DIRNAME (s) != nullptr)
 	    {
 	      srcpath = SYMTAB_DIRNAME (s);
 	      srcpath += SLASH_STRING;
@@ -1170,7 +1170,7 @@ open_source_file (struct symtab *s)
 	  const struct bfd_build_id *build_id = build_id_bfd_get (ofp->obfd);
 
 	  /* Query debuginfod for the source file.  */
-	  if (build_id != nullptr)
+	  if (build_id != nullptr && !srcpath.empty ())
 	    fd = debuginfod_source_query (build_id->data,
 					  build_id->size,
 					  srcpath.c_str (),
-- 
2.24.1
Simon Marchi Feb. 27, 2020, 11:37 p.m. | #5
On 2020-02-27 5:11 p.m., Aaron Merey wrote:
> On Thu, Feb 27, 2020 at 2:18 PM Simon Marchi <simark@simark.ca> wrote:

>> Please provide in the commit message an explanation of what this fixes, including

>> how to reproduce the crash.  Since this fixes the execution of an existing test

>> case, you can include the "make check" command line used to run it, like:

>>

>>   make check TESTS="gdb.dwarf2/dw2-ranges-base"

> 

> Fixed.

> 

>> Also, do you think you'd be able to send your patches using git-send-email?  That makes

>> it much easier to read and comment on.

> 

> Sure.

> 

> On Thu, Feb 27, 2020 at 2:33 PM Christian Biesinger <cbiesinger@google.com> wrote:

>> +   if (build_id != nullptr && srcpath.size () > 0)

>>

>> I usually prefer !srcpath.empty ()

> 

> Fixed.

> 

> Aaron

> 

> 

> From d048f84f027006782cd96dc66a10477ed5a78243 Mon Sep 17 00:00:00 2001

> From: Aaron Merey <amerey@redhat.com>

> Date: Thu, 27 Feb 2020 15:51:11 -0500

> Subject: [PATCH] gdb: Check for nullptr when computing srcpath

> 

> This fixes a regression caused by commit 0d79cdc494d5:

> 

>   $ make check TESTS="gdb.dwarf2/dw2-ranges-base.exp"

>   [...]

>   ERROR: GDB process no longer exists

> 

> This error is caused by an abort during the computation of srcpath

> when SYMTAB_DIRNAME (s) == NULL.

> 

> Computing srcpath only when SYMTAB_DIRNAME (s) is not NULL fixes this

> error. Also change the condition for calling debuginfod_source_query

> to include whether srcpath could be computed.

> 

> gdb/ChangeLog:

> 

> 2020-02-27  Aaron Merey  <amerey@redhat.com>

> 

>         * source.c (open_source_file): Check for nullptr when computing

>         srcpath.


Thanks, that LGTM.

Simon

Patch

From e3eb4852487b941a9efb4bbcc8b6fb5f522b8298 Mon Sep 17 00:00:00 2001
From: Aaron Merey <amerey@redhat.com>
Date: Thu, 27 Feb 2020 12:52:05 -0500
Subject: [PATCH] gdb: Check for nullptr when computing srcpath

gdb/ChangeLog:

2020-02-27  Aaron Merey  <amerey@redhat.com>

	* source.c (open_source_file): Check for nullptr when computing
	srcpath.
---
 gdb/ChangeLog | 4 ++++
 gdb/source.c  | 4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4376161673..371ef91421 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@ 
+2020-02-27  Aaron Merey  <amerey@redhat.com>
+	* source.c (open_source_file): Check for nullptr when computing
+	srcpath.
+
 2020-02-27  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* gdbtypes.c (create_array_type_with_stride): Use std::abs not
diff --git a/gdb/source.c b/gdb/source.c
index 051caf5c57..29eeb5341b 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1160,7 +1160,7 @@  open_source_file (struct symtab *s)
 	  std::string srcpath;
 	  if (IS_ABSOLUTE_PATH (s->filename))
 	    srcpath = s->filename;
-	  else
+	  else if (SYMTAB_DIRNAME (s) != nullptr)
 	    {
 	      srcpath = SYMTAB_DIRNAME (s);
 	      srcpath += SLASH_STRING;
@@ -1170,7 +1170,7 @@  open_source_file (struct symtab *s)
 	  const struct bfd_build_id *build_id = build_id_bfd_get (ofp->obfd);
 
 	  /* Query debuginfod for the source file.  */
-	  if (build_id != nullptr)
+	  if (build_id != nullptr && srcpath.size () > 0)
 	    fd = debuginfod_source_query (build_id->data,
 					  build_id->size,
 					  srcpath.c_str (),
-- 
2.24.1