diff mbox series

[3/3] gdb/corelow.c: do not try to reopen a file if open failed once

Message ID 20230531160406.3932028-4-lancelot.six@amd.com
State New
Headers
Series Fix use-after-free in gdb/corelow.c + cleanups |

Commit Message

Lancelot SIX May 31, 2023, 4:04 p.m. UTC 
  In the current implementation, core_target::build_file_mappings will try
to locate and open files which were mapped in the process for which the
core dump was produced.  If the file cannot be found or cannot be
opened, GDB will re-try to open it once for each time it was mapped in
the process's address space.

This patch makes it so GDB recognizes that it has already failed to open
a given file once and does not re-try the process for each mapping.
---
 gdb/corelow.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

Comments

Andrew Burgess June 1, 2023, 10:04 a.m. UTC | #1 
Lancelot SIX via Gdb-patches <gdb-patches@sourceware.org> writes:

> In the current implementation, core_target::build_file_mappings will try
> to locate and open files which were mapped in the process for which the
> core dump was produced.  If the file cannot be found or cannot be
> opened, GDB will re-try to open it once for each time it was mapped in
> the process's address space.
>
> This patch makes it so GDB recognizes that it has already failed to open
> a given file once and does not re-try the process for each mapping.

LGTM.

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

Thanks,
Andrew

> ---
>  gdb/corelow.c | 27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/gdb/corelow.c b/gdb/corelow.c
> index ce68f91132e..b831a90792c 100644
> --- a/gdb/corelow.c
> +++ b/gdb/corelow.c
> @@ -233,6 +233,16 @@ core_target::build_file_mappings ()
>  	   weed out non-file-backed mappings.  */
>  	gdb_assert (filename != nullptr);
>  
> +	if (unavailable_paths.find (filename) != unavailable_paths.end ())
> +	  {
> +	    /* We have already seen some mapping for FILENAME but failed to
> +	       find/open the file.  There is no point in trying the same
> +	       thing again so just record that the range [start, end) is
> +	       unavailable.  */
> +	    m_core_unavailable_mappings.emplace_back (start, end - start);
> +	    return;
> +	  }
> +
>  	struct bfd *bfd = bfd_map[filename];
>  	if (bfd == nullptr)
>  	  {
> @@ -250,11 +260,10 @@ core_target::build_file_mappings ()
>  	    if (expanded_fname == nullptr)
>  	      {
>  		m_core_unavailable_mappings.emplace_back (start, end - start);
> -		/* Print just one warning per path.  */
> -		if (unavailable_paths.insert (filename).second)
> -		  warning (_("Can't open file %s during file-backed mapping "
> -			     "note processing"),
> -			   filename);
> +		unavailable_paths.insert (filename);
> +		warning (_("Can't open file %s during file-backed mapping "
> +			   "note processing"),
> +			 filename);
>  		return;
>  	      }
>  
> @@ -264,10 +273,10 @@ core_target::build_file_mappings ()
>  	    if (bfd == nullptr || !bfd_check_format (bfd, bfd_object))
>  	      {
>  		m_core_unavailable_mappings.emplace_back (start, end - start);
> -		if (unavailable_paths.insert (filename).second)
> -		  warning (_("Can't open file %s which was expanded to %s "
> -			     "during file-backed mapping note processing"),
> -			   filename, expanded_fname.get ());
> +		unavailable_paths.insert (filename);
> +		warning (_("Can't open file %s which was expanded to %s "
> +			   "during file-backed mapping note processing"),
> +			 filename, expanded_fname.get ());
>  
>  		if (bfd != nullptr)
>  		  {
> -- 
> 2.34.1
diff mbox series

Patch

diff --git a/gdb/corelow.c b/gdb/corelow.c
index ce68f91132e..b831a90792c 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -233,6 +233,16 @@  core_target::build_file_mappings ()
 	   weed out non-file-backed mappings.  */
 	gdb_assert (filename != nullptr);
 
+	if (unavailable_paths.find (filename) != unavailable_paths.end ())
+	  {
+	    /* We have already seen some mapping for FILENAME but failed to
+	       find/open the file.  There is no point in trying the same
+	       thing again so just record that the range [start, end) is
+	       unavailable.  */
+	    m_core_unavailable_mappings.emplace_back (start, end - start);
+	    return;
+	  }
+
 	struct bfd *bfd = bfd_map[filename];
 	if (bfd == nullptr)
 	  {
@@ -250,11 +260,10 @@  core_target::build_file_mappings ()
 	    if (expanded_fname == nullptr)
 	      {
 		m_core_unavailable_mappings.emplace_back (start, end - start);
-		/* Print just one warning per path.  */
-		if (unavailable_paths.insert (filename).second)
-		  warning (_("Can't open file %s during file-backed mapping "
-			     "note processing"),
-			   filename);
+		unavailable_paths.insert (filename);
+		warning (_("Can't open file %s during file-backed mapping "
+			   "note processing"),
+			 filename);
 		return;
 	      }
 
@@ -264,10 +273,10 @@  core_target::build_file_mappings ()
 	    if (bfd == nullptr || !bfd_check_format (bfd, bfd_object))
 	      {
 		m_core_unavailable_mappings.emplace_back (start, end - start);
-		if (unavailable_paths.insert (filename).second)
-		  warning (_("Can't open file %s which was expanded to %s "
-			     "during file-backed mapping note processing"),
-			   filename, expanded_fname.get ());
+		unavailable_paths.insert (filename);
+		warning (_("Can't open file %s which was expanded to %s "
+			   "during file-backed mapping note processing"),
+			 filename, expanded_fname.get ());
 
 		if (bfd != nullptr)
 		  {