[gdb] Partially stabilize sort in compare_{symbols,msymbols}

In compare_symbols in gdb/linespec.c:
...
  uia = (uintptr_t) a.symbol->symtab ()->compunit ()->objfile ()->pspace ();
  uib = (uintptr_t) b.symbol->symtab ()->compunit ()->objfile ()->pspace ();

  if (uia < uib)
    return true;
  if (uia > uib)
     return false;
...
we compare pointers to struct program_space, which gives unstable sorting
results.

The assumption is that this doesn't matter, but as PR32202 demonstrates,
sometimes it does.

While PR32202 is fixed elsewhere, it seems like a good idea to stabilize this
comparison, because it comes at a small cost and possibly prevents
hard-to-reproduce user-visible ordering issues.

Fix this by comparing the program space IDs instead of the pointers.

Likewise in compare_msymbols.

Tested on x86_64-linux.

Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>

diff --git a/gdb/linespec.c b/gdb/linespec.c
index d42420ccfc2005d794eb5cecbe620f858a968577..53a59abfe90843e2155f9cd8584d94ae46f607d7 100644 (file)
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -3407,23 +3407,41 @@ lookup_prefix_sym (struct linespec_state *state,
   return collector.release_symbols ();
 }
 
-/* A std::sort comparison function for symbols.  The resulting order does
-   not actually matter; we just need to be able to sort them so that
-   symbols with the same program space end up next to each other.  */
+/* Compare pspace A and B based on program space ID.  Return 0 if equal,
+   1 if A->num > B->num, -1 otherwise (modeled on strcmp).  */
 
-static bool
-compare_symbols (const block_symbol &a, const block_symbol &b)
+static int
+compare_pspace (const struct program_space *a, const struct program_space *b)
 {
-  uintptr_t uia, uib;
+  if (a->num > b->num)
+    return 1;
 
-  uia = (uintptr_t) a.symbol->symtab ()->compunit ()->objfile ()->pspace ();
-  uib = (uintptr_t) b.symbol->symtab ()->compunit ()->objfile ()->pspace ();
+  if (a->num < b->num)
+    return -1;
 
-  if (uia < uib)
+  return 0;
+}
+
+/* An std::sort comparison function for symbols.  The requirement is that
+   symbols with the same program space end up next to each other.  This is for
+   the purpose of iterating over the symbols and doing something once for each
+   program space.  */
+
+static bool
+compare_symbols (const block_symbol &a, const block_symbol &b)
+{
+  /* To check for same program space, we could just use a pointer comparison,
+     which gives unstable sorting results.  While the assumption is that this
+     doesn't matter, play it safe and compare program space IDs instead.  */
+  int cmp
+    = compare_pspace (a.symbol->symtab ()->compunit ()->objfile ()->pspace (),
+                     b.symbol->symtab ()->compunit ()->objfile ()->pspace ());
+  if (cmp == -1)
     return true;
-  if (uia > uib)
+  if (cmp == 1)
     return false;
 
+  uintptr_t uia, uib;
   uia = (uintptr_t) a.symbol;
   uib = (uintptr_t) b.symbol;
 
@@ -3438,16 +3456,14 @@ compare_symbols (const block_symbol &a, const block_symbol &b)
 static bool
 compare_msymbols (const bound_minimal_symbol &a, const bound_minimal_symbol &b)
 {
-  uintptr_t uia, uib;
-
-  uia = (uintptr_t) a.objfile->pspace ();
-  uib = (uintptr_t) a.objfile->pspace ();
-
-  if (uia < uib)
+  /* See comment in compare_symbols for use of compare_pspace.  */
+  int cmp = compare_pspace (a.objfile->pspace (), a.objfile->pspace ());
+  if (cmp == -1)
     return true;
-  if (uia > uib)
+  if (cmp == 1)
     return false;
 
+  uintptr_t uia, uib;
   uia = (uintptr_t) a.minsym;
   uib = (uintptr_t) b.minsym;