Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH v1] gdbtypes: improve documentation of composite type helpers
@ 2026-08-13 17:27 Matthieu Longo
  2026-08-14  4:17 ` Simon Marchi
  0 siblings, 1 reply; 3+ messages in thread
From: Matthieu Longo @ 2026-08-13 17:27 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi, Matthieu Longo

Document how arch_composite_type handles a null NAME, and clarify the
semantics of a null or empty field name for the append_composite_type*
helpers.

Suggested-By: Simon Marchi <simark@simark.ca>
---
 gdb/gdbtypes.c | 21 ++++++++++++++-------
 gdb/gdbtypes.h | 13 +++++++++----
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 4b6c01910f4..7d8b7503da2 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -5413,7 +5413,8 @@ append_flags_type_flag (struct type *type, int bitpos, const char *name)
 }
 
 /* Allocate a TYPE_CODE_STRUCT or TYPE_CODE_UNION type structure (as
-   specified by CODE) associated with GDBARCH.  NAME is the type name.  */
+   specified by CODE) associated with GDBARCH.  NAME is the type name.
+   If NAME is nullptr, the type is anonymous.  */
 
 struct type *
 arch_composite_type (struct gdbarch *gdbarch, const char *name,
@@ -5428,9 +5429,11 @@ arch_composite_type (struct gdbarch *gdbarch, const char *name,
   return t;
 }
 
-/* Add new field with name NAME and type FIELD to composite type T.
-   Do not set the field's position or adjust the type's length;
-   the caller should do so.  Return the new field.  */
+/* Add a new field named NAME with type FIELD to composite type T.
+   This function does not set the field's position or adjust the length of T;
+   the caller is responsible for doing so.  If NAME is nullptr or an empty
+   string, the field represents an anonymous struct or union whose fields
+   are visible directly in the parent scope.  Return the newly added field.  */
 
 struct field *
 append_composite_type_field_raw (struct type *t, const char *name,
@@ -5448,8 +5451,10 @@ append_composite_type_field_raw (struct type *t, const char *name,
   return f;
 }
 
-/* Add new field with name NAME and type FIELD to composite type T.
-   ALIGNMENT (if non-zero) specifies the minimum field alignment.  */
+/* Add a new field named NAME with type FIELD to composite type T.
+   ALIGNMENT (if non-zero) specifies the minimum field alignment.
+   If NAME is nullptr or an empty string, the field represents an anonymous
+   struct or union whose fields are visible directly in the parent scope.   */
 
 void
 append_composite_type_field_aligned (struct type *t, const char *name,
@@ -5489,7 +5494,9 @@ append_composite_type_field_aligned (struct type *t, const char *name,
     }
 }
 
-/* Add new field with name NAME and type FIELD to composite type T.  */
+/* Add a new field named NAME with type FIELD to composite type T.
+   If NAME is nullptr or an empty string, the field represents an anonymous
+   struct or union whose fields are visible directly in the parent scope.  */
 
 void
 append_composite_type_field (struct type *t, const char *name,
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index dd2d24fa8e2..93bd938d0aa 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -2431,14 +2431,19 @@ extern struct type *init_pointer_type (type_allocator &alloc, int bit,
 extern struct type *init_fixed_point_type (type_allocator &, int, int,
 					   const char *);
 
-/* Helper functions to construct a struct or record type.  An
-   initially empty type is created using arch_composite_type().
-   Fields are then added using append_composite_type_field*().  A union
-   type has its size set to the largest field.  A struct type has each
+/* Helper functions to construct a struct or record type.  An initially empty
+   type is created using arch_composite_type().  Fields are then added using
+   append_composite_type_field*().
+   A union type has its size set to the largest field.  A struct type has each
    field packed against the previous.  */
 
+/* Note: if NAME is nullptr, the type is anonymous.  */
 extern struct type *arch_composite_type (struct gdbarch *gdbarch,
 					 const char *name, enum type_code code);
+
+/* Note: for all 3 functions below, if NAME is nullptr or an empty string,
+   the field represents an anonymous struct or union whose fields are visible
+   directly in the parent scope.  */
 extern void append_composite_type_field (struct type *t, const char *name,
 					 struct type *field);
 extern void append_composite_type_field_aligned (struct type *t,
-- 
2.55.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] gdbtypes: improve documentation of composite type helpers
  2026-08-13 17:27 [PATCH v1] gdbtypes: improve documentation of composite type helpers Matthieu Longo
@ 2026-08-14  4:17 ` Simon Marchi
  2026-08-14 10:51   ` Matthieu Longo
  0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi @ 2026-08-14  4:17 UTC (permalink / raw)
  To: Matthieu Longo, gdb-patches

On 8/13/26 1:27 PM, Matthieu Longo wrote:
> Document how arch_composite_type handles a null NAME, and clarify the
> semantics of a null or empty field name for the append_composite_type*
> helpers.
> 
> Suggested-By: Simon Marchi <simark@simark.ca>

Thanks for the patch!

It would be a good time to clean things up a bit, to bring them up to
the current standard.  The comments in the .c file should just be:

/* See gdbtypes.h.  */

And the proper comments should be moved to the .h file.

> @@ -5428,9 +5429,11 @@ arch_composite_type (struct gdbarch *gdbarch, const char *name,
>    return t;
>  }
>  
> -/* Add new field with name NAME and type FIELD to composite type T.
> -   Do not set the field's position or adjust the type's length;
> -   the caller should do so.  Return the new field.  */
> +/* Add a new field named NAME with type FIELD to composite type T.
> +   This function does not set the field's position or adjust the length of T;
> +   the caller is responsible for doing so.  If NAME is nullptr or an empty
> +   string, the field represents an anonymous struct or union whose fields
> +   are visible directly in the parent scope.  Return the newly added field.  */
>  
>  struct field *
>  append_composite_type_field_raw (struct type *t, const char *name,

I don't think that a field NAME can be nullptr.  For the "search through
anonymous structs/unions" code to work, the name specifically needs to
be a non-nullptr empty string:

https://gitlab.com/gnutools/binutils-gdb/-/blob/a760a797aeb6e9143d60f0b8b2b8f71da88cabcf/gdb/valops.c#L2014-2015

And I don't think it's actually safe to pass nullptr.  When I try to
change one of the siginfo fields to have a nullptr type, GDB crashes
here:

#2  0x00007efd9800ded2 in __ubsan_handle_nonnull_arg_abort () from /usr/lib/libubsan.so.1
#3  0x000055fa5ad2f2a5 in ui_file::puts (this=0x7b1d93e02310, str=0x0) at /home/smarchi/src/binutils-gdb/gdb/ui-file.h:77
#4  0x000055fa5e47b89d in gdb_puts (linebuffer=0x0, stream=0x7b1d93e02310) at /home/smarchi/src/binutils-gdb/gdb/utils.c:1912
#5  0x000055fa5e47bb9d in fputs_styled (linebuffer=0x0, style=..., stream=0x7b1d93e02310) at /home/smarchi/src/binutils-gdb/gdb/utils.c:1928

I think it would be safer to just say "empty string" here.

For types, I think that the name can be nullptr: it's possible to create
a type and never call set_name on it.

Simon

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] gdbtypes: improve documentation of composite type helpers
  2026-08-14  4:17 ` Simon Marchi
@ 2026-08-14 10:51   ` Matthieu Longo
  0 siblings, 0 replies; 3+ messages in thread
From: Matthieu Longo @ 2026-08-14 10:51 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 14/08/2026 05:17, Simon Marchi wrote:
> On 8/13/26 1:27 PM, Matthieu Longo wrote:
>> Document how arch_composite_type handles a null NAME, and clarify the
>> semantics of a null or empty field name for the append_composite_type*
>> helpers.
>>
>> Suggested-By: Simon Marchi <simark@simark.ca>
> 
> Thanks for the patch!
> 
> It would be a good time to clean things up a bit, to bring them up to
> the current standard.  The comments in the .c file should just be:
> 
> /* See gdbtypes.h.  */
> 
> And the proper comments should be moved to the .h file.
> 
>> @@ -5428,9 +5429,11 @@ arch_composite_type (struct gdbarch *gdbarch, const char *name,
>>    return t;
>>  }
>>  
>> -/* Add new field with name NAME and type FIELD to composite type T.
>> -   Do not set the field's position or adjust the type's length;
>> -   the caller should do so.  Return the new field.  */
>> +/* Add a new field named NAME with type FIELD to composite type T.
>> +   This function does not set the field's position or adjust the length of T;
>> +   the caller is responsible for doing so.  If NAME is nullptr or an empty
>> +   string, the field represents an anonymous struct or union whose fields
>> +   are visible directly in the parent scope.  Return the newly added field.  */
>>  
>>  struct field *
>>  append_composite_type_field_raw (struct type *t, const char *name,
> 
> I don't think that a field NAME can be nullptr.  For the "search through
> anonymous structs/unions" code to work, the name specifically needs to
> be a non-nullptr empty string:
> 
> https://gitlab.com/gnutools/binutils-gdb/-/blob/a760a797aeb6e9143d60f0b8b2b8f71da88cabcf/gdb/valops.c#L2014-2015
> 
> And I don't think it's actually safe to pass nullptr.  When I try to
> change one of the siginfo fields to have a nullptr type, GDB crashes
> here:
> 
> #2  0x00007efd9800ded2 in __ubsan_handle_nonnull_arg_abort () from /usr/lib/libubsan.so.1
> #3  0x000055fa5ad2f2a5 in ui_file::puts (this=0x7b1d93e02310, str=0x0) at /home/smarchi/src/binutils-gdb/gdb/ui-file.h:77
> #4  0x000055fa5e47b89d in gdb_puts (linebuffer=0x0, stream=0x7b1d93e02310) at /home/smarchi/src/binutils-gdb/gdb/utils.c:1912
> #5  0x000055fa5e47bb9d in fputs_styled (linebuffer=0x0, style=..., stream=0x7b1d93e02310) at /home/smarchi/src/binutils-gdb/gdb/utils.c:1928
> 
> I think it would be safer to just say "empty string" here.
> 
> For types, I think that the name can be nullptr: it's possible to create
> a type and never call set_name on it.
> 
> Simon

Addressed the above comments in v2.
https://inbox.sourceware.org/gdb-patches/20260814104941.522777-1-matthieu.longo@arm.com/

Matthieu

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-14 10:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-13 17:27 [PATCH v1] gdbtypes: improve documentation of composite type helpers Matthieu Longo
2026-08-14  4:17 ` Simon Marchi
2026-08-14 10:51   ` Matthieu Longo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox