Blame SOURCES/gdb-rhbz1639242-fix-dwarf2_find_containing_comp_unit-binary-search.patch

190f2a
From FEDORA_PATCHES Mon Sep 17 00:00:00 2001
190f2a
From: Sergio Durigan Junior <sergiodj@redhat.com>
190f2a
Date: Fri, 30 Nov 2018 15:20:27 -0500
190f2a
Subject: 
190f2a
 gdb-rhbz1639242-fix-dwarf2_find_containing_comp_unit-binary-search.patch
190f2a
190f2a
;; Fix for 'py-bt is broken, results in exception'.
190f2a
;; RHBZ 1639242
190f2a
190f2a
Fix dwarf2read.c:dwarf2_find_containing_comp_unit's binary search
190f2a
190f2a
First of all, I would like to express my gratitude to Keith Seitz, Jan
190f2a
Kratochvil and Tom Tromey, who were really kind and helped a lot with
190f2a
this bug.  The patch itself was authored by Jan.
190f2a
190f2a
This all began with:
190f2a
190f2a
  https://bugzilla.redhat.com/show_bug.cgi?id=1639242
190f2a
  py-bt is broken, results in exception
190f2a
190f2a
In summary, the error reported by the bug above is:
190f2a
190f2a
  $ gdb -args python3
190f2a
  GNU gdb (GDB) Fedora 8.1.1-3.fc28
190f2a
  (...)
190f2a
  Reading symbols from python3...Reading symbols from /usr/lib/debug/usr/bin/python3.6-3.6.6-1.fc28.x86_64.debug...done.
190f2a
  done.
190f2a
  Dwarf Error: could not find partial DIE containing offset 0x316 [in module /usr/lib/debug/usr/bin/python3.6-3.6.6-1.fc28.x86_64.debug]
190f2a
190f2a
After a long investigation, and after thinking that the problem might
190f2a
actually be on DWZ's side, we were able to determine that there's
190f2a
something wrong going on when
190f2a
dwarf2read.c:dwarf2_find_containing_comp_unit performs a binary search
190f2a
over all of the CUs belonging to an objfile in order to find the CU
190f2a
which contains a DIE at an specific offset.  The current algorithm is:
190f2a
190f2a
  static struct dwarf2_per_cu_data *
190f2a
  dwarf2_find_containing_comp_unit (sect_offset sect_off,
190f2a
				    unsigned int offset_in_dwz,
190f2a
				    struct dwarf2_per_objfile *dwarf2_per_objfile)
190f2a
  {
190f2a
    struct dwarf2_per_cu_data *this_cu;
190f2a
    int low, high;
190f2a
    const sect_offset *cu_off;
190f2a
190f2a
    low = 0;
190f2a
    high = dwarf2_per_objfile->all_comp_units.size () - 1;
190f2a
    while (high > low)
190f2a
      {
190f2a
	struct dwarf2_per_cu_data *mid_cu;
190f2a
	int mid = low + (high - low) / 2;
190f2a
190f2a
	mid_cu = dwarf2_per_objfile->all_comp_units[mid];
190f2a
	cu_off = &mid_cu->sect_off;
190f2a
	if (mid_cu->is_dwz > offset_in_dwz
190f2a
	    || (mid_cu->is_dwz == offset_in_dwz && *cu_off >= sect_off))
190f2a
	  high = mid;
190f2a
	else
190f2a
	  low = mid + 1;
190f2a
      }
190f2a
190f2a
For the sake of this example, let's consider that "sect_off =
190f2a
0x7d".
190f2a
190f2a
There are a few important things going on here.  First,
190f2a
"dwarf2_per_objfile->all_comp_units ()" will be sorted first by
190f2a
whether the CU is a DWZ CU, and then by cu->sect_off.  In this
190f2a
specific bug, "offset_in_dwz" is false, which means that, for the most
190f2a
part of the loop, we're going to do "high = mid" (i.e, we'll work with
190f2a
the lower part of the vector).
190f2a
190f2a
In our particular case, when we reach the part where "mid_cu->is_dwz
190f2a
== offset_in_dwz" (i.e, both are false), we end up with "high = 2" and
190f2a
"mid = 1".  I.e., there are only 2 elements in the vector who are not
190f2a
DWZ.  The vector looks like this:
190f2a
190f2a
  #0: cu->sect_off = 0;   length = 114;  is_dwz = false  <-- low
190f2a
  #1: cu->sect_off = 114; length = 7796; is_dwz = false  <-- mid
190f2a
  #2: cu->sect_off = 0;   length = 28;   is_dwz = true   <-- high
190f2a
  ...
190f2a
190f2a
The CU we want is #1, which is exactly where "mid" is.  Also, #1 is
190f2a
not DWZ, which is also exactly what we want.  So we perform the second
190f2a
comparison:
190f2a
190f2a
  (mid_cu->is_dwz == offset_in_dwz && *cu_off >= sect_off)
190f2a
                                      ^^^^^^^^^^^^^^^^^^^
190f2a
190f2a
Because "*cu_off = 114" and "sect_off = 0x7d", this evaluates to
190f2a
false, so we end up with "low = mid + 1 = 2", which actually gives us
190f2a
the wrong CU (i.e., a CU that is DWZ).  Next in the code, GDB does:
190f2a
190f2a
    gdb_assert (low == high);
190f2a
    this_cu = dwarf2_per_objfile->all_comp_units[low];
190f2a
    cu_off = &this_cu->sect_off;
190f2a
    if (this_cu->is_dwz != offset_in_dwz || *cu_off > sect_off)
190f2a
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
190f2a
      {
190f2a
	if (low == 0 || this_cu->is_dwz != offset_in_dwz)
190f2a
	  error (_("Dwarf Error: could not find partial DIE containing "
190f2a
		 "offset %s [in module %s]"),
190f2a
		 sect_offset_str (sect_off),
190f2a
		 bfd_get_filename (dwarf2_per_objfile->objfile->obfd));
190f2a
	...
190f2a
190f2a
Triggering the error we saw in the original bug report.
190f2a
190f2a
It's important to notice that we see the error message because the
190f2a
selected CU is a DWZ one, but we're looking for a non-DWZ CU here.
190f2a
However, even when the selected CU is *not* a DWZ (and we don't see
190f2a
any error message), we still end up with the wrong CU.  For example,
190f2a
suppose that the vector had:
190f2a
190f2a
  #0: cu->sect_off = 0;    length = 114;  is_dwz = false
190f2a
  #1: cu->sect_off = 114;  length = 7796; is_dwz = false
190f2a
  #2: cu->sect_off = 7910; length = 28;   is_dwz = false
190f2a
  ...
190f2a
190f2a
I.e., #2's "is_dwz" is false instead of true.  In this case, we still
190f2a
want #1, because that's where the DIE is located.  After the loop ends
190f2a
up in #2, we have "is_dwz" as false, which is what we wanted, so we
190f2a
compare offsets.  In this case, "7910 >= 0x7d", so we set "mid = high
190f2a
= 2".  Next iteration, we have "mid = 0 + (2 - 0) / 2 = 1", and thus
190f2a
we examining #1.  "is_dwz" is still false, but "114 >= 0x7d" also
190f2a
evaluates to false, so "low = mid + 1 = 2", which makes the loop stop.
190f2a
Therefore, we end up choosing #2 as our CU, even though #1 is the
190f2a
right one.
190f2a
190f2a
The problem here is happening because we're comparing "sect_off"
190f2a
directly against "*cu_off", while we should actually be comparing
190f2a
against "*cu_off + mid_cu->length" (i.e., the end offset):
190f2a
190f2a
  ...
190f2a
  || (mid_cu->is_dwz == offset_in_dwz
190f2a
      && *cu_off + mid_cu->length >= sect_off))
190f2a
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
190f2a
  ...
190f2a
190f2a
And this is what the patch does.  The idea is that if GDB is searching
190f2a
for an offset that falls above the *end* of the CU being
190f2a
analyzed (i.e., "mid"), then the next iteration should try a
190f2a
higher-offset CU next.  The previous algorithm was using
190f2a
the *beginning* of the CU.
190f2a
190f2a
Unfortunately, I could not devise a testcase for this problem, so I am
190f2a
proposing a fix with this huge explanation attached to it in the hope
190f2a
that it is sufficient.  After talking a bit to Keith (our testcase
190f2a
guru), it seems that one would have to create an objfile with both DWZ
190f2a
and non-DWZ sections, which may prove very hard to do, I think.
190f2a
190f2a
I ran this patch on our BuildBot, and no regressions were detected.
190f2a
190f2a
gdb/ChangeLog:
190f2a
2018-11-30  Jan Kratochvil  <jan.kratochvil@redhat.com>
190f2a
	    Keith Seitz  <keiths@redhat.com>
190f2a
	    Tom Tromey  <tom@tromey.com>
190f2a
	    Sergio Durigan Junior  <sergiodj@redhat.com>
190f2a
190f2a
	https://bugzilla.redhat.com/show_bug.cgi?id=1613614
190f2a
	* dwarf2read.c (dwarf2_find_containing_comp_unit): Add
190f2a
	'mid_cu->length' to '*cu_off' when checking if 'sect_off' is
190f2a
	inside the CU.
190f2a
190f2a
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
190f2a
--- a/gdb/dwarf2read.c
190f2a
+++ b/gdb/dwarf2read.c
190f2a
@@ -25039,7 +25039,8 @@ dwarf2_find_containing_comp_unit (sect_offset sect_off,
190f2a
       mid_cu = dwarf2_per_objfile->all_comp_units[mid];
190f2a
       cu_off = &mid_cu->sect_off;
190f2a
       if (mid_cu->is_dwz > offset_in_dwz
190f2a
-	  || (mid_cu->is_dwz == offset_in_dwz && *cu_off >= sect_off))
190f2a
+	  || (mid_cu->is_dwz == offset_in_dwz
190f2a
+	      && *cu_off + mid_cu->length >= sect_off))
190f2a
 	high = mid;
190f2a
       else
190f2a
 	low = mid + 1;