Bug 28404 - Verify bsearch precondition in find_pc_section
Summary: Verify bsearch precondition in find_pc_section
Status: RESOLVED WORKSFORME
Alias: None
Product: gdb
Classification: Unclassified
Component: gdb (show other bugs)
Version: HEAD
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-10-01 09:25 UTC by Tom de Vries
Modified: 2021-10-01 15:13 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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.