RE: [PATCH] gdb: fix handling of DW_AT_entry_pc of inlined subroutines

["Gerlicher, Klaus" <klaus.gerlicher@intel.com>]

Hi Andrew, Tom,

Just an observation on the concern for size of the block struct. Inline.

> -----Original Message-----
> From: Tom Tromey <tom@tromey.com>
> Sent: Thursday, October 17, 2024 10:03 PM
> To: Andrew Burgess <aburgess@redhat.com>
> Cc: gdb-patches@sourceware.org; Bernd Edlinger
> <bernd.edlinger@hotmail.de>
> Subject: Re: [PATCH] gdb: fix handling of DW_AT_entry_pc of inlined
> subroutines
> 
> >>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
> 
> Andrew> This commit extends the block class to carry the entry PC as a
> Andrew> separate member variable.  Then the DWARF reader is extended to
> read
> Andrew> and set the entry PC for the block.  Now in block::entry_pc(), if the
> Andrew> entry PC has been set to a specific value, this is the value returned.
> 
> I wonder what the size cost is of adding a field to block -- I mean the
> overall cost, not just sizeof(block).
> 
> But I suppose in the end it's not very important, since we normally
> operate on the assumption that most CUs won't be expanded.
> 
> Andrew>    CORE_ADDR entry_pc () const
> Andrew>    {
> Andrew> -    if (this->is_contiguous ())
> Andrew> +    if (m_entry_pc.has_value ())
> Andrew> +      return *m_entry_pc;
> Andrew> +    else if (this->is_contiguous ())
> Andrew>        return this->start ();
> Andrew>      else
> Andrew>        return this->ranges ()[0].start ();
> 
> Andrew> +
> Andrew> +  /* If set this is the entry PC for the block.  */
> Andrew> +
> Andrew> +  std::optional<CORE_ADDR> m_entry_pc;
> Andrew>  };
> 
> std::optional here means carrying an extra word for just the flag bit.
> 
> However I wonder if it would work to use CORE_ADDR here and use 0 as a
> sentinel to mean unset; or some other scheme making it so the entry PC
> and the 'start' are identical by default.
> 

If size of a block struct is a concern, couldn't we remove m_start and m_end in favour of only using 
the m_ranges member? This comment seems to indicate that they are kind of exclusive... 

  /* Address ranges for blocks with non-contiguous ranges.  If this
     is NULL, then there is only one range which is specified by
     startaddr and endaddr above.  */

  struct blockranges *m_ranges = nullptr;

Thanks
Klaus

> Tom

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928