Fix Fortran regression with variables in nested functions

Message ID 20190813182624.15135-1-tromey@adacore.com
State New, archived
Headers

Commit Message

Tom Tromey Aug. 13, 2019, 6:26 p.m. UTC
  Sergio pointed out that commit commit aa3b6533 ("Allow nested function
displays") regressed a few gdb.fortran tests.  I was able to reproduce
these failures with gcc head.

The bug is that some spots calling contained_in will in fact do the
wrong thing if nested functions are considered as contained.  In the
particular case of the Fortran regression, it was the call in
block_innermost_frame, being called from get_hosting_frame -- in this
case, the caller is specifically trying to avoid the nested case.

This patch fixes the problem by adding an "allow_nested" parameter to
contained_in, essentially reverting the change for most callers.

gdb/ChangeLog
2019-08-13  Tom Tromey  <tromey@adacore.com>

	* printcmd.c (do_one_display, info_display_command): Update.
	* block.h (contained_in): Return bool.  Add allow_nested
	parameter.
	* block.c (contained_in): Return bool.  Add allow_nested
	parameter.
---
 gdb/ChangeLog  |  8 ++++++++
 gdb/block.c    | 19 +++++++++++--------
 gdb/block.h    | 10 +++++++++-
 gdb/printcmd.c |  5 +++--
 4 files changed, 31 insertions(+), 11 deletions(-)
  

Comments

Kevin Buettner Aug. 14, 2019, 10:04 p.m. UTC | #1
On Tue, 13 Aug 2019 12:26:24 -0600
Tom Tromey <tromey@adacore.com> wrote:

> Sergio pointed out that commit commit aa3b6533 ("Allow nested function
> displays") regressed a few gdb.fortran tests.  I was able to reproduce
> these failures with gcc head.
> 
> The bug is that some spots calling contained_in will in fact do the
> wrong thing if nested functions are considered as contained.  In the
> particular case of the Fortran regression, it was the call in
> block_innermost_frame, being called from get_hosting_frame -- in this
> case, the caller is specifically trying to avoid the nested case.
> 
> This patch fixes the problem by adding an "allow_nested" parameter to
> contained_in, essentially reverting the change for most callers.
> 
> gdb/ChangeLog
> 2019-08-13  Tom Tromey  <tromey@adacore.com>
> 
> 	* printcmd.c (do_one_display, info_display_command): Update.
> 	* block.h (contained_in): Return bool.  Add allow_nested
> 	parameter.
> 	* block.c (contained_in): Return bool.  Add allow_nested
> 	parameter.

This patch fixes regressions that I was seeing in my OpenMP work,
so I'm in favor of it going in.

Kevin
  
Tom Tromey Aug. 19, 2019, 4:31 p.m. UTC | #2
Kevin> This patch fixes regressions that I was seeing in my OpenMP work,
Kevin> so I'm in favor of it going in.

I'm going to push it now.  Thanks.

Tom
  
Tom de Vries Aug. 20, 2019, 1:17 p.m. UTC | #3
On 19-08-19 18:31, Tom Tromey wrote:
> Kevin> This patch fixes regressions that I was seeing in my OpenMP work,
> Kevin> so I'm in favor of it going in.
> 
> I'm going to push it now.  Thanks.

This caused:
...
FAIL: gdb.ada/catch_ex.exp: insert catchpoint on Program_Error (GDB
internal error)
...

The internal error is an assertion failure:
...
src/gdb/dwarf2loc.c:727: internal-error: virtual void
dwarf_evaluate_loc_desc::get_frame_base(const gdb_byte**, size_t*):
Assertion `framefunc != NULL' failed.
...

Filed here ( https://sourceware.org/bugzilla/show_bug.cgi?id=24919 ).

Thanks,
- Tom
  

Patch

diff --git a/gdb/block.c b/gdb/block.c
index 5c6faa85045..ca4dc22cf30 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -65,25 +65,28 @@  block_gdbarch (const struct block *block)
   return get_objfile_arch (block_objfile (block));
 }
 
-/* Return Nonzero if block a is lexically nested within block b,
-   or if a and b have the same pc range.
-   Return zero otherwise.  */
+/* See block.h.  */
 
-int
-contained_in (const struct block *a, const struct block *b)
+bool
+contained_in (const struct block *a, const struct block *b,
+	      bool allow_nested)
 {
   if (!a || !b)
-    return 0;
+    return false;
 
   do
     {
       if (a == b)
-	return 1;
+	return true;
+      /* If A is a function block, then A cannot be contained in B,
+         except if A was inlined.  */
+      if (!allow_nested && BLOCK_FUNCTION (a) != NULL && !block_inlined_p (a))
+        return false;
       a = BLOCK_SUPERBLOCK (a);
     }
   while (a != NULL);
 
-  return 0;
+  return true;
 }
 
 
diff --git a/gdb/block.h b/gdb/block.h
index 9291deb6131..4c02e01d906 100644
--- a/gdb/block.h
+++ b/gdb/block.h
@@ -219,7 +219,15 @@  extern struct symbol *block_containing_function (const struct block *);
 
 extern int block_inlined_p (const struct block *block);
 
-extern int contained_in (const struct block *, const struct block *);
+/* Return true if block A is lexically nested within block B, or if a
+   and b have the same pc range.  Return false otherwise.  If
+   ALLOW_NESTED is true, then block A is considered to be in block B
+   if A is in a nested function in B's function.  If ALLOW_NESTED is
+   false (the default), then blocks in nested functions are not
+   considered to be contained.  */
+
+extern bool contained_in (const struct block *a, const struct block *b,
+			  bool allow_nested = false);
 
 extern const struct blockvector *blockvector_for_pc (CORE_ADDR,
 					       const struct block **);
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 7529842e73b..9b29b53ca74 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1936,7 +1936,8 @@  do_one_display (struct display *d)
   if (d->block)
     {
       if (d->pspace == current_program_space)
-	within_current_scope = contained_in (get_selected_block (0), d->block);
+	within_current_scope = contained_in (get_selected_block (0), d->block,
+					     true);
       else
 	within_current_scope = 0;
     }
@@ -2098,7 +2099,7 @@  Num Enb Expression\n"));
       else if (d->format.format)
 	printf_filtered ("/%c ", d->format.format);
       puts_filtered (d->exp_string);
-      if (d->block && !contained_in (get_selected_block (0), d->block))
+      if (d->block && !contained_in (get_selected_block (0), d->block, true))
 	printf_filtered (_(" (cannot be evaluated in the current context)"));
       printf_filtered ("\n");
     }