[RFA,1/6] Use std::vector in end_symtab_get_static_block

Message ID 20171020153318.AF81FD80819@oc3748833570.ibm.com
State New, archived
Headers

Commit Message

Ulrich Weigand Oct. 20, 2017, 3:33 p.m. UTC
  [Re-sent since the original seems to have gotten lost somehow.]

> >>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
> 
> Simon> That made me doubt for a second, since we're more used to see "<"
> Simon> in these functions.  I then saw that block_compar did sort them
> Simon> in descending order.  Could you add a comment here to indicate that?
> 
> Done, thanks.

This causes a number of regressions in the gdb.opt/inline-cmds.exp
test case for me.  Not sure exactly why, but changing the std::sort
to a std::stable_sort like below fixes those regressions for me.
Maybe the logic for handling inline function blocks somehow relied
on some (undocumented) behavior of qsort for handling elements that
compare as equal?

Bye,
Ulrich
  

Comments

Pedro Alves Oct. 20, 2017, 4:47 p.m. UTC | #1
On 10/20/2017 04:33 PM, Ulrich Weigand wrote:
> [Re-sent since the original seems to have gotten lost somehow.]
> 
>>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>>
>> Simon> That made me doubt for a second, since we're more used to see "<"
>> Simon> in these functions.  I then saw that block_compar did sort them
>> Simon> in descending order.  Could you add a comment here to indicate that?
>>
>> Done, thanks.
> 
> This causes a number of regressions in the gdb.opt/inline-cmds.exp
> test case for me.  Not sure exactly why, but changing the std::sort
> to a std::stable_sort like below fixes those regressions for me.
> Maybe the logic for handling inline function blocks somehow relied
> on some (undocumented) behavior of qsort for handling elements that
> compare as equal?
> 

Sounds like we should improve the sort predicate to disambiguate
better, define a total order?  I don't really know which sorting
is assumed, but e.g., if the block start addresses are the same,
compare the blocks' end addresses.  If those match as well,
repeat but with the blocks' superblocks.  And/or sort function
blocks before non-function blocks.  Etc.

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index c556ac1..b6d4e67 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -1249,7 +1249,7 @@  end_symtab_get_static_block (CORE_ADDR end_addr, int expandable, int required)
       for (pb = pending_blocks; pb != NULL; pb = pb->next)
 	barray.push_back (pb->block);
 
-      std::sort (barray.begin (), barray.end (),
+      std::stable_sort (barray.begin (), barray.end (),
 		 [] (const block *a, const block *b)
 		 {
 		   /* Sort blocks in descending order.  */