[binutils-gdb] Special case NULL pointers in dynamic type resolution

[binutils-gdb] Special case NULL pointers in dynamic type resolution

[Tom Tromey]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=3a3f1548fa383791f4ac4a1492b109422c1099d3

commit 3a3f1548fa383791f4ac4a1492b109422c1099d3
Author: Tom Tromey <tromey@adacore.com>
Date:   Tue Feb 13 11:47:38 2024 -0700

    Special case NULL pointers in dynamic type resolution
    
    commit f18fc7e5 ("gdb, types: Resolve pointer types dynamically")
    caused a regression on a test case in the AdaCore internal test suite.
    
    The issue here is that gdb would try to resolve the type of a dynamic
    pointer that happened to be NULL.  In this case, the "Location address
    is not set." error would end up being thrown from the DWARF expression
    evaluator.
    
    I think it makes more sense to special-case NULL pointers and not try
    to resolve their target type, as that type can't really be accessed
    anyway.
    
    This patch implements this idea, and also adds the missing Ada test
    case.

Diff:
---
 gdb/gdbtypes.c                                   | 13 +++++---
 gdb/testsuite/gdb.ada/recursive-variant.exp      | 31 +++++++++++++++++++
 gdb/testsuite/gdb.ada/recursive-variant/main.adb | 38 ++++++++++++++++++++++++
 gdb/testsuite/gdb.cp/vla-cxx.exp                 |  4 +--
 4 files changed, 80 insertions(+), 6 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index f1e9d7077d8..599a696839e 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2809,10 +2809,15 @@ resolve_dynamic_type_internal (struct type *type,
 	      pinfo.addr = read_memory_typed_address (addr_stack->addr, type);
 	    pinfo.next = addr_stack;
 
-	    resolved_type = copy_type (type);
-	    resolved_type->set_target_type
-	      (resolve_dynamic_type_internal (type->target_type (),
-					      &pinfo, frame, top_level));
+	    /* Special case a NULL pointer here -- we don't want to
+	       dereference it.  */
+	    if (pinfo.addr != 0)
+	      {
+		resolved_type = copy_type (type);
+		resolved_type->set_target_type
+		  (resolve_dynamic_type_internal (type->target_type (),
+						  &pinfo, frame, true));
+	      }
 	    break;
 	  }
 
diff --git a/gdb/testsuite/gdb.ada/recursive-variant.exp b/gdb/testsuite/gdb.ada/recursive-variant.exp
new file mode 100644
index 00000000000..ea13f83f2d5
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/recursive-variant.exp
@@ -0,0 +1,31 @@
+# Copyright 2024 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib "ada.exp"
+
+require allow_ada_tests
+
+standard_ada_testfile main
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable debug] != ""} {
+    return -1
+}
+
+clean_restart ${testfile}
+
+set bp_location [gdb_get_line_number "STOP" ${testdir}/main.adb]
+runto "main.adb:$bp_location"
+
+gdb_test "print *instance" [string_to_regexp "= (x => false, link => 0x0)"]
diff --git a/gdb/testsuite/gdb.ada/recursive-variant/main.adb b/gdb/testsuite/gdb.ada/recursive-variant/main.adb
new file mode 100644
index 00000000000..93b47e5002a
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/recursive-variant/main.adb
@@ -0,0 +1,38 @@
+--  Copyright 2024 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+procedure Main is
+
+   type Rec_Type;
+
+   type Rec_Type_Ref is access all Rec_Type;
+
+   type Rec_Type (X : Boolean) is record
+      Link : Rec_Type_Ref;
+      case X is
+         when True =>
+            Value : Integer;
+         when False =>
+            null;
+      end case;
+   end record;
+
+   Instance : Rec_Type_Ref := new Rec_Type'(X => False, Link => null);
+
+begin
+
+   null; -- STOP
+
+end Main;
diff --git a/gdb/testsuite/gdb.cp/vla-cxx.exp b/gdb/testsuite/gdb.cp/vla-cxx.exp
index 0033a968268..bf3ca792d58 100644
--- a/gdb/testsuite/gdb.cp/vla-cxx.exp
+++ b/gdb/testsuite/gdb.cp/vla-cxx.exp
@@ -26,10 +26,10 @@ if ![runto_main] {
 gdb_breakpoint [gdb_get_line_number "Before pointer assignment"]
 gdb_continue_to_breakpoint "Before pointer assignment"
 
-gdb_test "ptype ptr" "= int \\(\\*\\)\\\[3\\\]" \
+gdb_test "ptype ptr" "= int \\(\\*\\)\\\[variable length\\\]" \
     "ptype ptr, before pointer assignment"
 
-gdb_test "print ptr" "= \\(int \\(\\*\\)\\\[3\\\]\\) 0x0" \
+gdb_test "print ptr" "= \\(int \\(\\*\\)\\\[variable length\\\]\\) 0x0" \
     "print ptr, before pointer assignment"
 
 gdb_test "print *ptr" "Cannot access memory at address 0x0" \