This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH][PR gdb/19361] Fix invalid comparison functions


Hi all,

The attached patch fixes bugs in comparison functions qsort_cmp and compare_processes.

I've tested the patch on x86_64-pc-linux-gnu (no regressions in testsuite except for flakiness in gdb.threads and bigcore.exp).

These functions are passed to qsort(3) but do not obey standard symmetry requirements mandated by the standard (grep for "total ordering" in http://pubs.opengroup.org/onlinepubs/009695399/functions/qsort.html). This causes undefined behavior at runtime which can e.g. cause qsort to produce invalid results.

Compare_processes fails to properly compare process group leaders which is probably a serious problem (e.g. resulting in invalid sort).

Qsort_cmp fails to produce proper result when comparing same element. Sane qsort implementation probably don't call comparison callback on same element so this may not be a big problem in practice but I think it should still be fixed.

NB1: please Cc: me explicitly, I'm not subscribed to the list.
NB2: Bugs were found with SortChecker tool (https://github.com/yugr/sortcheck).

Best regards,
Yury Gribov
>From 5bc187e060a75f0dff83e2803adb53f41c2e6dcb Mon Sep 17 00:00:00 2001
From: Yury Gribov <y.gribov@samsung.com>
Date: Tue, 15 Dec 2015 10:56:06 +0300
Subject: [PATCH] Fix invalid comparison functions.

2015-12-15  Yury Gribov  <tetra2005@gmail.com>

	PR gdb/19361
        * nat/linux-osdata.c (compare_processes):
	Fix asymmetry in comparison function.
	* objfiles.c (qsort_cmp): Ditto.
---
 gdb/nat/linux-osdata.c | 4 ++--
 gdb/objfiles.c         | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c
index 56a8fe6..25a310f 100644
--- a/gdb/nat/linux-osdata.c
+++ b/gdb/nat/linux-osdata.c
@@ -420,9 +420,9 @@ compare_processes (const void *process1, const void *process2)
   else
     {
       /* Process group leaders always come first, else sort by PID.  */
-      if (pid1 == pgid1)
+      if (pid1 == pgid1 && pid2 != pgid2)
 	return -1;
-      else if (pid2 == pgid2)
+      else if (pid1 != pgid1 && pid2 == pgid2)
 	return 1;
       else if (pid1 < pid2)
 	return -1;
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index d33379f..4aa600b 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -1140,6 +1140,9 @@ qsort_cmp (const void *a, const void *b)
   const CORE_ADDR sect1_addr = obj_section_addr (sect1);
   const CORE_ADDR sect2_addr = obj_section_addr (sect2);
 
+  if (sect1 == sect2)
+    return 0;
+
   if (sect1_addr < sect2_addr)
     return -1;
   else if (sect1_addr > sect2_addr)
-- 
1.9.1


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]