[PATCH] Fix crash in f-typeprint.c

[PATCH] Fix crash in f-typeprint.c

[Tom Tromey]

I noticed a crash in f-typeprint.c that was hidden by an xfail:

XFAIL: gdb.fortran/vla-array.exp: print variable length string array type (GDB internal error) (PRMS gcc/101826)

I think this was introduced by commit 6594ca4a ("do not handle a NULL
linebuffer in pager_file::puts") but not detected due to the xfail.

It seems bad for an xfail to cover up a crash but I haven't
investigated that.

Meanwhile, this patch fixes the crash by checking for a NULL pointer
when calling gdb_puts.
---
 gdb/f-typeprint.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gdb/f-typeprint.c b/gdb/f-typeprint.c
index e96d27c537e..65c58b59b2d 100644
--- a/gdb/f-typeprint.c
+++ b/gdb/f-typeprint.c
@@ -414,7 +414,8 @@ f_language::f_type_print_base (struct type *type, struct ui_file *stream,
 
       gdb_puts (" ", stream);
 
-      gdb_puts (type->name (), stream);
+      if (type->name () != nullptr)
+	gdb_puts (type->name (), stream);
 
       /* According to the definition,
 	 we only print structure elements in case show > 0.  */
@@ -432,7 +433,8 @@ f_language::f_type_print_base (struct type *type, struct ui_file *stream,
 	      gdb_puts ("\n", stream);
 	    }
 	  gdb_printf (stream, "%*sEnd Type ", level, "");
-	  gdb_puts (type->name (), stream);
+	  if (type->name () != nullptr)
+	    gdb_puts (type->name (), stream);
 	}
       break;
 
-- 
2.49.0

Re: [PATCH] Fix crash in f-typeprint.c

[Kevin Buettner]

On Sat, 13 Sep 2025 13:48:11 -0600
Tom Tromey <tom@tromey.com> wrote:

> diff --git a/gdb/f-typeprint.c b/gdb/f-typeprint.c
> index e96d27c537e..65c58b59b2d 100644
> --- a/gdb/f-typeprint.c
> +++ b/gdb/f-typeprint.c
> @@ -414,7 +414,8 @@ f_language::f_type_print_base (struct type *type,
> struct ui_file *stream, 
>        gdb_puts (" ", stream);
>  
> -      gdb_puts (type->name (), stream);
> +      if (type->name () != nullptr)
> +	gdb_puts (type->name (), stream);

If you can't print the type, I'm wondering if it makes sense to print
the space before it? (I.e. look at the gdb_puts a few lines above...)

Kevin

Re: [PATCH] Fix crash in f-typeprint.c

[Tom Tromey]

>>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:

>> gdb_puts (" ", stream);
>> 
>> -      gdb_puts (type->name (), stream);
>> +      if (type->name () != nullptr)
>> +	gdb_puts (type->name (), stream);

Kevin> If you can't print the type, I'm wondering if it makes sense to print
Kevin> the space before it? (I.e. look at the gdb_puts a few lines above...)

I don't know but this change does preserve what happened before the
change to gdb_puts.

Tom

Re: [PATCH] Fix crash in f-typeprint.c

[Kevin Buettner]

On Sat, 13 Sep 2025 16:29:41 -0600
Tom Tromey <tom@tromey.com> wrote:

> >>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:  
> 
> >> gdb_puts (" ", stream);
> >> 
> >> -      gdb_puts (type->name (), stream);
> >> +      if (type->name () != nullptr)
> >> +	gdb_puts (type->name (), stream);  
> 
> Kevin> If you can't print the type, I'm wondering if it makes sense to
> Kevin> print the space before it? (I.e. look at the gdb_puts a few lines
> Kevin> above...)  
> 
> I don't know but this change does preserve what happened before the
> change to gdb_puts.

Since it fixes a crash and preserves the original behavior...

Approved-by: Kevin Buettner <kevinb@redhat.com>

Re: [PATCH] Fix crash in f-typeprint.c

[Tom Tromey]

>> I don't know but this change does preserve what happened before the
>> change to gdb_puts.

Kevin> Since it fixes a crash and preserves the original behavior...

Well, I took another look and I think that moving the space-printing
into the same 'if' seems ok.  It doesn't cause any test regressions
either.

Here's the updated version, let me know what you think.

Tom

commit 2c56513f148f24d9595a8bc95f15acd3e70353d3
Author: Tom Tromey <tom@tromey.com>
Date:   Sat Sep 13 13:44:10 2025 -0600

    Fix crash in f-typeprint.c
    
    I noticed a crash in f-typeprint.c that was hidden by an xfail:
    
    XFAIL: gdb.fortran/vla-array.exp: print variable length string array type (GDB internal error) (PRMS gcc/101826)
    
    I think this was introduced by commit 6594ca4a ("do not handle a NULL
    linebuffer in pager_file::puts") but not detected due to the xfail.
    
    It seems bad for an xfail to cover up a crash but I haven't
    investigated that.
    
    Meanwhile, this patch fixes the crash by checking for a NULL pointer
    when calling gdb_puts.

diff --git a/gdb/f-typeprint.c b/gdb/f-typeprint.c
index e96d27c537e..04eca74fb2b 100644
--- a/gdb/f-typeprint.c
+++ b/gdb/f-typeprint.c
@@ -412,9 +412,11 @@ f_language::f_type_print_base (struct type *type, struct ui_file *stream,
       if (show > 0)
 	f_type_print_derivation_info (type, stream);
 
-      gdb_puts (" ", stream);
-
-      gdb_puts (type->name (), stream);
+      if (type->name () != nullptr)
+	{
+	  gdb_puts (" ", stream);
+	  gdb_puts (type->name (), stream);
+	}
 
       /* According to the definition,
 	 we only print structure elements in case show > 0.  */
@@ -432,7 +434,8 @@ f_language::f_type_print_base (struct type *type, struct ui_file *stream,
 	      gdb_puts ("\n", stream);
 	    }
 	  gdb_printf (stream, "%*sEnd Type ", level, "");
-	  gdb_puts (type->name (), stream);
+	  if (type->name () != nullptr)
+	    gdb_puts (type->name (), stream);
 	}
       break;
 

Re: [PATCH] Fix crash in f-typeprint.c

[Kevin Buettner]

On Sat, 20 Sep 2025 14:58:31 -0600
Tom Tromey <tom@tromey.com> wrote:

> >> I don't know but this change does preserve what happened before the
> >> change to gdb_puts.  
> 
> Kevin> Since it fixes a crash and preserves the original behavior...  
> 
> Well, I took another look and I think that moving the space-printing
> into the same 'if' seems ok.  It doesn't cause any test regressions
> either.
> 
> Here's the updated version, let me know what you think.

Yeah, I like this one better.

It still has my approval...

Kevin

Re: [PATCH] Fix crash in f-typeprint.c

[Tom Tromey]

Kevin> Yeah, I like this one better.

Kevin> It still has my approval...

Thanks.

I'm going to backport this to gdb-17 as well, since it's a regression
with a trivial fix.

Tom