[PATCH] Fix htab_t pretty-printer for NULL values

[PATCH] Fix htab_t pretty-printer for NULL values

[Tom Tromey]

I was debugging gdb and was surprised to see this in a
compunit_symtab:

  m_call_site_htab = <error reading variable: Cannot access memory at address 0x28>,

Debugging showed that the field was not uninitialized; rather, the
htab_t pretty-printer did not cope with NULL.
---
 gdb/gdb-gdb.py.in                       | 5 +++++
 gdb/testsuite/gdb.gdb/python-helper.exp | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/gdb/gdb-gdb.py.in b/gdb/gdb-gdb.py.in
index 56d063cd506..567ec41b305 100644
--- a/gdb/gdb-gdb.py.in
+++ b/gdb/gdb-gdb.py.in
@@ -379,10 +379,15 @@ class HtabPrinter:
         return "array"
 
     def to_string(self):
+        if int(self._val) == 0:
+            return "(null)"
         n = int(self._val["n_elements"]) - int(self._val["n_deleted"])
         return "htab_t with {} elements".format(n)
 
     def children(self):
+        if int(self._val) == 0:
+            return
+
         size = int(self._val["size"])
         entries = self._val["entries"]
 
diff --git a/gdb/testsuite/gdb.gdb/python-helper.exp b/gdb/testsuite/gdb.gdb/python-helper.exp
index 16b419984cf..be0b311e8ca 100644
--- a/gdb/testsuite/gdb.gdb/python-helper.exp
+++ b/gdb/testsuite/gdb.gdb/python-helper.exp
@@ -211,6 +211,8 @@ proc test_python_helper {} {
     # Test the htab_t pretty-printer.
     gdb_test -prompt $outer_prompt_re "print all_bfds" "htab_t with ${::decimal} elements = \\{${::hex}.*\\}"
 
+    gdb_test -prompt $outer_prompt_re "print (htab_t) 0x0" \
+	[string_to_regexp " = (null)"]
     return 0
 }
 
-- 
2.39.1

Re: [PATCH] Fix htab_t pretty-printer for NULL values

[Simon Marchi]

On 3/8/23 15:56, Tom Tromey wrote:
> I was debugging gdb and was surprised to see this in a
> compunit_symtab:
> 
>   m_call_site_htab = <error reading variable: Cannot access memory at address 0x28>,
> 
> Debugging showed that the field was not uninitialized; rather, the
> htab_t pretty-printer did not cope with NULL.
> ---
>  gdb/gdb-gdb.py.in                       | 5 +++++
>  gdb/testsuite/gdb.gdb/python-helper.exp | 2 ++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/gdb/gdb-gdb.py.in b/gdb/gdb-gdb.py.in
> index 56d063cd506..567ec41b305 100644
> --- a/gdb/gdb-gdb.py.in
> +++ b/gdb/gdb-gdb.py.in
> @@ -379,10 +379,15 @@ class HtabPrinter:
>          return "array"
>  
>      def to_string(self):
> +        if int(self._val) == 0:
> +            return "(null)"
>          n = int(self._val["n_elements"]) - int(self._val["n_deleted"])
>          return "htab_t with {} elements".format(n)

It would make me tremendously happy if you put a blank line after the
return null :).

>  
>      def children(self):
> +        if int(self._val) == 0:
> +            return
> +
>          size = int(self._val["size"])
>          entries = self._val["entries"]
>  
> diff --git a/gdb/testsuite/gdb.gdb/python-helper.exp b/gdb/testsuite/gdb.gdb/python-helper.exp
> index 16b419984cf..be0b311e8ca 100644
> --- a/gdb/testsuite/gdb.gdb/python-helper.exp
> +++ b/gdb/testsuite/gdb.gdb/python-helper.exp
> @@ -211,6 +211,8 @@ proc test_python_helper {} {
>      # Test the htab_t pretty-printer.
>      gdb_test -prompt $outer_prompt_re "print all_bfds" "htab_t with ${::decimal} elements = \\{${::hex}.*\\}"
>  
> +    gdb_test -prompt $outer_prompt_re "print (htab_t) 0x0" \
> +	[string_to_regexp " = (null)"]

Instead of hardcoding (null), what about not selecting the pretty
printer when the value is nullptr?  Like this:


diff --git a/gdb/gdb-gdb.py.in b/gdb/gdb-gdb.py.in
index 567ec41b305b..5092512eaca9 100644
--- a/gdb/gdb-gdb.py.in
+++ b/gdb/gdb-gdb.py.in
@@ -379,15 +379,10 @@ class HtabPrinter:
         return "array"
 
     def to_string(self):
-        if int(self._val) == 0:
-            return "(null)"
         n = int(self._val["n_elements"]) - int(self._val["n_deleted"])
         return "htab_t with {} elements".format(n)
 
     def children(self):
-        if int(self._val) == 0:
-            return
-
         size = int(self._val["size"])
         entries = self._val["entries"]
 
@@ -418,7 +413,9 @@ def type_lookup_function(val):
     elif tag is not None and tag.startswith("intrusive_list<"):
         return IntrusiveListPrinter(val)
     elif name == "htab_t":
-        return HtabPrinter(val)
+        if int(val) != 0:
+            return HtabPrinter(val)
+
     return None
 
Can the value's value change after a pretty printer was selected for it?
In that case, my idea wouldn't work, because the value could become
nullptr under the pretty printer's feet.

Simon

Re: [PATCH] Fix htab_t pretty-printer for NULL values

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

>> +    gdb_test -prompt $outer_prompt_re "print (htab_t) 0x0" \
>> +	[string_to_regexp " = (null)"]

Simon> Instead of hardcoding (null), what about not selecting the pretty
Simon> printer when the value is nullptr?  Like this:

That seems fine, how about we go with your patch?

Simon> Can the value's value change after a pretty printer was selected for it?

Nope.

Tom