Re: [PATCHv5] Fix range end handling of inlined subroutines

Bernd Edlinger
2020-04-25T07:29:47Z

On 4/22/20 11:03 PM, Tom Tromey wrote:
>>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
>
>>> +void
>>> +buildsym_compunit::record_inline_range_end (CORE_ADDR end)
>>> +{
>>> + /* The performance of this function is very important,
>>> + it shall be O(n*log(n)) therefore we do not use std::vector
>>> + here since some compilers, e.g. visual studio, do not
>>> + guarantee that for vector::push_back. */
>
> Andrew> I think we're going to need more of a group discussion on this.
> Andrew> Simply saying std::vector is too slow doesn't I'm afraid convince me.
> Andrew> There seem to be lots of other places in GDB where both performance is
> Andrew> critical, and we use ::push_back.
>
> Andrew> Is your claim that we should move away from std::vector in all these
> Andrew> cases? Is this case somehow special?
>
> C++ documents push_back as having amortized constant complexity. If
> that's not the case for the MS compiler, that seems like a pretty
> serious bug there... I guess I'd like some documentation of some kind (a
> stackoverflow question, or maybe a test program that shows the vector
> growing linearly, or something like that).
>

The problem is, this is in an inner loop where the overall system
performance is severely affected by this bug which is not our fault.

BUT for the user this is a GDB bug, and guess what the microsoft debugger
will load faster. So why do we take that risk?

> I didn't understand the relevance or target of the "n*log(n)" comment.
>

Maybe give more context, I do not understand your question when all the context
is removed.

> Andrew> I don't think we should be doing this. This is defined quite clearly
> Andrew> in the DWARF spec as being an empty range. No code is associated with
> Andrew> this range. As such, it really shouldn't have an impact on how we
> Andrew> interpret the rest of the DWARF.
>
> Andrew> Again, I think you're trying too hard to work around GCC's broken
> Andrew> DWARF output.
>
> Do we know how long GCC has been generating this?
> And whether anybody is investigating a fix?
>

Since gcc-7 IIRC. So these versions can no longer be changed,
but they will be around for very long. As I said the only possible outcome
is disabling this feature for GCC a back-port of anything more complicated
will be way too dangerous. Even that is only gcc-10, gcc-9 and gcc-8.

So we missed gcc-7, everybody using that will be in trouble.

> I dislike adding workarounds for relatively modern versions of
> GCC... I'd prefer these things be fixed in GCC if possible. However, if
> that's not possible, I'm also flexible about it.
>
> Tom
>

Tom, once a bug has been unfixed for so long, the damage can no longer
be undone.

So I think we should work around the failuer,
which is IMHO neither GCC nor GDB's fault, but a flaw in the DWARF-2 spec.

I think the best solution would be to fix the spec, and work around
the issue until we have a new dwarf-version.

Thanks,
Bernd.