llvm dev - Jul 2020 - Switch to ld.bfd tombstone behavior by default

> I still think that we do bfd locs with a decent option to change for at
least the current release and sources and then, once we're a little more
certain we have everything that might want to parse dwarf (say by working with
dwarf-discuss), we can change the default.
Given what’s been found, I think Eric/Dave are correct, use bfd behavior by
default with an option to do the new thing.  The option can be coded to let Sony
(SCE debugger tuning) have it on by default.

Bringing it up on dwarf-discuss seems like a good idea too, you can refer to
http://dwarfstd.org/ShowIssue.php?issue=200609.1 as part of that.
--paulr

From: Eric Christopher <echristo at gmail.com>
Sent: Saturday, July 25, 2020 12:56 AM
To: Fāng-ruì Sòng <maskray at google.com>
Cc: Hans Wennborg <hans at chromium.org>; llvm-dev at lists.llvm.org;
Alexey Lapshin <a.v.lapshin at mail.ru>; George Rimar <grimar at
accesssoftek.com>; Robinson, Paul <paul.robinson at sony.com>; Adrian
Prantl <aprantl at apple.com>; Jonas Devlieghere <jdevlieghere at
apple.com>; James Henderson <jh7370.2008 at my.bristol.ac.uk>; Peter
Smith <Peter.Smith at arm.com>; David Blaikie <dblaikie at
gmail.com>; Pavel Labath <pavel at labath.sk>
Subject: Re: [llvm-dev] Switch to ld.bfd tombstone behavior by default

Hi Ray :)

While I understand the desire to say "we've updated everything
we've found" the problem is that we don't know that we've found
even much of the uses or can guarantee that people can upgrade, say their gdb or
breakpad, as fast as their compiler. Even worse it's a change in behavior
without much notice at all or even a particularly friendly way to recognize what
they should do to fix it.

I still think that we do bfd locs with a decent option to change for at least
the current release and sources and then, once we're a little more certain
we have everything that might want to parse dwarf (say by working with
dwarf-discuss), we can change the default.

-eric

On Fri, Jul 24, 2020 at 7:33 PM Fāng-ruì Sòng <maskray at
google.com<mailto:maskray at google.com>> wrote:
From my understanding the breakpad bug was also only related to .debug_line and
has been fixed by
https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2317730<https://urldefense.com/v3/__https:/chromium-review.googlesource.com/c/breakpad/breakpad/*/2317730__;Kw!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWYCu7N-Fg$>
> a) .debug_ranges&.debug_loc => -2, .debug_line => 0, other
.debug_* -> -1
> b) .debug_ranges&.debug_loc => -2, other .debug_* => 0
I am still of the opinion that we should just do a), not b).


On Fri, Jul 24, 2020 at 9:33 AM Hans Wennborg <hans at
chromium.org<mailto:hans at chromium.org>> wrote:
On Fri, Jul 24, 2020 at 5:12 PM Fangrui Song <maskray at
google.com<mailto:maskray at google.com>>
wrote:
>
> On 2020-07-24, Hans Wennborg via llvm-dev wrote:
> >Sounds good to me from a release perspective.
>
> I think we need more input from the triage of
>
https://chromium-review.googlesource.com/c/chromium/src/+/2291352<https://urldefense.com/v3/__https:/chromium-review.googlesource.com/c/chromium/src/*/2291352__;Kw!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWb5tE7cMg$>
> whether it is just .debug_line or .debug_*
The issue we hit in Chromium is tracked here:
https://bugs.chromium.org/p/chromium/issues/detail?id=1105559<https://urldefense.com/v3/__https:/bugs.chromium.org/p/chromium/issues/detail?id=1105559__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWanrbB9Cg$>
Doesn't
look like it has any more info at the moment.
>
> >On Fri, Jul 24, 2020 at 7:53 AM Eric Christopher via llvm-dev
> ><llvm-dev at lists.llvm.org<mailto:llvm-dev at
lists.llvm.org>> wrote:
> >>
> >> Hi All,
> >>
> >> In general I think we should adopt Dave's plan here. The
number of consumers that can (and have) been caught off guard by this change is
just too high.
> >>
> >> At the very least I think we should move this to opt in to the new
tombstoning behavior by default and at most migrate to bfd's behavior for
both the current release and in the current tree. If we want to make this sort
of change in the future by default I think we're going to need to provide
release notes about this and do aggressive outreach towards the consumers we do
know before making the change. If anyone wants to drive that effort I'll
happily provide any help or assistance in getting you in touch with at least the
consumers I know about.
> >>
> >> This change and the need for it is also probably worth a quick
message to dwarf-discuss at dwarfstd.org<mailto:dwarf-discuss at
dwarfstd.org> as well. Most of the major consumers and producers are on that
list and it's probably one of the easiest ways to get this change out there.
> >>
> >> Any strong objections to this path in the meantime?
> >>
> >> Thanks!
> >>
> >> -eric
> >>
> >> On Tue, Jul 21, 2020 at 12:18 PM David Blaikie via llvm-dev
<llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>>
wrote:
> >>>
> >>> On Tue, Jul 21, 2020 at 5:34 AM Alexey Lapshin
> >>> <alapshin at accesssoftek.com<mailto:alapshin at
accesssoftek.com>> wrote:
> >>> >
> >>> >
> >>> > >On Mon, Jul 20, 2020 at 10:32 AM Alexey Lapshin
> >>> > ><alapshin at accesssoftek.com<mailto:alapshin
at accesssoftek.com>> wrote:
> >>> > >>
> >>> > >> >On Fri, Jul 17, 2020 at 1:55 PM Alexey
Lapshin <a.v.lapshin at mail.ru<mailto:a.v.lapshin at mail.ru>>
wrote:
> >>> > >> >>
> >>> > >> >> >Пятница, 17 июля 2020, 19:42 +03:00
от David Blaikie <dblaikie at gmail.com<mailto:dblaikie at
gmail.com>>:
> >>> > >> >> >
> >>> > >> >> >On Fri, Jul 17, 2020 at 12:03 AM
Fangrui Song <maskray at google.com<mailto:maskray at google.com>>
wrote:
> >>> > >> >> >>
> >>> > >> >> >> Thanks for the write-up!
> >>> > >> >> >>
> >>> > >> >> >> On 2020-07-16, David Blaikie
wrote:
> >>> > >> >> >> >In short: Perhaps we
should switch lld to the bfd-style tombstoning
> >>> > >> >> >> >behavior for a release or
two, letting users opt-in to testing with the new
> >>> > >> >> >> >-1/-2 tombstoning in the
interim, before switching to the new tombstone by
> >>> > >> >> >> >default (while still
having the flag to switch back when users find
> >>> > >> >> >> >surprise places that
can't handle the new behavior).
> >>> > >> >> >> >
> >>> > >> >> >> >In long:
> >>> > >> >> >>
>https://reviews.llvm.org/D81784<https://urldefense.com/v3/__https:/reviews.llvm.org/D81784__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWYFUjv39g$>
and follow-on patches modified the behavior
> >>> > >> >> >> >of lld with regards to
resolving relocations from debug sections to dead
> >>> > >> >> >> >code (either comdat
deduplicated, or gc-sections use).
> >>> > >> >> >> >
> >>> > >> >> >> >A very quick summary of
the situation:
> >>> > >> >> >> >
> >>> > >> >> >> >Original Behavior:
> >>> > >> >> >> >
> >>> > >> >> >> > - bfd: 1 for
debug_ranges(0 would prematurely terminate the list), 0
> >>> > >> >> >> > elsewhere
> >>> > >> >> >> > - gold/lld: 0+addend
everywhere
> >>> > >> >> >> >
> >>> > >> >> >> >Limitations/bugs:
> >>> > >> >> >> >
> >>> > >> >> >> > - bfd/gold/lld
> >>> > >> >> >> > - doesn't support 0
as a valid executable address without ambiguities
> >>> > >> >> >> > - gold/lld
> >>> > >> >> >> > - ambiguities with large
gc'd functions combined with a .text mapping
> >>> > >> >> >> > that starts in relative
low addresses
> >>> > >> >> >> > - premature debug_range
termination with zero-length functions (Clang
> >>> > >> >> >> > produces these with
__builtin_unreachable or non-void return
> >>> > >> >> >> >type functions
> >>> > >> >> >> > without a return
statement)
> >>> > >> >> >> >
> >>> > >> >> >> >New behavior:
> >>> > >> >> >> >
> >>> > >> >> >> > - -2 for DWARFv4
debug_loc, debug_ranges (-1 is a base address specifier
> >>> > >> >> >> > there)
> >>> > >> >> >> > - -1 elsewhere
> >>> > >> >> >> > - linker flag to
customize to other values if desired
> >>> > >> >> >> >
> >>> > >> >> >> >Known issues:
> >>> > >> >> >> >
> >>> > >> >> >> > - lldb's line table
parsing can't handle -1 well at all (essentially
> >>> > >> >> >> > unusable)
> >>> > >> >> >>
> >>> > >> >> >> Pavel Labath will fix this
soon
https://reviews.llvm.org/D83957<https://urldefense.com/v3/__https:/reviews.llvm.org/D83957__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWZM6TUIWQ$>
> >>> > >> >> >> This is an unhandled
address-space wraparound problem.
> >>> > >> >> >> This pattern is potentially
common - and other downstream DWARF
> >>> > >> >> >> consumers might make similar
line table handling mistakes.
> >>> > >> >> >
> >>> > >> >> >That's the thing - I'm not
sure we can really classify them as
> >>> > >> >> >"mistakes". I think
bfd.ld's tombstoning behavior is about the only
> >>> > >> >> >thing we can reasonably say DWARF
consumers should /probably/ be
> >>> > >> >> >expected to handle - and I'd
imagine in many cases they haven't been
> >>> > >> >> >written intentionally to handle it,
but whatever behavior they have
> >>> > >> >> >has accidentally been sufficient
for their needs.
> >>> > >> >>
> >>> > >> >> >> > - gdb's line table
parsing ends up with different handling when breaking
> >>> > >> >> >> > on gc'd functions
(minor functionality issue)
> >>> > >> >> >>
> >>> > >> >> >> This is just a behavior
difference, not affecting users.
> >>> > >> >> >> It did break a test if linked
with LLD (gdb intrinsically has lots of
> >>> > >> >> >> failing tests even if built
with GCC+GNU ld).
> >>> > >> >> >>
> >>> > >> >> >> Previous behavior (when an
address is zero): a breakpoint on a
> >>> > >> >> >> --gc-sections discarded
function will be redirected to a larger line
> >>> > >> >> >> number with debug info, even
if that line can be an unrelated different
> >>> > >> >> >> function.
> >>> > >> >> >> New behavior is that the
breakpoint is on a wrapped-around small address.
> >>> > >> >> >>
> >>> > >> >> >> GDB 9.3 will restore the
previous behavior
> >>> > >> >> >>
(https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=a8caed5d7faa639a1e6769eba551d15d8ddd9510<https://urldefense.com/v3/__https:/sourceware.org/git/?p=binutils-gdb.git;a=commit;h=a8caed5d7faa639a1e6769eba551d15d8ddd9510__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWZQ1XTxqw$>)
> >>> > >> >> >>
> >>> > >> >> >> >
> >>> > >> >> >> >I think there's enough
risk in this work (even given the small number of
> >>> > >> >> >> >bugs found so far), given
there's a pretty wide array of debug info
> >>> > >> >> >> >consumers out there, that
we should change lld's default to match the
> >>> > >> >> >> >long-lived bfd strategy.
This would address my original motivation for
> >>> > >> >> >> >raising all this (empty
functions prematurely terminating the list), while
> >>> > >> >> >> >letting users who want to
experiment with it, or need it (like Alexey), can
> >>> > >> >> >> >opt-in to the -1/-2
behavior.
> >>> > >> >> >>
> >>> > >> >> >> I think we can only
confidently say that there is enough risk in using
> >>> > >> >> >> tombstone value -1 in
.debug_line, but I'd not say tombstone value -1 in
> >>> > >> >> >> other .debug_* can cause
problems.
> >>> > >> >>
> >>> > >> >> >Given how many DWARF parsers
we've had to cleanup or migrate off in
> >>> > >> >> >transitioning to DWARFv5 inside
Google, I think that's a fair bit of
> >>> > >> >> >evidence the set of parsers
isn't as narrow/closed as we'd like and
> >>> > >> >> >thus the number of places that
might have issues isn't known/easily
> >>> > >> >> >exhaustively tested. So I'm not
so concerned about the bugs we've
> >>> > >> >> >seen, but what that might indicate
about the things that we don't know
> >>> > >> >> >about and can't test (because
we don't know about them). (of course,
> >>> > >> >> >the flipside of that is that if we
don't know about them, we can't
> >>> > >> >> >tell people who own them to go
check if they work with this opt-in
> >>> > >> >> >feature - so they'll break
whenever we turn this on by default - but
> >>> > >> >> >perhaps in the interim we can get
at least a few big LLD customers to
> >>> > >> >> >deploy the feature and flush out
some of the issues - happy for Google
> >>> > >> >> >to use this internally, hopefully
we can encourage Apple folks, Sony
> >>> > >> >> >of course already has this semantic
so nothing to find there most
> >>> > >> >> >likely - Chromium, maybe Firefox,
etc)
> >>> > >> >>
> >>> > >> >> From the other side — when we already
switched new behavior ON as default —
> >>> > >> >> then it is easier to discover all these
unknown cases. We have an option restoring
> >>> > >> >> old behavior(which should be fine for
all of the current users). Thus, everybody
> >>> > >> >> who needs old behavior is able to
continue using it.
> >>> > >>
> >>> > >> >The cost to those users figuring out that
it's a problem and finding
> >>> > >> >the flags/adding them, etc, is non-zero.
> >>> > >> >But, yes, as I said - eventually some of
that will happen, sooner or
> >>> > >> >later. But reducing how much of it happens
is valuable too.
> >>> > >>
> >>> > >> I agree. Reducing the impact on customers is
important.
> >>> > >> Though in this case, as you said, it would be
done anyway(sooner or later).
> >>> > >> Finally, I do not know what is more important:
a) get feedback faster with
> >>> > >> the cost of involved users and requiring them to
make efforts
> >>> > >> to figure out the problem. or b) test with a
smaller set of users,
> >>> > >> find more problematic cases, make new tombstone
value to be default later.
> >>> > >> I am kind of preferring a). But, b) is more safe
and is OK.
> >>> > >>
> >>> > >>
> >>> > >> >> That makes transition more
understandable. All problems would be explicitly seen. Then,
> >>> > >> >> If we will know about new problems - we
could adapt the current solution. Similar to this >suggestion:
> >>> > >> >>
> >>> > >> >> a) .debug_ranges&.debug_loc =>
-2, .debug_line => 0, other .debug_* -> -1
> >>> > >>
> >>> > >> >The issue I have with this is that it's
reflective of the known
> >>> > >> >breakage in some tools that happen to have a
quick turnaround/feedback
> >>> > >> >- and assumes all the other sections are
significantly less likely to
> >>> > >> >have problems with this format - and I
don't think we have enough data
> >>> > >> >to make that estimate.
> >>> > >>
> >>> > >> We could extend that rule when/if new problems
become known.
> >>> > >> Or, customers could fix their code or use option
returning old behavior
> >>> > >> and it would probably not be necessary to extend
the rule.
> >>> > >>
> >>> > >> >I think having a clear flag for bfd behavior
or new behavior would be
> >>> > >> >good, keeping it bfd-by-default for now,
evangelizing (patch notes,
> >>> > >> >outreach to other projects and tool
developers) the new functionality
> >>> > >> >& then changing the default shortly
after an LLVM release (perhaps 12
> >>> > >> >- but perhaps longer) to give it time to
bake for trunk-following
> >>> > >> >users (ample time for them to pick it up on
their schedule, see what
> >>> > >> >other tools might break, whether they're
isolated enough to expect a
> >>> > >> >quick turnaround or whether we should get
them updated and then wait a
> >>> > >> >while longer.
> >>> > >>
> >>> > >> yes, doing clear flag for bfd behavior or new
behavior,
> >>> > >> and keep it bfd-by-default for now would be OK
and is more safe.
> >>> > >>
> >>> > >>
> >>> > >> >> This would help while dwarf users are
preparing their code to the new solution.
> >>> > >> >>
> >>> > >> >> Is option restoring old behavior not
enough to solve problems caused by new tombstone >value?
> >>> > >>
> >>> > >> >I don't personally think it is enough -
given some of the known
> >>> > >> >breakage - I think that points to a fair bit
of reasonably possible
> >>> > >> >unknown breakage. Having a few big (like
Apple and Google) users
> >>> > >> >switch over by default & potentially
flush out a few more issues (or
> >>> > >> >not -but build confidence that there
aren't more issues) I think is an
> >>> > >> >appropriate way to roll out a change like
this.
> >>> > >>
> >>> > >> >(minor doubt: I wonder how well the bfd
tombstoning works in DWARFv5
> >>> > >> >(rnglists/loclists) or in
debug_range/debug_loc that uses base address
> >>> > >> >specifiers, where the zero-without-addend
doesn't have a chance to
> >>> > >> >make an empty range (because it's not a
start/end pair, it's a
> >>> > >> >start+offset pair - so the offset remains
non-zero)... if bfd
> >>> > >> >tombstoning breaks DWARFv5 parsing (or base
address specifiers in
> >>> > >> >range/loc) in common consumers I might be
more inclined to support
> >>> > >> >enabling new tombstoning by default sooner
(though if we could enable
> >>> > >> >it just for DWARFv5 that might be nice - but
not practical, since the
> >>> > >> >linker doesn't know what DWARF it's
linking & could be linking
> >>> > >> >multiple versions))
> >>> > >>
> >>> > >> - From the point of view overlapping address
ranges,
> >>> > >> both bfd-solution(using 1) and old lld
solution(using 0) are equal.
> >>> >
> >>> > >gold and lld's solution was a bit more
complicated than using zero (&
> >>> > >bfd's solution uses 1 in debug_ranges, but not in
other debug
> >>> > >sections). lld and gold use 0+addend in ranges.
> >>> >
> >>> > right. I am talking about the same(though assumed that
context
> >>> > instead of detailed explanation). it should be
"bfd-solution(using 1
> >>> > for debug_ranges) and lld/gold solution(using
0+addend)".
> >>> > Thank you for pointing into that.
> >>> >
> >>> > >0 alone in ranges
> >>> > >would be more problematic than any of the other
deployed solutions,
> >>> > >because it would lead to the debug_ranges
contributions being
> >>> > >terminated early (0, 0 terminates the list - so
debug_ranges for a CU
> >>> > >with one gc'd function mid-way through the range
list would drop the
> >>> > >rest of the range lit) - that's why bfd uses 1
for debug_ranges rather
> >>> > >than 0 which it uses elsewhere (though the same fix
should,
> >>> > >technically, be applied to debug_loc too for the same
reasons as
> >>> > >debug_ranges). the 0+addend approach of gold/lld
works OK (except for
> >>> > >the low address overlap) except when there's a
zero-length function,
> >>> > >then 0+addend == 0 for the end of the range, andy ou
get a 0,0 entry
> >>> > >with the premature range list termination issues.
> >>> > >
> >>> > >> for the "start+offset pair" case they
could result in overlapping address ranges.
> >>> >
> >>> > >Not quite - the way bfd does things, at least in
debug_ranges without
> >>> > >base address specifiers (bit hard to isolate for a
consumer, really -
> >>> > >though they could special case zero base addresses...
sort of) the
> >>> > >empty ranges are (1, 1) - they're actually empty.
Whereas lld's
> >>> > >approach ends up with (0+addend, 0+addend) which is
usually (0, size)
> >>> > >which has issues with overlap on systems that have a
valid low address
> >>> > >range (or if you have sufficiently large functions).
> >>> > >
> >>> >
> >>> > There is a case when size of address range is set not by
relocated value but by constant value:
> >>> >
> >>> > DWARF5:
> >>> > DW_RLE_startx_length
> >>> > DW_RLE_start_length
> >>> >
> >>> > {address of deleted code, length}
> >>> > {address of existing code, length}
> >>> >
> >>> > DWARF4:
> >>> > base address selection entry: {-1, address of deleted
code}
> >>> > following range list entry: {0, length}
> >>> > base address selection entry: {-1, address of existing
code}
> >>> > following range list entry: {0, length}
> >>> >
> >>> > In these cases linker do not see the length of address
range
> >>> > and could not change it(using 1, or 0, or 0+addend) to be
zero length
> >>> > address range. Thus address ranges could overlap.
> >>> >
> >>> > So in both DWARF5 and DWARF4 there are cases when address
ranges
> >>> > could be overlapped and it could not be solved without
new meaning for tombstone value.
> >>> >
> >>> > >> - From the point of view correct parsing of
DWARF 5 - it looks like they are equally good.
> >>> > >
> >>> > >Yep, DWARFv5 is more complicated - bfd's approach
would be marginally
> >>> > >better even for addr+offset encodings in
debug_rnglists - since it'd
> >>> > >always produce a 0 for the addr, whereas
gold/lld's behavior can in
> >>> > >some cases (eg: "void f1() { }
__attribute__((nodebug)) void f2() { }
> >>> > >void f3() { }" with -fno-function-sections -
you'll end up with the
> >>> > >range for f3 starting at a non-zero addend - this
debug info will be
> >>> > >worse for bfd/ld (if the DWARF for this object was
included, but the
> >>> > >whole .text section was discarded (eg: maybe
there's a global variable
> >>> > >in this object which is linked in, but -gc-section is
still able to
> >>> > >discard the whole .text section as unreferenced) then
gold/lld's
> >>> > >approach would make f3 unidentifiable as dead code
(because it doesn't
> >>> > >have a tombstone start) but bfd's approach would
work (assuming zero
> >>> > >isn't a valid address))
> >>> >
> >>> > I see. thanks.
> >>> >
> >>> > >
> >>> > >> - From the point of view correct parsing of
DWARF 4 - it looks bfd-solution(using 1)
> >>> > >> is better than the old lld solution(using 0).
When range list entry contains
> >>> > >> addresses(start+end) which should be relocated
and for the zero-length functions,
> >>> > >> bfd-solution would result in range list entry:
{1, 1}, while old lld solution
> >>> > >> would result in {0, 0}, and match with the end
of list entry.
> >>> > >> That is the original problem that started this
thread.
> >>> > >
> >>> > >Only comes up for zero-length functions, because
gold/lld's approach
> >>> > >was 0+addend, not straight 0.
> >>> > >
> >>> >
> >>> > right. for zero-length functions.
> >>> >
> >>> > >> Though it looks like there still exist case when
range list could be terminated earlier:
> >>> > >>
> >>> > >> base address selection entry: {-1, address of
deleted code}
> >>> > >> following range list entry: {0, 0} <<
points to the same address as set by base address selection entry and has zero
size.
> >>> > >
> >>> > >That's a bug in the producer (though a good point
- I've probably made
> >>> > >that bug in LLVM) - the linker can't solve that
problem, since the
> >>> > >linker can't touch the literal unrelocated 0, 0.
> >>> > >
> >>> > >> after linker resolved relocations it would look
like this, for bfd case:
> >>> > >>
> >>> > >> base address selection entry: {-1, 1}
> >>> > >> following range list entry: {0, 0}
<<<<<<<<<<<
> >>> > >>
> >>> > >> So there still exists {0,0} entry which could be
considered as the end of list entry.
> >>> > >>
> >>> > >> But old lld solution has the same problem, thus
it would not be new.
> >>> > >>
> >>> > >> - Additionally, AFAIK gdb has special processing
for overlapped address
> >>> > >> ranges starting from 0. Using bfd tombstone
value could break that processing - I would check it.
> >>> > >
> >>> > >Not sure I understand - presumably gdb's special
processing is
> >>> > >intended to work with bfd's tombstoning, since
it's been the most
> >>> > >common/prolific unix linker, the one intended to work
with gdb (they
> >>> > >exist in the same repository) for decades, right?
> >>> >
> >>> > I think I misunderstood this: "I wonder how well the
bfd tombstoning works in DWARFv5
> >>> > (rnglists/loclists)". I read it as we would like to
use bfd tombstoning(1 for ranges)
> >>> > for rnglists/loclists also.
> >>> >
> >>> > So, right. bfd's tombstoning works correctly with gdb
until 1 is not used
> >>> > for rnglists/loclists as tombstone value.
> >>>
> >>> Not quite parsing this, but I think we're on the same page
- that
> >>> bfd's tombstoning "1 for debug_ranges/debug_loc, 0
(not 0+addend, but
> >>> absolute 0) for everything else (including debug_loclists and
> >>> debug_rnglists)" is probably the most likely to work for
gdb, since
> >>> it's been deployed for a long time.
> >>>
> >>> - Dave
> >>>
> >>> PS: Fair point about base address specifiers being able to
produce 0,0
> >>> entries - wouldn't mind fixing that in LLVM if I knew of
an easy way
> >>> to test at compile-time whether the difference between two
labels was
> >>> zero, then skip that entry in the lists entirely. Would save
space and
> >>> address the original issue I had with debug_ranges terminating
early.
> >>> (should've thought about that much earlier than my more
alarmist "oh
> >>> deer, we need to fundamentally change how linkers resolve
relocations
> >>> because everything's been broken and we just didn't
realize it" -
> >>> fixing the compiler not to produce zero-length ranges
would've been
> >>> less risky & probably still worth doing - though
addressing the
> >>> broader issue to help with your situation of 0 as a valid
address I
> >>> think is still good too)
> >>>
> >>> >
> >>> > Alexey.
> >>> >
> >>> > > > >> With consideration for satefy for the
upcoming release/11.x, we can make
> >>> > > > >> two choices:
> >>> > > > >>
> >>> > > > >> a) .debug_ranges&.debug_loc =>
-2, .debug_line => 0, other .debug_* -> -1
> >>> > > > >> b) .debug_ranges&.debug_loc =>
-2, other .debug_* => 0
> >>> > > > >>
> >>> > > > >> Delaying .debug_line => -1 for one
or two release sounds good to me.
> >>> > > > >> So LLD 11 or 12 linked binaries can be
debugged by LLDB 10. This is a
> >>> > > > >> nice property.
> >>> > > > >>
> >>> > > > >> This write-up proposes b), but I'd
say a) is likely sufficient. With the
> >>> > > > >> available information, I cannot yet
say that a) will have more risk.
> >>> > > > >
> >>> > > > >Risk is about the unknowns - and it still
seems like a lot of
> >>> > > > >unknowns. While there are probably many
more consumers that read
> >>> > > > >.debug_line than other sections, reading
debug_info (for instance) is
> >>> > > > >necessary for inline frames in symbolizing
- still probably one of the
> >>> > > > >most common uses of DWARF I'd guess.
(what about stack unwinding using
> >>> > > > >debug_frame? that'd worry me a bit if
anyone got /that/ wrong because
> >>> > > > >of this change)
> >>> > > > >
> >>> > > > >> > - chromium/firefox have some
tools that were broken:
> >>> > > > >> >
https://bugs.chromium.org/p/chromium/issues/detail?id=1102223#c5<https://urldefense.com/v3/__https:/bugs.chromium.org/p/chromium/issues/detail?id=1102223*c5__;Iw!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWa5dfJfdQ$>
> >>> > > > >>
> >>> > > > >> This is potentially related to other
.debug_* (not .debug_line)
> >>> > > > >> I hope Chromium developers can chime
in here:) The breakage was
> >>> > > > >> unfortunate but I don't know how
we could have avoided that. IMHO this
> >>> > > > >> is no different from "clang
started to emit a new DW_FORM_* and a
> >>> > > > >> postprocessing tool of .debug chokes
on that" Whether we want to
> >>> > > > >> suppress that particular DW_FORM_*
definitely should depend on how
> >>> > > > >> likely it can cause problems, but we
can't yet say we have to hold off
> >>> > > > >> on a feature for a solved (precisely,
mitigated) problem.
> >>> > > > >
> >>> > > > >LLVM has no custom forms and I'd be
super cautious about adding any
> >>> > > > >that were on by default because of how bad
that breakage would be.
> >>> > > > >
> >>> > > > >I'm not so concerned about the problems
we know - but what they tell
> >>> > > > >us about the problems that might arise from
use cases we don't know.
> >>> > > > >All the other projects out there that might
have custom DWARF parsers
> >>> > > > >to do some ad-hoc things.
> >>> > > > >
> >>> > > > >(also, ultimately - given how far-reaching
this is, I think we'll want
> >>> > > > >some tidier flags that are more
user-focussed. I'd hope for a flag
> >>> > > > >that gives BFD-like semantics (though
I'd be OK with fixing debug_loc
> >>> > > > >(using 1 instead of 0) to work the same as
debug_ranges while we're
> >>> > > > >there - a minor divergence from BFD, but
highly likely to not cause
> >>> > > > >problems/fall out naturally from a simple
implementation of parsing
> >>> > > > >that section) - something that's been
in-use and tested by basically
> >>> > > > >everyone for decades. And another flag for
the new semantics (-2 for
> >>> > > > >debug_loc/debug_ranges, -1 everywhere
else). Customizable per-section
> >>> > > > >expression-based support I think is a
recipe for platform divergence &
> >>> > > > >I'd rather it not be
available/supported at all, but if you really
> >>> > > > >want to keep it in, I'd at least rather
it not be the feature we
> >>> > > > >promote to users about how they can
test/opt in/out of the behavior
> >>> > > > >when they're seeing breakages or want
to test the future semantics)
> >>> > > > >
> >>> > > > >> >I'm not sure how to get the
word out to DWARF consumers that they should
> >>> > > > >> >consider this new experimental
behavior. Ray's done a good job
> >>> > > > >> >evangelizing/discussing this with
gdb and lldb at least - and of course
> >>> > > > >> >having turned it on by default
briefly has found some users (like Chromium)
> >>> > > > >> >that we probably wouldn't have
found no matter how long we left this as an
> >>> > > > >> >experimental option... so some
things are going to break when we switch no
> >>> > > > >> >matter what.
> >>> > > > >>
> >>> > > > >> Thank you for following up with some
GNU folks on their lists!
> >>> > > > >> If folks want to follow along the
thread:
> >>> > > > >>
https://sourceware.org/pipermail/binutils/2020-June/111376.html<https://urldefense.com/v3/__https:/sourceware.org/pipermail/binutils/2020-June/111376.html__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWbtQobFaQ$>
> >>> > > > >>
> >>> > > > >> We have informed binutils,
elfutils-devel (elfutils has a few debug
> >>> > > > >> tools) and gdb. I don't recall
that anyone has thought about problems
> >>> > > > >> with a tombstone value.
> >>> > > > >>
> >>> > > > >> >
> >>> > > > >> >P.S: Sony's already been using
the -1 technique with their debugger and
> >>> > > > >> >linker for a while, so they may
want to keep this on by default for SCE -
> >>> > > > >> >but I'm not sure how to do
that in-tree.
> >>> > > > >> >
> >>> > > > >> >
> >>> > > > >> >Clang doesn't know which lld
> >>> > > > >> >version it's running, so
whether the flag can be specified, I would think?
> >>> > > > >> >(so it'd be hard to have Clang
go "if SCE and LLD, pass the flag to use
> >>> > > > >> >-1", I think) - if there is a
way to make that decision in the compiler
> >>> > > > >.> >driver+linker, then we'd have
a question of "default new behavior except
> >>> > > > >> >when tuning for LLDB and GDB"
or "default bfd behavior except when tuning
> >>> > > > >> >for SCE".
> >>> > > > >>
> >>> > > > >> I've been involed in another
thread on SHF_LINK_ORDER
(https://sourceware.org/pipermail/binutils/2020-<https://urldefense.com/v3/__https:/sourceware.org/pipermail/binutils/2020-__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWbslmMhbQ$>
> >>> > > > >July/112415.html ).
> >>> > > > >> We may need a way to tell codegen
about the used linker.
> >>> > > > >>
> >>> > > > >> pcc proposed -mbinutils-version= -
This is nice in that some MC
> >>> > > > >> decisions related to
-fno-integrated-as can use this option as well.
> >>> > > > >> jyknight proposed -mlinker-version=
and syntax like -fuse-ld=bfd:2.34
> >>> > > > >>
> >>> > > > >> This may get more complex if the
generated object file want to be linked
> >>> > > > >> with more than one linker. This
discussion probably deserves its own
> >>> > > > >> thread.
> >>> > > >
> >>> > > >
> >>> > > > --
> >>> > > > Alexey Lapshin
> >>> > > >
> >>> > > _______________________________________________
> >>> > > LLVM Developers mailing list
> >>> > > llvm-dev at lists.llvm.org<mailto:llvm-dev at
lists.llvm.org>
> >>> > >
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev<https://urldefense.com/v3/__https:/lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWZikxq4BA$>
> >>> _______________________________________________
> >>> LLVM Developers mailing list
> >>> llvm-dev at lists.llvm.org<mailto:llvm-dev at
lists.llvm.org>
> >>>
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev<https://urldefense.com/v3/__https:/lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWZikxq4BA$>
> >>
> >> _______________________________________________
> >> LLVM Developers mailing list
> >> llvm-dev at lists.llvm.org<mailto:llvm-dev at
lists.llvm.org>
> >>
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev<https://urldefense.com/v3/__https:/lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWZikxq4BA$>
> >_______________________________________________
> >LLVM Developers mailing list
> >llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
>
>https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev<https://urldefense.com/v3/__https:/lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWZikxq4BA$>

--
宋方睿
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200727/0c757ffa/attachment-0001.html>

On Mon, Jul 27, 2020 at 9:11 AM Robinson, Paul <paul.robinson at sony.com>
wrote:
> > I still think that we do bfd locs with a decent option to change for
at
> least the current release and sources and then, once we're a little
more
> certain we have everything that might want to parse dwarf (say by working
> with dwarf-discuss), we can change the default.
>
>
>
> Given what’s been found, I think Eric/Dave are correct, use bfd behavior
> by default with an option to do the new thing.  The option can be coded to
> let Sony (SCE debugger tuning) have it on by default.
>
>
>
> Bringing it up on dwarf-discuss seems like a good idea too, you can refer
> to http://dwarfstd.org/ShowIssue.php?issue=200609.1 as part of that.
>
Thanks for the pointer! Posted here:
http://lists.dwarfstd.org/pipermail/dwarf-discuss-dwarfstd.org/2020-July/004683.html

> --paulr
>
>
>
> *From:* Eric Christopher <echristo at gmail.com>
> *Sent:* Saturday, July 25, 2020 12:56 AM
> *To:* Fāng-ruì Sòng <maskray at google.com>
> *Cc:* Hans Wennborg <hans at chromium.org>; llvm-dev at
lists.llvm.org; Alexey
> Lapshin <a.v.lapshin at mail.ru>; George Rimar <grimar at
accesssoftek.com>;
> Robinson, Paul <paul.robinson at sony.com>; Adrian Prantl <aprantl
at apple.com>;
> Jonas Devlieghere <jdevlieghere at apple.com>; James Henderson <
> jh7370.2008 at my.bristol.ac.uk>; Peter Smith <Peter.Smith at
arm.com>; David
> Blaikie <dblaikie at gmail.com>; Pavel Labath <pavel at
labath.sk>
> *Subject:* Re: [llvm-dev] Switch to ld.bfd tombstone behavior by default
>
>
>
> Hi Ray :)
>
>
>
> While I understand the desire to say "we've updated everything
we've
> found" the problem is that we don't know that we've found even
much of the
> uses or can guarantee that people can upgrade, say their gdb or breakpad,
> as fast as their compiler. Even worse it's a change in behavior without
> much notice at all or even a particularly friendly way to recognize what
> they should do to fix it.
>
>
>
> I still think that we do bfd locs with a decent option to change for at
> least the current release and sources and then, once we're a little
more
> certain we have everything that might want to parse dwarf (say by working
> with dwarf-discuss), we can change the default.
>
>
>
> -eric
>
>
>
> On Fri, Jul 24, 2020 at 7:33 PM Fāng-ruì Sòng <maskray at google.com>
wrote:
>
> From my understanding the breakpad bug was also only related to
> .debug_line and has been fixed by
> https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2317730
>
<https://urldefense.com/v3/__https:/chromium-review.googlesource.com/c/breakpad/breakpad/*/2317730__;Kw!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWYCu7N-Fg$>
>
>
>
> > a) .debug_ranges&.debug_loc => -2, .debug_line => 0, other
.debug_* -> -1
> > b) .debug_ranges&.debug_loc => -2, other .debug_* => 0
>
>
>
> I am still of the opinion that we should just do a), not b).
>
>
>
>
>
> On Fri, Jul 24, 2020 at 9:33 AM Hans Wennborg <hans at chromium.org>
wrote:
>
> On Fri, Jul 24, 2020 at 5:12 PM Fangrui Song <maskray at google.com>
wrote:
> >
> > On 2020-07-24, Hans Wennborg via llvm-dev wrote:
> > >Sounds good to me from a release perspective.
> >
> > I think we need more input from the triage of
> > https://chromium-review.googlesource.com/c/chromium/src/+/2291352
>
<https://urldefense.com/v3/__https:/chromium-review.googlesource.com/c/chromium/src/*/2291352__;Kw!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWb5tE7cMg$>
> > whether it is just .debug_line or .debug_*
>
> The issue we hit in Chromium is tracked here:
> https://bugs.chromium.org/p/chromium/issues/detail?id=1105559
>
<https://urldefense.com/v3/__https:/bugs.chromium.org/p/chromium/issues/detail?id=1105559__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWanrbB9Cg$>
> Doesn't
> look like it has any more info at the moment.
>
> >
> > >On Fri, Jul 24, 2020 at 7:53 AM Eric Christopher via llvm-dev
> > ><llvm-dev at lists.llvm.org> wrote:
> > >>
> > >> Hi All,
> > >>
> > >> In general I think we should adopt Dave's plan here. The
number of
> consumers that can (and have) been caught off guard by this change is just
> too high.
> > >>
> > >> At the very least I think we should move this to opt in to
the new
> tombstoning behavior by default and at most migrate to bfd's behavior
for
> both the current release and in the current tree. If we want to make this
> sort of change in the future by default I think we're going to need to
> provide release notes about this and do aggressive outreach towards the
> consumers we do know before making the change. If anyone wants to drive
> that effort I'll happily provide any help or assistance in getting you
in
> touch with at least the consumers I know about.
> > >>
> > >> This change and the need for it is also probably worth a
quick
> message to dwarf-discuss at dwarfstd.org as well. Most of the major
> consumers and producers are on that list and it's probably one of the
> easiest ways to get this change out there.
> > >>
> > >> Any strong objections to this path in the meantime?
> > >>
> > >> Thanks!
> > >>
> > >> -eric
> > >>
> > >> On Tue, Jul 21, 2020 at 12:18 PM David Blaikie via llvm-dev
<
> llvm-dev at lists.llvm.org> wrote:
> > >>>
> > >>> On Tue, Jul 21, 2020 at 5:34 AM Alexey Lapshin
> > >>> <alapshin at accesssoftek.com> wrote:
> > >>> >
> > >>> >
> > >>> > >On Mon, Jul 20, 2020 at 10:32 AM Alexey Lapshin
> > >>> > ><alapshin at accesssoftek.com> wrote:
> > >>> > >>
> > >>> > >> >On Fri, Jul 17, 2020 at 1:55 PM Alexey
Lapshin <
> a.v.lapshin at mail.ru> wrote:
> > >>> > >> >>
> > >>> > >> >> >Пятница, 17 июля 2020, 19:42
+03:00 от David Blaikie <
> dblaikie at gmail.com>:
> > >>> > >> >> >
> > >>> > >> >> >On Fri, Jul 17, 2020 at 12:03
AM Fangrui Song <
> maskray at google.com> wrote:
> > >>> > >> >> >>
> > >>> > >> >> >> Thanks for the write-up!
> > >>> > >> >> >>
> > >>> > >> >> >> On 2020-07-16, David
Blaikie wrote:
> > >>> > >> >> >> >In short: Perhaps we
should switch lld to the bfd-style
> tombstoning
> > >>> > >> >> >> >behavior for a
release or two, letting users opt-in to
> testing with the new
> > >>> > >> >> >> >-1/-2 tombstoning in
the interim, before switching to
> the new tombstone by
> > >>> > >> >> >> >default (while still
having the flag to switch back when
> users find
> > >>> > >> >> >> >surprise places that
can't handle the new behavior).
> > >>> > >> >> >> >
> > >>> > >> >> >> >In long:
> > >>> > >> >> >>
>https://reviews.llvm.org/D81784
>
<https://urldefense.com/v3/__https:/reviews.llvm.org/D81784__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWYFUjv39g$>
> and follow-on patches modified the behavior
> > >>> > >> >> >> >of lld with regards
to resolving relocations from debug
> sections to dead
> > >>> > >> >> >> >code (either comdat
deduplicated, or gc-sections use).
> > >>> > >> >> >> >
> > >>> > >> >> >> >A very quick summary
of the situation:
> > >>> > >> >> >> >
> > >>> > >> >> >> >Original Behavior:
> > >>> > >> >> >> >
> > >>> > >> >> >> > - bfd: 1 for
debug_ranges(0 would prematurely terminate
> the list), 0
> > >>> > >> >> >> > elsewhere
> > >>> > >> >> >> > - gold/lld: 0+addend
everywhere
> > >>> > >> >> >> >
> > >>> > >> >> >> >Limitations/bugs:
> > >>> > >> >> >> >
> > >>> > >> >> >> > - bfd/gold/lld
> > >>> > >> >> >> > - doesn't
support 0 as a valid executable address
> without ambiguities
> > >>> > >> >> >> > - gold/lld
> > >>> > >> >> >> > - ambiguities with
large gc'd functions combined with a
> .text mapping
> > >>> > >> >> >> > that starts in
relative low addresses
> > >>> > >> >> >> > - premature
debug_range termination with zero-length
> functions (Clang
> > >>> > >> >> >> > produces these with
__builtin_unreachable or non-void
> return
> > >>> > >> >> >> >type functions
> > >>> > >> >> >> > without a return
statement)
> > >>> > >> >> >> >
> > >>> > >> >> >> >New behavior:
> > >>> > >> >> >> >
> > >>> > >> >> >> > - -2 for DWARFv4
debug_loc, debug_ranges (-1 is a base
> address specifier
> > >>> > >> >> >> > there)
> > >>> > >> >> >> > - -1 elsewhere
> > >>> > >> >> >> > - linker flag to
customize to other values if desired
> > >>> > >> >> >> >
> > >>> > >> >> >> >Known issues:
> > >>> > >> >> >> >
> > >>> > >> >> >> > - lldb's line
table parsing can't handle -1 well at all
> (essentially
> > >>> > >> >> >> > unusable)
> > >>> > >> >> >>
> > >>> > >> >> >> Pavel Labath will fix
this soon
> https://reviews.llvm.org/D83957
>
<https://urldefense.com/v3/__https:/reviews.llvm.org/D83957__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWZM6TUIWQ$>
> > >>> > >> >> >> This is an unhandled
address-space wraparound problem.
> > >>> > >> >> >> This pattern is
potentially common - and other downstream
> DWARF
> > >>> > >> >> >> consumers might make
similar line table handling mistakes.
> > >>> > >> >> >
> > >>> > >> >> >That's the thing - I'm
not sure we can really classify them
> as
> > >>> > >> >> >"mistakes". I think
bfd.ld's tombstoning behavior is about
> the only
> > >>> > >> >> >thing we can reasonably say
DWARF consumers should
> /probably/ be
> > >>> > >> >> >expected to handle - and
I'd imagine in many cases they
> haven't been
> > >>> > >> >> >written intentionally to
handle it, but whatever behavior
> they have
> > >>> > >> >> >has accidentally been
sufficient for their needs.
> > >>> > >> >>
> > >>> > >> >> >> > - gdb's line
table parsing ends up with different
> handling when breaking
> > >>> > >> >> >> > on gc'd
functions (minor functionality issue)
> > >>> > >> >> >>
> > >>> > >> >> >> This is just a behavior
difference, not affecting users.
> > >>> > >> >> >> It did break a test if
linked with LLD (gdb intrinsically
> has lots of
> > >>> > >> >> >> failing tests even if
built with GCC+GNU ld).
> > >>> > >> >> >>
> > >>> > >> >> >> Previous behavior (when
an address is zero): a breakpoint
> on a
> > >>> > >> >> >> --gc-sections discarded
function will be redirected to a
> larger line
> > >>> > >> >> >> number with debug info,
even if that line can be an
> unrelated different
> > >>> > >> >> >> function.
> > >>> > >> >> >> New behavior is that the
breakpoint is on a
> wrapped-around small address.
> > >>> > >> >> >>
> > >>> > >> >> >> GDB 9.3 will restore the
previous behavior
> > >>> > >> >> >> (
>
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=a8caed5d7faa639a1e6769eba551d15d8ddd9510
>
<https://urldefense.com/v3/__https:/sourceware.org/git/?p=binutils-gdb.git;a=commit;h=a8caed5d7faa639a1e6769eba551d15d8ddd9510__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWZQ1XTxqw$>
> )
> > >>> > >> >> >>
> > >>> > >> >> >> >
> > >>> > >> >> >> >I think there's
enough risk in this work (even given the
> small number of
> > >>> > >> >> >> >bugs found so far),
given there's a pretty wide array of
> debug info
> > >>> > >> >> >> >consumers out there,
that we should change lld's default
> to match the
> > >>> > >> >> >> >long-lived bfd
strategy. This would address my original
> motivation for
> > >>> > >> >> >> >raising all this
(empty functions prematurely
> terminating the list), while
> > >>> > >> >> >> >letting users who
want to experiment with it, or need it
> (like Alexey), can
> > >>> > >> >> >> >opt-in to the -1/-2
behavior.
> > >>> > >> >> >>
> > >>> > >> >> >> I think we can only
confidently say that there is enough
> risk in using
> > >>> > >> >> >> tombstone value -1 in
.debug_line, but I'd not say
> tombstone value -1 in
> > >>> > >> >> >> other .debug_* can cause
problems.
> > >>> > >> >>
> > >>> > >> >> >Given how many DWARF parsers
we've had to cleanup or
> migrate off in
> > >>> > >> >> >transitioning to DWARFv5
inside Google, I think that's a
> fair bit of
> > >>> > >> >> >evidence the set of parsers
isn't as narrow/closed as we'd
> like and
> > >>> > >> >> >thus the number of places that
might have issues isn't
> known/easily
> > >>> > >> >> >exhaustively tested. So
I'm not so concerned about the bugs
> we've
> > >>> > >> >> >seen, but what that might
indicate about the things that we
> don't know
> > >>> > >> >> >about and can't test
(because we don't know about them).
> (of course,
> > >>> > >> >> >the flipside of that is that
if we don't know about them,
> we can't
> > >>> > >> >> >tell people who own them to go
check if they work with this
> opt-in
> > >>> > >> >> >feature - so they'll break
whenever we turn this on by
> default - but
> > >>> > >> >> >perhaps in the interim we can
get at least a few big LLD
> customers to
> > >>> > >> >> >deploy the feature and flush
out some of the issues - happy
> for Google
> > >>> > >> >> >to use this internally,
hopefully we can encourage Apple
> folks, Sony
> > >>> > >> >> >of course already has this
semantic so nothing to find
> there most
> > >>> > >> >> >likely - Chromium, maybe
Firefox, etc)
> > >>> > >> >>
> > >>> > >> >> From the other side — when we
already switched new behavior
> ON as default —
> > >>> > >> >> then it is easier to discover all
these unknown cases. We
> have an option restoring
> > >>> > >> >> old behavior(which should be fine
for all of the current
> users). Thus, everybody
> > >>> > >> >> who needs old behavior is able to
continue using it.
> > >>> > >>
> > >>> > >> >The cost to those users figuring out
that it's a problem and
> finding
> > >>> > >> >the flags/adding them, etc, is
non-zero.
> > >>> > >> >But, yes, as I said - eventually some
of that will happen,
> sooner or
> > >>> > >> >later. But reducing how much of it
happens is valuable too.
> > >>> > >>
> > >>> > >> I agree. Reducing the impact on customers
is important.
> > >>> > >> Though in this case, as you said, it would
be done
> anyway(sooner or later).
> > >>> > >> Finally, I do not know what is more
important: a) get feedback
> faster with
> > >>> > >> the cost of involved users and requiring
them to make efforts
> > >>> > >> to figure out the problem. or b) test with
a smaller set of
> users,
> > >>> > >> find more problematic cases, make new
tombstone value to be
> default later.
> > >>> > >> I am kind of preferring a). But, b) is more
safe and is OK.
> > >>> > >>
> > >>> > >>
> > >>> > >> >> That makes transition more
understandable. All problems
> would be explicitly seen. Then,
> > >>> > >> >> If we will know about new problems
- we could adapt the
> current solution. Similar to this >suggestion:
> > >>> > >> >>
> > >>> > >> >> a) .debug_ranges&.debug_loc
=> -2, .debug_line => 0, other
> .debug_* -> -1
> > >>> > >>
> > >>> > >> >The issue I have with this is that
it's reflective of the known
> > >>> > >> >breakage in some tools that happen to
have a quick
> turnaround/feedback
> > >>> > >> >- and assumes all the other sections
are significantly less
> likely to
> > >>> > >> >have problems with this format - and I
don't think we have
> enough data
> > >>> > >> >to make that estimate.
> > >>> > >>
> > >>> > >> We could extend that rule when/if new
problems become known.
> > >>> > >> Or, customers could fix their code or use
option returning old
> behavior
> > >>> > >> and it would probably not be necessary to
extend the rule.
> > >>> > >>
> > >>> > >> >I think having a clear flag for bfd
behavior or new behavior
> would be
> > >>> > >> >good, keeping it bfd-by-default for
now, evangelizing (patch
> notes,
> > >>> > >> >outreach to other projects and tool
developers) the new
> functionality
> > >>> > >> >& then changing the default shortly
after an LLVM release
> (perhaps 12
> > >>> > >> >- but perhaps longer) to give it time
to bake for
> trunk-following
> > >>> > >> >users (ample time for them to pick it
up on their schedule,
> see what
> > >>> > >> >other tools might break, whether
they're isolated enough to
> expect a
> > >>> > >> >quick turnaround or whether we should
get them updated and
> then wait a
> > >>> > >> >while longer.
> > >>> > >>
> > >>> > >> yes, doing clear flag for bfd behavior or
new behavior,
> > >>> > >> and keep it bfd-by-default for now would be
OK and is more safe.
> > >>> > >>
> > >>> > >>
> > >>> > >> >> This would help while dwarf users
are preparing their code
> to the new solution.
> > >>> > >> >>
> > >>> > >> >> Is option restoring old behavior
not enough to solve
> problems caused by new tombstone >value?
> > >>> > >>
> > >>> > >> >I don't personally think it is
enough - given some of the known
> > >>> > >> >breakage - I think that points to a
fair bit of reasonably
> possible
> > >>> > >> >unknown breakage. Having a few big
(like Apple and Google)
> users
> > >>> > >> >switch over by default &
potentially flush out a few more
> issues (or
> > >>> > >> >not -but build confidence that there
aren't more issues) I
> think is an
> > >>> > >> >appropriate way to roll out a change
like this.
> > >>> > >>
> > >>> > >> >(minor doubt: I wonder how well the bfd
tombstoning works in
> DWARFv5
> > >>> > >> >(rnglists/loclists) or in
debug_range/debug_loc that uses base
> address
> > >>> > >> >specifiers, where the
zero-without-addend doesn't have a
> chance to
> > >>> > >> >make an empty range (because it's
not a start/end pair, it's a
> > >>> > >> >start+offset pair - so the offset
remains non-zero)... if bfd
> > >>> > >> >tombstoning breaks DWARFv5 parsing (or
base address specifiers
> in
> > >>> > >> >range/loc) in common consumers I might
be more inclined to
> support
> > >>> > >> >enabling new tombstoning by default
sooner (though if we could
> enable
> > >>> > >> >it just for DWARFv5 that might be nice
- but not practical,
> since the
> > >>> > >> >linker doesn't know what DWARF
it's linking & could be linking
> > >>> > >> >multiple versions))
> > >>> > >>
> > >>> > >> - From the point of view overlapping
address ranges,
> > >>> > >> both bfd-solution(using 1) and old lld
solution(using 0) are
> equal.
> > >>> >
> > >>> > >gold and lld's solution was a bit more
complicated than using
> zero (&
> > >>> > >bfd's solution uses 1 in debug_ranges, but
not in other debug
> > >>> > >sections). lld and gold use 0+addend in ranges.
> > >>> >
> > >>> > right. I am talking about the same(though assumed
that context
> > >>> > instead of detailed explanation). it should be
"bfd-solution(using
> 1
> > >>> > for debug_ranges) and lld/gold solution(using
0+addend)".
> > >>> > Thank you for pointing into that.
> > >>> >
> > >>> > >0 alone in ranges
> > >>> > >would be more problematic than any of the other
deployed
> solutions,
> > >>> > >because it would lead to the debug_ranges
contributions being
> > >>> > >terminated early (0, 0 terminates the list - so
debug_ranges for
> a CU
> > >>> > >with one gc'd function mid-way through the
range list would drop
> the
> > >>> > >rest of the range lit) - that's why bfd uses
1 for debug_ranges
> rather
> > >>> > >than 0 which it uses elsewhere (though the same
fix should,
> > >>> > >technically, be applied to debug_loc too for the
same reasons as
> > >>> > >debug_ranges). the 0+addend approach of gold/lld
works OK (except
> for
> > >>> > >the low address overlap) except when there's
a zero-length
> function,
> > >>> > >then 0+addend == 0 for the end of the range,
andy ou get a 0,0
> entry
> > >>> > >with the premature range list termination
issues.
> > >>> > >
> > >>> > >> for the "start+offset pair" case
they could result in
> overlapping address ranges.
> > >>> >
> > >>> > >Not quite - the way bfd does things, at least in
debug_ranges
> without
> > >>> > >base address specifiers (bit hard to isolate for
a consumer,
> really -
> > >>> > >though they could special case zero base
addresses... sort of) the
> > >>> > >empty ranges are (1, 1) - they're actually
empty. Whereas lld's
> > >>> > >approach ends up with (0+addend, 0+addend) which
is usually (0,
> size)
> > >>> > >which has issues with overlap on systems that
have a valid low
> address
> > >>> > >range (or if you have sufficiently large
functions).
> > >>> > >
> > >>> >
> > >>> > There is a case when size of address range is set
not by relocated
> value but by constant value:
> > >>> >
> > >>> > DWARF5:
> > >>> > DW_RLE_startx_length
> > >>> > DW_RLE_start_length
> > >>> >
> > >>> > {address of deleted code, length}
> > >>> > {address of existing code, length}
> > >>> >
> > >>> > DWARF4:
> > >>> > base address selection entry: {-1, address of
deleted code}
> > >>> > following range list entry: {0, length}
> > >>> > base address selection entry: {-1, address of
existing code}
> > >>> > following range list entry: {0, length}
> > >>> >
> > >>> > In these cases linker do not see the length of
address range
> > >>> > and could not change it(using 1, or 0, or 0+addend)
to be zero
> length
> > >>> > address range. Thus address ranges could overlap.
> > >>> >
> > >>> > So in both DWARF5 and DWARF4 there are cases when
address ranges
> > >>> > could be overlapped and it could not be solved
without new meaning
> for tombstone value.
> > >>> >
> > >>> > >> - From the point of view correct parsing of
DWARF 5 - it looks
> like they are equally good.
> > >>> > >
> > >>> > >Yep, DWARFv5 is more complicated - bfd's
approach would be
> marginally
> > >>> > >better even for addr+offset encodings in
debug_rnglists - since
> it'd
> > >>> > >always produce a 0 for the addr, whereas
gold/lld's behavior can
> in
> > >>> > >some cases (eg: "void f1() { }
__attribute__((nodebug)) void f2()
> { }
> > >>> > >void f3() { }" with -fno-function-sections
- you'll end up with
> the
> > >>> > >range for f3 starting at a non-zero addend -
this debug info will
> be
> > >>> > >worse for bfd/ld (if the DWARF for this object
was included, but
> the
> > >>> > >whole .text section was discarded (eg: maybe
there's a global
> variable
> > >>> > >in this object which is linked in, but
-gc-section is still able
> to
> > >>> > >discard the whole .text section as unreferenced)
then gold/lld's
> > >>> > >approach would make f3 unidentifiable as dead
code (because it
> doesn't
> > >>> > >have a tombstone start) but bfd's approach
would work (assuming
> zero
> > >>> > >isn't a valid address))
> > >>> >
> > >>> > I see. thanks.
> > >>> >
> > >>> > >
> > >>> > >> - From the point of view correct parsing of
DWARF 4 - it looks
> bfd-solution(using 1)
> > >>> > >> is better than the old lld solution(using
0). When range list
> entry contains
> > >>> > >> addresses(start+end) which should be
relocated and for the
> zero-length functions,
> > >>> > >> bfd-solution would result in range list
entry: {1, 1}, while
> old lld solution
> > >>> > >> would result in {0, 0}, and match with the
end of list entry.
> > >>> > >> That is the original problem that started
this thread.
> > >>> > >
> > >>> > >Only comes up for zero-length functions, because
gold/lld's
> approach
> > >>> > >was 0+addend, not straight 0.
> > >>> > >
> > >>> >
> > >>> > right. for zero-length functions.
> > >>> >
> > >>> > >> Though it looks like there still exist case
when range list
> could be terminated earlier:
> > >>> > >>
> > >>> > >> base address selection entry: {-1, address
of deleted code}
> > >>> > >> following range list entry: {0, 0} <<
points to the same
> address as set by base address selection entry and has zero size.
> > >>> > >
> > >>> > >That's a bug in the producer (though a good
point - I've probably
> made
> > >>> > >that bug in LLVM) - the linker can't solve
that problem, since the
> > >>> > >linker can't touch the literal unrelocated
0, 0.
> > >>> > >
> > >>> > >> after linker resolved relocations it would
look like this, for
> bfd case:
> > >>> > >>
> > >>> > >> base address selection entry: {-1, 1}
> > >>> > >> following range list entry: {0, 0}
<<<<<<<<<<<
> > >>> > >>
> > >>> > >> So there still exists {0,0} entry which
could be considered as
> the end of list entry.
> > >>> > >>
> > >>> > >> But old lld solution has the same problem,
thus it would not be
> new.
> > >>> > >>
> > >>> > >> - Additionally, AFAIK gdb has special
processing for overlapped
> address
> > >>> > >> ranges starting from 0. Using bfd tombstone
value could break
> that processing - I would check it.
> > >>> > >
> > >>> > >Not sure I understand - presumably gdb's
special processing is
> > >>> > >intended to work with bfd's tombstoning,
since it's been the most
> > >>> > >common/prolific unix linker, the one intended to
work with gdb
> (they
> > >>> > >exist in the same repository) for decades,
right?
> > >>> >
> > >>> > I think I misunderstood this: "I wonder how
well the bfd
> tombstoning works in DWARFv5
> > >>> > (rnglists/loclists)". I read it as we would
like to use bfd
> tombstoning(1 for ranges)
> > >>> > for rnglists/loclists also.
> > >>> >
> > >>> > So, right. bfd's tombstoning works correctly
with gdb until 1 is
> not used
> > >>> > for rnglists/loclists as tombstone value.
> > >>>
> > >>> Not quite parsing this, but I think we're on the same
page - that
> > >>> bfd's tombstoning "1 for debug_ranges/debug_loc,
0 (not 0+addend, but
> > >>> absolute 0) for everything else (including debug_loclists
and
> > >>> debug_rnglists)" is probably the most likely to work
for gdb, since
> > >>> it's been deployed for a long time.
> > >>>
> > >>> - Dave
> > >>>
> > >>> PS: Fair point about base address specifiers being able
to produce
> 0,0
> > >>> entries - wouldn't mind fixing that in LLVM if I knew
of an easy way
> > >>> to test at compile-time whether the difference between
two labels was
> > >>> zero, then skip that entry in the lists entirely. Would
save space
> and
> > >>> address the original issue I had with debug_ranges
terminating early.
> > >>> (should've thought about that much earlier than my
more alarmist "oh
> > >>> deer, we need to fundamentally change how linkers resolve
relocations
> > >>> because everything's been broken and we just
didn't realize it" -
> > >>> fixing the compiler not to produce zero-length ranges
would've been
> > >>> less risky & probably still worth doing - though
addressing the
> > >>> broader issue to help with your situation of 0 as a valid
address I
> > >>> think is still good too)
> > >>>
> > >>> >
> > >>> > Alexey.
> > >>> >
> > >>> > > > >> With consideration for satefy for
the upcoming
> release/11.x, we can make
> > >>> > > > >> two choices:
> > >>> > > > >>
> > >>> > > > >> a) .debug_ranges&.debug_loc
=> -2, .debug_line => 0, other
> .debug_* -> -1
> > >>> > > > >> b) .debug_ranges&.debug_loc
=> -2, other .debug_* => 0
> > >>> > > > >>
> > >>> > > > >> Delaying .debug_line => -1 for
one or two release sounds
> good to me.
> > >>> > > > >> So LLD 11 or 12 linked binaries
can be debugged by LLDB 10.
> This is a
> > >>> > > > >> nice property.
> > >>> > > > >>
> > >>> > > > >> This write-up proposes b), but
I'd say a) is likely
> sufficient. With the
> > >>> > > > >> available information, I cannot
yet say that a) will have
> more risk.
> > >>> > > > >
> > >>> > > > >Risk is about the unknowns - and it
still seems like a lot of
> > >>> > > > >unknowns. While there are probably
many more consumers that
> read
> > >>> > > > >.debug_line than other sections,
reading debug_info (for
> instance) is
> > >>> > > > >necessary for inline frames in
symbolizing - still probably
> one of the
> > >>> > > > >most common uses of DWARF I'd
guess. (what about stack
> unwinding using
> > >>> > > > >debug_frame? that'd worry me a bit
if anyone got /that/ wrong
> because
> > >>> > > > >of this change)
> > >>> > > > >
> > >>> > > > >> > - chromium/firefox have some
tools that were broken:
> > >>> > > > >> >
> https://bugs.chromium.org/p/chromium/issues/detail?id=1102223#c5
>
<https://urldefense.com/v3/__https:/bugs.chromium.org/p/chromium/issues/detail?id=1102223*c5__;Iw!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWa5dfJfdQ$>
> > >>> > > > >>
> > >>> > > > >> This is potentially related to
other .debug_* (not
> .debug_line)
> > >>> > > > >> I hope Chromium developers can
chime in here:) The breakage
> was
> > >>> > > > >> unfortunate but I don't know
how we could have avoided
> that. IMHO this
> > >>> > > > >> is no different from "clang
started to emit a new DW_FORM_*
> and a
> > >>> > > > >> postprocessing tool of .debug
chokes on that" Whether we
> want to
> > >>> > > > >> suppress that particular
DW_FORM_* definitely should depend
> on how
> > >>> > > > >> likely it can cause problems, but
we can't yet say we have
> to hold off
> > >>> > > > >> on a feature for a solved
(precisely, mitigated) problem.
> > >>> > > > >
> > >>> > > > >LLVM has no custom forms and I'd
be super cautious about
> adding any
> > >>> > > > >that were on by default because of how
bad that breakage
> would be.
> > >>> > > > >
> > >>> > > > >I'm not so concerned about the
problems we know - but what
> they tell
> > >>> > > > >us about the problems that might arise
from use cases we
> don't know.
> > >>> > > > >All the other projects out there that
might have custom DWARF
> parsers
> > >>> > > > >to do some ad-hoc things.
> > >>> > > > >
> > >>> > > > >(also, ultimately - given how
far-reaching this is, I think
> we'll want
> > >>> > > > >some tidier flags that are more
user-focussed. I'd hope for a
> flag
> > >>> > > > >that gives BFD-like semantics (though
I'd be OK with fixing
> debug_loc
> > >>> > > > >(using 1 instead of 0) to work the
same as debug_ranges while
> we're
> > >>> > > > >there - a minor divergence from BFD,
but highly likely to not
> cause
> > >>> > > > >problems/fall out naturally from a
simple implementation of
> parsing
> > >>> > > > >that section) - something that's
been in-use and tested by
> basically
> > >>> > > > >everyone for decades. And another flag
for the new semantics
> (-2 for
> > >>> > > > >debug_loc/debug_ranges, -1 everywhere
else). Customizable
> per-section
> > >>> > > > >expression-based support I think is a
recipe for platform
> divergence &
> > >>> > > > >I'd rather it not be
available/supported at all, but if you
> really
> > >>> > > > >want to keep it in, I'd at least
rather it not be the feature
> we
> > >>> > > > >promote to users about how they can
test/opt in/out of the
> behavior
> > >>> > > > >when they're seeing breakages or
want to test the future
> semantics)
> > >>> > > > >
> > >>> > > > >> >I'm not sure how to get
the word out to DWARF consumers
> that they should
> > >>> > > > >> >consider this new
experimental behavior. Ray's done a good
> job
> > >>> > > > >> >evangelizing/discussing this
with gdb and lldb at least -
> and of course
> > >>> > > > >> >having turned it on by
default briefly has found some
> users (like Chromium)
> > >>> > > > >> >that we probably wouldn't
have found no matter how long we
> left this as an
> > >>> > > > >> >experimental option... so
some things are going to break
> when we switch no
> > >>> > > > >> >matter what.
> > >>> > > > >>
> > >>> > > > >> Thank you for following up with
some GNU folks on their
> lists!
> > >>> > > > >> If folks want to follow along the
thread:
> > >>> > > > >>
> https://sourceware.org/pipermail/binutils/2020-June/111376.html
>
<https://urldefense.com/v3/__https:/sourceware.org/pipermail/binutils/2020-June/111376.html__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWbtQobFaQ$>
> > >>> > > > >>
> > >>> > > > >> We have informed binutils,
elfutils-devel (elfutils has a
> few debug
> > >>> > > > >> tools) and gdb. I don't
recall that anyone has thought
> about problems
> > >>> > > > >> with a tombstone value.
> > >>> > > > >>
> > >>> > > > >> >
> > >>> > > > >> >P.S: Sony's already been
using the -1 technique with their
> debugger and
> > >>> > > > >> >linker for a while, so they
may want to keep this on by
> default for SCE -
> > >>> > > > >> >but I'm not sure how to
do that in-tree.
> > >>> > > > >> >
> > >>> > > > >> >
> > >>> > > > >> >Clang doesn't know which
lld
> > >>> > > > >> >version it's running, so
whether the flag can be
> specified, I would think?
> > >>> > > > >> >(so it'd be hard to have
Clang go "if SCE and LLD, pass
> the flag to use
> > >>> > > > >> >-1", I think) - if there
is a way to make that decision in
> the compiler
> > >>> > > > >.> >driver+linker, then we'd
have a question of "default new
> behavior except
> > >>> > > > >> >when tuning for LLDB and
GDB" or "default bfd behavior
> except when tuning
> > >>> > > > >> >for SCE".
> > >>> > > > >>
> > >>> > > > >> I've been involed in another
thread on SHF_LINK_ORDER (
> https://sourceware.org/pipermail/binutils/2020-
>
<https://urldefense.com/v3/__https:/sourceware.org/pipermail/binutils/2020-__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWbslmMhbQ$>
> > >>> > > > >July/112415.html ).
> > >>> > > > >> We may need a way to tell codegen
about the used linker.
> > >>> > > > >>
> > >>> > > > >> pcc proposed -mbinutils-version=
- This is nice in that
> some MC
> > >>> > > > >> decisions related to
-fno-integrated-as can use this option
> as well.
> > >>> > > > >> jyknight proposed
-mlinker-version= and syntax like
> -fuse-ld=bfd:2.34
> > >>> > > > >>
> > >>> > > > >> This may get more complex if the
generated object file want
> to be linked
> > >>> > > > >> with more than one linker. This
discussion probably
> deserves its own
> > >>> > > > >> thread.
> > >>> > > >
> > >>> > > >
> > >>> > > > --
> > >>> > > > Alexey Lapshin
> > >>> > > >
> > >>> > > _______________________________________________
> > >>> > > LLVM Developers mailing list
> > >>> > > llvm-dev at lists.llvm.org
> > >>> > >
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
<https://urldefense.com/v3/__https:/lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWZikxq4BA$>
> > >>> _______________________________________________
> > >>> LLVM Developers mailing list
> > >>> llvm-dev at lists.llvm.org
> > >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
<https://urldefense.com/v3/__https:/lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWZikxq4BA$>
> > >>
> > >> _______________________________________________
> > >> LLVM Developers mailing list
> > >> llvm-dev at lists.llvm.org
> > >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
<https://urldefense.com/v3/__https:/lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWZikxq4BA$>
> > >_______________________________________________
> > >LLVM Developers mailing list
> > >llvm-dev at lists.llvm.org
> > >https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
<https://urldefense.com/v3/__https:/lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev__;!!JmoZiZGBv3RvKRSx!ofVK1otXjQ1pPSV3TC8HS9Gwe9JwuC91OgFyvfCtZ57p55u8aHljgBgPTWZikxq4BA$>
>
>
>
>
> --
>
> 宋方睿
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200727/b16615a6/attachment-0001.html>

Created https://reviews.llvm.org/D84825 to be used for release/11.x

I haven't seen a strong argument for changing other .debug_*  but in
any case I don't want to continue debating on this topic.

* .debug_ranges & .debug_loc: -2 (lld<11: 0+addend)
* .debug_*: 0 (lld<11: 0+addend, lld HEAD: -1)

On Mon, Jul 27, 2020 at 12:47 PM David Blaikie <dblaikie at gmail.com>
wrote:
>
>
>
> On Mon, Jul 27, 2020 at 9:11 AM Robinson, Paul <paul.robinson at
sony.com> wrote:
>>
>> > I still think that we do bfd locs with a decent option to change
for at least the current release and sources and then, once we're a little
more certain we have everything that might want to parse dwarf (say by working
with dwarf-discuss), we can change the default.
>>
>>
>>
>> Given what’s been found, I think Eric/Dave are correct, use bfd
behavior by default with an option to do the new thing.  The option can be coded
to let Sony (SCE debugger tuning) have it on by default.
>>
>>
>>
>> Bringing it up on dwarf-discuss seems like a good idea too, you can
refer to http://dwarfstd.org/ShowIssue.php?issue=200609.1 as part of that.
>
>
> Thanks for the pointer! Posted here:
http://lists.dwarfstd.org/pipermail/dwarf-discuss-dwarfstd.org/2020-July/004683.html
>
>>
>> --paulr
>>
>>
>>
>> From: Eric Christopher <echristo at gmail.com>
>> Sent: Saturday, July 25, 2020 12:56 AM
>> To: Fāng-ruì Sòng <maskray at google.com>
>> Cc: Hans Wennborg <hans at chromium.org>; llvm-dev at
lists.llvm.org; Alexey Lapshin <a.v.lapshin at mail.ru>; George Rimar
<grimar at accesssoftek.com>; Robinson, Paul <paul.robinson at
sony.com>; Adrian Prantl <aprantl at apple.com>; Jonas Devlieghere
<jdevlieghere at apple.com>; James Henderson <jh7370.2008 at
my.bristol.ac.uk>; Peter Smith <Peter.Smith at arm.com>; David Blaikie
<dblaikie at gmail.com>; Pavel Labath <pavel at labath.sk>
>> Subject: Re: [llvm-dev] Switch to ld.bfd tombstone behavior by default
>>
>>
>>
>> Hi Ray :)
>>
>>
>>
>> While I understand the desire to say "we've updated everything
we've found" the problem is that we don't know that we've found
even much of the uses or can guarantee that people can upgrade, say their gdb or
breakpad, as fast as their compiler. Even worse it's a change in behavior
without much notice at all or even a particularly friendly way to recognize what
they should do to fix it.
>>
>>
>>
>> I still think that we do bfd locs with a decent option to change for at
least the current release and sources and then, once we're a little more
certain we have everything that might want to parse dwarf (say by working with
dwarf-discuss), we can change the default.
>>
>>
>>
>> -eric
>>
>>
>>
>> On Fri, Jul 24, 2020 at 7:33 PM Fāng-ruì Sòng <maskray at
google.com> wrote:
>>
>> From my understanding the breakpad bug was also only related to
.debug_line and has been fixed by
https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2317730
>>
>>
>>
>> > a) .debug_ranges&.debug_loc => -2, .debug_line => 0,
other .debug_* -> -1
>> > b) .debug_ranges&.debug_loc => -2, other .debug_* => 0
>>
>>
>>
>> I am still of the opinion that we should just do a), not b).
>>
>>
>>
>>
>>
>> On Fri, Jul 24, 2020 at 9:33 AM Hans Wennborg <hans at
chromium.org> wrote:
>>
>> On Fri, Jul 24, 2020 at 5:12 PM Fangrui Song <maskray at
google.com> wrote:
>> >
>> > On 2020-07-24, Hans Wennborg via llvm-dev wrote:
>> > >Sounds good to me from a release perspective.
>> >
>> > I think we need more input from the triage of
>> > https://chromium-review.googlesource.com/c/chromium/src/+/2291352
>> > whether it is just .debug_line or .debug_*
>>
>> The issue we hit in Chromium is tracked here:
>> https://bugs.chromium.org/p/chromium/issues/detail?id=1105559
Doesn't
>> look like it has any more info at the moment.
>>
>> >
>> > >On Fri, Jul 24, 2020 at 7:53 AM Eric Christopher via llvm-dev
>> > ><llvm-dev at lists.llvm.org> wrote:
>> > >>
>> > >> Hi All,
>> > >>
>> > >> In general I think we should adopt Dave's plan here.
The number of consumers that can (and have) been caught off guard by this change
is just too high.
>> > >>
>> > >> At the very least I think we should move this to opt in
to the new tombstoning behavior by default and at most migrate to bfd's
behavior for both the current release and in the current tree. If we want to
make this sort of change in the future by default I think we're going to
need to provide release notes about this and do aggressive outreach towards the
consumers we do know before making the change. If anyone wants to drive that
effort I'll happily provide any help or assistance in getting you in touch
with at least the consumers I know about.
>> > >>
>> > >> This change and the need for it is also probably worth a
quick message to dwarf-discuss at dwarfstd.org as well. Most of the major
consumers and producers are on that list and it's probably one of the
easiest ways to get this change out there.
>> > >>
>> > >> Any strong objections to this path in the meantime?
>> > >>
>> > >> Thanks!
>> > >>
>> > >> -eric
>> > >>
>> > >> On Tue, Jul 21, 2020 at 12:18 PM David Blaikie via
llvm-dev <llvm-dev at lists.llvm.org> wrote:
>> > >>>
>> > >>> On Tue, Jul 21, 2020 at 5:34 AM Alexey Lapshin
>> > >>> <alapshin at accesssoftek.com> wrote:
>> > >>> >
>> > >>> >
>> > >>> > >On Mon, Jul 20, 2020 at 10:32 AM Alexey
Lapshin
>> > >>> > ><alapshin at accesssoftek.com> wrote:
>> > >>> > >>
>> > >>> > >> >On Fri, Jul 17, 2020 at 1:55 PM
Alexey Lapshin <a.v.lapshin at mail.ru> wrote:
>> > >>> > >> >>
>> > >>> > >> >> >Пятница, 17 июля 2020,
19:42 +03:00 от David Blaikie <dblaikie at gmail.com>:
>> > >>> > >> >> >
>> > >>> > >> >> >On Fri, Jul 17, 2020 at
12:03 AM Fangrui Song <maskray at google.com> wrote:
>> > >>> > >> >> >>
>> > >>> > >> >> >> Thanks for the
write-up!
>> > >>> > >> >> >>
>> > >>> > >> >> >> On 2020-07-16, David
Blaikie wrote:
>> > >>> > >> >> >> >In short: Perhaps
we should switch lld to the bfd-style tombstoning
>> > >>> > >> >> >> >behavior for a
release or two, letting users opt-in to testing with the new
>> > >>> > >> >> >> >-1/-2 tombstoning
in the interim, before switching to the new tombstone by
>> > >>> > >> >> >> >default (while
still having the flag to switch back when users find
>> > >>> > >> >> >> >surprise places
that can't handle the new behavior).
>> > >>> > >> >> >> >
>> > >>> > >> >> >> >In long:
>> > >>> > >> >> >>
>https://reviews.llvm.org/D81784 and follow-on patches modified the behavior
>> > >>> > >> >> >> >of lld with
regards to resolving relocations from debug sections to dead
>> > >>> > >> >> >> >code (either
comdat deduplicated, or gc-sections use).
>> > >>> > >> >> >> >
>> > >>> > >> >> >> >A very quick
summary of the situation:
>> > >>> > >> >> >> >
>> > >>> > >> >> >> >Original
Behavior:
>> > >>> > >> >> >> >
>> > >>> > >> >> >> > - bfd: 1 for
debug_ranges(0 would prematurely terminate the list), 0
>> > >>> > >> >> >> > elsewhere
>> > >>> > >> >> >> > - gold/lld:
0+addend everywhere
>> > >>> > >> >> >> >
>> > >>> > >> >> >> >Limitations/bugs:
>> > >>> > >> >> >> >
>> > >>> > >> >> >> > - bfd/gold/lld
>> > >>> > >> >> >> > - doesn't
support 0 as a valid executable address without ambiguities
>> > >>> > >> >> >> > - gold/lld
>> > >>> > >> >> >> > - ambiguities
with large gc'd functions combined with a .text mapping
>> > >>> > >> >> >> > that starts in
relative low addresses
>> > >>> > >> >> >> > - premature
debug_range termination with zero-length functions (Clang
>> > >>> > >> >> >> > produces these
with __builtin_unreachable or non-void return
>> > >>> > >> >> >> >type functions
>> > >>> > >> >> >> > without a return
statement)
>> > >>> > >> >> >> >
>> > >>> > >> >> >> >New behavior:
>> > >>> > >> >> >> >
>> > >>> > >> >> >> > - -2 for DWARFv4
debug_loc, debug_ranges (-1 is a base address specifier
>> > >>> > >> >> >> > there)
>> > >>> > >> >> >> > - -1 elsewhere
>> > >>> > >> >> >> > - linker flag to
customize to other values if desired
>> > >>> > >> >> >> >
>> > >>> > >> >> >> >Known issues:
>> > >>> > >> >> >> >
>> > >>> > >> >> >> > - lldb's
line table parsing can't handle -1 well at all (essentially
>> > >>> > >> >> >> > unusable)
>> > >>> > >> >> >>
>> > >>> > >> >> >> Pavel Labath will fix
this soon https://reviews.llvm.org/D83957
>> > >>> > >> >> >> This is an unhandled
address-space wraparound problem.
>> > >>> > >> >> >> This pattern is
potentially common - and other downstream DWARF
>> > >>> > >> >> >> consumers might make
similar line table handling mistakes.
>> > >>> > >> >> >
>> > >>> > >> >> >That's the thing -
I'm not sure we can really classify them as
>> > >>> > >> >> >"mistakes". I
think bfd.ld's tombstoning behavior is about the only
>> > >>> > >> >> >thing we can reasonably
say DWARF consumers should /probably/ be
>> > >>> > >> >> >expected to handle - and
I'd imagine in many cases they haven't been
>> > >>> > >> >> >written intentionally to
handle it, but whatever behavior they have
>> > >>> > >> >> >has accidentally been
sufficient for their needs.
>> > >>> > >> >>
>> > >>> > >> >> >> > - gdb's line
table parsing ends up with different handling when breaking
>> > >>> > >> >> >> > on gc'd
functions (minor functionality issue)
>> > >>> > >> >> >>
>> > >>> > >> >> >> This is just a
behavior difference, not affecting users.
>> > >>> > >> >> >> It did break a test
if linked with LLD (gdb intrinsically has lots of
>> > >>> > >> >> >> failing tests even if
built with GCC+GNU ld).
>> > >>> > >> >> >>
>> > >>> > >> >> >> Previous behavior
(when an address is zero): a breakpoint on a
>> > >>> > >> >> >> --gc-sections
discarded function will be redirected to a larger line
>> > >>> > >> >> >> number with debug
info, even if that line can be an unrelated different
>> > >>> > >> >> >> function.
>> > >>> > >> >> >> New behavior is that
the breakpoint is on a wrapped-around small address.
>> > >>> > >> >> >>
>> > >>> > >> >> >> GDB 9.3 will restore
the previous behavior
>> > >>> > >> >> >>
(https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=a8caed5d7faa639a1e6769eba551d15d8ddd9510)
>> > >>> > >> >> >>
>> > >>> > >> >> >> >
>> > >>> > >> >> >> >I think
there's enough risk in this work (even given the small number of
>> > >>> > >> >> >> >bugs found so
far), given there's a pretty wide array of debug info
>> > >>> > >> >> >> >consumers out
there, that we should change lld's default to match the
>> > >>> > >> >> >> >long-lived bfd
strategy. This would address my original motivation for
>> > >>> > >> >> >> >raising all this
(empty functions prematurely terminating the list), while
>> > >>> > >> >> >> >letting users who
want to experiment with it, or need it (like Alexey), can
>> > >>> > >> >> >> >opt-in to the
-1/-2 behavior.
>> > >>> > >> >> >>
>> > >>> > >> >> >> I think we can only
confidently say that there is enough risk in using
>> > >>> > >> >> >> tombstone value -1 in
.debug_line, but I'd not say tombstone value -1 in
>> > >>> > >> >> >> other .debug_* can
cause problems.
>> > >>> > >> >>
>> > >>> > >> >> >Given how many DWARF
parsers we've had to cleanup or migrate off in
>> > >>> > >> >> >transitioning to DWARFv5
inside Google, I think that's a fair bit of
>> > >>> > >> >> >evidence the set of
parsers isn't as narrow/closed as we'd like and
>> > >>> > >> >> >thus the number of places
that might have issues isn't known/easily
>> > >>> > >> >> >exhaustively tested. So
I'm not so concerned about the bugs we've
>> > >>> > >> >> >seen, but what that might
indicate about the things that we don't know
>> > >>> > >> >> >about and can't test
(because we don't know about them). (of course,
>> > >>> > >> >> >the flipside of that is
that if we don't know about them, we can't
>> > >>> > >> >> >tell people who own them
to go check if they work with this opt-in
>> > >>> > >> >> >feature - so they'll
break whenever we turn this on by default - but
>> > >>> > >> >> >perhaps in the interim we
can get at least a few big LLD customers to
>> > >>> > >> >> >deploy the feature and
flush out some of the issues - happy for Google
>> > >>> > >> >> >to use this internally,
hopefully we can encourage Apple folks, Sony
>> > >>> > >> >> >of course already has this
semantic so nothing to find there most
>> > >>> > >> >> >likely - Chromium, maybe
Firefox, etc)
>> > >>> > >> >>
>> > >>> > >> >> From the other side — when we
already switched new behavior ON as default —
>> > >>> > >> >> then it is easier to discover
all these unknown cases. We have an option restoring
>> > >>> > >> >> old behavior(which should be
fine for all of the current users). Thus, everybody
>> > >>> > >> >> who needs old behavior is able
to continue using it.
>> > >>> > >>
>> > >>> > >> >The cost to those users figuring
out that it's a problem and finding
>> > >>> > >> >the flags/adding them, etc, is
non-zero.
>> > >>> > >> >But, yes, as I said - eventually
some of that will happen, sooner or
>> > >>> > >> >later. But reducing how much of it
happens is valuable too.
>> > >>> > >>
>> > >>> > >> I agree. Reducing the impact on
customers is important.
>> > >>> > >> Though in this case, as you said, it
would be done anyway(sooner or later).
>> > >>> > >> Finally, I do not know what is more
important: a) get feedback faster with
>> > >>> > >> the cost of involved users and
requiring them to make efforts
>> > >>> > >> to figure out the problem. or b) test
with a smaller set of users,
>> > >>> > >> find more problematic cases, make new
tombstone value to be default later.
>> > >>> > >> I am kind of preferring a). But, b) is
more safe and is OK.
>> > >>> > >>
>> > >>> > >>
>> > >>> > >> >> That makes transition more
understandable. All problems would be explicitly seen. Then,
>> > >>> > >> >> If we will know about new
problems - we could adapt the current solution. Similar to this >suggestion:
>> > >>> > >> >>
>> > >>> > >> >> a)
.debug_ranges&.debug_loc => -2, .debug_line => 0, other .debug_* ->
-1
>> > >>> > >>
>> > >>> > >> >The issue I have with this is that
it's reflective of the known
>> > >>> > >> >breakage in some tools that happen
to have a quick turnaround/feedback
>> > >>> > >> >- and assumes all the other
sections are significantly less likely to
>> > >>> > >> >have problems with this format -
and I don't think we have enough data
>> > >>> > >> >to make that estimate.
>> > >>> > >>
>> > >>> > >> We could extend that rule when/if new
problems become known.
>> > >>> > >> Or, customers could fix their code or
use option returning old behavior
>> > >>> > >> and it would probably not be necessary
to extend the rule.
>> > >>> > >>
>> > >>> > >> >I think having a clear flag for bfd
behavior or new behavior would be
>> > >>> > >> >good, keeping it bfd-by-default for
now, evangelizing (patch notes,
>> > >>> > >> >outreach to other projects and tool
developers) the new functionality
>> > >>> > >> >& then changing the default
shortly after an LLVM release (perhaps 12
>> > >>> > >> >- but perhaps longer) to give it
time to bake for trunk-following
>> > >>> > >> >users (ample time for them to pick
it up on their schedule, see what
>> > >>> > >> >other tools might break, whether
they're isolated enough to expect a
>> > >>> > >> >quick turnaround or whether we
should get them updated and then wait a
>> > >>> > >> >while longer.
>> > >>> > >>
>> > >>> > >> yes, doing clear flag for bfd behavior
or new behavior,
>> > >>> > >> and keep it bfd-by-default for now
would be OK and is more safe.
>> > >>> > >>
>> > >>> > >>
>> > >>> > >> >> This would help while dwarf
users are preparing their code to the new solution.
>> > >>> > >> >>
>> > >>> > >> >> Is option restoring old
behavior not enough to solve problems caused by new tombstone >value?
>> > >>> > >>
>> > >>> > >> >I don't personally think it is
enough - given some of the known
>> > >>> > >> >breakage - I think that points to a
fair bit of reasonably possible
>> > >>> > >> >unknown breakage. Having a few big
(like Apple and Google) users
>> > >>> > >> >switch over by default &
potentially flush out a few more issues (or
>> > >>> > >> >not -but build confidence that
there aren't more issues) I think is an
>> > >>> > >> >appropriate way to roll out a
change like this.
>> > >>> > >>
>> > >>> > >> >(minor doubt: I wonder how well the
bfd tombstoning works in DWARFv5
>> > >>> > >> >(rnglists/loclists) or in
debug_range/debug_loc that uses base address
>> > >>> > >> >specifiers, where the
zero-without-addend doesn't have a chance to
>> > >>> > >> >make an empty range (because
it's not a start/end pair, it's a
>> > >>> > >> >start+offset pair - so the offset
remains non-zero)... if bfd
>> > >>> > >> >tombstoning breaks DWARFv5 parsing
(or base address specifiers in
>> > >>> > >> >range/loc) in common consumers I
might be more inclined to support
>> > >>> > >> >enabling new tombstoning by default
sooner (though if we could enable
>> > >>> > >> >it just for DWARFv5 that might be
nice - but not practical, since the
>> > >>> > >> >linker doesn't know what DWARF
it's linking & could be linking
>> > >>> > >> >multiple versions))
>> > >>> > >>
>> > >>> > >> - From the point of view overlapping
address ranges,
>> > >>> > >> both bfd-solution(using 1) and old lld
solution(using 0) are equal.
>> > >>> >
>> > >>> > >gold and lld's solution was a bit more
complicated than using zero (&
>> > >>> > >bfd's solution uses 1 in debug_ranges,
but not in other debug
>> > >>> > >sections). lld and gold use 0+addend in
ranges.
>> > >>> >
>> > >>> > right. I am talking about the same(though
assumed that context
>> > >>> > instead of detailed explanation). it should be
"bfd-solution(using 1
>> > >>> > for debug_ranges) and lld/gold solution(using
0+addend)".
>> > >>> > Thank you for pointing into that.
>> > >>> >
>> > >>> > >0 alone in ranges
>> > >>> > >would be more problematic than any of the
other deployed solutions,
>> > >>> > >because it would lead to the debug_ranges
contributions being
>> > >>> > >terminated early (0, 0 terminates the list -
so debug_ranges for a CU
>> > >>> > >with one gc'd function mid-way through
the range list would drop the
>> > >>> > >rest of the range lit) - that's why bfd
uses 1 for debug_ranges rather
>> > >>> > >than 0 which it uses elsewhere (though the
same fix should,
>> > >>> > >technically, be applied to debug_loc too for
the same reasons as
>> > >>> > >debug_ranges). the 0+addend approach of
gold/lld works OK (except for
>> > >>> > >the low address overlap) except when
there's a zero-length function,
>> > >>> > >then 0+addend == 0 for the end of the range,
andy ou get a 0,0 entry
>> > >>> > >with the premature range list termination
issues.
>> > >>> > >
>> > >>> > >> for the "start+offset pair"
case they could result in overlapping address ranges.
>> > >>> >
>> > >>> > >Not quite - the way bfd does things, at
least in debug_ranges without
>> > >>> > >base address specifiers (bit hard to isolate
for a consumer, really -
>> > >>> > >though they could special case zero base
addresses... sort of) the
>> > >>> > >empty ranges are (1, 1) - they're
actually empty. Whereas lld's
>> > >>> > >approach ends up with (0+addend, 0+addend)
which is usually (0, size)
>> > >>> > >which has issues with overlap on systems
that have a valid low address
>> > >>> > >range (or if you have sufficiently large
functions).
>> > >>> > >
>> > >>> >
>> > >>> > There is a case when size of address range is
set not by relocated value but by constant value:
>> > >>> >
>> > >>> > DWARF5:
>> > >>> > DW_RLE_startx_length
>> > >>> > DW_RLE_start_length
>> > >>> >
>> > >>> > {address of deleted code, length}
>> > >>> > {address of existing code, length}
>> > >>> >
>> > >>> > DWARF4:
>> > >>> > base address selection entry: {-1, address of
deleted code}
>> > >>> > following range list entry: {0, length}
>> > >>> > base address selection entry: {-1, address of
existing code}
>> > >>> > following range list entry: {0, length}
>> > >>> >
>> > >>> > In these cases linker do not see the length of
address range
>> > >>> > and could not change it(using 1, or 0, or
0+addend) to be zero length
>> > >>> > address range. Thus address ranges could
overlap.
>> > >>> >
>> > >>> > So in both DWARF5 and DWARF4 there are cases
when address ranges
>> > >>> > could be overlapped and it could not be solved
without new meaning for tombstone value.
>> > >>> >
>> > >>> > >> - From the point of view correct
parsing of DWARF 5 - it looks like they are equally good.
>> > >>> > >
>> > >>> > >Yep, DWARFv5 is more complicated - bfd's
approach would be marginally
>> > >>> > >better even for addr+offset encodings in
debug_rnglists - since it'd
>> > >>> > >always produce a 0 for the addr, whereas
gold/lld's behavior can in
>> > >>> > >some cases (eg: "void f1() { }
__attribute__((nodebug)) void f2() { }
>> > >>> > >void f3() { }" with
-fno-function-sections - you'll end up with the
>> > >>> > >range for f3 starting at a non-zero addend -
this debug info will be
>> > >>> > >worse for bfd/ld (if the DWARF for this
object was included, but the
>> > >>> > >whole .text section was discarded (eg: maybe
there's a global variable
>> > >>> > >in this object which is linked in, but
-gc-section is still able to
>> > >>> > >discard the whole .text section as
unreferenced) then gold/lld's
>> > >>> > >approach would make f3 unidentifiable as
dead code (because it doesn't
>> > >>> > >have a tombstone start) but bfd's
approach would work (assuming zero
>> > >>> > >isn't a valid address))
>> > >>> >
>> > >>> > I see. thanks.
>> > >>> >
>> > >>> > >
>> > >>> > >> - From the point of view correct
parsing of DWARF 4 - it looks bfd-solution(using 1)
>> > >>> > >> is better than the old lld
solution(using 0). When range list entry contains
>> > >>> > >> addresses(start+end) which should be
relocated and for the zero-length functions,
>> > >>> > >> bfd-solution would result in range list
entry: {1, 1}, while old lld solution
>> > >>> > >> would result in {0, 0}, and match with
the end of list entry.
>> > >>> > >> That is the original problem that
started this thread.
>> > >>> > >
>> > >>> > >Only comes up for zero-length functions,
because gold/lld's approach
>> > >>> > >was 0+addend, not straight 0.
>> > >>> > >
>> > >>> >
>> > >>> > right. for zero-length functions.
>> > >>> >
>> > >>> > >> Though it looks like there still exist
case when range list could be terminated earlier:
>> > >>> > >>
>> > >>> > >> base address selection entry: {-1,
address of deleted code}
>> > >>> > >> following range list entry: {0, 0}
<< points to the same address as set by base address selection entry and
has zero size.
>> > >>> > >
>> > >>> > >That's a bug in the producer (though a
good point - I've probably made
>> > >>> > >that bug in LLVM) - the linker can't
solve that problem, since the
>> > >>> > >linker can't touch the literal
unrelocated 0, 0.
>> > >>> > >
>> > >>> > >> after linker resolved relocations it
would look like this, for bfd case:
>> > >>> > >>
>> > >>> > >> base address selection entry: {-1, 1}
>> > >>> > >> following range list entry: {0, 0}
<<<<<<<<<<<
>> > >>> > >>
>> > >>> > >> So there still exists {0,0} entry which
could be considered as the end of list entry.
>> > >>> > >>
>> > >>> > >> But old lld solution has the same
problem, thus it would not be new.
>> > >>> > >>
>> > >>> > >> - Additionally, AFAIK gdb has special
processing for overlapped address
>> > >>> > >> ranges starting from 0. Using bfd
tombstone value could break that processing - I would check it.
>> > >>> > >
>> > >>> > >Not sure I understand - presumably gdb's
special processing is
>> > >>> > >intended to work with bfd's tombstoning,
since it's been the most
>> > >>> > >common/prolific unix linker, the one
intended to work with gdb (they
>> > >>> > >exist in the same repository) for decades,
right?
>> > >>> >
>> > >>> > I think I misunderstood this: "I wonder how
well the bfd tombstoning works in DWARFv5
>> > >>> > (rnglists/loclists)". I read it as we would
like to use bfd tombstoning(1 for ranges)
>> > >>> > for rnglists/loclists also.
>> > >>> >
>> > >>> > So, right. bfd's tombstoning works correctly
with gdb until 1 is not used
>> > >>> > for rnglists/loclists as tombstone value.
>> > >>>
>> > >>> Not quite parsing this, but I think we're on the
same page - that
>> > >>> bfd's tombstoning "1 for
debug_ranges/debug_loc, 0 (not 0+addend, but
>> > >>> absolute 0) for everything else (including
debug_loclists and
>> > >>> debug_rnglists)" is probably the most likely to
work for gdb, since
>> > >>> it's been deployed for a long time.
>> > >>>
>> > >>> - Dave
>> > >>>
>> > >>> PS: Fair point about base address specifiers being
able to produce 0,0
>> > >>> entries - wouldn't mind fixing that in LLVM if I
knew of an easy way
>> > >>> to test at compile-time whether the difference
between two labels was
>> > >>> zero, then skip that entry in the lists entirely.
Would save space and
>> > >>> address the original issue I had with debug_ranges
terminating early.
>> > >>> (should've thought about that much earlier than
my more alarmist "oh
>> > >>> deer, we need to fundamentally change how linkers
resolve relocations
>> > >>> because everything's been broken and we just
didn't realize it" -
>> > >>> fixing the compiler not to produce zero-length ranges
would've been
>> > >>> less risky & probably still worth doing - though
addressing the
>> > >>> broader issue to help with your situation of 0 as a
valid address I
>> > >>> think is still good too)
>> > >>>
>> > >>> >
>> > >>> > Alexey.
>> > >>> >
>> > >>> > > > >> With consideration for satefy
for the upcoming release/11.x, we can make
>> > >>> > > > >> two choices:
>> > >>> > > > >>
>> > >>> > > > >> a)
.debug_ranges&.debug_loc => -2, .debug_line => 0, other .debug_* ->
-1
>> > >>> > > > >> b)
.debug_ranges&.debug_loc => -2, other .debug_* => 0
>> > >>> > > > >>
>> > >>> > > > >> Delaying .debug_line => -1
for one or two release sounds good to me.
>> > >>> > > > >> So LLD 11 or 12 linked
binaries can be debugged by LLDB 10. This is a
>> > >>> > > > >> nice property.
>> > >>> > > > >>
>> > >>> > > > >> This write-up proposes b),
but I'd say a) is likely sufficient. With the
>> > >>> > > > >> available information, I
cannot yet say that a) will have more risk.
>> > >>> > > > >
>> > >>> > > > >Risk is about the unknowns - and
it still seems like a lot of
>> > >>> > > > >unknowns. While there are probably
many more consumers that read
>> > >>> > > > >.debug_line than other sections,
reading debug_info (for instance) is
>> > >>> > > > >necessary for inline frames in
symbolizing - still probably one of the
>> > >>> > > > >most common uses of DWARF I'd
guess. (what about stack unwinding using
>> > >>> > > > >debug_frame? that'd worry me a
bit if anyone got /that/ wrong because
>> > >>> > > > >of this change)
>> > >>> > > > >
>> > >>> > > > >> > - chromium/firefox have
some tools that were broken:
>> > >>> > > > >> >
https://bugs.chromium.org/p/chromium/issues/detail?id=1102223#c5
>> > >>> > > > >>
>> > >>> > > > >> This is potentially related
to other .debug_* (not .debug_line)
>> > >>> > > > >> I hope Chromium developers
can chime in here:) The breakage was
>> > >>> > > > >> unfortunate but I don't
know how we could have avoided that. IMHO this
>> > >>> > > > >> is no different from
"clang started to emit a new DW_FORM_* and a
>> > >>> > > > >> postprocessing tool of .debug
chokes on that" Whether we want to
>> > >>> > > > >> suppress that particular
DW_FORM_* definitely should depend on how
>> > >>> > > > >> likely it can cause problems,
but we can't yet say we have to hold off
>> > >>> > > > >> on a feature for a solved
(precisely, mitigated) problem.
>> > >>> > > > >
>> > >>> > > > >LLVM has no custom forms and
I'd be super cautious about adding any
>> > >>> > > > >that were on by default because of
how bad that breakage would be.
>> > >>> > > > >
>> > >>> > > > >I'm not so concerned about the
problems we know - but what they tell
>> > >>> > > > >us about the problems that might
arise from use cases we don't know.
>> > >>> > > > >All the other projects out there
that might have custom DWARF parsers
>> > >>> > > > >to do some ad-hoc things.
>> > >>> > > > >
>> > >>> > > > >(also, ultimately - given how
far-reaching this is, I think we'll want
>> > >>> > > > >some tidier flags that are more
user-focussed. I'd hope for a flag
>> > >>> > > > >that gives BFD-like semantics
(though I'd be OK with fixing debug_loc
>> > >>> > > > >(using 1 instead of 0) to work the
same as debug_ranges while we're
>> > >>> > > > >there - a minor divergence from
BFD, but highly likely to not cause
>> > >>> > > > >problems/fall out naturally from a
simple implementation of parsing
>> > >>> > > > >that section) - something
that's been in-use and tested by basically
>> > >>> > > > >everyone for decades. And another
flag for the new semantics (-2 for
>> > >>> > > > >debug_loc/debug_ranges, -1
everywhere else). Customizable per-section
>> > >>> > > > >expression-based support I think
is a recipe for platform divergence &
>> > >>> > > > >I'd rather it not be
available/supported at all, but if you really
>> > >>> > > > >want to keep it in, I'd at
least rather it not be the feature we
>> > >>> > > > >promote to users about how they
can test/opt in/out of the behavior
>> > >>> > > > >when they're seeing breakages
or want to test the future semantics)
>> > >>> > > > >
>> > >>> > > > >> >I'm not sure how to
get the word out to DWARF consumers that they should
>> > >>> > > > >> >consider this new
experimental behavior. Ray's done a good job
>> > >>> > > > >> >evangelizing/discussing
this with gdb and lldb at least - and of course
>> > >>> > > > >> >having turned it on by
default briefly has found some users (like Chromium)
>> > >>> > > > >> >that we probably
wouldn't have found no matter how long we left this as an
>> > >>> > > > >> >experimental option... so
some things are going to break when we switch no
>> > >>> > > > >> >matter what.
>> > >>> > > > >>
>> > >>> > > > >> Thank you for following up
with some GNU folks on their lists!
>> > >>> > > > >> If folks want to follow along
the thread:
>> > >>> > > > >>
https://sourceware.org/pipermail/binutils/2020-June/111376.html
>> > >>> > > > >>
>> > >>> > > > >> We have informed binutils,
elfutils-devel (elfutils has a few debug
>> > >>> > > > >> tools) and gdb. I don't
recall that anyone has thought about problems
>> > >>> > > > >> with a tombstone value.
>> > >>> > > > >>
>> > >>> > > > >> >
>> > >>> > > > >> >P.S: Sony's already
been using the -1 technique with their debugger and
>> > >>> > > > >> >linker for a while, so
they may want to keep this on by default for SCE -
>> > >>> > > > >> >but I'm not sure how
to do that in-tree.
>> > >>> > > > >> >
>> > >>> > > > >> >
>> > >>> > > > >> >Clang doesn't know
which lld
>> > >>> > > > >> >version it's running,
so whether the flag can be specified, I would think?
>> > >>> > > > >> >(so it'd be hard to
have Clang go "if SCE and LLD, pass the flag to use
>> > >>> > > > >> >-1", I think) - if
there is a way to make that decision in the compiler
>> > >>> > > > >.> >driver+linker, then
we'd have a question of "default new behavior except
>> > >>> > > > >> >when tuning for LLDB and
GDB" or "default bfd behavior except when tuning
>> > >>> > > > >> >for SCE".
>> > >>> > > > >>
>> > >>> > > > >> I've been involed in
another thread on SHF_LINK_ORDER
(https://sourceware.org/pipermail/binutils/2020-
>> > >>> > > > >July/112415.html ).
>> > >>> > > > >> We may need a way to tell
codegen about the used linker.
>> > >>> > > > >>
>> > >>> > > > >> pcc proposed
-mbinutils-version= - This is nice in that some MC
>> > >>> > > > >> decisions related to
-fno-integrated-as can use this option as well.
>> > >>> > > > >> jyknight proposed
-mlinker-version= and syntax like -fuse-ld=bfd:2.34
>> > >>> > > > >>
>> > >>> > > > >> This may get more complex if
the generated object file want to be linked
>> > >>> > > > >> with more than one linker.
This discussion probably deserves its own
>> > >>> > > > >> thread.
>> > >>> > > >
>> > >>> > > >
>> > >>> > > > --
>> > >>> > > > Alexey Lapshin
>> > >>> > > >
>> > >>> > >
_______________________________________________
>> > >>> > > LLVM Developers mailing list
>> > >>> > > llvm-dev at lists.llvm.org
>> > >>> > >
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> > >>> _______________________________________________
>> > >>> LLVM Developers mailing list
>> > >>> llvm-dev at lists.llvm.org
>> > >>>
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> > >>
>> > >> _______________________________________________
>> > >> LLVM Developers mailing list
>> > >> llvm-dev at lists.llvm.org
>> > >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> > >_______________________________________________
>> > >LLVM Developers mailing list
>> > >llvm-dev at lists.llvm.org
>> > >https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>>
>>
>>
>> --
>>
>> 宋方睿


-- 
宋方睿