Re: [PATCH v3 6/8] Introduce find_pc_partial_entry_range and use it in infrun.c

[Kevin Buettner <kevinb@redhat.com>]

On Tue, 21 Aug 2018 12:19:13 -0400
Simon Marchi <simon.marchi@ericsson.com> wrote:

> On 2018-08-20 06:45 PM, Kevin Buettner wrote:
> > An earlier version of this patch used the returned block in conjunction
> > with BLOCK_ENTRY_PC to set stop_func_start in fill_in_stop_func() in
> > infrun.c.  While I think this was the correct thing to do, changes
> > to find_inferior_partial_function could potentially end up with
> > stop_func_end < stop_func_start, which is definitely wrong.  For
> > this case, we want to set both stop_func_start and stop_func_end
> > to the start and end of the range containing the function's entry
> > pc.
> > 
> > I think that this functionality will be useful in many other places
> > too - it probably ought to be used in all of the various prologue
> > analyzers in GDB.
> > 
> > The change to infrun.c was simple: the call to find_pc_partial_function
> > was replaced with a call to find_pc_partial_entry_range.  The difference
> > between these two functions is that find_pc_partial_entry_function
> > will (potentially) return the start and end address corresponding to
> > the range in which PC is found, but find_pc_partial_entry_range
> > will (again, potentially) return the start and end address of the
> > range containing the entry pc.  find_pc_partial_function has the
> > property that *ADDRESS <= PC < *ENDADDR.  This condition does not
> > necessarily hold for the outputs of find_pc_partial_entry_range.
> > 
> > It should be noted that for functions which contain only a single
> > range, the outputs of find_pc_partial_{function,entry_range} are
> > identical.
> > 
> > I think it might happen that find_pc_partial_entry_range will come
> > to be used in place of many of the calls to find_pc_partial_function
> > within GDB.  Care must be taken in making this change, however, since
> > some of this code depends on the *ADDRESS <= PC < *ENDADDR property.  
> 
> This LGTM.
> 
> I just find the naming of all these finc_pc_* funtions confusing, like
> what does the "partial" mean in "find_pc_partial_function"?  Why does
> the new function name not include "function"?  I don't really have a
> better alternative to suggest, but if there's a logic behind the new name
> I'd like to know it so I understand the naming scheme better.

I (also) don't understand the original intent/meaning behind the use of
"partial" in the name.

As noted elsewhere, I've made a change to what the return values
*ADDRESS and *ENDADDR refer to: the start and end address now refer
only to the part (range) of the function in which PC is found.  So
that's what "partial" might now refer to, though I'll admit that I
hadn't put much thought into it until just now.

With regard to "find_pc_partial_entry_range", it seems to me that
"find_pc_partial_function_entry_range" is overly long.  Maybe
"find_pc_function_entry_range" is better?

Kevin