[PATCH v3 2/5] objdump: New feature display of global variable information based on DWARF info section
Jan Beulich
jbeulich@suse.com
Wed Sep 3 09:46:01 GMT 2025
On 01.09.2025 19:50, Guillaume VACHERIAS wrote:
> Add in objdump option --map-global-vars to display global variable information
As for patch 1, some words on what good this does would be nice.
> binutils/
>
> * objdump.c (dump_global_vars_info): New function. Entry point of
> display of variable information option.
> (dump_global_variable_info): New function. Load and go through dwarf
> info section to process information.
> * dwarf.c (insert_element_in_list): New function. Retrieve valuable
> information from dwarf attributes of all DWARF debug information
> entries.
> (get_location_expression): New function. Useful to retrieve
> DW_AT_location value which gives the data object location.
> (read_and_display_attr_value): Add arguments useful to identify DWARF
> information entry, and instrument with function
> insert_element_in_list.
> (read_and_display_attr): Add arguments useful to identify DWARF
> information entry.
> (process_debug_info): Add argument to process DWARF information entry
> for the display of global variable information.
> (display_variable_mapping_info): New function.
>
> binutils/doc/
>
> * binutils.texi: Add documentation for new option --map-global-vars.
>
> Signed-off-by: Guillaume VACHERIAS <guillaume.vacherias@foss.st.com>
> ---
> binutils/ChangeLog | 22 ++-
> binutils/doc/binutils.texi | 7 +
> binutils/dwarf.c | 382 +++++++++++++++++++++++++++++++++++--
> binutils/dwarf.h | 1 +
> binutils/objdump.c | 86 ++++++++-
> 5 files changed, 478 insertions(+), 20 deletions(-)
>
> diff --git a/binutils/ChangeLog b/binutils/ChangeLog
> index 0c061efcb17..72f0ed9eb0a 100644
> --- a/binutils/ChangeLog
> +++ b/binutils/ChangeLog
> @@ -1,8 +1,28 @@
> 2025-09-01 Guillaume Vacherias <guillaume.vacherias@foss.st.com>
> - * dwarf.c (DO_LOC, DO_TYPES): Define.
> + * dwarf.c (DO_LOC, DO_TYPES, DO_GLOBAL_VARS): Define.
> (process_debug_info): Change arguments do_loc and do_types
> to a single unsigned int do_flags.
> + (insert_element_in_list): New function. Retrieve valuable
> + information from DWARF attributes of all DWARF debug information
> + entries.
> + (get_location_expression): New function. Useful to retrieve
> + DW_AT_location value which gives the data object location.
> + (read_and_display_attr_value): Add arguments useful to identify DWARF
> + information entry, and instrument with function
> + insert_element_in_list.
> (find_cu_tu_set_v2): Change parameter do_types from int to bool.
> + (read_and_display_attr): Add arguments useful to identify DWARF
> + information entry.
> + (process_debug_info): Add argument to process DWARF information entry
> + for the display of global variable information.
> + (display_variable_mapping_info): New function.
> +
> + * objdump.c (dump_global_vars_info): New function. Entry point of
> + display of variable information option.
> + (dump_global_variable_info): New function. Load and go through DWARF
> + info section to process information.
> +
> + * doc/binutils.texi: Add documentation for new option --map-global-vars.
First, see my respective comment on patch 1. Then, even if you were to modify
the file directly, you definitely don't want to alter an existing entry.
> @@ -2435,6 +2571,53 @@ display_lang (uint64_t uvalue)
> }
> }
>
> +static void
> +insert_element_in_list (enum dwarf_tag dw_tag,
> + enum dwarf_attribute dw_attr ATTRIBUTE_UNUSED,
> + uint64_t die_offset ATTRIBUTE_UNUSED,
> + uint64_t *uvalue ATTRIBUTE_UNUSED,
> + int64_t *svalue ATTRIBUTE_UNUSED,
> + const unsigned char *data ATTRIBUTE_UNUSED)
> +{
> + /* TODO: Retrieve attributes information from
> + below dwarf entries tag. */
> + switch (dw_tag)
> + {
> + /* Base type. */
> + case DW_TAG_base_type:
> + break;
> + /* typedef type. */
With "typedef" being a keyword in C, having it all lower-case may be okay
here, but ...
> + case DW_TAG_typedef:
> + break;
> + /* enumeration type. */
"enumeration" clearly isn't a keyword, and hence wants starting with a capital
letter, just like the other comments here have it. Also, isn't this more the
comment for ...
> + case DW_TAG_enumerator:
> + break;
> + case DW_TAG_enumeration_type:
> + break;
... this case?
> + /* Array type. */
> + case DW_TAG_subrange_type:
> + break;
> + case DW_TAG_array_type:
> + break;
Same here - misplaced (and missing) comment?
> + /* Member type. */
> + case DW_TAG_member:
> + break;
> + /* Structure type. */
> + case DW_TAG_structure_type:
> + /* Union type. */
> + case DW_TAG_union_type:
> + break;
> + case DW_TAG_const_type:
> + case DW_TAG_volatile_type:
> + case DW_TAG_pointer_type:
> + break;
> + case DW_TAG_variable:
> + break;
No comments for these last four? I'm not convinced of the need for these
comments, but imo if you add them, then please do so consistently.
> @@ -2451,7 +2634,10 @@ read_and_display_attr_value (unsigned long attribute,
> struct dwarf_section *section,
> struct cu_tu_set *this_set,
> char delimiter,
> - int level)
> + int level,
> + int do_var_map,
bool? (applicable elsewhere as well)
> @@ -2855,14 +3098,25 @@ read_and_display_attr_value (unsigned long attribute,
>
> /* We have already displayed the form name. */
> if (idx != (uint64_t) -1)
> - printf (_("%c(index: %#" PRIx64 "): %#" PRIx64),
> + {
> + printf (_("%c(index: %#" PRIx64 "): %#" PRIx64),
You shouldn't change this line's indentation without ...
> delimiter, uvalue, idx);
... this one's.
> @@ -3143,6 +3397,76 @@ read_and_display_attr_value (unsigned long attribute,
> }
> }
>
> + if (do_var_map && attribute != 0)
Why the check of attribute, when the default case of the switch (doing nothing)
would handle it fine?
> + {
> + switch (attribute)
> + {
> + case DW_AT_frame_base:
> + case DW_AT_location:
> + case DW_AT_loclists_base:
> + case DW_AT_rnglists_base:
> + case DW_AT_str_offsets_base:
> + case DW_AT_string_length:
> + case DW_AT_return_addr:
> + case DW_AT_data_member_location:
> + case DW_AT_vtable_elem_location:
> + case DW_AT_segment:
> + case DW_AT_static_link:
> + case DW_AT_use_location:
> + case DW_AT_call_value:
> + case DW_AT_GNU_call_site_value:
> + case DW_AT_call_data_value:
> + case DW_AT_GNU_call_site_data_value:
> + case DW_AT_call_target:
> + case DW_AT_GNU_call_site_target:
> + case DW_AT_call_target_clobbered:
> + case DW_AT_GNU_call_site_target_clobbered:
> + case DW_AT_allocated:
> + case DW_AT_associated:
> + case DW_AT_data_location:
> + case DW_AT_stride:
> + case DW_AT_upper_bound:
> + case DW_AT_lower_bound:
> + case DW_AT_rank:
> + if (block_start)
> + {
> + get_location_expression (die_tag,
> + attribute,
> + block_start,
> + pointer_size,
> + offset_size,
> + dwarf_version,
> + die_offset,
> + uvalue,
> + cu_offset,
> + section);
You may want to make better use of line length here.
> + }
Omitting the figure braces also would reduce indentation a tiny bit.
(Both applicable again below.)
> @@ -3834,7 +4164,7 @@ process_debug_info (struct dwarf_section * section,
> alloc_num_debug_info_entries = num_units;
> }
>
> - if (!(do_flags & DO_LOC))
> + if (!(do_flags & DO_LOC) || do_flags & DO_GLOBAL_VARS)
Prentheses please to visually separate the & expression from the || one.
> @@ -7683,6 +8020,13 @@ display_debug_str (struct dwarf_section *section,
> return 1;
> }
>
> +static int
> +display_variable_mapping_info (struct dwarf_section *section, void *file)
> +{
> + return process_debug_info (section, file, section->abbrev_sec,
> + DO_LOC | DO_GLOBAL_VARS);
Why also DO_LOC?
> @@ -4528,6 +4533,42 @@ dump_dwarf_section (bfd *abfd, asection *section,
> }
> }
>
> +static void
> +dump_global_variable_info (bfd *abfd, asection *section,
> + void *arg)
> +{
> + const char *name = bfd_section_name (section);
> + const char *match;
> + bool is_mainfile = *(bool *) arg;
> +
> + if (*name == 0)
> + return;
> +
> + if (!is_mainfile && !process_links
> + && (section->flags & SEC_DEBUGGING) == 0)
> + return;
> +
> + if (startswith (name, ".gnu.linkonce.wi."))
> + match = ".debug_info";
> + else
> + match = name;
> +
> + if (((strcmp (debug_displays[info].section.uncompressed_name,match) == 0
> + || strcmp (debug_displays[info].section.compressed_name, match) == 0
> + || strcmp (debug_displays[info].section.xcoff_name, match) == 0))
> + && debug_displays[info].enabled != NULL
> + && *debug_displays[info].enabled)
I'm pretty sure I did already comment on this or a similar indentation issue?
> @@ -4560,6 +4601,41 @@ dump_dwarf (bfd *abfd, bool is_mainfile)
> bfd_map_over_sections (abfd, dump_dwarf_section, (void *) &is_mainfile);
> }
>
> +
Please don't introduce double blank lines, as they can make patch context
less meaningful.
> +/* Dump all global variable information in memory. */
> +
> +static void
> +dump_global_vars_info (bfd *abfd, bool is_mainfile)
> +{
> + /* The byte_get pointer should have been set at the start of dump_bfd (). */
> + if (byte_get == NULL)
> + {
> + warn (_ ("File %s does not contain any dwarf debug information\n"),
> + bfd_get_filename (abfd));
> + return;
> + }
> +
> + switch (bfd_get_arch (abfd))
> + {
> + case bfd_arch_s12z:
> + /* S12Z has a 24 bit address space. But the only known
> + producer of dwarf_info encodes addresses into 32 bits. */
> + eh_addr_size = 4;
> + break;
> +
> + default:
> + eh_addr_size = bfd_arch_bits_per_address (abfd) / 8;
> + break;
> + }
I pretty strongly think such logic wants isolating into a helper rather
than having in two places, where thngs can easily go out of sync.
Jan
More information about the Binutils
mailing list