diff mbox

[PR,gdb/19361] Fix invalid comparison functions

Message ID 566FFEE2.4010300@samsung.com
State New, archived
Headers

Commit Message

Yury Gribov Dec. 15, 2015, 11:52 a.m. UTC 
  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
diff mbox

Patch

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