Re: What should we do re: "[patch] Speed up find_pc_section"

[Paul Pluzhnikov <ppluzhnikov at google dot com>]

On Tue, Sep 8, 2009 at 11:36 AM, Joel Brobecker <brobecker@adacore.com> wrote:

> According to Tristan Gingold, this optimization is currently causing
> the Darwin debugger to fail completely (I think Paul knows).

I certainly do. I also have a fix in the works, and Tristan confirmed that
what I have now fixes the problem for him.

> Basically,
> what happens is that, on Darwin, the debugging info is in the .o files,
> and the sections there will necessarily overlap with the sections in
> the executable.

Rather, all the .o files are relocatable, so they all start at zero
and overlap each other.

> On MacOS, the way things are currently setup, I think
> that we read the .o files as extra object files, rather than sources
> of debug info (a-la .gnu_debug_link).

Correct.

> I don't know how dodgy it was in the first place to load the entire
> .o as opposed to just its debugging info, but fixing this will require
> a non-trivial effort.

I don't believe there is non-trivial effort in fixing this.
In fact, I believe (but have not yet confirmed) this may just be fixed by
the other patch:
  http://sourceware.org/ml/gdb-patches/2009-08/msg00437.html

Specifically, I believe all the .o files will have lma != vma and will
get filtered out as if they are overlay sections:

+static int
+insert_section_p (const struct bfd *abfd,
+		  const struct bfd_section *section)
+{
+  const bfd_vma lma = bfd_section_lma (abfd, section);
+
+  if (lma != 0 && lma != bfd_section_vma (abfd, section)
+      && (bfd_get_file_flags (abfd) & BFD_IN_MEMORY) == 0)
+    /* This is an overlay section.  IN_MEMORY check is needed to avoid
+       discarding sections from the "system supplied DSO" (aka vdso)
+       on Linux.  */
+    return 0;


> Tristan has some ideas on how to do this, but
> it would be too late for the release - we're probably looking at fixing
> this for 7.1. In the meantime, the MacOS port is badly broken.
>
> I would like us to consider the idea of reverting that patch (and all
> followup patches), only to re-apply it after the gdb-7.0 branch is cut.
> The reason behind this suggestion is that we're still negotiating some
> issues uncovered by this patch (I understand that some of the issues
> were latent), and we're just so close to the branch time that I think
> it would be less risky to release without it.
>
> Paul, before we discuss the merits of that suggestion, do you think
> that it would be doable to revert? I know that several patches have
> already been applied on top in order to fix some of the issues we
> later uncovered.

It appears to me that reverting this patch is more work than fixing the
overlay and MacOS problems exposed by it. But I haven't looked in detail
into what it will take to revert.

> Other ideas?
>
> Perhaps one possiblity would be to remove that assertion from the code
> and let it return one section at random that matches the PC? Paul seems
> to say that, if the assertion is tripped, it means that, before his
> patch, we were already potentially returning the wrong section anyway.

I believe that's correct.

> Does removing the assertion bring us back to that situation?

Almost, but not exactly: before the patch, the order of adding objfiles
mattered: the earlier objfile with overlapping section was the one that
was always found. Now the order of objfiles doesn't matter, as sections
are sorted independently.

> One last suggestion that Tristan did make this morning, is to downgrade
> Darwin support to DSYM files only, as opposed to automatically load
> all .o files. That would be too bad, however, as this introduces an
> extra step in the build that's inconvenient, and potentially time-
> consuming.

Note that the separate '*.o' addition has its own bug: the sections from
all the .o files are added, but never removed (and this is unrelated to
my patch in any way). This is in fact why Tristan said that my earlier patch
"works, but makes GDB spend 10 minutes in CPU loop".

What really happens there is that with lots of separate .o files to load for
a shared library, the first run takes 10s (600+ *.o files loaded), the
second 150s (1200+ *.o loaded), the third 600s.

This is also the part I haven't finished fixing yet.

Now that I am back from vacation, I will confirm that the
gdb-find_pc_section-20090825.txt from
  http://sourceware.org/ml/gdb-patches/2009-08/msg00437.html
in fact does also fix MacOS, and will work on eliminating unbounded growth
of the separate .o files.

If someone could just review the above patch, we'll be good to go :=)

Thanks,
--
Paul Pluzhnikov