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

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

[Kevin Buettner]

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

In this v2 series, I've addressed the concerns from Simon Marchi's
review of the earlier patch set.  I've also changed my mind about how
return values *ADDRESS and *ENDADDR ought to be set for
find_pc_partial_function.  I'll 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 v2 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 v2 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 blockvec 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 4ad0527..5c5ee42 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -14722,6 +14722,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)
 	{
@@ -14730,7 +14731,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);
 	  record_block_range (block, start, end - 1);
+	  blockvec.emplace_back (start, end);
 	});
+
+      BLOCK_RANGES(block) = make_blockranges (objfile, blockvec);
     }
 }
 

[PATCH v2 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 a
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_pc_partial_entry_range
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 with requires prologue
analysis which also 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 | 78 +++++++++++++++++++++++++++++++++++++++++++-------------
 gdb/symtab.h     | 40 ++++++++++++++++++++++++++---
 2 files changed, 96 insertions(+), 22 deletions(-)

diff --git a/gdb/blockframe.c b/gdb/blockframe.c
index b3c9aa3..85be327 100644
--- a/gdb/blockframe.c
+++ b/gdb/blockframe.c
@@ -159,6 +159,7 @@ 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.  */
 
@@ -169,24 +170,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;
@@ -228,17 +219,64 @@ 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_low = BLOCK_START (b);
+	  cache_pc_function_high = BLOCK_END (b);
 	  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;
 	}
     }
@@ -268,6 +306,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:
 
@@ -292,12 +331,15 @@ find_pc_partial_function (CORE_ADDR pc, const char **name, CORE_ADDR *address,
 	     then add one to that.  */
 
 	  *endaddr = 1 + overlay_unmapped_address (cache_pc_function_high - 1,
-						   section);
+	                                           section);
 	}
       else
 	*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 84fc897..e518603 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1683,10 +1683,42 @@ extern struct symbol *find_pc_sect_function (CORE_ADDR, struct obj_section *);
 
 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 v2 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 v2 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 will still be the
very lowest address in the block - might not be the same as
BLOCK_ENTRY_PC.

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 85be327..05ff2aa 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);
 	}
     }
 
@@ -350,7 +350,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 43de7df..936e170 100644
--- a/gdb/compile/compile-c-symbols.c
+++ b/gdb/compile/compile-c-symbols.c
@@ -184,7 +184,7 @@ convert_one_symbol (struct 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;
@@ -497,7 +497,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 01c0bc4..be1cca0 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -272,10 +272,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 730934f..2da088e 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -361,7 +361,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;
@@ -833,7 +833,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 8ad5e25..09e5c29 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 5c5faf7..2b17957 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1554,7 +1554,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 3edd5b2..53830fd 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++;
 }
 
@@ -333,8 +333,8 @@ skip_inline_frames (ptid_t ptid, 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 ae0200b..1736e59 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 acb08f2..cd08879 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -1147,7 +1147,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;
 
@@ -1161,7 +1161,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 696285e..3c6e4be 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
@@ -1520,7 +1520,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 97ebc8b..5d3110d 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 3e594e7..4eb8a4f 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:
@@ -3620,7 +3620,7 @@ find_function_start_sal (symbol *sym, bool funfirstline)
 {
   fixup_symbol_section (sym, NULL);
   symtab_and_line sal
-    = find_function_start_sal (BLOCK_START (SYMBOL_BLOCK_VALUE (sym)),
+    = find_function_start_sal (BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (sym)),
 			       SYMBOL_OBJ_SECTION (symbol_objfile (sym), sym),
 			       funfirstline);
   sal.symbol = sym;
@@ -3700,7 +3700,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);
     }
@@ -3761,7 +3761,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)))
@@ -3965,7 +3965,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;
@@ -4970,7 +4970,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 5af3cfe..a1a3524 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -2519,7 +2519,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
 	    {
@@ -2596,7 +2596,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 9f9e78e..7c5c79e 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3051,7 +3051,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 v2 6/8] Introduce find_pc_partial_entry_range 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_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.

gdb/ChangeLog:
    
    	* infrun.c (fill_in_stop_func): Use find_pc_partial_entry_range
    	in place of find_pc_partial_function.
    	* blockframe.c (find_pc_partial_entry_range): New function.
    	* symtab.h (find_pc_partial_entry_range): Declare and document.
---
 gdb/blockframe.c | 37 +++++++++++++++++++++++++++++++++++++
 gdb/infrun.c     |  6 ++++--
 gdb/symtab.h     | 20 +++++++++++++++++++-
 3 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/gdb/blockframe.c b/gdb/blockframe.c
index 05ff2aa..38006aa 100644
--- a/gdb/blockframe.c
+++ b/gdb/blockframe.c
@@ -345,6 +345,43 @@ find_pc_partial_function (CORE_ADDR pc, const char **name, CORE_ADDR *address,
 
 /* See symtab.h.  */
 
+bool
+find_pc_partial_entry_range (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_pc_partial_entry_range"));
+    }
+
+  return status;
+}
+
+/* See symtab.h.  */
+
 struct type *
 find_function_type (CORE_ADDR pc)
 {
diff --git a/gdb/infrun.c b/gdb/infrun.c
index f455af2..027de22 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4297,10 +4297,12 @@ fill_in_stop_func (struct gdbarch *gdbarch,
 {
   if (!ecs->stop_func_filled_in)
     {
+
       /* 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 (stop_pc, &ecs->stop_func_name,
-				&ecs->stop_func_start, &ecs->stop_func_end);
+      find_pc_partial_entry_range (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 e518603..ff598c6 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1714,12 +1714,30 @@ 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_pc_partial_entry_range.  */
 
 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_pc_partial_entry_range (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 v2 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 95c39cf..7e4e5d3 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 v2 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 | 400 +++++++++++++++++++++++++++
 2 files changed, 478 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..227c753
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp
@@ -0,0 +1,400 @@
+# 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
+    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
+    lassign [function_range baz [list ${srcdir}/${subdir}/$srcfile]] \
+	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. I attempted 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_start+$main_len}
+	    {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_start+$foo_len}
+	    {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_start$foo_low_len}
+	    {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_start + $main_len}
+	    {range {$bar_start} $bar_start + $bar_len}
+	    {range {$baz_start} $baz_start + $baz_len}
+	}
+    }
+}
+
+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"
+}
+
+clean_restart ${testfile}
+if ![runto_main] {
+    return -1
+}
+
+with_test_prefix "step-test-2" {
+    # 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.
+# Note: I couldn't get $hex to work in the gdb_test_sequence regex.
+gdb_test_sequence "disassemble foo" "" {
+    "Dump of assembler code for function foo:"
+    "Address range 0x[\\dabcdefABCDEF]+ to 0x[\\dabcdefABCDEF]+:"
+    "   0x[\\dabcdefABCDEF]+ <\\+0>:"
+    "Address range 0x[\\dabcdefABCDEF]+ to 0x[\\dabcdefABCDEF]+:"
+    "   0x[\\dabcdefABCDEF]+ <(.+?)>:"
+    "End of assembler dump\\." 
+}
+
+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 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 (2nd time)"
+
+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"
+
+clean_restart ${testfile}
+if ![runto_main] {
+    return -1
+}
+
+with_test_prefix "step-test-3" {
+    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" work with some versions
+    # of gcc, though it's not clear to me that they should.  This example,
+    # which causes foo_low 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.  If a subroutine
+    # call were used, it would be considered to be a separate subprogram
+    # and the issues that I see wouldn't be encountered.
+    #
+    # For the moment though, I've left these tests in place, but disabled,
+    # in the event that 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 v2 0/8] Non-contiguous address range support

[Simon Marchi]

On 2018-08-13 7:50 p.m., Kevin Buettner wrote:
> This is version 2 of an eight part patch series which adds further
> support for non-contiguous address ranges to GDB.
> 
> In this v2 series, I've addressed the concerns from Simon Marchi's
> review of the earlier patch set.  I've also changed my mind about how
> return values *ADDRESS and *ENDADDR ought to be set for
> find_pc_partial_function.  I'll 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...
> 

Hi Kevin,

The patchset seems quite outdated (based on an old commit).  Could you
send a rebased version?

Thanks,

Simon

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

[Kevin Buettner]

> The patchset seems quite outdated (based on an old commit).  Could you
> send a rebased version?

Done.  The v3 0/8 patch is here:

https://sourceware.org/ml/gdb-patches/2018-08/msg00467.html

(I thought that I had rebased v2 prior to posting, but perhaps I hadn't...)

Kevin

Re: [PATCH v2 3/8] Add support for non-contiguous blocks to find_pc_partial_function

[Simon Marchi]

On 2018-08-13 08:04 PM, Kevin Buettner wrote:
> 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 a
> 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_pc_partial_entry_range
> 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 with requires prologue
> analysis which also occupies non-contiguous ranges.

LGTM, just some nits.

As I mentioned previously, cache_pc_function_{low,high} could be renamed
to reflect that they now represent the low/high addresses of the range.
If you think it's not necessary, it's also fine, but I just want to make
sure the comment didn't just fall through the cracks.

> @@ -228,17 +219,64 @@ 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_low = BLOCK_START (b);
> +	  cache_pc_function_high = BLOCK_END (b);

I think these last two lines are unnecessary, because we are guaranteed to set them
lower in any case.

> @@ -292,12 +331,15 @@ find_pc_partial_function (CORE_ADDR pc, const char **name, CORE_ADDR *address,
>  	     then add one to that.  */
>  
>  	  *endaddr = 1 + overlay_unmapped_address (cache_pc_function_high - 1,
> -						   section);
> +	                                           section);

Spurious change here.

Simon

Re: [PATCH v2 3/8] Add support for non-contiguous blocks to find_pc_partial_function

[Kevin Buettner]

On Tue, 21 Aug 2018 11:56:52 -0400
Simon Marchi <simon.marchi@ericsson.com> wrote:

> As I mentioned previously, cache_pc_function_{low,high} could be renamed
> to reflect that they now represent the low/high addresses of the range.
> If you think it's not necessary, it's also fine, but I just want to make
> sure the comment didn't just fall through the cracks.

Once I realized that find_pc_partial_function no longer needed to
track both the minimum/maximum function addresses AND the range in
which PC is found, I decided to leave the name alone.  (We only track
the latter now.)  For a while, I had rewritten it as you had suggested
in your earlier review.  It definitely made sense to do this when
find_pc_partial_function returned values for *ADDRESS and *ENDADDR
which referred to the min/max addresses of the function.

I'll take another look at it though.  If I decide to leave these names
unchanged, I'll extend the comment in the declaration for
cache_pc_function_{low,high}.

Kevin

Re: [PATCH v2 8/8] Test case for functions with non-contiguous ranges

[Pedro Alves]

On 08/14/2018 01:14 AM, Kevin Buettner wrote:
> 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 | 400 +++++++++++++++++++++++++++
>  2 files changed, 478 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..227c753
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp
> @@ -0,0 +1,400 @@
> +# 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
> +    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
> +    lassign [function_range baz [list ${srcdir}/${subdir}/$srcfile]] \
> +	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}
> +            }

Tab vs spaces mixup here.

> +	    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. I attempted to make it reasonably
> +	# accurate as it made debugging the test case easier.

Double space after ".".  

Nit: I'm not a big fan of using "I", "me", etc. in comments, it reads
a bit awkwardly personal to me.  If several people edit these
comments later on, who becomes the "I" then?

> +clean_restart ${testfile}
> +if ![runto_main] {
> +    return -1
> +}

Should move these "clean_restart / runto_main" calls into the
following with_test_prefix, in case the runto_main issues
an internal FAIL.

> +
> +with_test_prefix "step-test-2" {
> +    # 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.
> +# Note: I couldn't get $hex to work in the gdb_test_sequence regex.

Did you try to use [list .....] instead of {.....} ?

> +gdb_test_sequence "disassemble foo" "" {
> +    "Dump of assembler code for function foo:"
> +    "Address range 0x[\\dabcdefABCDEF]+ to 0x[\\dabcdefABCDEF]+:"
> +    "   0x[\\dabcdefABCDEF]+ <\\+0>:"
> +    "Address range 0x[\\dabcdefABCDEF]+ to 0x[\\dabcdefABCDEF]+:"
> +    "   0x[\\dabcdefABCDEF]+ <(.+?)>:"
> +    "End of assembler dump\\." 
> +}
> +
> +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

Spaces vs tabs mixup, seemingly.

Also, this leaves "foo_low_addr" unset if the gdb_test_multiple
ever fails, leading to a TCL error when next foo_low_addr is
referenced.

The usual pattern around this is to default the variable,
something like:

set foo_low_addr -1
gdb_test_multiple $test $test {
    -re "   ($hex) <foo.*?>.*${gdb_prompt}" {
        set foo_low_addr $expect_out(1,string)
	pass $test
    }

> +    }
> +}
> +
> +set test "x/i foo"
> +gdb_test_multiple $test $test {
> +    -re "   ($hex) <foo.*?>.*${gdb_prompt}" {
> +        set foo_addr $expect_out(1,string)
> +	pass $test

Ditto, indentation and default.

> +    }
> +}
> +
> +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 (2nd time)"

Don't use tail parens for "(2nd time)":

  https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages

Use e.g., "break foo, 2nd time", or create a with_test_prefix
block, if it makes sense.

> +
> +gdb_test "break baz" \
> +    "Breakpoint.*at.* file .*$srcfile, line \\d+\\."
> +
> +gdb_test "continue" \
> +    "Breakpoint \\d+, foo \\(\\).*" \
> +    "Continue to foo"

Lowercase "continue".

> +
> +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"

Ditto.

> +
> +clean_restart ${testfile}
> +if ![runto_main] {
> +    return -1
> +}
> +
Thanks,
Pedro Alves