Bug 28404

Summary:	Verify bsearch precondition in find_pc_section
Product:	gdb	Reporter:	Tom de Vries <vries>
Component:	gdb	Assignee:	Not yet assigned to anyone <unassigned>
Status:	RESOLVED WORKSFORME
Severity:	normal
Priority:	P2
Version:	HEAD
Target Milestone:	---
Host:		Target:
Build:		Last reconfirmed:
Project(s) to access:		ssh public key:

Description Tom de Vries 2021-10-01 09:25:13 UTC

While debugging some problem I came across a bsearch in find_pc_section, which relies on the sections array being in ascending sorted order.

Visual inspection revealed that the precondition was not met.

It would probably be good to write an assert checking the precondition, and use it to see if indeed it triggers.

Perhaps even add a verify_bsearch to gdbsupport that can be dropped in place to check the precondition.

Comment 1 Tom de Vries 2021-10-01 15:10:08 UTC

I wrote this:
...
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index b65fa8820ca..cc9a47eaad8 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -1227,6 +1227,19 @@ find_pc_section (CORE_ADDR pc)
       return NULL;
     }
 
+  {
+    struct obj_section *prev_elem = nullptr;
+    struct obj_section *elem = nullptr;
+    for (int i = 0; i < pspace_info->num_sections; (prev_elem = elem), ++i)

+      {
+       elem = pspace_info->sections[i];
+       gdb_assert (elem->addr () <= elem->endaddr ());
+       if (prev_elem == nullptr)
+         continue;
+       gdb_assert (prev_elem->endaddr () <= elem->addr ());
+      }
+  }re--
+
   sp = (struct obj_section **) bsearch (&pc,
                                        pspace_info->sections,
                                        pspace_info->num_sections,
... 

And tested using target board unix/-fPIE/-pie.  No regression.

So, I guess I was looking at the consequences of the patch series I was playing around with.

Comment 2 Tom de Vries 2021-10-01 15:13:02 UTC

(In reply to Tom de Vries from comment #0)
> Perhaps even add a verify_bsearch to gdbsupport that can be dropped in place
> to check the precondition.

That doesn't seem to be possible, since the check that we're trying to do uses a different compare function (elem vs elem) than bsearch does (key vs elem).

Adding this code as a regular check seems somewhat expensive.  So I'm leaving things as they are for now.

Marking this fixed-worksforme.