[PATCH v4 0/8] Non-contiguous address range support

[PATCH v4 0/8] Non-contiguous address range support

[Kevin Buettner]

This is version 4 of an eight part patch series which adds further
support for non-contiguous address ranges to GDB.

This v4 series addresses concerns from Simon Marchi and Pedro Alves.
Only parts 3, 6, and 8 have changed since v3.

The v3 series had been rebased against more recent (current at time
of posting) sources.

In the v2 series, I addressed the concerns from Simon Marchi's
review of the v1 patch set.  I also changed my mind about how
return values *ADDRESS and *ENDADDR ought to be set for
find_pc_partial_function.  I discuss this matter in the remarks
preceding the relevant patches.

Everything below this point was copy/pasted from the introductory
message for the v1 patch set...

This sequence of patches was motivated by GCC bug 84550:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84550

There is a test case posted to that bug along with some analysis of
the underlying problem.

There is also a GDB bug for the same issue; it's 23021, but at the
moment, there is little there aside from a link to the GCC bug
mentioned above.  But here's a link anyway:

https://sourceware.org/bugzilla/show_bug.cgi?id=23021

A quick synopsis of the problem is as follows...

Recent versions of gcc can generate code in which a function is split
into at least two non-contiguous address ranges.  As I understand it,
the idea here is to separate code which gcc does not expect to execute
in normal operation from the rest of the code.  Doing this may result
in better cache locality for the normal case.  The generated code for
the example in GCC bug 84550 separated a call to abort() from the rest
of the code comprising the function.

In the course of my investigation, I identified at least four
problems:

1) Stepping into a function from a function which occupies non-contiguous
   address ranges does not always work.  It was not uncommon to see the
   program run to completion when attempting to do a step.

2) Setting a breakpoint on a function with non-contiguous address ranges
   causes a breakpoint to be placed on more than one location.  When a
   breakpoint is set on the "cold" address range, this is almost certainly
   incorrect.  The breakpoint should instead be set only on code near the
   entry point(s).

3) The disassemble command did not work correctly.  E.g. here is what I
   found during my analysis of 84550:

	(gdb) x/i 'main.cold.0'
	   0x4010e0 <main()>:   mov    %rax,%rdi
	(gdb) x/i main
	   0x4011a0 <main>:     push   %r12
	(gdb) disassemble main
	Dump of assembler code for function main():
	   0x00000000004010e0 <+0>:     mov    %rax,%rdi
	   ...
        [No addresses starting at 0x4011a0 are shown]

4) Display of addresses associated with the non-contiguous function are
   confusing.  E.g. in the above example, note that GDB thinks that
   the address associated with main.cold.0 is <main()>, but that there's
   also a minsym called main which is displayed as <main>.

There are probably several other problems which are related those
identified above.

I discovered that the stepping problem could be "fixed" by disabling
the find_pc_partial_function cache.  This cache keeps track of the
most recent result (of calling find_pc_partial_function).  If
find_pc_partial_function is called with an address which falls within
the cache range, then that's considered to be a cache hit and the most
recent result is returned.  Obviously, this won't work correctly for
functions which occupy non-contiguous (disjoint) address ranges where
other functions might be placed in the gap.

So one of the problems that needed to be solved was to make the
caching code work correctly.  It is interesting to note that stepping
_did_ work when the cache was disabled.  This is/was due to GDB
already having some (albeit incomplete) support for non-contiguous
addresses in the form of blockvector address maps.  Code responsible
for mapping addresses to blocks (which form the lower levels of
find_pc_partial_function) handle this case correctly.

To solve the problem of incorrect disassembly, we need to be able
to iterate over all of the ranges associated with a block.

Finally, we need to distinguish between the entry pc and the lowest
address in a block.  I discovered that this lack of distinction was
the cause of the remainder of the bugs including some which seemed to
be introduced by fixing the problems noted above.  Once this
distinction is made, it will be straightforward to add full support for
DW_AT_entry_pc.  I considered adding this support as part of this
patch series, but decided to wait until the community weighs in on my
work thus far...

[PATCH v4 1/8] Add block range data structure for blocks with non-contiguous address ranges

[Kevin Buettner]

This patch does the following:

- Introduces a block range data structure which is accessed via
  a new field in struct block.
- Defines several macros for accessing block ranges.
- Defines a new function, make_blockrange, which is responsible for
  creating the new data structure.

It should be noted that some support for non-contiguous ranges already
existed in GDB in the form of blockvector addrmaps.  This support
allowed GDB to quickly find a block containing a particular address
even when the block consists of non-contiguous addresses.  See
find_block_in_blockvector() in block.c, dwarf2_record_block_ranges()
in dwarf2read.c, and record_block_range() in buildsym.c.

Addrmaps do not provide a convenient way to examine address ranges
associated with a particular block.  This data structure (and its
interface) is set up for quickly finding the value (which in this case
is a block) associated with a particular address.  The interface
does not include a method for doing a reverse mapping from blocks to
addresses.  A linear time mapping might be attempted via use of the
addrmap's foreach method, but this is not as straightforward as it
might first appear due to the fact that blocks corresponding to inline
function instances and lexical blocks w/ variables end up getting
interspersed in in the set of transitions.

Note:  If this approach is deemed to be too expensive in terms of
space, an alternate approach might be to attempt the linear time
mapping noted above.  find_pc_partial_function() needs to be able to
quickly know whether there are discontiguous ranges, so a flag for
this property would have to be added to struct block.  Also integral
to this set of changes is the concept of an "entry pc" which might be
different from the block's start address.  An entry_pc field would
also need to be added to struct block.  This does not result in any
space savings in struct block though since the space for the flag and
entry_pc use more space than the blockranges struct pointer that I've
added.  There would, however, be some space savings due to the fact
that the new data structures that I've added for this patch would not
need to be allocated.  (I happen to like the approach I've come up
with, but I wanted to mention another possibility just in case someone
does not.)

gdb/ChangeLog:
    
    	* block.h (blockrange, blockranges): New struct declarations.
    	(struct block): Add new field named `ranges'.
    	(BLOCK_RANGES, BLOCK_NRANGES, BLOCK_RANGE, BLOCK_CONTIGUOUS_P)
    	(BLOCK_RANGE_START, BLOCK_RANGE_END, BLOCK_ENTRY_PC): New
    	macros for accessing ranges in struct block.
    	(make_blockranges): New declaration.
    	block.c (make_blockranges): New function.
---
 gdb/block.c | 21 +++++++++++++++
 gdb/block.h | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 106 insertions(+)

diff --git a/gdb/block.c b/gdb/block.c
index f26d169..85e6c61 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -807,3 +807,24 @@ block_find_non_opaque_type_preferred (struct symbol *sym, void *data)
   *best = sym;
   return 0;
 }
+
+/* See block.h.  */
+
+struct blockranges *
+make_blockranges (struct objfile *objfile,
+                  const std::vector<blockrange> &rangevec)
+{
+  struct blockranges *blr;
+  size_t n = rangevec.size();
+
+  blr = (struct blockranges *)
+    obstack_alloc (&objfile->objfile_obstack,
+                   sizeof (struct blockranges)
+		   + (n - 1) * sizeof (struct blockrange));
+
+  blr->nranges = n;
+  for (int i = 0; i < n; i++)
+    blr->range[i] = rangevec[i];
+  return blr;
+}
+
diff --git a/gdb/block.h b/gdb/block.h
index ff12725..0050eac 100644
--- a/gdb/block.h
+++ b/gdb/block.h
@@ -31,6 +31,37 @@ struct using_direct;
 struct obstack;
 struct addrmap;
 
+/* Blocks can occupy non-contiguous address ranges.  When this occurs,
+   startaddr and endaddr within struct block (still) specify the lowest
+   and highest addresses of all ranges, but each individual range is
+   specified by the addresses in struct blockrange.  */
+
+struct blockrange
+{
+  blockrange (CORE_ADDR startaddr_, CORE_ADDR endaddr_)
+    : startaddr (startaddr_),
+      endaddr (endaddr_)
+  {
+  }
+
+  /* Lowest address in this range.  */
+
+  CORE_ADDR startaddr;
+
+  /* One past the highest address in the range.  */
+
+  CORE_ADDR endaddr;
+};
+
+/* Two or more non-contiguous ranges in the same order as that provided
+   via the debug info.  */
+
+struct blockranges
+{
+  int nranges;
+  struct blockrange range[1];
+};
+
 /* All of the name-scope contours of the program
    are represented by `struct block' objects.
    All of these objects are pointed to by the blockvector.
@@ -86,6 +117,12 @@ struct block
      using directives and the current namespace scope.  */
 
   struct block_namespace_info *namespace_info;
+
+  /* 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 *ranges;
 };
 
 /* The global block is singled out so that we can provide a back-link
@@ -109,6 +146,49 @@ struct global_block
 #define BLOCK_DICT(bl)		(bl)->dict
 #define BLOCK_NAMESPACE(bl)	(bl)->namespace_info
 
+/* Accessor for ranges field within block BL.  */
+
+#define BLOCK_RANGES(bl)	(bl)->ranges
+
+/* Number of ranges within a block.  */
+
+#define BLOCK_NRANGES(bl)	(bl)->ranges->nranges
+
+/* Access range array for block BL.  */
+
+#define BLOCK_RANGE(bl)		(bl)->ranges->range
+
+/* Are all addresses within a block contiguous?  */
+
+#define BLOCK_CONTIGUOUS_P(bl)	(BLOCK_RANGES (bl) == nullptr \
+				 || BLOCK_NRANGES (bl) <= 1)
+
+/* Obtain the start address of the Nth range for block BL.  */
+
+#define BLOCK_RANGE_START(bl,n) (BLOCK_RANGE (bl)[n].startaddr)
+
+/* Obtain the end address of the Nth range for block BL.  */
+
+#define BLOCK_RANGE_END(bl,n)	(BLOCK_RANGE (bl)[n].endaddr)
+
+/* Define the "entry pc" for a block BL to be the lowest (start) address
+   for the block when all addresses within the block are contiguous.  If
+   non-contiguous, then use the start address for the first range in the
+   block.
+
+   At the moment, this almost matches what DWARF specifies as the entry
+   pc.  (The missing bit is support for DW_AT_entry_pc which should be
+   preferred over range data and the low_pc.)
+
+   Once support for DW_AT_entry_pc is added, I expect that an entry_pc
+   field will be added to one of these data structures.  Once that's done,
+   the entry_pc field can be set from the dwarf reader (and other readers
+   too).  BLOCK_ENTRY_PC can then be redefined to be less DWARF-centric.  */
+
+#define BLOCK_ENTRY_PC(bl)	(BLOCK_CONTIGUOUS_P (bl) \
+				 ? BLOCK_START (bl) \
+				 : BLOCK_RANGE_START (bl,0))
+
 struct blockvector
 {
   /* Number of blocks in the list.  */
@@ -322,4 +402,9 @@ extern int block_find_non_opaque_type_preferred (struct symbol *sym,
        (sym) != NULL;							\
        (sym) = block_iter_match_next ((name), &(iter)))
 
+/* Given a vector of pairs, allocate and build an obstack allocated
+   blockranges struct for a block.  */
+struct blockranges *make_blockranges (struct objfile *objfile,
+                                      const std::vector<blockrange> &rangevec);
+
 #endif /* BLOCK_H */

[PATCH v4 2/8] Record explicit block ranges from dwarf2read.c

[Kevin Buettner]

This change sets BLOCK_RANGES for the block under consideration by
calling make_blockranges().  This action is performed in
dwarf2_record_block_ranges().

It should be noted that dwarf2_record_block_ranges() already does some
recording of the range via a call to record_block_range().  The ranges
recorded in that fashion end up in the address map associated with the
blockvector for the compilation unit's symtab.  Given an address, the
addrmap provides a fast way of finding the block containing that
address.  The address map does not, however, provide a convenient way
of determining which address ranges make up a particular block.

While reading a set of ranges, a vector of pairs is used to collect
the starting and ending addresses for each range in the block.  Once
all of the ranges for a block have been collected, make_blockranges()
is called to fill in BLOCK_RANGES for the block.

The ranges are stored for the block in the order that they're read
from the debug info.  For DWARF, the starting address of the first
range of the block will be the entry pc in cases where DW_AT_entry_pc
is not present.  (Well, that would ideally be the case.  At the moment
DW_AT_entry_pc is not being handled.)

gdb/ChangeLog:
    
    	* dwarf2read.c (dwarf2_record_block_ranges): Fill in BLOCK_RANGES
    	for block.
---
 gdb/dwarf2read.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 81a0087..8834d08 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -14846,6 +14846,7 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
       unsigned long offset = (DW_UNSND (attr)
 			      + (need_ranges_base ? cu->ranges_base : 0));
 
+      std::vector<blockrange> blockvec;
       dwarf2_ranges_process (offset, cu,
 	[&] (CORE_ADDR start, CORE_ADDR end)
 	{
@@ -14854,7 +14855,10 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
 	  start = gdbarch_adjust_dwarf2_addr (gdbarch, start);
 	  end = gdbarch_adjust_dwarf2_addr (gdbarch, end);
 	  cu->builder->record_block_range (block, start, end - 1);
+	  blockvec.emplace_back (start, end);
 	});
+
+      BLOCK_RANGES(block) = make_blockranges (objfile, blockvec);
     }
 }
 

[PATCH v4 3/8] Add support for non-contiguous blocks to find_pc_partial_function

[Kevin Buettner]

This change adds an optional output parameter BLOCK to
find_pc_partial_function.  If BLOCK is non-null, then *BLOCK will be
set to the address of the block corresponding to the function symbol
if such a symbol was found during lookup.  Otherwise it's set to the
NULL value.  Callers may wish to use the block information to
determine whether the block contains any non-contiguous ranges.  The
caller may also iterate over or examine those ranges.

When I first started looking at the broken stepping behavior associated
with functions w/ non-contiguous ranges, I found that I could "fix"
the problem by disabling the find_pc_partial_function cache.  It would
sometimes happen that the PC passed in would be between the low and
high cache values, but would be in some other function that happens to
be placed in between the ranges for the cached function.  This caused
incorrect values to be returned.

So dealing with this cache turns out to be very important for fixing
this problem.  I explored three different ways of dealing with the
cache.

My first approach was to clear the cache when a block was encountered
with more than one range.  This would cause the non-cache pathway to
be executed on the next call to find_pc_partial_function.

Another approach, which I suspect is slightly faster, checks to see
whether the PC is within one of the ranges associated with the cached
block.  If so, then the cached values can be used.  It falls back to
the original behavior if there is no cached block.

The current approach, suggested by Simon Marchi, is to restrict the
low/high pc values recorded for the cache to the beginning and end of
the range containing the PC value under consideration.  This allows us
to retain the simple (and fast) test for determining whether the
memoized (cached) values apply to the PC passed to
find_pc_partial_function.

Another choice that had to be made regards setting *ADDRESS and
*ENDADDR.  There are three possibilities which might make sense:

1) *ADDRESS and *ENDADDR represent the lowest and highest address
   of the function.
2) *ADDRESS and *ENDADDR are set to the start and end address of
   the range containing the entry pc.
3) *ADDRESS and *ENDADDR are set to the start and end address of
   the range in which PC is found.

An earlier version of this patch implemented option #1.  I found out
that it's not very useful though and, in fact, returns results that
are incorrect when used in the context of determining the start and
end of the function for doing prologue analysis.  While debugging a
function in which the entry pc was in the second range (of a function
containing two non-contiguous ranges), I noticed that
amd64_skip_prologue called find_pc_partial_function - the returned
start address was set to the beginning of the first range.  This is
incorrect for this function.  What was also interesting was that this
first invocation of find_pc_partial_function correctly set the cache
for the PC on which it had been invoked, but a slightly later call
from skip_prologue_using_sal could not use this cached value because
it was now being used to lookup the very lowest address of the
function - which is in a range not containing the entry pc.

Option #2 is attractive as it would provide a desirable result
when used in the context of prologue analysis.  However, many callers,
including some which do prologue analysis want the condition
*ADDRESS <= PC < *ENDADDR to hold.  This will not be the case when
find_pc_partial_function is called on a PC that's in a non-entry-pc
range.  A later patch to this series adds
find_function_entry_range_from_pc as a wrapper of
find_pc_partial_function.

Option #3 causes the *ADDRESS <= PC < *ENDADDR property to hold.  If
find_pc_partial_function is called with a PC that's within entry pc's
range, then it will correctly return the limits of that range.  So, if
the result of a minsym search is passed to find_pc_partial_function
to find the limits, then correct results will be achieved.  Returned
limits (for prologue analysis) won't be correct when PC is within some
other (non-entry-pc) range.  I don't yet know how big of a problem
this might be; I'm guessing that it won't be a serious problem - if a
compiler generates functions which have non-contiguous ranges, then it
also probably generates DWARF2 CFI which makes a lot of the old
prologue analysis moot.

I've implemented option #3 for this version of the patch.  I don't see
any regressions for x86-64.  Moreover, I don't expect to see
regressions for other targets either simply because
find_pc_partial_function behaves the same as it did before for the
contiguous address range case.  That said, there may be some
adjustments needed if GDB encounters a function requiring prologue
analysis which occupies non-contiguous ranges.

gdb/ChangeLog:
    
    	* symtab.h (find_pc_partial_function): Add new parameter `block'.
    	* blockframe.c (cache_pc_function_block): New static global.
    	(clear_pc_function_cache): Clear cache_pc_function_block.
    	(find_pc_partial_function): Move comment to symtab.h.  Add
    	support for non-contiguous blocks.
---
 gdb/blockframe.c | 99 +++++++++++++++++++++++++++++++++++++++++++++-----------
 gdb/symtab.h     | 40 ++++++++++++++++++++---
 2 files changed, 116 insertions(+), 23 deletions(-)

diff --git a/gdb/blockframe.c b/gdb/blockframe.c
index 6a018cc..6dc736e 100644
--- a/gdb/blockframe.c
+++ b/gdb/blockframe.c
@@ -165,13 +165,35 @@ find_pc_sect_containing_function (CORE_ADDR pc, struct obj_section *section)
   return block_containing_function (bl);
 }
 
-/* These variables are used to cache the most recent result
-   of find_pc_partial_function.  */
+/* These variables are used to cache the most recent result of
+   find_pc_partial_function.
+
+   The addresses cache_pc_function_low and cache_pc_function_high
+   record the range in which PC was found during the most recent
+   successful lookup.  When the function occupies a single contiguous
+   address range, these values correspond to the low and high
+   addresses of the function.  (The high address is actually one byte
+   beyond the last byte of the function.)  For a function with more
+   than one (non-contiguous) range, the range in which PC was found is
+   used to set the cache bounds.
+
+   When determining whether or not these cached values apply to a
+   particular PC value, PC must be within the range specified by
+   cache_pc_function_low and cache_pc_function_high.  In addition to
+   PC being in that range, cache_pc_section must also match PC's
+   section.  See find_pc_partial_function() for details on both the
+   comparison as well as how PC's section is determined.
+
+   The other values aren't used for determining whether the cache
+   applies, but are used for setting the outputs from
+   find_pc_partial_function.  cache_pc_function_low and
+   cache_pc_function_high are used to set outputs as well.  */
 
 static CORE_ADDR cache_pc_function_low = 0;
 static CORE_ADDR cache_pc_function_high = 0;
 static const char *cache_pc_function_name = 0;
 static struct obj_section *cache_pc_function_section = NULL;
+static const struct block *cache_pc_function_block = nullptr;
 
 /* Clear cache, e.g. when symbol table is discarded.  */
 
@@ -182,24 +204,14 @@ clear_pc_function_cache (void)
   cache_pc_function_high = 0;
   cache_pc_function_name = (char *) 0;
   cache_pc_function_section = NULL;
+  cache_pc_function_block = nullptr;
 }
 
-/* Finds the "function" (text symbol) that is smaller than PC but
-   greatest of all of the potential text symbols in SECTION.  Sets
-   *NAME and/or *ADDRESS conditionally if that pointer is non-null.
-   If ENDADDR is non-null, then set *ENDADDR to be the end of the
-   function (exclusive), but passing ENDADDR as non-null means that
-   the function might cause symbols to be read.  This function either
-   succeeds or fails (not halfway succeeds).  If it succeeds, it sets
-   *NAME, *ADDRESS, and *ENDADDR to real information and returns 1.
-   If it fails, it sets *NAME, *ADDRESS and *ENDADDR to zero and
-   returns 0.  */
-
-/* Backward compatibility, no section argument.  */
+/* See symtab.h.  */
 
 int
 find_pc_partial_function (CORE_ADDR pc, const char **name, CORE_ADDR *address,
-			  CORE_ADDR *endaddr)
+			  CORE_ADDR *endaddr, const struct block **block)
 {
   struct obj_section *section;
   struct symbol *f;
@@ -241,17 +253,62 @@ find_pc_partial_function (CORE_ADDR pc, const char **name, CORE_ADDR *address,
   if (compunit_symtab != NULL)
     {
       /* Checking whether the msymbol has a larger value is for the
-	 "pathological" case mentioned in print_frame_info.  */
+	 "pathological" case mentioned in stack.c:find_frame_funname.
+
+	 We use BLOCK_ENTRY_PC instead of BLOCK_START_PC for this
+	 comparison because the minimal symbol should refer to the
+	 function's entry pc which is not necessarily the lowest
+	 address of the function.  This will happen when the function
+	 has more than one range and the entry pc is not within the
+	 lowest range of addresses.  */
       f = find_pc_sect_function (mapped_pc, section);
       if (f != NULL
 	  && (msymbol.minsym == NULL
-	      || (BLOCK_START (SYMBOL_BLOCK_VALUE (f))
+	      || (BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (f))
 		  >= BMSYMBOL_VALUE_ADDRESS (msymbol))))
 	{
-	  cache_pc_function_low = BLOCK_START (SYMBOL_BLOCK_VALUE (f));
-	  cache_pc_function_high = BLOCK_END (SYMBOL_BLOCK_VALUE (f));
+	  const struct block *b = SYMBOL_BLOCK_VALUE (f);
+
 	  cache_pc_function_name = SYMBOL_LINKAGE_NAME (f);
 	  cache_pc_function_section = section;
+	  cache_pc_function_block = b;
+
+	  /* For blocks occupying contiguous addresses (i.e. no gaps),
+	     the low and high cache addresses are simply the start
+	     and end of the block.
+
+	     For blocks with non-contiguous ranges, we have to search
+	     for the range containing mapped_pc and then use the start
+	     and end of that range.
+
+	     This causes the returned *ADDRESS and *ENDADDR values to
+	     be limited to the range in which mapped_pc is found.  See
+	     comment preceding declaration of find_pc_partial_function
+	     in symtab.h for more information.  */
+
+	  if (BLOCK_CONTIGUOUS_P (b))
+	    {
+	      cache_pc_function_low = BLOCK_START (b);
+	      cache_pc_function_high = BLOCK_END (b);
+	    }
+	  else
+	    {
+	      int i;
+	      for (i = 0; i < BLOCK_NRANGES (b); i++)
+	        {
+		  if (BLOCK_RANGE_START (b, i) <= mapped_pc
+		      && mapped_pc < BLOCK_RANGE_END (b, i))
+		    {
+		      cache_pc_function_low = BLOCK_RANGE_START (b, i);
+		      cache_pc_function_high = BLOCK_RANGE_END (b, i);
+		      break;
+		    }
+		}
+	      /* Above loop should exit via the break.  */
+	      gdb_assert (i < BLOCK_NRANGES (b));
+	    }
+
+
 	  goto return_cached_value;
 	}
     }
@@ -281,6 +338,7 @@ find_pc_partial_function (CORE_ADDR pc, const char **name, CORE_ADDR *address,
   cache_pc_function_name = MSYMBOL_LINKAGE_NAME (msymbol.minsym);
   cache_pc_function_section = section;
   cache_pc_function_high = minimal_symbol_upper_bound (msymbol);
+  cache_pc_function_block = nullptr;
 
  return_cached_value:
 
@@ -311,6 +369,9 @@ find_pc_partial_function (CORE_ADDR pc, const char **name, CORE_ADDR *address,
 	*endaddr = cache_pc_function_high;
     }
 
+  if (block != nullptr)
+    *block = cache_pc_function_block;
+
   return 1;
 }
 
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 0b155d0..746b5e6 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1694,10 +1694,42 @@ extern struct symbol *find_pc_sect_containing_function
 
 extern struct symbol *find_symbol_at_address (CORE_ADDR);
 
-/* lookup function from address, return name, start addr and end addr.  */
-
-extern int find_pc_partial_function (CORE_ADDR, const char **, CORE_ADDR *,
-				     CORE_ADDR *);
+/* Finds the "function" (text symbol) that is smaller than PC but
+   greatest of all of the potential text symbols in SECTION.  Sets
+   *NAME and/or *ADDRESS conditionally if that pointer is non-null.
+   If ENDADDR is non-null, then set *ENDADDR to be the end of the
+   function (exclusive).  If the optional parameter BLOCK is non-null,
+   then set *BLOCK to the address of the block corresponding to the
+   function symbol, if such a symbol could be found during the lookup;
+   nullptr is used as a return value for *BLOCK if no block is found. 
+   This function either succeeds or fails (not halfway succeeds).  If
+   it succeeds, it sets *NAME, *ADDRESS, and *ENDADDR to real
+   information and returns 1.  If it fails, it sets *NAME, *ADDRESS
+   and *ENDADDR to zero and returns 0.
+   
+   If the function in question occupies non-contiguous ranges,
+   *ADDRESS and *ENDADDR are (subject to the conditions noted above) set
+   to the start and end of the range in which PC is found.  Thus
+   *ADDRESS <= PC < *ENDADDR with no intervening gaps (in which ranges
+   from other functions might be found).
+   
+   This property allows find_pc_partial_function to be used (as it had
+   been prior to the introduction of non-contiguous range support) by
+   various tdep files for finding a start address and limit address
+   for prologue analysis.  This still isn't ideal, however, because we
+   probably shouldn't be doing prologue analysis (in which
+   instructions are scanned to determine frame size and stack layout)
+   for any range that doesn't contain the entry pc.  Moreover, a good
+   argument can be made that prologue analysis ought to be performed
+   starting from the entry pc even when PC is within some other range.
+   This might suggest that *ADDRESS and *ENDADDR ought to be set to the
+   limits of the entry pc range, but that will cause the 
+   *ADDRESS <= PC < *ENDADDR condition to be violated; many of the
+   callers of find_pc_partial_function expect this condition to hold.  */
+
+extern int find_pc_partial_function (CORE_ADDR pc, const char **name,
+				     CORE_ADDR *address, CORE_ADDR *endaddr,
+				     const struct block **block = nullptr);
 
 /* Return the type of a function with its first instruction exactly at
    the PC address.  Return NULL otherwise.  */

[PATCH v4 4/8] Disassemble blocks with non-contiguous ranges

[Kevin Buettner]

This patch adds support for disassembly of blocks with non-contiguous
ranges.  These blocks are printed as follows:

(gdb) disassemble foo
Dump of assembler code for function foo:
Address range 0x401136 to 0x401151:
   0x0000000000401136 <+0>:     push   %rbp
   0x0000000000401137 <+1>:     mov    %rsp,%rbp
   0x000000000040113a <+4>:     callq  0x401134 <bar>
   0x000000000040113f <+9>:     mov    0x2eef(%rip),%eax        # 0x404034 <e>
   0x0000000000401145 <+15>:    test   %eax,%eax
   0x0000000000401147 <+17>:    je     0x40114e <foo+24>
   0x0000000000401149 <+19>:    callq  0x401128 <foo+4294967282>
   0x000000000040114e <+24>:    nop
   0x000000000040114f <+25>:    pop    %rbp
   0x0000000000401150 <+26>:    retq
Address range 0x401128 to 0x401134:
   0x0000000000401128 <+-14>:   push   %rbp
   0x0000000000401129 <+-13>:   mov    %rsp,%rbp
   0x000000000040112c <+-10>:   callq  0x401126 <baz>
   0x0000000000401131 <+-5>:    nop
   0x0000000000401132 <+-4>:    pop    %rbp
   0x0000000000401133 <+-3>:    retq
End of assembler dump.

This is an actual dump from the test case that I constructed for
this work.  The ranges are printed in the order encountered in the
debug info. For the above example, note that the second range occupies
lower addresses than the first range.

Functions with contiguous ranges are still printed as follows:

(gdb) disassemble main
Dump of assembler code for function main:
   0x0000000000401151 <+0>:     push   %rbp
   0x0000000000401152 <+1>:     mov    %rsp,%rbp
   0x0000000000401155 <+4>:     callq  0x401136 <foo>
   0x000000000040115a <+9>:     mov    $0x0,%eax
   0x000000000040115f <+14>:    pop    %rbp
   0x0000000000401160 <+15>:    retq
End of assembler dump.

gdb/ChangeLog:
    
    	* cli/cli-cmds.c (block.h): Include.
    	(print_disassembly): Handle printing of non-contiguous blocks.
    	(disassemble_current_function): Likewise.
    	(disassemble_command): Likewise.
---
 gdb/cli/cli-cmds.c | 43 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 11 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 5c5d6dc..4694553 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -38,6 +38,7 @@
 #include "tracepoint.h"
 #include "filestuff.h"
 #include "location.h"
+#include "block.h"
 
 #include "ui-out.h"
 
@@ -1091,11 +1092,15 @@ list_command (const char *arg, int from_tty)
    Perform the disassembly.
    NAME is the name of the function if known, or NULL.
    [LOW,HIGH) are the range of addresses to disassemble.
+   BLOCK is the block to disassemble; it needs to be provided
+   when non-contiguous blocks are disassembled; otherwise
+   it can be NULL.
    MIXED is non-zero to print source with the assembler.  */
 
 static void
 print_disassembly (struct gdbarch *gdbarch, const char *name,
 		   CORE_ADDR low, CORE_ADDR high,
+		   const struct block *block,
 		   gdb_disassembly_flags flags)
 {
 #if defined(TUI)
@@ -1104,14 +1109,28 @@ print_disassembly (struct gdbarch *gdbarch, const char *name,
     {
       printf_filtered ("Dump of assembler code ");
       if (name != NULL)
-        printf_filtered ("for function %s:\n", name);
-      else
-        printf_filtered ("from %s to %s:\n",
-			 paddress (gdbarch, low), paddress (gdbarch, high));
-
-      /* Dump the specified range.  */
-      gdb_disassembly (gdbarch, current_uiout, flags, -1, low, high);
+	printf_filtered ("for function %s:\n", name);
+      if (block == nullptr || BLOCK_CONTIGUOUS_P (block))
+        {
+	  if (name == NULL)
+	    printf_filtered ("from %s to %s:\n",
+			     paddress (gdbarch, low), paddress (gdbarch, high));
 
+	  /* Dump the specified range.  */
+	  gdb_disassembly (gdbarch, current_uiout, flags, -1, low, high);
+	}
+      else
+        {
+	  for (int i = 0; i < BLOCK_NRANGES (block); i++)
+	    {
+	      CORE_ADDR low = BLOCK_RANGE_START (block, i);
+	      CORE_ADDR high = BLOCK_RANGE_END (block, i);
+	      printf_filtered (_("Address range %s to %s:\n"),
+			       paddress (gdbarch, low),
+			       paddress (gdbarch, high));
+	      gdb_disassembly (gdbarch, current_uiout, flags, -1, low, high);
+	    }
+	}
       printf_filtered ("End of assembler dump.\n");
       gdb_flush (gdb_stdout);
     }
@@ -1133,11 +1152,12 @@ disassemble_current_function (gdb_disassembly_flags flags)
   struct gdbarch *gdbarch;
   CORE_ADDR low, high, pc;
   const char *name;
+  const struct block *block;
 
   frame = get_selected_frame (_("No frame selected."));
   gdbarch = get_frame_arch (frame);
   pc = get_frame_address_in_block (frame);
-  if (find_pc_partial_function (pc, &name, &low, &high) == 0)
+  if (find_pc_partial_function (pc, &name, &low, &high, &block) == 0)
     error (_("No function contains program counter for selected frame."));
 #if defined(TUI)
   /* NOTE: cagney/2003-02-13 The `tui_active' was previously
@@ -1148,7 +1168,7 @@ disassemble_current_function (gdb_disassembly_flags flags)
 #endif
   low += gdbarch_deprecated_function_start_offset (gdbarch);
 
-  print_disassembly (gdbarch, name, low, high, flags);
+  print_disassembly (gdbarch, name, low, high, block, flags);
 }
 
 /* Dump a specified section of assembly code.
@@ -1184,6 +1204,7 @@ disassemble_command (const char *arg, int from_tty)
   CORE_ADDR pc;
   gdb_disassembly_flags flags;
   const char *p;
+  const struct block *block = nullptr;
 
   p = arg;
   name = NULL;
@@ -1234,7 +1255,7 @@ disassemble_command (const char *arg, int from_tty)
   if (p[0] == '\0')
     {
       /* One argument.  */
-      if (find_pc_partial_function (pc, &name, &low, &high) == 0)
+      if (find_pc_partial_function (pc, &name, &low, &high, &block) == 0)
 	error (_("No function contains specified address."));
 #if defined(TUI)
       /* NOTE: cagney/2003-02-13 The `tui_active' was previously
@@ -1262,7 +1283,7 @@ disassemble_command (const char *arg, int from_tty)
 	high += low;
     }
 
-  print_disassembly (gdbarch, name, low, high, flags);
+  print_disassembly (gdbarch, name, low, high, block, flags);
 }
 
 static void

[PATCH v4 5/8] Use BLOCK_ENTRY_PC in place of most uses of BLOCK_START

[Kevin Buettner]

This change/patch substitues BLOCK_ENTRY_PC for BLOCK_START in
places where BLOCK_START is used to obtain the address at which
execution should enter the block.  Since blocks can now contain
non-contiguous ranges, the BLOCK_START - which is still be the
very lowest address in the block - might not be the same as
BLOCK_ENTRY_PC.

There is a change to infrun.c which is less obvious and less mechanical.
I'm posting it as a separate patch.

gdb/ChangeLog:
    
    	* ax-gdb.c (gen_var_ref): Use BLOCK_ENTRY_PC in place of
    	BLOCK_START.
    	* blockframe.c (get_pc_function_start): Likewise.
    	* compile/compile-c-symbols.c (convert_one_symbol): Likewise.
    	(gcc_symbol_address): Likewise.
    	* compile/compile-object-run.c (compile_object_run): Likewise.
    	* compile/compile.c (get_expr_block_and_pc): Likewise.
    	* dwarf2loc.c (dwarf2_find_location_expression): Likewise.
    	(func_addr_to_tail_call_list): Likewise.
    	* findvar.c (default_read_var_value): Likewise.
    	* inline-frame.c (inline_frame_this_id): Likewise.
    	(skip-inline_frames): Likewise.
    	* infcmd.c (until_next_command): Likewise.
    	* linespec.c (convert_linespec_to_sals): Likewise.
    	* parse.c (parse_exp_in_context_1): Likewise.
    	* printcmd.c (build_address_symbolic): likewise.
    	(info_address_command): Likewise.
    	symtab.c (find_function_start_sal): Likewise.
    	(skip_prologue_sal): Likewise.
    	(find_function_alias_target): Likewise.
    	(find_gnu_ifunc): Likewise.
    	* stack.c (find_frame_funname): Likewise.
    	* symtab.c (fixup_symbol_section): Likewise.
    	(find_function_start_sal): Likewise.
    	(skip_prologue_sal): Likewsie.
    	(find_function_alias_target): Likewise.
    	(find_gnu_ifunc): Likewise.
    	* tracepoint.c (info_scope_command): Likewise.
    	* value.c (value_fn_field): Likewise.
---
 gdb/ax-gdb.c                     |  2 +-
 gdb/blockframe.c                 |  4 ++--
 gdb/compile/compile-c-symbols.c  |  4 ++--
 gdb/compile/compile-object-run.c |  2 +-
 gdb/compile/compile.c            |  4 ++--
 gdb/dwarf2loc.c                  |  4 ++--
 gdb/findvar.c                    |  4 ++--
 gdb/infcmd.c                     |  2 +-
 gdb/inline-frame.c               |  6 +++---
 gdb/linespec.c                   |  2 +-
 gdb/parse.c                      |  4 ++--
 gdb/printcmd.c                   |  4 ++--
 gdb/stack.c                      |  2 +-
 gdb/symtab.c                     | 12 ++++++------
 gdb/tracepoint.c                 |  4 ++--
 gdb/value.c                      |  2 +-
 16 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/gdb/ax-gdb.c b/gdb/ax-gdb.c
index 2468061..9abd939 100644
--- a/gdb/ax-gdb.c
+++ b/gdb/ax-gdb.c
@@ -679,7 +679,7 @@ gen_var_ref (struct agent_expr *ax, struct axs_value *value, struct symbol *var)
       break;
 
     case LOC_BLOCK:
-      ax_const_l (ax, BLOCK_START (SYMBOL_BLOCK_VALUE (var)));
+      ax_const_l (ax, BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (var)));
       value->kind = axs_rvalue;
       break;
 
diff --git a/gdb/blockframe.c b/gdb/blockframe.c
index 6dc736e..6ea0965 100644
--- a/gdb/blockframe.c
+++ b/gdb/blockframe.c
@@ -96,7 +96,7 @@ get_pc_function_start (CORE_ADDR pc)
       if (symbol)
 	{
 	  bl = SYMBOL_BLOCK_VALUE (symbol);
-	  return BLOCK_START (bl);
+	  return BLOCK_ENTRY_PC (bl);
 	}
     }
 
@@ -382,7 +382,7 @@ find_function_type (CORE_ADDR pc)
 {
   struct symbol *sym = find_pc_function (pc);
 
-  if (sym != NULL && BLOCK_START (SYMBOL_BLOCK_VALUE (sym)) == pc)
+  if (sym != NULL && BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym)) == pc)
     return SYMBOL_TYPE (sym);
 
   return NULL;
diff --git a/gdb/compile/compile-c-symbols.c b/gdb/compile/compile-c-symbols.c
index ecb0c16..e7423d1 100644
--- a/gdb/compile/compile-c-symbols.c
+++ b/gdb/compile/compile-c-symbols.c
@@ -93,7 +93,7 @@ convert_one_symbol (compile_c_instance *context,
 
 	case LOC_BLOCK:
 	  kind = GCC_C_SYMBOL_FUNCTION;
-	  addr = BLOCK_START (SYMBOL_BLOCK_VALUE (sym.symbol));
+	  addr = BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym.symbol));
 	  if (is_global && TYPE_GNU_IFUNC (SYMBOL_TYPE (sym.symbol)))
 	    addr = gnu_ifunc_resolve_addr (target_gdbarch (), addr);
 	  break;
@@ -405,7 +405,7 @@ gcc_symbol_address (void *datum, struct gcc_c_context *gcc_context,
 	    fprintf_unfiltered (gdb_stdlog,
 				"gcc_symbol_address \"%s\": full symbol\n",
 				identifier);
-	  result = BLOCK_START (SYMBOL_BLOCK_VALUE (sym));
+	  result = BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym));
 	  if (TYPE_GNU_IFUNC (SYMBOL_TYPE (sym)))
 	    result = gnu_ifunc_resolve_addr (target_gdbarch (), result);
 	  found = 1;
diff --git a/gdb/compile/compile-object-run.c b/gdb/compile/compile-object-run.c
index e8a95e3..0846c73 100644
--- a/gdb/compile/compile-object-run.c
+++ b/gdb/compile/compile-object-run.c
@@ -152,7 +152,7 @@ compile_object_run (struct compile_module *module)
 
       gdb_assert (TYPE_CODE (func_type) == TYPE_CODE_FUNC);
       func_val = value_from_pointer (lookup_pointer_type (func_type),
-				   BLOCK_START (SYMBOL_BLOCK_VALUE (func_sym)));
+				   BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (func_sym)));
 
       vargs = XALLOCAVEC (struct value *, TYPE_NFIELDS (func_type));
       if (TYPE_NFIELDS (func_type) >= 1)
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 31ae597..20d81cb 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -439,10 +439,10 @@ get_expr_block_and_pc (CORE_ADDR *pc)
 	block = BLOCKVECTOR_BLOCK (SYMTAB_BLOCKVECTOR (cursal.symtab),
 				   STATIC_BLOCK);
       if (block != NULL)
-	*pc = BLOCK_START (block);
+	*pc = BLOCK_ENTRY_PC (block);
     }
   else
-    *pc = BLOCK_START (block);
+    *pc = BLOCK_ENTRY_PC (block);
 
   return block;
 }
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index df2f231..200fa03 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -367,7 +367,7 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
 	  if (pc_block)
 	    pc_func = block_linkage_function (pc_block);
 
-	  if (pc_func && pc == BLOCK_START (SYMBOL_BLOCK_VALUE (pc_func)))
+	  if (pc_func && pc == BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (pc_func)))
 	    {
 	      *locexpr_length = length;
 	      return loc_ptr;
@@ -871,7 +871,7 @@ func_addr_to_tail_call_list (struct gdbarch *gdbarch, CORE_ADDR addr)
   struct symbol *sym = find_pc_function (addr);
   struct type *type;
 
-  if (sym == NULL || BLOCK_START (SYMBOL_BLOCK_VALUE (sym)) != addr)
+  if (sym == NULL || BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym)) != addr)
     throw_error (NO_ENTRY_VALUE_ERROR,
 		 _("DW_TAG_call_site resolving failed to find function "
 		   "name for address %s"),
diff --git a/gdb/findvar.c b/gdb/findvar.c
index ebaff92..9256833 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -702,10 +702,10 @@ default_read_var_value (struct symbol *var, const struct block *var_block,
     case LOC_BLOCK:
       if (overlay_debugging)
 	addr = symbol_overlayed_address
-	  (BLOCK_START (SYMBOL_BLOCK_VALUE (var)),
+	  (BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (var)),
 	   SYMBOL_OBJ_SECTION (symbol_objfile (var), var));
       else
-	addr = BLOCK_START (SYMBOL_BLOCK_VALUE (var));
+	addr = BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (var));
       break;
 
     case LOC_REGISTER:
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 74d5956..860909f 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1552,7 +1552,7 @@ until_next_command (int from_tty)
     {
       sal = find_pc_line (pc, 0);
 
-      tp->control.step_range_start = BLOCK_START (SYMBOL_BLOCK_VALUE (func));
+      tp->control.step_range_start = BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (func));
       tp->control.step_range_end = sal.end;
     }
   tp->control.may_range_step = 1;
diff --git a/gdb/inline-frame.c b/gdb/inline-frame.c
index c6caf9d..b408b71 100644
--- a/gdb/inline-frame.c
+++ b/gdb/inline-frame.c
@@ -165,7 +165,7 @@ inline_frame_this_id (struct frame_info *this_frame,
      in the frame ID (and eventually, to set breakpoints).  */
   func = get_frame_function (this_frame);
   gdb_assert (func != NULL);
-  (*this_id).code_addr = BLOCK_START (SYMBOL_BLOCK_VALUE (func));
+  (*this_id).code_addr = BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (func));
   (*this_id).artificial_depth++;
 }
 
@@ -341,8 +341,8 @@ skip_inline_frames (thread_info *thread, bpstat stop_chain)
 	  if (block_inlined_p (cur_block))
 	    {
 	      /* See comments in inline_frame_this_id about this use
-		 of BLOCK_START.  */
-	      if (BLOCK_START (cur_block) == this_pc
+		 of BLOCK_ENTRY_PC.  */
+	      if (BLOCK_ENTRY_PC (cur_block) == this_pc
 		  || block_starting_point_at (this_pc, cur_block))
 		{
 		  /* Do not skip the inlined frame if execution
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 892d141..73fbe4a 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2283,7 +2283,7 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls)
 		   && SYMBOL_CLASS (sym) == LOC_BLOCK)
 		{
 		  const CORE_ADDR addr
-		    = BLOCK_START (SYMBOL_BLOCK_VALUE (sym));
+		    = BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym));
 
 		  bound_minimal_symbol_d *elem;
 		  for (int m = 0;
diff --git a/gdb/parse.c b/gdb/parse.c
index 0cbdb48..1b58073 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -1146,7 +1146,7 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc,
   if (!expression_context_block)
     expression_context_block = get_selected_block (&expression_context_pc);
   else if (pc == 0)
-    expression_context_pc = BLOCK_START (expression_context_block);
+    expression_context_pc = BLOCK_ENTRY_PC (expression_context_block);
   else
     expression_context_pc = pc;
 
@@ -1160,7 +1160,7 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc,
 	  = BLOCKVECTOR_BLOCK (SYMTAB_BLOCKVECTOR (cursal.symtab),
 			       STATIC_BLOCK);
       if (expression_context_block)
-	expression_context_pc = BLOCK_START (expression_context_block);
+	expression_context_pc = BLOCK_ENTRY_PC (expression_context_block);
     }
 
   if (language_mode == language_mode_auto && block != NULL)
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 2ecbd54..1a3d972 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -609,7 +609,7 @@ build_address_symbolic (struct gdbarch *gdbarch,
 	 pointer is <function+3>.  This matches the ISA behavior.  */
       addr = gdbarch_addr_bits_remove (gdbarch, addr);
 
-      name_location = BLOCK_START (SYMBOL_BLOCK_VALUE (symbol));
+      name_location = BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (symbol));
       if (do_demangle || asm_demangle)
 	name_temp = SYMBOL_PRINT_NAME (symbol);
       else
@@ -1519,7 +1519,7 @@ info_address_command (const char *exp, int from_tty)
 
     case LOC_BLOCK:
       printf_filtered (_("a function at address "));
-      load_addr = BLOCK_START (SYMBOL_BLOCK_VALUE (sym));
+      load_addr = BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym));
       fputs_filtered (paddress (gdbarch, load_addr), gdb_stdout);
       if (section_is_overlay (section))
 	{
diff --git a/gdb/stack.c b/gdb/stack.c
index 366687e..40ff99b 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1079,7 +1079,7 @@ find_frame_funname (struct frame_info *frame, enum language *funlang,
 
       if (msymbol.minsym != NULL
 	  && (BMSYMBOL_VALUE_ADDRESS (msymbol)
-	      > BLOCK_START (SYMBOL_BLOCK_VALUE (func))))
+	      > BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (func))))
 	{
 	  /* We also don't know anything about the function besides
 	     its address and name.  */
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 73c7983..0a1caa5 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -1746,7 +1746,7 @@ fixup_symbol_section (struct symbol *sym, struct objfile *objfile)
       addr = SYMBOL_VALUE_ADDRESS (sym);
       break;
     case LOC_BLOCK:
-      addr = BLOCK_START (SYMBOL_BLOCK_VALUE (sym));
+      addr = BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym));
       break;
 
     default:
@@ -3637,7 +3637,7 @@ find_function_start_sal (symbol *sym, bool funfirstline)
 {
   fixup_symbol_section (sym, NULL);
   symtab_and_line sal
-    = find_function_start_sal_1 (BLOCK_START (SYMBOL_BLOCK_VALUE (sym)),
+    = find_function_start_sal_1 (BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym)),
 				 SYMBOL_OBJ_SECTION (symbol_objfile (sym), sym),
 				 funfirstline);
   sal.symbol = sym;
@@ -3717,7 +3717,7 @@ skip_prologue_sal (struct symtab_and_line *sal)
       fixup_symbol_section (sym, NULL);
 
       objfile = symbol_objfile (sym);
-      pc = BLOCK_START (SYMBOL_BLOCK_VALUE (sym));
+      pc = BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym));
       section = SYMBOL_OBJ_SECTION (objfile, sym);
       name = SYMBOL_LINKAGE_NAME (sym);
     }
@@ -3778,7 +3778,7 @@ skip_prologue_sal (struct symtab_and_line *sal)
       /* Check if gdbarch_skip_prologue left us in mid-line, and the next
 	 line is still part of the same function.  */
       if (skip && start_sal.pc != pc
-	  && (sym ? (BLOCK_START (SYMBOL_BLOCK_VALUE (sym)) <= start_sal.end
+	  && (sym ? (BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym)) <= start_sal.end
 		     && start_sal.end < BLOCK_END (SYMBOL_BLOCK_VALUE (sym)))
 	      : (lookup_minimal_symbol_by_pc_section (start_sal.end, section).minsym
 		 == lookup_minimal_symbol_by_pc_section (pc, section).minsym)))
@@ -3982,7 +3982,7 @@ find_function_alias_target (bound_minimal_symbol msymbol)
   symbol *sym = find_pc_function (func_addr);
   if (sym != NULL
       && SYMBOL_CLASS (sym) == LOC_BLOCK
-      && BLOCK_START (SYMBOL_BLOCK_VALUE (sym)) == func_addr)
+      && BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym)) == func_addr)
     return sym;
 
   return NULL;
@@ -4987,7 +4987,7 @@ find_gnu_ifunc (const symbol *sym)
 				symbol_name_match_type::SEARCH_NAME);
   struct objfile *objfile = symbol_objfile (sym);
 
-  CORE_ADDR address = BLOCK_START (SYMBOL_BLOCK_VALUE (sym));
+  CORE_ADDR address = BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym));
   minimal_symbol *ifunc = NULL;
 
   iterate_over_minimal_symbols (objfile, lookup_name,
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index cd538f4..a96f56a 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -2536,7 +2536,7 @@ info_scope_command (const char *args_in, int from_tty)
 
 	  if (SYMBOL_COMPUTED_OPS (sym) != NULL)
 	    SYMBOL_COMPUTED_OPS (sym)->describe_location (sym,
-							  BLOCK_START (block),
+							  BLOCK_ENTRY_PC (block),
 							  gdb_stdout);
 	  else
 	    {
@@ -2613,7 +2613,7 @@ info_scope_command (const char *args_in, int from_tty)
 		case LOC_BLOCK:
 		  printf_filtered ("a function at address ");
 		  printf_filtered ("%s",
-				   paddress (gdbarch, BLOCK_START (SYMBOL_BLOCK_VALUE (sym))));
+				   paddress (gdbarch, BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym))));
 		  break;
 		case LOC_UNRESOLVED:
 		  msym = lookup_minimal_symbol (SYMBOL_LINKAGE_NAME (sym),
diff --git a/gdb/value.c b/gdb/value.c
index b0f22f1..38fc18e 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3050,7 +3050,7 @@ value_fn_field (struct value **arg1p, struct fn_field *f,
   VALUE_LVAL (v) = lval_memory;
   if (sym)
     {
-      set_value_address (v, BLOCK_START (SYMBOL_BLOCK_VALUE (sym)));
+      set_value_address (v, BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym)));
     }
   else
     {

[PATCH v4 6/8] Introduce find_function_entry_range_from_pc and use it in infrun.c

[Kevin Buettner]

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_function_entry_range_from_pc.  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_function_entry_range_from_pc 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_function_entry_range_from_pc.

It should be noted that for functions which contain only a single
range, the outputs of find_pc_partial_function and
find_function_entry_range_from_pc are identical.

I think it might happen that find_function_entry_range_from_pc 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.

Finally, a note regarding the name: I had initially chosen a different
name with a find_pc_partial_ prefix, but Simon suggested the current
name citing the goal of eventually making naming consistent using
the form find_X_from_Y.  In this case X is "function_entry_range" and
Y is "pc".  Both the name and rationale made sense to me, so that's
how it came to be.

gdb/ChangeLog:
    
    	* infrun.c (fill_in_stop_func): Use find_function_entry_range_from_pc
    	in place of find_pc_partial_function.
    	* blockframe.c (find_function_entry_range_from_pc): New function.
    	* symtab.h (find_function_entry_range_from_pc): Declare and document.
---
 gdb/blockframe.c | 37 +++++++++++++++++++++++++++++++++++++
 gdb/infrun.c     |  7 ++++---
 gdb/symtab.h     | 21 ++++++++++++++++++++-
 3 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/gdb/blockframe.c b/gdb/blockframe.c
index 6ea0965..f6dd861 100644
--- a/gdb/blockframe.c
+++ b/gdb/blockframe.c
@@ -377,6 +377,43 @@ find_pc_partial_function (CORE_ADDR pc, const char **name, CORE_ADDR *address,
 
 /* See symtab.h.  */
 
+bool
+find_function_entry_range_from_pc (CORE_ADDR pc, const char **name,
+				   CORE_ADDR *address, CORE_ADDR *endaddr)
+{
+  const struct block *block;
+  bool status = find_pc_partial_function (pc, name, address, endaddr, &block);
+
+  if (status && block != nullptr && !BLOCK_CONTIGUOUS_P (block))
+    {
+      CORE_ADDR entry_pc = BLOCK_ENTRY_PC (block);
+
+      for (int i = 0; i < BLOCK_NRANGES (block); i++)
+        {
+	  if (BLOCK_RANGE_START (block, i) <= entry_pc
+	      && entry_pc < BLOCK_RANGE_END (block, i))
+	    {
+	      if (address != nullptr)
+	        *address = BLOCK_RANGE_START (block, i);
+
+	      if (endaddr != nullptr)
+	        *endaddr = BLOCK_RANGE_END (block, i);
+
+	      return status;
+	    }
+	}
+
+      /* It's an internal error if we exit the above loop without finding
+         the range.  */
+      internal_error (__FILE__, __LINE__,
+                      _("Entry block not found in find_function_entry_range_from_pc"));
+    }
+
+  return status;
+}
+
+/* See symtab.h.  */
+
 struct type *
 find_function_type (CORE_ADDR pc)
 {
diff --git a/gdb/infrun.c b/gdb/infrun.c
index b25d745..7731ccd 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4287,9 +4287,10 @@ fill_in_stop_func (struct gdbarch *gdbarch,
     {
       /* Don't care about return value; stop_func_start and stop_func_name
 	 will both be 0 if it doesn't work.  */
-      find_pc_partial_function (ecs->event_thread->suspend.stop_pc,
-				&ecs->stop_func_name,
-				&ecs->stop_func_start, &ecs->stop_func_end);
+      find_function_entry_range_from_pc (ecs->event_thread->suspend.stop_pc,
+				         &ecs->stop_func_name,
+				         &ecs->stop_func_start,
+					 &ecs->stop_func_end);
       ecs->stop_func_start
 	+= gdbarch_deprecated_function_start_offset (gdbarch);
 
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 746b5e6..eb14f34 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1725,12 +1725,31 @@ extern struct symbol *find_symbol_at_address (CORE_ADDR);
    This might suggest that *ADDRESS and *ENDADDR ought to be set to the
    limits of the entry pc range, but that will cause the 
    *ADDRESS <= PC < *ENDADDR condition to be violated; many of the
-   callers of find_pc_partial_function expect this condition to hold.  */
+   callers of find_pc_partial_function expect this condition to hold. 
+
+   Callers which require the start and/or end addresses for the range
+   containing the entry pc should instead call
+   find_function_entry_range_from_pc.  */
 
 extern int find_pc_partial_function (CORE_ADDR pc, const char **name,
 				     CORE_ADDR *address, CORE_ADDR *endaddr,
 				     const struct block **block = nullptr);
 
+/* Like find_pc_partial_function, above, but *ADDRESS and *ENDADDR are
+   set to start and end addresses of the range containing the entry pc.
+
+   Note that it is not necessarily the case that (for non-NULL ADDRESS
+   and ENDADDR arguments) the *ADDRESS <= PC < *ENDADDR condition will
+   hold.
+
+   See comment for find_pc_partial_function, above, for further
+   explanation.  */
+
+extern bool find_function_entry_range_from_pc (CORE_ADDR pc,
+					       const char **name,
+					       CORE_ADDR *address,
+					       CORE_ADDR *endaddr);
+
 /* Return the type of a function with its first instruction exactly at
    the PC address.  Return NULL otherwise.  */
 

[PATCH v4 7/8] Relocate block range start and end addresses

[Kevin Buettner]

gdb/ChangeLog:
    
    	* objfiles.c (objfile_relocate1): Relocate start and end addresses
    	for each range in a block.
---
 gdb/objfiles.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index df28da5..4bffd20 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -831,6 +831,14 @@ objfile_relocate1 (struct objfile *objfile,
 	  BLOCK_START (b) += ANOFFSET (delta, block_line_section);
 	  BLOCK_END (b) += ANOFFSET (delta, block_line_section);
 
+	  if (BLOCK_RANGES (b) != nullptr)
+	    for (int j = 0; j < BLOCK_NRANGES (b); j++)
+	      {
+		BLOCK_RANGE_START (b, j)
+		  += ANOFFSET (delta, block_line_section);
+		BLOCK_RANGE_END (b, j) += ANOFFSET (delta, block_line_section);
+	      }
+
 	  /* We only want to iterate over the local symbols, not any
 	     symbols in included symtabs.  */
 	  ALL_DICT_SYMBOLS (BLOCK_DICT (b), iter, sym)

[PATCH v4 8/8] Test case for functions with non-contiguous ranges

[Kevin Buettner]

See comments in the new files for what this is about - I tried to
explain it all there.

gdb/testsuite/ChangeLog:
    
    	* gdb.dwarf2/dw2-ranges-func.c: New file.
    	* gdb.dwarf2/dw2-ranges-func.exp: New file.
---
 gdb/testsuite/gdb.dwarf2/dw2-ranges-func.c   |  78 ++++++
 gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp | 405 +++++++++++++++++++++++++++
 2 files changed, 483 insertions(+)

diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.c b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.c
new file mode 100644
index 0000000..864803c
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.c
@@ -0,0 +1,78 @@
+/* Copyright 2018 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* The idea here is to, via use of the dwarf assembler, create a function
+   which occupies two non-contiguous address ranges.
+
+   foo_low and foo will be combined into a single function foo with a
+   function bar in between these two ranges.
+
+   This test case was motivated by a bug in which a function which
+   occupied two non-contiguous address ranges was calling another
+   function which resides in between these ranges.  So we end up with
+   a situation in which the low/start address of our constructed foo
+   (in this case) will be less than any of the addresses in bar, but
+   the high/end address of foo will be greater than any of bar's
+   addresses.
+
+   This situation was causing a problem in the caching code of
+   find_pc_partial_function:  When the low and high addresses of foo
+   are placed in the cache, the simple check that was used to see if
+   the cache was applicable would (incorrectly) succeed when presented
+   with an address in bar.  I.e. an address in bar presented as an
+   input to find_pc_partial_function could produce the answer "this
+   address belongs to foo".  */
+
+volatile int e = 0;
+
+void
+baz (void)
+{
+  asm ("baz_label: .globl baz_label");
+}						/* baz end */
+
+void
+foo_low (void)
+{						/* foo_low prologue */
+  asm ("foo_low_label: .globl foo_low_label");
+  baz ();					/* foo_low baz call */
+  asm ("foo_low_label2: .globl foo_low_label2");
+}						/* foo_low end */
+
+void
+bar (void)
+{
+  asm ("bar_label: .globl bar_label");
+}						/* bar end */
+
+void
+foo (void)
+{						/* foo prologue */
+  asm ("foo_label: .globl foo_label");
+  bar ();					/* foo bar call */
+  asm ("foo_label2: .globl foo_label2");
+  if (e) foo_low ();				/* foo foo_low call */
+  asm ("foo_label3: .globl foo_label3");
+}						/* foo end */
+
+int
+main (void)
+{						/* main prologue */
+  asm ("main_label: .globl main_label");
+  foo ();					/* main foo call */
+  asm ("main_label2: .globl main_label2");
+  return 0;					/* main return */
+}						/* main end */
+
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp
new file mode 100644
index 0000000..6f1f2d2
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp
@@ -0,0 +1,405 @@
+# Copyright 2018 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+load_lib dwarf.exp
+
+# Test DW_AT_ranges in the context of a subprogram scope.
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    unsupported "dwarf2 support required for this test"
+    return 0
+}
+
+if [get_compiler_info] {
+    return -1
+}
+if !$gcc_compiled {
+    unsupported "gcc required for this test"
+    return 0
+}
+
+standard_testfile dw2-ranges-func.c dw2-ranges-func-dw.S
+
+# We need to know the size of integer and address types in order to
+# write some of the debugging info we'd like to generate.
+#
+# For that, we ask GDB by debugging our test program.  Any program
+# would do, but since we already have it specifically for this
+# testcase, might as well use that.
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
+    return -1
+}
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    global srcdir subdir srcfile srcfile2
+    declare_labels integer_label volatile_label func_ranges_label cu_ranges_label L
+    set int_size [get_sizeof "int" 4]
+
+    # Find start address and length for our functions.
+    lassign [function_range main [list ${srcdir}/${subdir}/$srcfile]] \
+	main_start main_len
+    set main_end "$main_start + $main_len"
+    lassign [function_range foo [list ${srcdir}/${subdir}/$srcfile]] \
+	foo_start foo_len
+    set foo_end "$foo_start + $foo_len"
+    lassign [function_range foo_low [list ${srcdir}/${subdir}/$srcfile]] \
+	foo_low_start foo_low_len
+    set foo_low_end "$foo_low_start + $foo_low_len"
+    lassign [function_range bar [list ${srcdir}/${subdir}/$srcfile]] \
+	bar_start bar_len
+    set bar_end "$bar_start + $bar_len"
+    lassign [function_range baz [list ${srcdir}/${subdir}/$srcfile]] \
+	baz_start baz_len
+    set baz_end "$baz_start + $baz_len"
+
+    set e_var [gdb_target_symbol e]
+
+    cu {} {
+	compile_unit {
+	    {language @DW_LANG_C}
+	    {name dw-ranges-func.c}
+	    {stmt_list $L DW_FORM_sec_offset}
+	    {low_pc 0 addr}
+	    {ranges ${cu_ranges_label} DW_FORM_sec_offset}
+	} {
+	    integer_label: DW_TAG_base_type {
+		{DW_AT_byte_size $int_size DW_FORM_sdata}
+		{DW_AT_encoding  @DW_ATE_signed}
+		{DW_AT_name      integer}
+	    }
+	    volatile_label: DW_TAG_volatile_type {
+		{type :$integer_label}
+	    }
+	    DW_TAG_variable {
+		{name e}
+		{external 1 flag}
+		{type :$volatile_label}
+		{location {addr $e_var} SPECIAL_expr}
+	    }
+	    subprogram {
+		{external 1 flag}
+		{name main}
+		{DW_AT_type :$integer_label}
+		{low_pc $main_start addr}
+		{high_pc $main_len DW_FORM_data4}
+	    }
+	    subprogram {
+		{external 1 flag}
+		{name foo}
+		{ranges ${func_ranges_label} DW_FORM_sec_offset}
+	    }
+	    subprogram {
+		{external 1 flag}
+		{name bar}
+		{low_pc $bar_start addr}
+		{high_pc $bar_len DW_FORM_data4}
+	    }
+	    subprogram {
+		{external 1 flag}
+		{name baz}
+		{low_pc $baz_start addr}
+		{high_pc $baz_len DW_FORM_data4}
+	    }
+	}
+    }
+
+    lines {version 2} L {
+	include_dir "${srcdir}/${subdir}"
+	file_name "$srcfile" 1
+
+	# Generate a line table program.  An attempt was made to make it
+	# reasonably accurate as it made debugging the test case easier.
+	program {
+	    {DW_LNE_set_address $main_start}
+	    {DW_LNS_advance_line [expr [gdb_get_line_number "main prologue"] - 1]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address main_label}
+	    {DW_LNS_advance_line [expr [gdb_get_line_number "main foo call"] - [gdb_get_line_number "main prologue"]]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address main_label2}
+	    {DW_LNS_advance_line [expr [gdb_get_line_number "main return"] - [gdb_get_line_number "main foo call"]]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address $main_end}
+	    {DW_LNS_advance_line [expr [gdb_get_line_number "main end"] - [gdb_get_line_number "main return"] + 1]}
+	    {DW_LNS_copy}
+	    {DW_LNE_end_sequence}
+
+	    {DW_LNE_set_address $foo_start}
+	    {DW_LNS_advance_line [expr [gdb_get_line_number "foo prologue"] - 1] }
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address foo_label}
+	    {DW_LNS_advance_line [expr [gdb_get_line_number "foo bar call"] - [gdb_get_line_number "foo prologue"]]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address foo_label2}
+	    {DW_LNS_advance_line [expr [gdb_get_line_number "foo foo_low call"] - [gdb_get_line_number "foo bar call"]]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address foo_label3}
+	    {DW_LNS_advance_line [expr [gdb_get_line_number "foo end"] - [gdb_get_line_number "foo foo_low call"]]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address $foo_end}
+	    {DW_LNS_advance_line 1}
+	    {DW_LNS_copy}
+	    {DW_LNE_end_sequence}
+
+	    {DW_LNE_set_address $bar_start}
+	    {DW_LNS_advance_line [expr [gdb_get_line_number "bar end"] - 1]}
+	    {DW_LNS_copy}
+	    {DW_LNS_advance_pc $bar_len}
+	    {DW_LNS_advance_line 1}
+	    {DW_LNS_copy}
+	    {DW_LNE_end_sequence}
+
+	    {DW_LNE_set_address $baz_start}
+	    {DW_LNS_advance_line [expr [gdb_get_line_number "baz end"] - 1]}
+	    {DW_LNS_copy}
+	    {DW_LNS_advance_pc $baz_len}
+	    {DW_LNS_advance_line 1}
+	    {DW_LNS_copy}
+	    {DW_LNE_end_sequence}
+
+	    {DW_LNE_set_address $foo_low_start}
+	    {DW_LNS_advance_line [expr [gdb_get_line_number "foo_low prologue"] - 1]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address foo_low_label}
+	    {DW_LNS_advance_line [expr [gdb_get_line_number "foo_low baz call"] - [gdb_get_line_number "foo_low prologue"]]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address foo_low_label2}
+	    {DW_LNS_advance_line [expr [gdb_get_line_number "foo_low end"] - [gdb_get_line_number "foo_low baz call"]]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address $foo_low_end}
+	    {DW_LNS_advance_line 1}
+	    {DW_LNS_copy}
+	    {DW_LNE_end_sequence}
+	}
+    }
+
+    # Generate ranges data.
+    ranges {is_64 [is_64_target]} {
+	func_ranges_label: sequence {
+	    {range {$foo_start } $foo_end}
+	    {range {$foo_low_start} $foo_low_end}
+	}
+	cu_ranges_label: sequence {
+	    {range {$foo_start } $foo_end}
+	    {range {$foo_low_start} $foo_low_end}
+	    {range {$main_start} $main_end}
+	    {range {$bar_start} $bar_end}
+	    {range {$baz_start} $baz_end}
+	}
+    }
+}
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+set main_prologue_line_num [gdb_get_line_number "main prologue"]
+# Do a sanity check to make sure that line number info is available.
+gdb_test "info line main" \
+    "Line ${main_prologue_line_num} of .* starts at address .* and ends at .*"
+
+with_test_prefix "step-test-1" {
+    set bp_foo_bar [gdb_get_line_number "foo bar call"]
+
+    gdb_test "break $bp_foo_bar" \
+	"Breakpoint.*at.* file .*$srcfile, line $bp_foo_bar\\." \
+	"break at call to bar"
+
+    gdb_test "continue" \
+	"Continuing\\..*Breakpoint \[0-9\]+, foo \\(\\).*$bp_foo_bar\\s+bar\\s\\(\\);.*foo bar call.*" \
+	"continue to call of bar"
+
+    gdb_test "step" \
+	"bar \\(\\).*bar end.*" \
+	"step into bar"
+
+    gdb_test "step" \
+	"foo \\(\\).*foo foo_low call.*" \
+	"step out of bar, back into foo"
+}
+
+with_test_prefix "step-test-2" {
+    clean_restart ${testfile}
+    if ![runto_main] {
+	return -1
+    }
+
+    # Note that the RE used for the following test will fail when the
+    # breakpoint has been set on multiple locations. E.g. "(2 locations)". 
+    # This is intentional since that behavior is one of the bugs that
+    # this test case tests for.
+    gdb_test "break foo" \
+	"Breakpoint.*at.* file .*$srcfile, line \\d+\\." \
+	"break foo"
+
+    # Continue to foo.  Allow execution to stop either on the prologue
+    # or on the call to bar since either behavior is acceptable though
+    # the latter is preferred.
+    set test "continue to foo"
+    gdb_test_multiple "continue" $test {
+	-re "Breakpoint \\d+, foo \\(\\).*foo prologue.*${gdb_prompt}" {
+	    pass $test
+	    gdb_test "step" \
+		     "foo bar call .*" \
+		     "step to call of bar after landing on prologue"
+	}
+	-re "Breakpoint \\d+, foo \\(\\).*foo bar call.*${gdb_prompt}" {
+	    pass $test
+	}
+    }
+
+    gdb_test "step" \
+	"bar \\(\\).*bar end.*" \
+	"step into bar"
+
+    gdb_test "step" \
+	"foo \\(\\).*foo foo_low call.*" \
+	"step out of bar, back into foo"
+}
+
+clean_restart ${testfile}
+if ![runto_main] {
+    return -1
+}
+
+# Disassembly of foo should have multiple address ranges.
+gdb_test_sequence "disassemble foo" "" [list \
+    "Dump of assembler code for function foo:" \
+    "Address range $hex to $hex:" \
+    "   $hex <\\+0>:" \
+    "Address range $hex to $hex:" \
+    "   $hex <(.+?)>:" \
+    "End of assembler dump\\." \
+]
+
+set foo_low_addr -1
+set test "x/i foo_low"
+gdb_test_multiple $test $test {
+    -re "   ($hex) <foo.*?>.*${gdb_prompt}" {
+	set foo_low_addr $expect_out(1,string)
+	pass $test
+    }
+}
+
+set foo_addr -1
+set test "x/i foo"
+gdb_test_multiple $test $test {
+    -re "   ($hex) <foo.*?>.*${gdb_prompt}" {
+	set foo_addr $expect_out(1,string)
+	pass $test
+    }
+}
+
+gdb_assert {$foo_low_addr != $foo_addr} "foo and foo_low are at different addresses"
+
+# This more permissive RE for "break foo" will allow a breakpoint on
+# multiple locations to PASS.  */
+gdb_test "break foo" \
+    "Breakpoint.*at.*" \
+    "break foo"
+
+gdb_test "break baz" \
+    "Breakpoint.*at.* file .*$srcfile, line \\d+\\."
+
+gdb_test "continue" \
+    "Breakpoint \\d+, foo \\(\\).*" \
+    "continue to foo"
+
+gdb_test_no_output "set variable e=1"
+
+# If GDB incorrectly places the foo breakpoint on multiple locations,
+# then GDB will (incorrectly) stop in foo_low instead of in baz.
+gdb_test "continue" \
+    "Breakpoint \\d+, (?:$hex in )?baz \\(\\).*" \
+    "continue to baz"
+
+with_test_prefix "step-test-3" {
+    clean_restart ${testfile}
+    if ![runto_main] {
+	return -1
+    }
+
+    gdb_test "step" \
+	     "foo \\(\\).*bar \\(\\);.*foo bar call.*" \
+	     "step into foo from main"
+
+    gdb_test "step" \
+	     "bar \\(\\).*\}.* bar end.*" \
+	     "step into bar from foo"
+
+    gdb_test "step" \
+	     "foo(_label2)? \\(\\).*foo_low \\(\\);.*foo foo_low call.*" \
+	     "step out of bar to foo"
+
+    # The tests in the "enable_foo_low_stepping" section, below, work
+    # with some versions of gcc, though it's not clear that they
+    # should.  This test case causes foo_low, originally a separate
+    # function invoked via a subroutine call, to be considered as part
+    # of foo via use of DW_AT_ranges.  Real code that I've looked at
+    # uses a branch instruction to cause code in the "cold" range to
+    # be executed. 
+    #
+    # For the moment though, these tests have been left in place, but
+    # disabled, in case we decide that making such a subroutine call
+    # is a reasonable thing to do that should also be supported by
+    # GDB.
+
+    set enable_foo_low_stepping false
+
+    if { $enable_foo_low_stepping } {
+	gdb_test_no_output "set variable e=1"
+
+	set test "step into foo_low from foo"
+	gdb_test_multiple "step" $test {
+	    -re "foo(_low)? \\(\\).*\{.*foo_low prologue.*${gdb_prompt}" {
+		pass $test
+		gdb_test "step" \
+			 "foo \\(\\).*baz \\(\\);.*foo_low baz call.*" \
+			 "step to baz call in foo_low"
+
+	    }
+	    -re "foo(_low)? \\(\\).*baz \\(\\);.*foo_low baz call.*${gdb_prompt}" {
+		pass $test
+	    }
+	}
+
+	gdb_test "step" \
+		 "baz \\(\\).*\}.*baz end.*" \
+		 "step into baz from foo_low"
+
+	gdb_test "step" \
+		 "foo(?:_low(?:_label2)?)? \\(\\).*\}.*foo_low end.*" \
+		 "step out of baz to foo_low"
+
+	gdb_test "step" \
+		 "foo(?:_label3)? \\(\\).*\}.*foo end.*" \
+		 "step out of foo_low to foo"
+    } else {
+	gdb_test "next" \
+		 ".*foo end.*" \
+		 "next over foo_low call"
+    }
+
+    gdb_test "step" \
+	     "main(?:_label2)? \\(\\).*" \
+	     "step out of foo to main"
+}

Re: [PATCH v4 0/8] Non-contiguous address range support

[Simon Marchi]

On 2018-08-22 12:57, Kevin Buettner wrote:
> This is version 4 of an eight part patch series which adds further
> support for non-contiguous address ranges to GDB.
> 
> This v4 series addresses concerns from Simon Marchi and Pedro Alves.
> Only parts 3, 6, and 8 have changed since v3.
> 
> The v3 series had been rebased against more recent (current at time
> of posting) sources.
> 
> In the v2 series, I addressed the concerns from Simon Marchi's
> review of the v1 patch set.  I also changed my mind about how
> return values *ADDRESS and *ENDADDR ought to be set for
> find_pc_partial_function.  I discuss this matter in the remarks
> preceding the relevant patches.
> 
> Everything below this point was copy/pasted from the introductory
> message for the v1 patch set...
> 
> This sequence of patches was motivated by GCC bug 84550:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84550
> 
> There is a test case posted to that bug along with some analysis of
> the underlying problem.
> 
> There is also a GDB bug for the same issue; it's 23021, but at the
> moment, there is little there aside from a link to the GCC bug
> mentioned above.  But here's a link anyway:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=23021
> 
> A quick synopsis of the problem is as follows...
> 
> Recent versions of gcc can generate code in which a function is split
> into at least two non-contiguous address ranges.  As I understand it,
> the idea here is to separate code which gcc does not expect to execute
> in normal operation from the rest of the code.  Doing this may result
> in better cache locality for the normal case.  The generated code for
> the example in GCC bug 84550 separated a call to abort() from the rest
> of the code comprising the function.
> 
> In the course of my investigation, I identified at least four
> problems:
> 
> 1) Stepping into a function from a function which occupies 
> non-contiguous
>    address ranges does not always work.  It was not uncommon to see the
>    program run to completion when attempting to do a step.
> 
> 2) Setting a breakpoint on a function with non-contiguous address 
> ranges
>    causes a breakpoint to be placed on more than one location.  When a
>    breakpoint is set on the "cold" address range, this is almost 
> certainly
>    incorrect.  The breakpoint should instead be set only on code near 
> the
>    entry point(s).
> 
> 3) The disassemble command did not work correctly.  E.g. here is what I
>    found during my analysis of 84550:
> 
> 	(gdb) x/i 'main.cold.0'
> 	   0x4010e0 <main()>:   mov    %rax,%rdi
> 	(gdb) x/i main
> 	   0x4011a0 <main>:     push   %r12
> 	(gdb) disassemble main
> 	Dump of assembler code for function main():
> 	   0x00000000004010e0 <+0>:     mov    %rax,%rdi
> 	   ...
>         [No addresses starting at 0x4011a0 are shown]
> 
> 4) Display of addresses associated with the non-contiguous function are
>    confusing.  E.g. in the above example, note that GDB thinks that
>    the address associated with main.cold.0 is <main()>, but that 
> there's
>    also a minsym called main which is displayed as <main>.
> 
> There are probably several other problems which are related those
> identified above.
> 
> I discovered that the stepping problem could be "fixed" by disabling
> the find_pc_partial_function cache.  This cache keeps track of the
> most recent result (of calling find_pc_partial_function).  If
> find_pc_partial_function is called with an address which falls within
> the cache range, then that's considered to be a cache hit and the most
> recent result is returned.  Obviously, this won't work correctly for
> functions which occupy non-contiguous (disjoint) address ranges where
> other functions might be placed in the gap.
> 
> So one of the problems that needed to be solved was to make the
> caching code work correctly.  It is interesting to note that stepping
> _did_ work when the cache was disabled.  This is/was due to GDB
> already having some (albeit incomplete) support for non-contiguous
> addresses in the form of blockvector address maps.  Code responsible
> for mapping addresses to blocks (which form the lower levels of
> find_pc_partial_function) handle this case correctly.
> 
> To solve the problem of incorrect disassembly, we need to be able
> to iterate over all of the ranges associated with a block.
> 
> Finally, we need to distinguish between the entry pc and the lowest
> address in a block.  I discovered that this lack of distinction was
> the cause of the remainder of the bugs including some which seemed to
> be introduced by fixing the problems noted above.  Once this
> distinction is made, it will be straightforward to add full support for
> DW_AT_entry_pc.  I considered adding this support as part of this
> patch series, but decided to wait until the community weighs in on my
> work thus far...


Thanks, it looks good from my side.  I'll let Pedro take a look at the 
test again.

Simon

Re: [PATCH v4 0/8] Non-contiguous address range support

[Pedro Alves]

On 08/22/2018 07:22 PM, Simon Marchi wrote:

> Thanks, it looks good from my side.  I'll let Pedro take a look at the test again.

Thanks.  I have no further comments.

Pedro Alves

Re: [PATCH v4 0/8] Non-contiguous address range support

[Kevin Buettner]

I've pushed this series.

Thanks to Simon and Pedro for the reviews.

Kevin