Re: [PATCH v1] gdbtypes: improve documentation of composite type helpers

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 <[email protected]>
> 
> 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/[email protected]/

Matthieu