[PATCH v2 1/5] Add suitable defines to use at call and use sites

[Guillaume VACHERIAS - foss]

I'll correct what you've pointed out in v3.
Thank you !

Guillaume

On 27.08.2025 19:00, Guillaume VACHERIAS wrote:
> Changes since v1:
> - Added new prerequisite patch to better handle call and use sites.
> - Change function process_debug_info arguments bool to unsigned int.
> - Change option name to "--map-global-vars" instead of "-Y/--map-file".
> - Added information on DWARF dependencues of this option.
> - Correct switch-case braces alignment.
> - Correct type of structure container field holding DW_AT_name from 
> (unsigned char *) to plain text (char *).
> - Change to sizeof (<expression>) to avoid risk of variable's types 
> being changed.
> - Replace error messages with asserts.
> - Rename enumeration identifier UNKNOWN to UNKNWON_TYPE to be less generic.
> - Remove unnecessary typedef aliases.
> 
> --

The above isn't supposed to go into the eventual commit, is it? In its place (more precisely, ahead of it, followed by a --- separator) a non-empty description would be nice. Even if it's only a single sentence saying why the change is being done. That, after all, ...

> binutils/
> 
> 	* dwarf.c (process_debug_info): Change arguments do_loc and do_types
> 	to single unsigned int do_flags.

.... isn't being expressed by ChangeLog entries.

> --- a/binutils/dwarf.c
> +++ b/binutils/dwarf.c
> @@ -51,6 +51,9 @@
>  #define MAX(a, b) ((a) > (b) ? (a) : (b))  #define MIN(a, b) ((a) < 
> (b) ? (a) : (b))
>  
> +#define DO_LOC     0X1
> +#define DO_TYPES   0X2

Nit: Lower-case x-es please.

> @@ -3891,7 +3893,7 @@ process_debug_info (struct dwarf_section * 
> section,
>  
>        SAFE_BYTE_GET_AND_INC (compunit.cu_version, hdrptr, 2, end_cu);
>  
> -      this_set = find_cu_tu_set_v2 (cu_offset, do_types);
> +      this_set = find_cu_tu_set_v2 (cu_offset, (do_flags & 
> + DO_TYPES));

For this, find_cu_tu_set_v2() 2nd parameter would better also switch to bool.

> @@ -3902,7 +3904,7 @@ process_debug_info (struct dwarf_section * section,
>        else
>  	{
>  	  SAFE_BYTE_GET_AND_INC (compunit.cu_unit_type, hdrptr, 1, end_cu);
> -	  do_types = (compunit.cu_unit_type == DW_UT_type);
> +	  do_flags |= (compunit.cu_unit_type == DW_UT_type) ? DO_TYPES : 0;

Original code set the flag anew; you only ever set, but never clear the flag.
(Same issue elsewhere as it looks.)

> @@ -4022,7 +4024,7 @@ process_debug_info (struct dwarf_section * section,
>  	  compunit.cu_pointer_size = offset_size;
>  	}
>  
> -      if (do_types)
> +      if ((do_flags & DO_TYPES))

I'd recommend against the extra parentheses, as they (slightly) hamper readability.

> @@ -4303,7 +4306,7 @@ process_debug_info (struct dwarf_section * section,
>  					    offset_size,
>  					    compunit.cu_version,
>  					    debug_info_p,
> -					    do_loc || ! do_printing,
> +					    do_flags & DO_LOC|| ! do_printing,

Here, otoh, you will want to add parentheses (and a blank).

> @@ -7675,19 +7679,19 @@ display_debug_str (struct dwarf_section 
> *section,  static int  display_debug_info (struct dwarf_section 
> *section, void *file)  {
> -  return process_debug_info (section, file, section->abbrev_sec, 
> false, false);
> +  return process_debug_info (section, file, section->abbrev_sec, 
> + 0x0);

Preferably just 0 please.

Jan