gold vs libc

gold vs libc

[Roland McGrath]

On the glibc.git branch roland/gold-vs-libc is the trivial configure change
necessary to let glibc attempt to build using gold as the linker.  (The
version check with that change is excessively loose--to boot, it admits
current gold versions so you can test, even though they don't work enough.)
i.e.:

	$ git clone git://sourceware.org/git/glibc.git
	$ git checkout roland/gold-vs-libc
	... make the ld in your path and found by the compiler be gold ...
	$ mkdir build
	$ cd build
	$ ../configure --enable-add-ons --prefix=/usr

The configure will fail the __ehdr_start check, which I've already posted
on the binutils list about.  This is not fatal.

	$ make -j...
	$ make -j... check

This will get test failures for nptl/tst-*-static (all but one of them).
(For me, there is one other FAIL, which is not a regression from libc built
by BFD ld.)  The affected tests are all statically-linked programs using
cancellation, i.e. unwinding.  I examined one in detail: tst-sem11-static.
I assume that the others are all suffering from the same gold bug.

I suspect the bug is in gold's .eh_frame optimization pass.  For me (with
trunk gold), the __EH_FRAME_BEGIN__ symbol (from gcc's crtbeginT.o--meant
to attach to the start of the .eh_frame output section) winds up pointing
at the end of .eh_frame rather than the beginning.  In the binary created
by BFD ld, the symbol points 48 bytes past the true start of .eh_frame,
which is exactly the size of crt1.o's .eh_frame contribution (so what we'd
expect given the input file order, albeit not what would be really right).

The .eh_frame in gold's output is a little smaller, perhaps suggesting that
it de-dup'd some CIEs and BFD ld didn't.  I'm not sure whether BFD ld does
.eh_frame optimization without --eh-frame-hdr (which isn't passed in this
static link).

Perhaps .eh_frame optimization just never really works with static linking
because there is no right answer for what a symbol like __EH_FRAME_BEGIN__
should stick to?  Hmm.  If the first CIE there happens to be one that's
de-dup'd then maybe it appears to make sense for the label to move to the
remaining other equivalent CIE, which might be way off into the section.

OTOH, crtbeginT.o is actually an input file that defines __EH_FRAME_BEGIN__
as a symbol in a zero-length .eh_frame input section.  So it seems more
unambiguous that this symbol should stick to that input file's relative
position in the output section, which I think is well-defined in theory
even when the input section is empty--rather than sticking to the
immediately adjacent content of another (file's) input section going into
the same output section, which if it's a de-dup'd CIE could be considered
to have "moved" to elsewhere in the output section.

At any rate, the symbol winding up exactly at the end of the .eh_frame
output section, as it does, seems clearly wrong.

gold hackers wanting to help gold finally get over the hump to being able
to build glibc could try the build from the branch and debug this issue.
glibc hackers wanting to help gold hackers achieve the same end could work
on reducing and isolating a test case that demonstrates the misbehavior
with the __EH_FRAME_BEGIN__ symbol without needing a whole pile of libc
build .o and .a files to make it happen.


Thanks,
Roland

Re: gold vs libc

[Alan Modra]

On Sat, Mar 29, 2014 at 09:25:16PM -0700, Roland McGrath wrote:
> I suspect the bug is in gold's .eh_frame optimization pass.  For me (with
> trunk gold), the __EH_FRAME_BEGIN__ symbol (from gcc's crtbeginT.o--meant
> to attach to the start of the .eh_frame output section) winds up pointing
> at the end of .eh_frame rather than the beginning.  In the binary created
> by BFD ld, the symbol points 48 bytes past the true start of .eh_frame,
> which is exactly the size of crt1.o's .eh_frame contribution (so what we'd
> expect given the input file order, albeit not what would be really right).

https://sourceware.org/bugzilla/show_bug.cgi?id=14675

-- 
Alan Modra
Australia Development Lab, IBM

Re: gold vs libc

[Roland McGrath]

> https://sourceware.org/bugzilla/show_bug.cgi?id=14675

Thanks.  In fact, CFI in crt1.o is useful for the debugger (which doesn't
care where __EH_FRAME_BEGIN__ points).  See:
	https://sourceware.org/ml/libc-alpha/2012-03/msg00573.html
for why we added it.

In the (more common) dynamic linking case, it works fine because
__EH_FRAME_BEGIN__ is not used.  Insteaad, --eh-frame-hdr is used at link
time so PT_GNU_EH_FRAME can be used at runtime.  I don't know why we don't
do it that way for static linking too.  The binary-search table is a
worthwhile optimization for everybody.

AFAICT if we just started passing --eh-frame-hdr and stopped using
crt{begin,end}T.o instead of crt{begin,end}.o under -static it would work
as is.  dl_iterate_phdr in the static case should already find the
program's own phdr so libgcc can find PT_GNU_EH_FRAME in there and be happy.

But given the status quo, gold should do something more like what ld does.


Thanks,
Roland

Re: gold vs libc

[Carlos O'Donell]

On 03/30/2014 12:25 AM, Roland McGrath wrote:
> On the glibc.git branch roland/gold-vs-libc is the trivial configure change
> necessary to let glibc attempt to build using gold as the linker.  (The
> version check with that change is excessively loose--to boot, it admits
> current gold versions so you can test, even though they don't work enough.)

Enabling gold is BZ #14995.
https://sourceware.org/bugzilla/show_bug.cgi?id=14995

... and to link to the old discussion with more patches for
configure:

https://sourceware.org/ml/libc-alpha/2013-01/msg00274.html

Cheres,
Carlos.

Re: gold vs libc

[Ian Lance Taylor]

On Sat, Mar 29, 2014 at 10:06 PM, Roland McGrath <roland@hack.frob.com> wrote:

>> https://sourceware.org/bugzilla/show_bug.cgi?id=14675
>
> Thanks.  In fact, CFI in crt1.o is useful for the debugger (which doesn't
> care where __EH_FRAME_BEGIN__ points).  See:
>         https://sourceware.org/ml/libc-alpha/2012-03/msg00573.html
> for why we added it.
>
> In the (more common) dynamic linking case, it works fine because
> __EH_FRAME_BEGIN__ is not used.  Insteaad, --eh-frame-hdr is used at link
> time so PT_GNU_EH_FRAME can be used at runtime.  I don't know why we don't
> do it that way for static linking too.  The binary-search table is a
> worthwhile optimization for everybody.
>
> AFAICT if we just started passing --eh-frame-hdr and stopped using
> crt{begin,end}T.o instead of crt{begin,end}.o under -static it would work
> as is.  dl_iterate_phdr in the static case should already find the
> program's own phdr so libgcc can find PT_GNU_EH_FRAME in there and be happy.
>
> But given the status quo, gold should do something more like what ld does.

I don't fully understand this, but it seems to me that gold's
behaviour is correct and GNU ld's behaviour is incorrect.  GNU ld is
effectively discarding the exception frame information in crt1.o, and
gold is retaining it.

To put it another way: what principle should gold follow to get the
result you want?

Ian

Re: gold vs libc

[Roland McGrath]

> I don't fully understand this, but it seems to me that gold's
> behaviour is correct and GNU ld's behaviour is incorrect.  GNU ld is
> effectively discarding the exception frame information in crt1.o, and
> gold is retaining it.

That is neither true nor relevant.  I did not lodge any complaint
about what contents wind up in the .eh_frame output section.  The
problem is how it's treating a symbol defined in an input section.

GNU ld produces an __EH_FRAME_BEGIN__ symbol that points to the place
in the output .eh_frame section that corresponds to the order of input
files' .eh_frame sections.  This does indeed point past the crt1.o
CFI, but that is correct given the input file order (crt1.o and its
.eh_frame content before crtbeginT.o and its __EH_FRAME_BEGIN__
symbol).

Gold produces an __EH_FRAME_BEGIN__ symbol that points to the end of
the .eh_frame output section.  The effect of this is that gold is
"effectively discarding" the entirety of the CFI.  There is no logic
of input sections and symbols and their order by which this result
makes any kind of sense.

The fact that it's CFI data is secondary IMHO.  If the linker wants to
optimize CFI data, then that's a fine thing.  But its starting mandate
has to be that it doesn't break the general rules of how link order
and symbol values would come out if it were any other random rodata
section rather than .eh_frame.

> To put it another way: what principle should gold follow to get the
> result you want?

When an input file contains a symbol pointing to a location in an
input section, the output file should define that symbol so it points
to the part of the output section that corresponds to the origin input
location.  When the symbol points to input contents of at least one
byte, what this means is pretty incontrovertibly clear.  In this case,
it points to an empty input section.  But I claim that it's adequately
clear what it should mean in this case too.


Thanks,
Roland

Re: gold vs libc

[Ian Lance Taylor]

On Mon, Mar 31, 2014 at 1:04 PM, Roland McGrath <roland@hack.frob.com> wrote:
>
> When an input file contains a symbol pointing to a location in an
> input section, the output file should define that symbol so it points
> to the part of the output section that corresponds to the origin input
> location.  When the symbol points to input contents of at least one
> byte, what this means is pretty incontrovertibly clear.  In this case,
> it points to an empty input section.  But I claim that it's adequately
> clear what it should mean in this case too.

It's really not.  When the eh_frame section is being optimized, there
is no longer any correspondence between symbols defined in the input
sections and data defined in the output section.  All the input data
is tossed in a heap, and the output section is constructed out of that
heap.  I agree that when an input symbol points to some data in some
input section, and when we copy that input data unchanged, then the
symbol should clearly point to the same data in the output section.
That works fine for merge sections, for example.  But that's not the
case here.  There is nothing to locate the input symbol at any
location in the output section at all.

The current code has a simple algorithm that usually produces the
result we want: we simply copy .eh_frame sections until we find one we
can optimize.  Perhaps we could change to a different algorithm: put
all unoptimized .eh_frame sections first, then all optimized .eh_frame
sections.

Ian

Re: gold vs libc

[Roland McGrath]

> On Mon, Mar 31, 2014 at 1:04 PM, Roland McGrath <roland@hack.frob.com> wrote:
> >
> > When an input file contains a symbol pointing to a location in an
> > input section, the output file should define that symbol so it points
> > to the part of the output section that corresponds to the origin input
> > location.  When the symbol points to input contents of at least one
> > byte, what this means is pretty incontrovertibly clear.  In this case,
> > it points to an empty input section.  But I claim that it's adequately
> > clear what it should mean in this case too.
> 
> It's really not.  When the eh_frame section is being optimized, there
> is no longer any correspondence between symbols defined in the input
> sections and data defined in the output section.

I understand that's what's going on underneath.  But the user didn't ask
you to fiddle with his .eh_frame contents.  He did ask you to place some
symbols in his .eh_frame section.  If .eh_frame optimization is not
transparent, then it's broken.

IMHO it would be acceptable to simply disable .eh_frame optimization when
there are any symbols in input .eh_frame sections that will survive to
affect the output of anything except the .eh_frame output section's
contents.  (That tortured wording is to distinguish __EH_FRAME_BEGIN__,
whose value is used by relocs in other sections like .text, from the
various .L* symbols used within individual input sections that are only
used in arithmetic producing values inside the section.)  What's not
acceptable is breaking the core semantics of linking that would apply if
nobody had ever thought of .eh_frame optimization.

> The current code has a simple algorithm that usually produces the
> result we want: we simply copy .eh_frame sections until we find one we
> can optimize.  Perhaps we could change to a different algorithm: put
> all unoptimized .eh_frame sections first, then all optimized .eh_frame
> sections.

In the concrete scenario, that would violate the abstract principles I
described but would deliver an even better practical result.  The empty
.eh_frame section (and its __EH_FRAME_BEGIN__ symbol) from crtbeginT.o
count as "unoptimized", while the .eh_frame content from crt1.o could count
as "optimized".  So it would place the emptiness and __EH_FRAME_BEGIN__
first, and everything else (including crt1.o's contributions) after, even
though crt1.o appears before crtbeginT.o in the link.

I think either this or disabling the optimization entirely are acceptable
resolutions to the bug.


Thanks,
Roland

Re: gold vs libc

[Ian Lance Taylor]

On Mon, Mar 31, 2014 at 2:40 PM, Roland McGrath <roland@hack.frob.com> wrote:
>> On Mon, Mar 31, 2014 at 1:04 PM, Roland McGrath <roland@hack.frob.com> wrote:
>> >
>> > When an input file contains a symbol pointing to a location in an
>> > input section, the output file should define that symbol so it points
>> > to the part of the output section that corresponds to the origin input
>> > location.  When the symbol points to input contents of at least one
>> > byte, what this means is pretty incontrovertibly clear.  In this case,
>> > it points to an empty input section.  But I claim that it's adequately
>> > clear what it should mean in this case too.
>>
>> It's really not.  When the eh_frame section is being optimized, there
>> is no longer any correspondence between symbols defined in the input
>> sections and data defined in the output section.
>
> I understand that's what's going on underneath.  But the user didn't ask
> you to fiddle with his .eh_frame contents.  He did ask you to place some
> symbols in his .eh_frame section.  If .eh_frame optimization is not
> transparent, then it's broken.

The user is trying to do something special, and the linker is trying
to do something special, and the two are colliding.  The user is
defining a symbol in one file and assuming that it will name data
defined in a different file.  The linker is applying special treatment
to a particular section type.  The two uses collide.  This collision
can happen in a number of different ways.  For example, the same kind
of thing will happen for a symbol attached to no data in a SHF_MERGE
section.  Or a .note.GNU-stack section.  Or a .gnu.attributes section.
And sections can be eliminated due to ICF.  And incremental linking
can change section ordering.  So can GC and ICF.

You are assuming that the linker is simpler than it is.  In the
absence of a linker script, the linker will reshuffle sections and
symbols as it sees fit.  It makes certain obvious guarantees.  For
symbols attached to no data, the kind of handling you expect is not
one of them.


> IMHO it would be acceptable to simply disable .eh_frame optimization when
> there are any symbols in input .eh_frame sections that will survive to
> affect the output of anything except the .eh_frame output section's
> contents.  (That tortured wording is to distinguish __EH_FRAME_BEGIN__,
> whose value is used by relocs in other sections like .text, from the
> various .L* symbols used within individual input sections that are only
> used in arithmetic producing values inside the section.)  What's not
> acceptable is breaking the core semantics of linking that would apply if
> nobody had ever thought of .eh_frame optimization.

I would say that the handling of a symbol attached to no data is not
part of the core semantics of linking.

I freely grant that GCC's crtbegin.o file tries this trick in a number
of different ways--even worse, crtend.o has trailing symbols.  Because
of this existing behaviour, gold has various special cases to make it
continue to work.  One of those special cases is for
__EH_FRAME_BEGIN__.  As you've seen, the existing special case does
not work any more.  This is an unfortunate interaction.  I don't think
it's an obvious bug.


In any case I think we've identified the problem and we've identified
a fix.  Someone just needs to implement it.

Ian

Re: gold vs libc

[Cary Coutant]

Adding Rafael, since he ran into this issue again...

>> IMHO it would be acceptable to simply disable .eh_frame optimization when
>> there are any symbols in input .eh_frame sections that will survive to
>> affect the output of anything except the .eh_frame output section's
>> contents.  (That tortured wording is to distinguish __EH_FRAME_BEGIN__,
>> whose value is used by relocs in other sections like .text, from the
>> various .L* symbols used within individual input sections that are only
>> used in arithmetic producing values inside the section.)  What's not
>> acceptable is breaking the core semantics of linking that would apply if
>> nobody had ever thought of .eh_frame optimization.
>
> I would say that the handling of a symbol attached to no data is not
> part of the core semantics of linking.

I wouldn't go that far -- it's a common-enough trick that I think it
really is one of the core features. Preserving link order is important
enough that we only violate that principle when explicitly told to do
so (e.g., merge strings or constants, optimize sections, etc.). And in
the case of optimized .eh_frame data, we actually do go to some
trouble to preserve the key expectations.

> I freely grant that GCC's crtbegin.o file tries this trick in a number
> of different ways--even worse, crtend.o has trailing symbols.  Because
> of this existing behaviour, gold has various special cases to make it
> continue to work.  One of those special cases is for
> __EH_FRAME_BEGIN__.  As you've seen, the existing special case does
> not work any more.  This is an unfortunate interaction.  I don't think
> it's an obvious bug.

The problem isn't so much the bracketing symbols in crtbegin.o and
crtend.o. When gold optimizes the .eh_frame section, it tries to
preserve the order by placing the optimized contents where the first
optimizable input section appeared in the link order. Assuming that
the starting symbol actually precedes the first optimizable input
section, this produces the results you'd expect, and the optimized
data will appear between the two bracketing symbols.

When there's an optimizable input section that appears before the
bracketing symbol, it's not clear to me why the linker should be
expected to figure out what is really meant. Should we put the
optimized data in place of the second non-empty, optimizable, input
section? The third? The last?

> In any case I think we've identified the problem and we've identified
> a fix.  Someone just needs to implement it.

Have we identified a fix? From my point of view, the proper fix is to
reorder the crt files so that the __EH_FRAME_BEGIN__ symbol *precedes*
any non-empty .eh_frame sections. Why is this not possible? Don't we
all agree that it's silly to have the start symbol follow some actual
content?

If you want to disable .eh_frame optimization in this case, what are
the specific conditions under which we should not do it? When
--eh-frame-hdr is not specified? When we're doing static linking? When
we see a zero-length section with a symbol that comes after a
non-zero-length section, but isn't an end bracket? (How should we tell
the start bracket apart from the end bracket?) When we see a non-zero
length section in a file named "crt1.o"? None of these strikes me as
particularly elegant (or future-proof).

If the .eh_frame data in crt1.o really does need to come before
__EH_FRAME_BEGIN__, another thing you could do is simply make it so
gold treats it as non-optimizable. Adding a null relocation to the
first word of the section should do it; inserting a zero-length entry
anywhere but the end would do it (if that doesn't have adverse affects
elsewhere).

-cary

Re: gold vs libc

[Rafael Espíndola]

> If the .eh_frame data in crt1.o really does need to come before
> __EH_FRAME_BEGIN__, another thing you could do is simply make it so
> gold treats it as non-optimizable. Adding a null relocation to the
> first word of the section should do it; inserting a zero-length entry
> anywhere but the end would do it (if that doesn't have adverse affects
> elsewhere).

Since the problem comes from an optimizations that knows what
.eh_frame is, maybe it could learn that __EH_FRAME_BEGIN__ and
__EH_FRAME_END__ are special symbols marking the start and end of the
section?

Cheers,
Rafael

Re: gold vs libc

[Roland McGrath]

> Have we identified a fix?

I had to re-read the old discussion to come back up to speed on the
details.  See https://sourceware.org/ml/libc-alpha/2014-03/msg00971.html
for the full old thread (Ian's last message is in the April archive and so
doesn't get included in the thread link chain from there).  (Just to make
this revival of the thread a bit more self-contained so people don't have
to find all the old bits I did, I'll note here that
https://sourceware.org/bugzilla/show_bug.cgi?id=14675 is a report of this
same issue.)  In https://sourceware.org/ml/libc-alpha/2014-03/msg01012.html
I said:

    IMHO it would be acceptable to simply disable .eh_frame optimization when
    there are any symbols in input .eh_frame sections that will survive to
    affect the output of anything except the .eh_frame output section's
    contents.  (That tortured wording is to distinguish __EH_FRAME_BEGIN__,
    whose value is used by relocs in other sections like .text, from the
    various .L* symbols used within individual input sections that are only
    used in arithmetic producing values inside the section.)  What's not
    acceptable is breaking the core semantics of linking that would apply if
    nobody had ever thought of .eh_frame optimization.

and also, in response to the cited suggestion from Ian:

    > The current code has a simple algorithm that usually produces the
    > result we want: we simply copy .eh_frame sections until we find one we
    > can optimize.  Perhaps we could change to a different algorithm: put
    > all unoptimized .eh_frame sections first, then all optimized .eh_frame
    > sections.

    In the concrete scenario, that would violate the abstract principles I
    described but would deliver an even better practical result.  The empty
    .eh_frame section (and its __EH_FRAME_BEGIN__ symbol) from crtbeginT.o
    count as "unoptimized", while the .eh_frame content from crt1.o could count
    as "optimized".  So it would place the emptiness and __EH_FRAME_BEGIN__
    first, and everything else (including crt1.o's contributions) after, even
    though crt1.o appears before crtbeginT.o in the link.

    I think either this or disabling the optimization entirely are acceptable
    resolutions to the bug.

I think that when Ian said, "we've identified a fix," he was referring to,
"put all unoptimized .eh_frame sections first, then all optimized .eh_frame
sections."

> From my point of view, the proper fix is to reorder the crt files so that
> the __EH_FRAME_BEGIN__ symbol *precedes* any non-empty .eh_frame
> sections. Why is this not possible? Don't we all agree that it's silly to
> have the start symbol follow some actual content?

I think everyone agrees that putting crt1.o before crtbegin.o is
undesireable.  It's certainly possible to change that.  But it's one of
those nasty interactions between our loosely-coordinated components: it's
GCC (or another compiler driver) that both provides crtbegin.o and decides
on the link order; it's glibc (or another C library) that provides crt1.o
and decides whether it should or does contain CFI; it's the linker
(whichever one) that the compiler and libc together rely on to do The Right
Thing.

AFAIK nothing prevents us from changing GCC.  But it will be years before
we can rely on people using a GCC so changed.  That's no reason to avoid
changing GCC so that (eventually) no other fixes or workarounds will be
required; but it is a key reason why "pass the buck to GCC" is not an
acceptable resolution to the situation today.

The fundamental mindset of my part of this conversation has been that Gold
is billed as a drop-in replacement for BFD ld and this is a situation in
which it does not fit that bill.  It might well be right to call this a
"bug-compatibility" situation and argue that Gold is more Right than BFD ld
is.  But that doesn't deliver a practical solution to the real problem.

> If you want to disable .eh_frame optimization in this case, what are
> the specific conditions under which we should not do it? When
> --eh-frame-hdr is not specified? When we're doing static linking? When
> we see a zero-length section with a symbol that comes after a
> non-zero-length section, but isn't an end bracket? (How should we tell
> the start bracket apart from the end bracket?) When we see a non-zero
> length section in a file named "crt1.o"? None of these strikes me as
> particularly elegant (or future-proof).

Of the things you listed above, off hand the only thing that seems
approximately sensible to me is not to optimize when not using
--eh-frame-hdr.  (The rationale there is that --eh-frame-hdr is an explicit
request for the linker to grok .eh_frame's format completely and produce
"optimal, semantically equivalent" results with the clear expectation that
PT_GNU_EH_FRAME will be the only means of bootstrapping access to the data.
Otherwise, the implicit expectation is that the linker just behave like a
linker and paste together input sections and their symbols in their input
order.)  The other predicates you listed are indeed too picayune and
fragile.  In March I suggested a predicate that I think is the closest to
"exactly right" for the general case of things like bracket symbols; that's
cited at the top of this message.  That might well be overly fragile as
well, and the notion that people really care about optimizing the space use
of .eh_frame at all when they aren't bothering to optimize its runtime
access cost seems dubious.

> If the .eh_frame data in crt1.o really does need to come before
> __EH_FRAME_BEGIN__, another thing you could do is simply make it so
> gold treats it as non-optimizable. Adding a null relocation to the
> first word of the section should do it; inserting a zero-length entry
> anywhere but the end would do it (if that doesn't have adverse affects
> elsewhere).

It doesn't need to and in fact it's undesireable that it do so.  That is
one reason in favor of Ian's suggestion: though it doesn't adhere to any
clear abstract principle I can see, as a pragmatic solution to the actual
case in point, it magically makes something even better in practice than
the status quo ante (ante as in before switching linkers) long before a fix
for the underlying wrongness (in GCC's link order) will be available.
OTOH, it certainly seems like it has more risk of unintended consequences
we aren't now able to predict than does simply disabling optimization.

Incidentally, Ian mentioned Gold having had a special case for the
__EH_FRAME_BEGIN__ symbol.  But 'git log -G__EH_FRAME_BEGIN -- gold' finds
no point in the history where Gold's source code mentioned that symbol.  Do
you know what Ian was referring to?  From
https://sourceware.org/ml/libc-alpha/2014-04/msg00021.html:

    I freely grant that GCC's crtbegin.o file tries this trick in a number
    of different ways--even worse, crtend.o has trailing symbols.  Because
    of this existing behaviour, gold has various special cases to make it
    continue to work.  One of those special cases is for
    __EH_FRAME_BEGIN__.  As you've seen, the existing special case does
    not work any more.  This is an unfortunate interaction.  I don't think
    it's an obvious bug.

In that vein, there is Rafael's suggestion:

> Since the problem comes from an optimizations that knows what
> .eh_frame is, maybe it could learn that __EH_FRAME_BEGIN__ and
> __EH_FRAME_END__ are special symbols marking the start and end of the
> section?

I think that would be acceptable too.  It would have the same benefits of
"magically fixing" the crt1/crtbegin ordering snafu, but it seems much
simpler and thus it seems that predicting possible downsides would be
easier (I haven't thought of any).

I've said a lot in this thread about abstract principles for how a linker
should behave and how those boil down for Gold in this case, and as to
abstraction and principles I stick by everything I've said so far.  But the
bottom line is that from the glibc perspective, we'll be happy with any
solution that makes Gold deliver results as usable or more usable than BFD
ld's results for the scenario we know about without depending on GCC to
change the link order.


Thanks,
Roland

Re: gold vs libc

[Ian Lance Taylor]

On Wed, Sep 10, 2014 at 3:52 PM, Roland McGrath <roland@hack.frob.com> wrote:
>
> Incidentally, Ian mentioned Gold having had a special case for the
> __EH_FRAME_BEGIN__ symbol.  But 'git log -G__EH_FRAME_BEGIN -- gold' finds
> no point in the history where Gold's source code mentioned that symbol.  Do
> you know what Ian was referring to?  From
> https://sourceware.org/ml/libc-alpha/2014-04/msg00021.html:
>
>     I freely grant that GCC's crtbegin.o file tries this trick in a number
>     of different ways--even worse, crtend.o has trailing symbols.  Because
>     of this existing behaviour, gold has various special cases to make it
>     continue to work.  One of those special cases is for
>     __EH_FRAME_BEGIN__.  As you've seen, the existing special case does
>     not work any more.  This is an unfortunate interaction.  I don't think
>     it's an obvious bug.

I shouldn't have said __EH_FRAME_BEGIN__.  There is no special case
for the symbol.  There is a special case for the sections found in
typical crtbegin/crtend files.  It's these lines in
Eh_frame::add_ehframe_input_section in gold/ehframe.cc:

  if (contents_len == 0)
    return false;

  // If this is the marker section for the end of the data, then
  // return false to force it to be handled as an ordinary input
  // section.  If we don't do this, we won't correctly handle the case
  // of unrecognized .eh_frame sections.
  if (contents_len == 4
      && elfcpp::Swap<32, big_endian>::readval(pcontents) == 0)
    return false;

Ian

Re: gold vs libc

[Alan Modra]

On Wed, Sep 10, 2014 at 06:05:26PM -0400, Rafael Espíndola wrote:
> > If the .eh_frame data in crt1.o really does need to come before
> > __EH_FRAME_BEGIN__, another thing you could do is simply make it so
> > gold treats it as non-optimizable. Adding a null relocation to the
> > first word of the section should do it; inserting a zero-length entry
> > anywhere but the end would do it (if that doesn't have adverse affects
> > elsewhere).
> 
> Since the problem comes from an optimizations that knows what
> .eh_frame is, maybe it could learn that __EH_FRAME_BEGIN__ and
> __EH_FRAME_END__ are special symbols marking the start and end of the
> section?

I'm inclined to think this is the best solution (and made a comment to
that effect in pr17366 before reading this thread).  I don't like the
idea of making .eh_frame optimisation depend on --eh-frame-hdr.
Providing a sorted list of FDEs is really separate to optimising those
FDEs and CIEs.

-- 
Alan Modra
Australia Development Lab, IBM

Re: gold vs libc

[Cary Coutant]

> In https://sourceware.org/ml/libc-alpha/2014-03/msg01012.html
> I said:
>
>     IMHO it would be acceptable to simply disable .eh_frame optimization when
>     there are any symbols in input .eh_frame sections that will survive to
>     affect the output of anything except the .eh_frame output section's
>     contents.  (That tortured wording is to distinguish __EH_FRAME_BEGIN__,
>     whose value is used by relocs in other sections like .text, from the
>     various .L* symbols used within individual input sections that are only
>     used in arithmetic producing values inside the section.)  What's not
>     acceptable is breaking the core semantics of linking that would apply if
>     nobody had ever thought of .eh_frame optimization.

As hard as that is to describe, it seems like it would be fragile.

> and also, in response to the cited suggestion from Ian:
>
>     > The current code has a simple algorithm that usually produces the
>     > result we want: we simply copy .eh_frame sections until we find one we
>     > can optimize.  Perhaps we could change to a different algorithm: put
>     > all unoptimized .eh_frame sections first, then all optimized .eh_frame
>     > sections.
>
>     In the concrete scenario, that would violate the abstract principles I
>     described but would deliver an even better practical result.  The empty
>     .eh_frame section (and its __EH_FRAME_BEGIN__ symbol) from crtbeginT.o
>     count as "unoptimized", while the .eh_frame content from crt1.o could count
>     as "optimized".  So it would place the emptiness and __EH_FRAME_BEGIN__
>     first, and everything else (including crt1.o's contributions) after, even
>     though crt1.o appears before crtbeginT.o in the link.
>
>     I think either this or disabling the optimization entirely are acceptable
>     resolutions to the bug.
>
> I think that when Ian said, "we've identified a fix," he was referring to,
> "put all unoptimized .eh_frame sections first, then all optimized .eh_frame
> sections."

That solution will end up putting the unoptimized end-bracket section
from crtend.o before the optimized contents. I've already fixed a bug
where --sort-section made that happen. Sure, that could be fixed, but
only by piling on another special case.

What we really want to do is put the optimized sections in between the
two bracketing sections. Identifying those bracketing sections is the
challenge.

>> If the .eh_frame data in crt1.o really does need to come before
>> __EH_FRAME_BEGIN__, another thing you could do is simply make it so
>> gold treats it as non-optimizable. Adding a null relocation to the
>> first word of the section should do it; inserting a zero-length entry
>> anywhere but the end would do it (if that doesn't have adverse affects
>> elsewhere).
>
> It doesn't need to and in fact it's undesireable that it do so.  That is
> one reason in favor of Ian's suggestion: though it doesn't adhere to any
> clear abstract principle I can see, as a pragmatic solution to the actual
> case in point, it magically makes something even better in practice than
> the status quo ante (ante as in before switching linkers) long before a fix
> for the underlying wrongness (in GCC's link order) will be available.
> OTOH, it certainly seems like it has more risk of unintended consequences
> we aren't now able to predict than does simply disabling optimization.

So that's an argument against just turning off the optimization.

>> Since the problem comes from an optimizations that knows what
>> .eh_frame is, maybe it could learn that __EH_FRAME_BEGIN__ and
>> __EH_FRAME_END__ are special symbols marking the start and end of the
>> section?
>
> I think that would be acceptable too.  It would have the same benefits of
> "magically fixing" the crt1/crtbegin ordering snafu, but it seems much
> simpler and thus it seems that predicting possible downsides would be
> easier (I haven't thought of any).

I don't like the idea of checking for specific symbol name to give
special treatment to a section. I wouldn't mind simply making
__EH_FRAME_BEGIN__ and __EH_FRAME_END__ linker-defined symbols that
would override any definitions found in the object files.

I could also special case by filename -- check
is_in_system_directory(), and if true, check the filename to see if it
contains "begin" or "end".

I think I'd prefer to do the first. We have precedent for providing
linker-defined bracketing symbols -- not for having them override any
definitions already present, but I don't think that's a big problem in
this case. It's also simple, and lets us preserve the simple rule of
putting the linker-generated contents in the place of the first
optimized section.

BTW, my copy of crtend.o doesn't define __EH_FRAME_END__. It does
define __FRAME_END, but it's a local symbol. Having the linker provide
__EH_FRAME_END__ would be consistent, and shouldn't break anything.
With this proposal, __FRAME_END would get the right value anyway.
(Until, that is, someone comes along with another crtend-like file and
decides it needs CFI as well!)

-cary

Re: gold vs libc

[Cary Coutant]

> I wouldn't mind simply making
> __EH_FRAME_BEGIN__ and __EH_FRAME_END__ linker-defined symbols that
> would override any definitions found in the object files.

Ah, now I see that this is what Alan was referring to. What I didn't
like about Rafael's suggestion was the idea that the linker would
special-case the sections based on what symbols were defined in them.
If his suggestion was simply to override any definitions with the
linker's own definitions, then my objection was misfounded.

-cary

Re: gold vs libc

[Ian Lance Taylor]

On Thu, Sep 11, 2014 at 9:00 AM, Cary Coutant <ccoutant@google.com> wrote:
>
> I could also special case by filename -- check
> is_in_system_directory(), and if true, check the filename to see if it
> contains "begin" or "end".

Note that we already special case crtbegin and crtend in
Output_section::Input_section_sort_compare::operator().  This is to
emulate the GNU linker's special casing of crtbegin and crtend in
ld/scripttempl/elf.sc.

Ian

Re: gold vs libc

[Rafael Espíndola]

On 11 September 2014 12:05, Cary Coutant <ccoutant@google.com> wrote:
>> I wouldn't mind simply making
>> __EH_FRAME_BEGIN__ and __EH_FRAME_END__ linker-defined symbols that
>> would override any definitions found in the object files.
>
> Ah, now I see that this is what Alan was referring to. What I didn't
> like about Rafael's suggestion was the idea that the linker would
> special-case the sections based on what symbols were defined in them.
> If his suggestion was simply to override any definitions with the
> linker's own definitions, then my objection was misfounded.

Yes, that is what I meant. The linker would just know
__EH_FRAME_BEGIN__ to part of the format of .eh_frame.

Cheers,
Rafael

Re: gold vs libc

[Roland McGrath]

> I don't like the idea of checking for specific symbol name to give
> special treatment to a section. I wouldn't mind simply making
> __EH_FRAME_BEGIN__ and __EH_FRAME_END__ linker-defined symbols that
> would override any definitions found in the object files.

The notion as I read it was that when the linker decides to rewrite
.eh_frame data, it defines these symbols accordingly.

> I could also special case by filename -- check is_in_system_directory(),
> and if true, check the filename to see if it contains "begin" or "end".

The horror, the horror.

> BTW, my copy of crtend.o doesn't define __EH_FRAME_END__. It does
> define __FRAME_END, but it's a local symbol. Having the linker provide
> __EH_FRAME_END__ would be consistent, and shouldn't break anything.
> With this proposal, __FRAME_END would get the right value anyway.
> (Until, that is, someone comes along with another crtend-like file and
> decides it needs CFI as well!)

Nothing needs or uses an end symbol (there's an in-band terminator)
and there is no canonical name for one, so I don't think you should
define one.

In fact, __EH_FRAME_BEGIN__ is not a global symbol either.  It's not
actually a canonical name.  It's just used inside crtbeginT.o, which
defines it in .eh_frame and uses it in a reloc in .text.  There is no such
symbol around at all by link time that matters.  There is just a reloc
using the STT_SECTION symbol for .eh_frame.  (At least that's what I see on
by x86_64 system.  I've never been very clear on when the assembler uses a
specific local symbol in a reloc vs reducing it to a section symbol, and I
suspect it varies across machines.)  So the symbol idea doesn't fly.

You could instead special-case a reloc against .eh_frame+0 in an input file
with an empty .eh_frame.  Off hand, I think this might well be the only
reloc against a .eh_frame input section you will ever see.  So perhaps
really the right thing to do is verify that there is at most one reloc
against .eh_frame anywhere (and perhaps that it is against an empty input
.eh_frame section) and treat it specially if so--but either error or
disable .eh_frame optimization if there are any more relocs into .eh_frame
sections.  I can't really imagine any situation with a reloc pointing into
.eh_frame whose intended semantics wouldn't be probably broken by .eh_frame
optimization.


Thanks,
Roland

PATCH: PR gold/14675: No eh_frame info registered in exception_static_test

[H.J. Lu]

On Wed, Sep 10, 2014 at 05:04:41PM -0700, Ian Lance Taylor wrote:
> On Wed, Sep 10, 2014 at 3:52 PM, Roland McGrath <roland@hack.frob.com> wrote:
> >
> > Incidentally, Ian mentioned Gold having had a special case for the
> > __EH_FRAME_BEGIN__ symbol.  But 'git log -G__EH_FRAME_BEGIN -- gold' finds
> > no point in the history where Gold's source code mentioned that symbol.  Do
> > you know what Ian was referring to?  From
> > https://sourceware.org/ml/libc-alpha/2014-04/msg00021.html:
> >
> >     I freely grant that GCC's crtbegin.o file tries this trick in a number
> >     of different ways--even worse, crtend.o has trailing symbols.  Because
> >     of this existing behaviour, gold has various special cases to make it
> >     continue to work.  One of those special cases is for
> >     __EH_FRAME_BEGIN__.  As you've seen, the existing special case does
> >     not work any more.  This is an unfortunate interaction.  I don't think
> >     it's an obvious bug.
> 
> I shouldn't have said __EH_FRAME_BEGIN__.  There is no special case
> for the symbol.  There is a special case for the sections found in
> typical crtbegin/crtend files.  It's these lines in
> Eh_frame::add_ehframe_input_section in gold/ehframe.cc:
> 
>   if (contents_len == 0)
>     return false;
> 
>   // If this is the marker section for the end of the data, then
>   // return false to force it to be handled as an ordinary input
>   // section.  If we don't do this, we won't correctly handle the case
>   // of unrecognized .eh_frame sections.
>   if (contents_len == 4
>       && elfcpp::Swap<32, big_endian>::readval(pcontents) == 0)
>     return false;
> 
> Ian

Here is an idea.  Does it look OK?

Thanks.

H.J.
---
From 4a5d5d94ca76cbf730d7f0379601e75f9469670e Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sat, 20 Dec 2014 05:45:51 -0800
Subject: [PATCH] Treat .eh_frame section before crtbegin as normal input

Force the exception frame section from input files before the crtbegin
file to be handled as an ordinary input section if we aren't creating
the exception frame header.  If we don't do this, we won't correctly
handle the special marker symbol in the exception frame section in the
crtbegin file.

	PR gold/14675
	* ehframe.cc (Eh_frame::add_ehframe_input_section): Force the
	exception frame section from input files before the crtbegin
	file to be handled as an ordinary input section if we aren't
	creating the exception frame header.
	(Eh_frame::add_ehframe_input_section<32, false>): Updated.
	(Eh_frame::add_ehframe_input_section<32, true>): Likewise.
	(Eh_frame::add_ehframe_input_section<64, false>): Likewise.
	(Eh_frame::add_ehframe_input_section<64, true>): Likewise.
	* ehframe.h (Eh_frame::add_ehframe_input_section): Add a
	bool parameter to indicate if the crtbegin file has been
	processed.
	* layout.cc (Layout::Layout): Initialize seen_crtbegin_.
	(Layout::layout_eh_frame): Pass this->seen_crtbegin_ to
	Eh_frame::add_ehframe_input_section.
	(Layout::make_eh_frame_section): Set this->seen_crtbegin_ to
	true when processing the crtbegin file.
	* layout.h (Layout): Add a seen_crtbegin_ field.
---
 gold/ChangeLog  | 21 +++++++++++++++++++++
 gold/ehframe.cc | 34 ++++++++++++++++++++++++----------
 gold/ehframe.h  |  6 ++++--
 gold/layout.cc  | 10 +++++++++-
 gold/layout.h   |  3 +++
 5 files changed, 61 insertions(+), 13 deletions(-)

diff --git a/gold/ChangeLog b/gold/ChangeLog
index 9edf043..9cd3f8f 100644
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,3 +1,24 @@
+2014-12-20  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR gold/14675
+	* ehframe.cc (Eh_frame::add_ehframe_input_section): Force the
+	exception frame section from input files before the crtbegin
+	file to be handled as an ordinary input section if we aren't
+	creating the exception frame header.
+	(Eh_frame::add_ehframe_input_section<32, false>): Updated.
+	(Eh_frame::add_ehframe_input_section<32, true>): Likewise.
+	(Eh_frame::add_ehframe_input_section<64, false>): Likewise.
+	(Eh_frame::add_ehframe_input_section<64, true>): Likewise.
+	* ehframe.h (Eh_frame::add_ehframe_input_section): Add a
+	bool parameter to indicate if the crtbegin file has been
+	processed.
+	* layout.cc (Layout::Layout): Initialize seen_crtbegin_.
+	(Layout::layout_eh_frame): Pass this->seen_crtbegin_ to
+	Eh_frame::add_ehframe_input_section.
+	(Layout::make_eh_frame_section): Set this->seen_crtbegin_ to
+	true when processing the crtbegin file.
+	* layout.h (Layout): Add a seen_crtbegin_ field.
+
 2014-12-16  Cary Coutant  <ccoutant@google.com>
 
 	* mapfile.cc (Mapfile::print_input_section): Print uncompressed sizes.
diff --git a/gold/ehframe.cc b/gold/ehframe.cc
index c711bac..dde90df 100644
--- a/gold/ehframe.cc
+++ b/gold/ehframe.cc
@@ -576,7 +576,8 @@ Eh_frame::add_ehframe_input_section(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type)
+    unsigned int reloc_type,
+    bool crtbegin_seen)
 {
   // Get the section contents.
   section_size_type contents_len;
@@ -595,15 +596,24 @@ Eh_frame::add_ehframe_input_section(
     return false;
 
   New_cies new_cies;
-  if (!this->do_add_ehframe_input_section(object, symbols, symbols_size,
+  bool recognized_eh_frame_section
+    = this->do_add_ehframe_input_section(object, symbols, symbols_size,
 					  symbol_names, symbol_names_size,
 					  shndx, reloc_shndx,
 					  reloc_type, pcontents,
-					  contents_len, &new_cies))
+					  contents_len, &new_cies);
+  if (!recognized_eh_frame_section && this->eh_frame_hdr_ != NULL)
+    this->eh_frame_hdr_->found_unrecognized_eh_frame_section();
+
+  // If we don't unrecognize the exception frame section or we haven't
+  // seen the crtbegin file and we aren't creating the exception frame
+  // header, then return false to force it to be handled as an ordinary
+  // input section.  If we don't do this, we won't correctly handle the
+  // special marker symbol in the exception frame section in the crtbegin
+  // file.
+  if (!recognized_eh_frame_section
+      || (this->eh_frame_hdr_ == NULL && !crtbegin_seen))
     {
-      if (this->eh_frame_hdr_ != NULL)
-	this->eh_frame_hdr_->found_unrecognized_eh_frame_section();
-
       for (New_cies::iterator p = new_cies.begin();
 	   p != new_cies.end();
 	   ++p)
@@ -1224,7 +1234,8 @@ Eh_frame::add_ehframe_input_section<32, false>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool crtbegin_seen);
 #endif
 
 #ifdef HAVE_TARGET_32_BIG
@@ -1238,7 +1249,8 @@ Eh_frame::add_ehframe_input_section<32, true>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool crtbegin_seen);
 #endif
 
 #ifdef HAVE_TARGET_64_LITTLE
@@ -1252,7 +1264,8 @@ Eh_frame::add_ehframe_input_section<64, false>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool crtbegin_seen);
 #endif
 
 #ifdef HAVE_TARGET_64_BIG
@@ -1266,7 +1279,8 @@ Eh_frame::add_ehframe_input_section<64, true>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool crtbegin_seen);
 #endif
 
 } // End namespace gold.
diff --git a/gold/ehframe.h b/gold/ehframe.h
index 2ae12e0..b0effa0 100644
--- a/gold/ehframe.h
+++ b/gold/ehframe.h
@@ -371,7 +371,8 @@ class Eh_frame : public Output_section_data
   // is the relocation section if any (0 for none, -1U for multiple).
   // RELOC_TYPE is the type of the relocation section if any.  This
   // returns whether the section was incorporated into the .eh_frame
-  // data.
+  // data.  CRTBEGIN_SEEN is true if the crtbegin file has been
+  // processed.
   template<int size, bool big_endian>
   bool
   add_ehframe_input_section(Sized_relobj_file<size, big_endian>* object,
@@ -380,7 +381,8 @@ class Eh_frame : public Output_section_data
 			    const unsigned char* symbol_names,
 			    section_size_type symbol_names_size,
 			    unsigned int shndx, unsigned int reloc_shndx,
-			    unsigned int reloc_type);
+			    unsigned int reloc_type,
+			    bool crtbegin_seen);
 
   // Add a CIE and an FDE for a PLT section, to permit unwinding
   // through a PLT.  The FDE data should start with 8 bytes of zero,
diff --git a/gold/layout.cc b/gold/layout.cc
index 0a71a2a..6649f9f 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -469,6 +469,7 @@ Layout::Layout(int number_of_input_files, Script_options* script_options)
     unique_segment_for_sections_specified_(false),
     incremental_inputs_(NULL),
     record_output_section_data_from_script_(false),
+    seen_crtbegin_(false),
     script_output_section_data_list_(),
     segment_states_(NULL),
     relaxation_debug_check_(NULL),
@@ -1428,7 +1429,8 @@ Layout::layout_eh_frame(Sized_relobj_file<size, big_endian>* object,
 							 symbol_names_size,
 							 shndx,
 							 reloc_shndx,
-							 reloc_type))
+							 reloc_type,
+							 this->seen_crtbegin_))
     {
       os->update_flags_for_input_section(shdr.get_sh_flags());
 
@@ -1485,6 +1487,12 @@ Layout::make_eh_frame_section(const Relobj* object)
   if (os == NULL)
     return NULL;
 
+  if (object != NULL && Layout::match_file_name(object, "crtbegin"))
+    {
+      gold_assert(!this->seen_crtbegin_);
+      this->seen_crtbegin_ = true;
+    }
+
   if (this->eh_frame_section_ == NULL)
     {
       this->eh_frame_section_ = os;
diff --git a/gold/layout.h b/gold/layout.h
index 032f5f3..74e5ec1 100644
--- a/gold/layout.h
+++ b/gold/layout.h
@@ -1420,6 +1420,9 @@ class Layout
   Incremental_inputs* incremental_inputs_;
   // Whether we record output section data created in script
   bool record_output_section_data_from_script_;
+  // Whether we have seen the crtbegin file when processing the exception
+  // frame sections.
+  bool seen_crtbegin_;
   // List of output data that needs to be removed at relaxation clean up.
   Output_section_data_list script_output_section_data_list_;
   // Structure to save segment states before entering the relaxation loop.
-- 
1.9.3

Re: PATCH: PR gold/14675: No eh_frame info registered in exception_static_test

[Cary Coutant]

> Here is an idea.  Does it look OK?

I don't like the idea of making a file named "crtbegin" magic, and
with this patch, any link *without* such a file will no longer
optimize eh data.

-cary

Re: PATCH: PR gold/14675: No eh_frame info registered in exception_static_test

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 1870 bytes --]

On Mon, Dec 22, 2014 at 9:37 AM, Cary Coutant <ccoutant@google.com> wrote:
>> Here is an idea.  Does it look OK?
>
> I don't like the idea of making a file named "crtbegin" magic, and
> with this patch, any link *without* such a file will no longer
> optimize eh data.
>
> -cary

How about this?  We do this only when there is a crtbeginT file
on command line?

-- 
H.J.
---
PR gold/14675
* ehframe.cc (Eh_frame::add_ehframe_input_section): Force the
exception frame section from input files if it can't be
optimized.
(Eh_frame::add_ehframe_input_section<32, false>): Updated.
(Eh_frame::add_ehframe_input_section<32, true>): Likewise.
(Eh_frame::add_ehframe_input_section<64, false>): Likewise.
(Eh_frame::add_ehframe_input_section<64, true>): Likewise.
* ehframe.h (Eh_frame::add_ehframe_input_section): Add a
bool parameter to indicate if the exception frame section
can be optimized.
* layout.cc (Layout::Layout): Initialize has_crtbeginT_ to
has_crtbeginT and optimize_ehframe_ to !has_crtbeginT.
(Layout::layout_eh_frame): Pass this->optimize_ehframe_ to
Eh_frame::add_ehframe_input_section.
(Layout::make_eh_frame_section): Set this->optimize_ehframe_
to true when processing the crtbeginT file if it is on command
line.
(Layout::match_file_name (const char*, const char*)): New.
(Layout::match_file_name(const Relobj*, const char*): Use it.
* layout.h (Layout::Layout): Add has_crtbeginT.
(Layout::match_file_name (const char*, const char*)): New.
(Layout): Add has_crtbeginT_ and optimize_ehframe_ members.
* main.cc (main): Update layout.
* options.cc (Input_arguments::add_file): Set
this->has_crtbeginT_ to true if there is a crtbeginT file.
* options.h (Input_arguments::Input_arguments): Initialize
has_crtbeginT_.
(Input_arguments::has_crtbeginT): New function.
(Input_arguments::has_crtbeginT_): New bool member.
(Command_line::crtbeginT): New function.

[-- Attachment #2: 0001-Treat-.eh_frame-section-before-crtbeginT-as-normal-i.patch --]
[-- Type: text/x-patch, Size: 15240 bytes --]

From b4e73bc925dfff0da1fcca6159a9b0c5d7792ea9 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sat, 20 Dec 2014 05:45:51 -0800
Subject: [PATCH] Treat .eh_frame section before crtbeginT as normal input

Force the exception frame section from input files before the crtbeginT
file to be handled as an ordinary input section if we aren't creating
the exception frame header.  If we don't do this, we won't correctly
handle the special marker symbol in the exception frame section in the
crtbeginT file.

	PR gold/14675
	* ehframe.cc (Eh_frame::add_ehframe_input_section): Force the
	exception frame section from input files if it can't be
	optimized.
	(Eh_frame::add_ehframe_input_section<32, false>): Updated.
	(Eh_frame::add_ehframe_input_section<32, true>): Likewise.
	(Eh_frame::add_ehframe_input_section<64, false>): Likewise.
	(Eh_frame::add_ehframe_input_section<64, true>): Likewise.
	* ehframe.h (Eh_frame::add_ehframe_input_section): Add a
	bool parameter to indicate if the exception frame section
	can be optimized.
	* layout.cc (Layout::Layout): Initialize has_crtbeginT_ to
	has_crtbeginT and optimize_ehframe_ to !has_crtbeginT.
	(Layout::layout_eh_frame): Pass this->optimize_ehframe_ to
	Eh_frame::add_ehframe_input_section.
	(Layout::make_eh_frame_section): Set this->optimize_ehframe_
	to true when processing the crtbeginT file if it is on command
	line.
	(Layout::match_file_name (const char*, const char*)): New.
	(Layout::match_file_name(const Relobj*, const char*): Use it.
	* layout.h (Layout::Layout): Add has_crtbeginT.
	(Layout::match_file_name (const char*, const char*)): New.
	(Layout): Add has_crtbeginT_ and optimize_ehframe_ members.
	* main.cc (main): Update layout.
	* options.cc (Input_arguments::add_file): Set
	this->has_crtbeginT_ to true if there is a crtbeginT file.
	* options.h (Input_arguments::Input_arguments): Initialize
	has_crtbeginT_.
	(Input_arguments::has_crtbeginT): New function.
	(Input_arguments::has_crtbeginT_): New bool member.
	(Command_line::crtbeginT): New function.
---
 gold/ChangeLog  | 34 ++++++++++++++++++++++++++++++++++
 gold/ehframe.cc | 31 +++++++++++++++++++++----------
 gold/ehframe.h  |  6 ++++--
 gold/layout.cc  | 35 +++++++++++++++++++++++++++--------
 gold/layout.h   | 12 +++++++++++-
 gold/main.cc    |  3 ++-
 gold/options.cc |  4 ++++
 gold/options.h  | 15 ++++++++++++++-
 8 files changed, 117 insertions(+), 23 deletions(-)

diff --git a/gold/ChangeLog b/gold/ChangeLog
index f67df17..1e846c9 100644
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,3 +1,37 @@
+2014-12-22  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR gold/14675
+	* ehframe.cc (Eh_frame::add_ehframe_input_section): Force the
+	exception frame section from input files if it can't be
+	optimized.
+	(Eh_frame::add_ehframe_input_section<32, false>): Updated.
+	(Eh_frame::add_ehframe_input_section<32, true>): Likewise.
+	(Eh_frame::add_ehframe_input_section<64, false>): Likewise.
+	(Eh_frame::add_ehframe_input_section<64, true>): Likewise.
+	* ehframe.h (Eh_frame::add_ehframe_input_section): Add a
+	bool parameter to indicate if the exception frame section
+	can be optimized.
+	* layout.cc (Layout::Layout): Initialize has_crtbeginT_ to
+	has_crtbeginT and optimize_ehframe_ to !has_crtbeginT.
+	(Layout::layout_eh_frame): Pass this->optimize_ehframe_ to
+	Eh_frame::add_ehframe_input_section.
+	(Layout::make_eh_frame_section): Set this->optimize_ehframe_
+	to true when processing the crtbeginT file if it is on command
+	line.
+	(Layout::match_file_name (const char*, const char*)): New.
+	(Layout::match_file_name(const Relobj*, const char*): Use it.
+	* layout.h (Layout::Layout): Add has_crtbeginT.
+	(Layout::match_file_name (const char*, const char*)): New.
+	(Layout): Add has_crtbeginT_ and optimize_ehframe_ members.
+	* main.cc (main): Update layout.
+	* options.cc (Input_arguments::add_file): Set
+	this->has_crtbeginT_ to true if there is a crtbeginT file.
+	* options.h (Input_arguments::Input_arguments): Initialize
+	has_crtbeginT_.
+	(Input_arguments::has_crtbeginT): New function.
+	(Input_arguments::has_crtbeginT_): New bool member.
+	(Command_line::crtbeginT): New function.
+
 2014-12-20  H.J. Lu  <hongjiu.lu@intel.com>
 
 	PR gold/14608
diff --git a/gold/ehframe.cc b/gold/ehframe.cc
index c711bac..ce3741b 100644
--- a/gold/ehframe.cc
+++ b/gold/ehframe.cc
@@ -576,7 +576,8 @@ Eh_frame::add_ehframe_input_section(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type)
+    unsigned int reloc_type,
+    bool optimize_ehframe)
 {
   // Get the section contents.
   section_size_type contents_len;
@@ -595,15 +596,21 @@ Eh_frame::add_ehframe_input_section(
     return false;
 
   New_cies new_cies;
-  if (!this->do_add_ehframe_input_section(object, symbols, symbols_size,
+  bool recognized_eh_frame_section
+    = this->do_add_ehframe_input_section(object, symbols, symbols_size,
 					  symbol_names, symbol_names_size,
 					  shndx, reloc_shndx,
 					  reloc_type, pcontents,
-					  contents_len, &new_cies))
+					  contents_len, &new_cies);
+  if (!recognized_eh_frame_section && this->eh_frame_hdr_ != NULL)
+    this->eh_frame_hdr_->found_unrecognized_eh_frame_section();
+
+  // If we don't unrecognize the exception frame section or it can't
+  // be optimized, then return false to force it to be handled as an
+  // ordinary input section.
+  if (!recognized_eh_frame_section
+      || (this->eh_frame_hdr_ == NULL && !optimize_ehframe))
     {
-      if (this->eh_frame_hdr_ != NULL)
-	this->eh_frame_hdr_->found_unrecognized_eh_frame_section();
-
       for (New_cies::iterator p = new_cies.begin();
 	   p != new_cies.end();
 	   ++p)
@@ -1224,7 +1231,8 @@ Eh_frame::add_ehframe_input_section<32, false>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool optimize_ehframe);
 #endif
 
 #ifdef HAVE_TARGET_32_BIG
@@ -1238,7 +1246,8 @@ Eh_frame::add_ehframe_input_section<32, true>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool optimize_ehframe);
 #endif
 
 #ifdef HAVE_TARGET_64_LITTLE
@@ -1252,7 +1261,8 @@ Eh_frame::add_ehframe_input_section<64, false>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool optimize_ehframe);
 #endif
 
 #ifdef HAVE_TARGET_64_BIG
@@ -1266,7 +1276,8 @@ Eh_frame::add_ehframe_input_section<64, true>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool optimize_ehframe);
 #endif
 
 } // End namespace gold.
diff --git a/gold/ehframe.h b/gold/ehframe.h
index 2ae12e0..93f7268 100644
--- a/gold/ehframe.h
+++ b/gold/ehframe.h
@@ -371,7 +371,8 @@ class Eh_frame : public Output_section_data
   // is the relocation section if any (0 for none, -1U for multiple).
   // RELOC_TYPE is the type of the relocation section if any.  This
   // returns whether the section was incorporated into the .eh_frame
-  // data.
+  // data.  OPTIMIZE_EHFRAME is true if the exception frame section
+  // can be optimized.
   template<int size, bool big_endian>
   bool
   add_ehframe_input_section(Sized_relobj_file<size, big_endian>* object,
@@ -380,7 +381,8 @@ class Eh_frame : public Output_section_data
 			    const unsigned char* symbol_names,
 			    section_size_type symbol_names_size,
 			    unsigned int shndx, unsigned int reloc_shndx,
-			    unsigned int reloc_type);
+			    unsigned int reloc_type,
+			    bool optimize_ehframe);
 
   // Add a CIE and an FDE for a PLT section, to permit unwinding
   // through a PLT.  The FDE data should start with 8 bytes of zero,
diff --git a/gold/layout.cc b/gold/layout.cc
index 0a71a2a..05873b5 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -418,7 +418,8 @@ Layout_task_runner::run(Workqueue* workqueue, const Task* task)
 
 // Layout methods.
 
-Layout::Layout(int number_of_input_files, Script_options* script_options)
+Layout::Layout(int number_of_input_files, Script_options* script_options,
+	       bool has_crtbeginT)
   : number_of_input_files_(number_of_input_files),
     script_options_(script_options),
     namepool_(),
@@ -469,6 +470,8 @@ Layout::Layout(int number_of_input_files, Script_options* script_options)
     unique_segment_for_sections_specified_(false),
     incremental_inputs_(NULL),
     record_output_section_data_from_script_(false),
+    has_crtbeginT_(has_crtbeginT),
+    optimize_ehframe_(!has_crtbeginT),
     script_output_section_data_list_(),
     segment_states_(NULL),
     relaxation_debug_check_(NULL),
@@ -1428,7 +1431,8 @@ Layout::layout_eh_frame(Sized_relobj_file<size, big_endian>* object,
 							 symbol_names_size,
 							 shndx,
 							 reloc_shndx,
-							 reloc_type))
+							 reloc_type,
+							 this->optimize_ehframe_))
     {
       os->update_flags_for_input_section(shdr.get_sh_flags());
 
@@ -1485,6 +1489,11 @@ Layout::make_eh_frame_section(const Relobj* object)
   if (os == NULL)
     return NULL;
 
+  if (object != NULL
+      && this->has_crtbeginT_
+      && Layout::match_file_name(object, "crtbeginT"))
+    this->optimize_ehframe_ = true;
+
   if (this->eh_frame_section_ == NULL)
     {
       this->eh_frame_section_ = os;
@@ -5092,19 +5101,18 @@ Layout::output_section_name(const Relobj* relobj, const char* name,
   return name;
 }
 
-// Return true if RELOBJ is an input file whose base name matches
-// FILE_NAME.  The base name must have an extension of ".o", and must
-// be exactly FILE_NAME.o or FILE_NAME, one character, ".o".  This is
+// Return true if FILE is an input file whose base name matches
+// MATCH.  The base name must have an extension of ".o", and must
+// be exactly MATCH.o or MATCH, one character, ".o".  This is
 // to match crtbegin.o as well as crtbeginS.o without getting confused
 // by other possibilities.  Overall matching the file name this way is
 // a dreadful hack, but the GNU linker does it in order to better
 // support gcc, and we need to be compatible.
 
 bool
-Layout::match_file_name(const Relobj* relobj, const char* match)
+Layout::match_file_name(const char* file, const char* match)
 {
-  const std::string& file_name(relobj->name());
-  const char* base_name = lbasename(file_name.c_str());
+  const char* base_name = lbasename(file);
   size_t match_len = strlen(match);
   if (strncmp(base_name, match, match_len) != 0)
     return false;
@@ -5114,6 +5122,17 @@ Layout::match_file_name(const Relobj* relobj, const char* match)
   return memcmp(base_name + base_len - 2, ".o", 2) == 0;
 }
 
+// Return true if FILE is an input file whose base name matches
+// MATCH.  The base name must have an extension of ".o", and must
+// be exactly MATCH.o or MATCH, one character, ".o".
+
+bool
+Layout::match_file_name(const Relobj* relobj, const char* match)
+{
+  const std::string& file_name(relobj->name());
+  return Layout::match_file_name(file_name.c_str(), match);
+}
+
 // Check if a comdat group or .gnu.linkonce section with the given
 // NAME is selected for the link.  If there is already a section,
 // *KEPT_SECTION is set to point to the existing section and the
diff --git a/gold/layout.h b/gold/layout.h
index 032f5f3..02ebf1f 100644
--- a/gold/layout.h
+++ b/gold/layout.h
@@ -488,7 +488,7 @@ enum Output_section_order
 class Layout
 {
  public:
-  Layout(int number_of_input_files, Script_options*);
+  Layout(int number_of_input_files, Script_options*, bool has_crtbeginT);
 
   ~Layout()
   {
@@ -757,6 +757,12 @@ class Layout
 	    || strncmp(name, ".stab", sizeof(".stab") - 1) == 0);
   }
 
+  // Return true if FILE is an input file whose base name matches
+  // FILE_NAME.  The base name must have an extension of ".o", and
+  // must be exactly FILE_NAME.o or FILE_NAME, one character, ".o".
+  static bool
+  match_file_name(const char* file, const char* file_name);
+
   // Return true if RELOBJ is an input file whose base name matches
   // FILE_NAME.  The base name must have an extension of ".o", and
   // must be exactly FILE_NAME.o or FILE_NAME, one character, ".o".
@@ -1420,6 +1426,10 @@ class Layout
   Incremental_inputs* incremental_inputs_;
   // Whether we record output section data created in script
   bool record_output_section_data_from_script_;
+  // Whether there is a crtbeginT file.
+  bool has_crtbeginT_;
+  // Whether to optimize the exception frame section.
+  bool optimize_ehframe_;
   // List of output data that needs to be removed at relaxation clean up.
   Output_section_data_list script_output_section_data_list_;
   // Structure to save segment states before entering the relaxation loop.
diff --git a/gold/main.cc b/gold/main.cc
index bb86613..bbeff65 100644
--- a/gold/main.cc
+++ b/gold/main.cc
@@ -227,7 +227,8 @@ main(int argc, char** argv)
 
   // The layout object.
   Layout layout(command_line.number_of_input_files(),
-		&command_line.script_options());
+		&command_line.script_options(),
+		command_line.crtbeginT());
 
   if (layout.incremental_inputs() != NULL)
     layout.incremental_inputs()->report_command_line(argc, argv);
diff --git a/gold/options.cc b/gold/options.cc
index 731061d..cf7af2d 100644
--- a/gold/options.cc
+++ b/gold/options.cc
@@ -38,6 +38,7 @@
 #include "script.h"
 #include "target-select.h"
 #include "options.h"
+#include "layout.h"
 #include "plugin.h"
 
 namespace gold
@@ -1346,6 +1347,9 @@ Input_arguments::add_file(Input_file_argument& file)
       return this->input_argument_list_.back().lib()->add_file(file);
     }
   this->input_argument_list_.push_back(Input_argument(file));
+  if (!this->has_crtbeginT_
+      && Layout::match_file_name(file.name(), "crtbeginT"))
+    this->has_crtbeginT_ = true;
   return this->input_argument_list_.back();
 }
 
diff --git a/gold/options.h b/gold/options.h
index 6d827f1..7e881df 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -1970,7 +1970,8 @@ class Input_arguments
   typedef Input_argument_list::const_iterator const_iterator;
 
   Input_arguments()
-    : input_argument_list_(), in_group_(false), in_lib_(false), file_count_(0)
+    : input_argument_list_(), in_group_(false), in_lib_(false),
+      file_count_(0), has_crtbeginT_(false)
   { }
 
   // Add a file.
@@ -2030,11 +2031,18 @@ class Input_arguments
   number_of_input_files() const
   { return this->file_count_; }
 
+  // Return whether there is a crtbeginT file.
+  bool
+  has_crtbeginT() const
+  { return this->has_crtbeginT_; }
+
  private:
   Input_argument_list input_argument_list_;
   bool in_group_;
   bool in_lib_;
   unsigned int file_count_;
+  // Whether there is a crtbeginT file.
+  bool has_crtbeginT_;
 };
 
 
@@ -2103,6 +2111,11 @@ class Command_line
   end() const
   { return this->inputs_.end(); }
 
+  // Whether there is a crtbeginT file.
+  bool
+  crtbeginT() const
+  { return this->inputs_.has_crtbeginT(); }
+
  private:
   Command_line(const Command_line&);
   Command_line& operator=(const Command_line&);
-- 
1.9.3

Re: PATCH: PR gold/14675: No eh_frame info registered in exception_static_test

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 2148 bytes --]

On Mon, Dec 22, 2014 at 2:40 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Dec 22, 2014 at 9:37 AM, Cary Coutant <ccoutant@google.com> wrote:
>>> Here is an idea.  Does it look OK?
>>
>> I don't like the idea of making a file named "crtbegin" magic, and
>> with this patch, any link *without* such a file will no longer
>> optimize eh data.
>>
>> -cary
>
> How about this?  We do this only when there is a crtbeginT file
> on command line?
>
> --
> H.J.
> ---
> PR gold/14675
> * ehframe.cc (Eh_frame::add_ehframe_input_section): Force the
> exception frame section from input files if it can't be
> optimized.
> (Eh_frame::add_ehframe_input_section<32, false>): Updated.
> (Eh_frame::add_ehframe_input_section<32, true>): Likewise.
> (Eh_frame::add_ehframe_input_section<64, false>): Likewise.
> (Eh_frame::add_ehframe_input_section<64, true>): Likewise.
> * ehframe.h (Eh_frame::add_ehframe_input_section): Add a
> bool parameter to indicate if the exception frame section
> can be optimized.
> * layout.cc (Layout::Layout): Initialize has_crtbeginT_ to
> has_crtbeginT and optimize_ehframe_ to !has_crtbeginT.
> (Layout::layout_eh_frame): Pass this->optimize_ehframe_ to
> Eh_frame::add_ehframe_input_section.
> (Layout::make_eh_frame_section): Set this->optimize_ehframe_
> to true when processing the crtbeginT file if it is on command
> line.
> (Layout::match_file_name (const char*, const char*)): New.
> (Layout::match_file_name(const Relobj*, const char*): Use it.
> * layout.h (Layout::Layout): Add has_crtbeginT.
> (Layout::match_file_name (const char*, const char*)): New.
> (Layout): Add has_crtbeginT_ and optimize_ehframe_ members.
> * main.cc (main): Update layout.
> * options.cc (Input_arguments::add_file): Set
> this->has_crtbeginT_ to true if there is a crtbeginT file.
> * options.h (Input_arguments::Input_arguments): Initialize
> has_crtbeginT_.
> (Input_arguments::has_crtbeginT): New function.
> (Input_arguments::has_crtbeginT_): New bool member.
> (Command_line::crtbeginT): New function.

Slightly updated patch.  We don't need to add a has_crtbeginT_
member to Layout.  optimize_ehframe_ is sufficient.

-- 
H.J.

[-- Attachment #2: 0001-Treat-.eh_frame-section-before-crtbeginT-as-normal-i.patch --]
[-- Type: text/x-patch, Size: 15333 bytes --]

From cade3c669933278d78d5a8900458324aadd68b14 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sat, 20 Dec 2014 05:45:51 -0800
Subject: [PATCH] Treat .eh_frame section before crtbeginT as normal input

Force the exception frame section from input files before the crtbeginT
file to be handled as an ordinary input section if we aren't creating
the exception frame header.  If we don't do this, we won't correctly
handle the special marker symbol in the exception frame section in the
crtbeginT file.

	PR gold/14675
	* ehframe.cc (Eh_frame::add_ehframe_input_section): Force the
	exception frame section from input files if it can't be
	optimized.
	(Eh_frame::add_ehframe_input_section<32, false>): Updated.
	(Eh_frame::add_ehframe_input_section<32, true>): Likewise.
	(Eh_frame::add_ehframe_input_section<64, false>): Likewise.
	(Eh_frame::add_ehframe_input_section<64, true>): Likewise.
	* ehframe.h (Eh_frame::add_ehframe_input_section): Add a
	bool parameter to indicate if the exception frame section
	can be optimized.
	* layout.cc (Layout::Layout): Initialize optimize_ehframe_ to
	!has_crtbeginT.
	(Layout::layout_eh_frame): Pass this->optimize_ehframe_ to
	Eh_frame::add_ehframe_input_section.
	(Layout::make_eh_frame_section): Set this->optimize_ehframe_
	to true when processing the crtbeginT file if it is on command
	line.
	(Layout::match_file_name (const char*, const char*)): New.
	(Layout::match_file_name(const Relobj*, const char*): Use it.
	* layout.h (Layout::Layout): Add has_crtbeginT.
	(Layout::match_file_name (const char*, const char*)): New.
	(Layout): Add an optimize_ehframe_ member.
	* main.cc (main): Update layout.
	* options.cc: Include "layout.h".
	(Input_arguments::add_file): Set this->has_crtbeginT_ to true
	if there is a crtbeginT file.
	* options.h (Input_arguments::Input_arguments): Initialize
	has_crtbeginT_ to false.
	(Input_arguments::has_crtbeginT): New function.
	(Input_arguments::has_crtbeginT_): New bool member.
	(Command_line::crtbeginT): New function.
---
 gold/ChangeLog  | 35 +++++++++++++++++++++++++++++++++++
 gold/ehframe.cc | 31 +++++++++++++++++++++----------
 gold/ehframe.h  |  6 ++++--
 gold/layout.cc  | 37 +++++++++++++++++++++++++++++--------
 gold/layout.h   | 10 +++++++++-
 gold/main.cc    |  3 ++-
 gold/options.cc |  4 ++++
 gold/options.h  | 15 ++++++++++++++-
 8 files changed, 118 insertions(+), 23 deletions(-)

diff --git a/gold/ChangeLog b/gold/ChangeLog
index 457fa6b..ee56b98 100644
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,3 +1,38 @@
+2014-12-22  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR gold/14675
+	* ehframe.cc (Eh_frame::add_ehframe_input_section): Force the
+	exception frame section from input files if it can't be
+	optimized.
+	(Eh_frame::add_ehframe_input_section<32, false>): Updated.
+	(Eh_frame::add_ehframe_input_section<32, true>): Likewise.
+	(Eh_frame::add_ehframe_input_section<64, false>): Likewise.
+	(Eh_frame::add_ehframe_input_section<64, true>): Likewise.
+	* ehframe.h (Eh_frame::add_ehframe_input_section): Add a
+	bool parameter to indicate if the exception frame section
+	can be optimized.
+	* layout.cc (Layout::Layout): Initialize optimize_ehframe_ to
+	!has_crtbeginT.
+	(Layout::layout_eh_frame): Pass this->optimize_ehframe_ to
+	Eh_frame::add_ehframe_input_section.
+	(Layout::make_eh_frame_section): Set this->optimize_ehframe_
+	to true when processing the crtbeginT file if it is on command
+	line.
+	(Layout::match_file_name (const char*, const char*)): New.
+	(Layout::match_file_name(const Relobj*, const char*): Use it.
+	* layout.h (Layout::Layout): Add has_crtbeginT.
+	(Layout::match_file_name (const char*, const char*)): New.
+	(Layout): Add an optimize_ehframe_ member.
+	* main.cc (main): Update layout.
+	* options.cc: Include "layout.h".
+	(Input_arguments::add_file): Set this->has_crtbeginT_ to true
+	if there is a crtbeginT file.
+	* options.h (Input_arguments::Input_arguments): Initialize
+	has_crtbeginT_ to false.
+	(Input_arguments::has_crtbeginT): New function.
+	(Input_arguments::has_crtbeginT_): New bool member.
+	(Command_line::crtbeginT): New function.
+
 2014-12-22  Cary Coutant  <ccoutant@google.com>
 
 	* powerpc.cc (Target_powerpc::relocate): Fix overflow check.
diff --git a/gold/ehframe.cc b/gold/ehframe.cc
index c711bac..ce3741b 100644
--- a/gold/ehframe.cc
+++ b/gold/ehframe.cc
@@ -576,7 +576,8 @@ Eh_frame::add_ehframe_input_section(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type)
+    unsigned int reloc_type,
+    bool optimize_ehframe)
 {
   // Get the section contents.
   section_size_type contents_len;
@@ -595,15 +596,21 @@ Eh_frame::add_ehframe_input_section(
     return false;
 
   New_cies new_cies;
-  if (!this->do_add_ehframe_input_section(object, symbols, symbols_size,
+  bool recognized_eh_frame_section
+    = this->do_add_ehframe_input_section(object, symbols, symbols_size,
 					  symbol_names, symbol_names_size,
 					  shndx, reloc_shndx,
 					  reloc_type, pcontents,
-					  contents_len, &new_cies))
+					  contents_len, &new_cies);
+  if (!recognized_eh_frame_section && this->eh_frame_hdr_ != NULL)
+    this->eh_frame_hdr_->found_unrecognized_eh_frame_section();
+
+  // If we don't unrecognize the exception frame section or it can't
+  // be optimized, then return false to force it to be handled as an
+  // ordinary input section.
+  if (!recognized_eh_frame_section
+      || (this->eh_frame_hdr_ == NULL && !optimize_ehframe))
     {
-      if (this->eh_frame_hdr_ != NULL)
-	this->eh_frame_hdr_->found_unrecognized_eh_frame_section();
-
       for (New_cies::iterator p = new_cies.begin();
 	   p != new_cies.end();
 	   ++p)
@@ -1224,7 +1231,8 @@ Eh_frame::add_ehframe_input_section<32, false>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool optimize_ehframe);
 #endif
 
 #ifdef HAVE_TARGET_32_BIG
@@ -1238,7 +1246,8 @@ Eh_frame::add_ehframe_input_section<32, true>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool optimize_ehframe);
 #endif
 
 #ifdef HAVE_TARGET_64_LITTLE
@@ -1252,7 +1261,8 @@ Eh_frame::add_ehframe_input_section<64, false>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool optimize_ehframe);
 #endif
 
 #ifdef HAVE_TARGET_64_BIG
@@ -1266,7 +1276,8 @@ Eh_frame::add_ehframe_input_section<64, true>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool optimize_ehframe);
 #endif
 
 } // End namespace gold.
diff --git a/gold/ehframe.h b/gold/ehframe.h
index 2ae12e0..93f7268 100644
--- a/gold/ehframe.h
+++ b/gold/ehframe.h
@@ -371,7 +371,8 @@ class Eh_frame : public Output_section_data
   // is the relocation section if any (0 for none, -1U for multiple).
   // RELOC_TYPE is the type of the relocation section if any.  This
   // returns whether the section was incorporated into the .eh_frame
-  // data.
+  // data.  OPTIMIZE_EHFRAME is true if the exception frame section
+  // can be optimized.
   template<int size, bool big_endian>
   bool
   add_ehframe_input_section(Sized_relobj_file<size, big_endian>* object,
@@ -380,7 +381,8 @@ class Eh_frame : public Output_section_data
 			    const unsigned char* symbol_names,
 			    section_size_type symbol_names_size,
 			    unsigned int shndx, unsigned int reloc_shndx,
-			    unsigned int reloc_type);
+			    unsigned int reloc_type,
+			    bool optimize_ehframe);
 
   // Add a CIE and an FDE for a PLT section, to permit unwinding
   // through a PLT.  The FDE data should start with 8 bytes of zero,
diff --git a/gold/layout.cc b/gold/layout.cc
index 0a71a2a..a723d03 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -418,7 +418,8 @@ Layout_task_runner::run(Workqueue* workqueue, const Task* task)
 
 // Layout methods.
 
-Layout::Layout(int number_of_input_files, Script_options* script_options)
+Layout::Layout(int number_of_input_files, Script_options* script_options,
+	       bool has_crtbeginT)
   : number_of_input_files_(number_of_input_files),
     script_options_(script_options),
     namepool_(),
@@ -469,6 +470,7 @@ Layout::Layout(int number_of_input_files, Script_options* script_options)
     unique_segment_for_sections_specified_(false),
     incremental_inputs_(NULL),
     record_output_section_data_from_script_(false),
+    optimize_ehframe_(!has_crtbeginT),
     script_output_section_data_list_(),
     segment_states_(NULL),
     relaxation_debug_check_(NULL),
@@ -1428,7 +1430,8 @@ Layout::layout_eh_frame(Sized_relobj_file<size, big_endian>* object,
 							 symbol_names_size,
 							 shndx,
 							 reloc_shndx,
-							 reloc_type))
+							 reloc_type,
+							 this->optimize_ehframe_))
     {
       os->update_flags_for_input_section(shdr.get_sh_flags());
 
@@ -1485,6 +1488,14 @@ Layout::make_eh_frame_section(const Relobj* object)
   if (os == NULL)
     return NULL;
 
+  // optimize_ehframe_ is false only if there is a crtbeginT file on
+  // command line.  We can't optimize the exception frame section
+  // until we have seen the crtbeginT file.
+  if (!this->optimize_ehframe_
+      && object != NULL
+      && Layout::match_file_name(object, "crtbeginT"))
+    this->optimize_ehframe_ = true;
+
   if (this->eh_frame_section_ == NULL)
     {
       this->eh_frame_section_ = os;
@@ -5092,19 +5103,18 @@ Layout::output_section_name(const Relobj* relobj, const char* name,
   return name;
 }
 
-// Return true if RELOBJ is an input file whose base name matches
-// FILE_NAME.  The base name must have an extension of ".o", and must
-// be exactly FILE_NAME.o or FILE_NAME, one character, ".o".  This is
+// Return true if FILE is an input file whose base name matches
+// MATCH.  The base name must have an extension of ".o", and must
+// be exactly MATCH.o or MATCH, one character, ".o".  This is
 // to match crtbegin.o as well as crtbeginS.o without getting confused
 // by other possibilities.  Overall matching the file name this way is
 // a dreadful hack, but the GNU linker does it in order to better
 // support gcc, and we need to be compatible.
 
 bool
-Layout::match_file_name(const Relobj* relobj, const char* match)
+Layout::match_file_name(const char* file, const char* match)
 {
-  const std::string& file_name(relobj->name());
-  const char* base_name = lbasename(file_name.c_str());
+  const char* base_name = lbasename(file);
   size_t match_len = strlen(match);
   if (strncmp(base_name, match, match_len) != 0)
     return false;
@@ -5114,6 +5124,17 @@ Layout::match_file_name(const Relobj* relobj, const char* match)
   return memcmp(base_name + base_len - 2, ".o", 2) == 0;
 }
 
+// Return true if FILE is an input file whose base name matches
+// MATCH.  The base name must have an extension of ".o", and must
+// be exactly MATCH.o or MATCH, one character, ".o".
+
+bool
+Layout::match_file_name(const Relobj* relobj, const char* match)
+{
+  const std::string& file_name(relobj->name());
+  return Layout::match_file_name(file_name.c_str(), match);
+}
+
 // Check if a comdat group or .gnu.linkonce section with the given
 // NAME is selected for the link.  If there is already a section,
 // *KEPT_SECTION is set to point to the existing section and the
diff --git a/gold/layout.h b/gold/layout.h
index 032f5f3..31724a2 100644
--- a/gold/layout.h
+++ b/gold/layout.h
@@ -488,7 +488,7 @@ enum Output_section_order
 class Layout
 {
  public:
-  Layout(int number_of_input_files, Script_options*);
+  Layout(int number_of_input_files, Script_options*, bool has_crtbeginT);
 
   ~Layout()
   {
@@ -757,6 +757,12 @@ class Layout
 	    || strncmp(name, ".stab", sizeof(".stab") - 1) == 0);
   }
 
+  // Return true if FILE is an input file whose base name matches
+  // FILE_NAME.  The base name must have an extension of ".o", and
+  // must be exactly FILE_NAME.o or FILE_NAME, one character, ".o".
+  static bool
+  match_file_name(const char* file, const char* file_name);
+
   // Return true if RELOBJ is an input file whose base name matches
   // FILE_NAME.  The base name must have an extension of ".o", and
   // must be exactly FILE_NAME.o or FILE_NAME, one character, ".o".
@@ -1420,6 +1426,8 @@ class Layout
   Incremental_inputs* incremental_inputs_;
   // Whether we record output section data created in script
   bool record_output_section_data_from_script_;
+  // Whether to optimize the exception frame section.
+  bool optimize_ehframe_;
   // List of output data that needs to be removed at relaxation clean up.
   Output_section_data_list script_output_section_data_list_;
   // Structure to save segment states before entering the relaxation loop.
diff --git a/gold/main.cc b/gold/main.cc
index bb86613..bbeff65 100644
--- a/gold/main.cc
+++ b/gold/main.cc
@@ -227,7 +227,8 @@ main(int argc, char** argv)
 
   // The layout object.
   Layout layout(command_line.number_of_input_files(),
-		&command_line.script_options());
+		&command_line.script_options(),
+		command_line.crtbeginT());
 
   if (layout.incremental_inputs() != NULL)
     layout.incremental_inputs()->report_command_line(argc, argv);
diff --git a/gold/options.cc b/gold/options.cc
index 731061d..cf7af2d 100644
--- a/gold/options.cc
+++ b/gold/options.cc
@@ -38,6 +38,7 @@
 #include "script.h"
 #include "target-select.h"
 #include "options.h"
+#include "layout.h"
 #include "plugin.h"
 
 namespace gold
@@ -1346,6 +1347,9 @@ Input_arguments::add_file(Input_file_argument& file)
       return this->input_argument_list_.back().lib()->add_file(file);
     }
   this->input_argument_list_.push_back(Input_argument(file));
+  if (!this->has_crtbeginT_
+      && Layout::match_file_name(file.name(), "crtbeginT"))
+    this->has_crtbeginT_ = true;
   return this->input_argument_list_.back();
 }
 
diff --git a/gold/options.h b/gold/options.h
index 6d827f1..7e881df 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -1970,7 +1970,8 @@ class Input_arguments
   typedef Input_argument_list::const_iterator const_iterator;
 
   Input_arguments()
-    : input_argument_list_(), in_group_(false), in_lib_(false), file_count_(0)
+    : input_argument_list_(), in_group_(false), in_lib_(false),
+      file_count_(0), has_crtbeginT_(false)
   { }
 
   // Add a file.
@@ -2030,11 +2031,18 @@ class Input_arguments
   number_of_input_files() const
   { return this->file_count_; }
 
+  // Return whether there is a crtbeginT file.
+  bool
+  has_crtbeginT() const
+  { return this->has_crtbeginT_; }
+
  private:
   Input_argument_list input_argument_list_;
   bool in_group_;
   bool in_lib_;
   unsigned int file_count_;
+  // Whether there is a crtbeginT file.
+  bool has_crtbeginT_;
 };
 
 
@@ -2103,6 +2111,11 @@ class Command_line
   end() const
   { return this->inputs_.end(); }
 
+  // Whether there is a crtbeginT file.
+  bool
+  crtbeginT() const
+  { return this->inputs_.has_crtbeginT(); }
+
  private:
   Command_line(const Command_line&);
   Command_line& operator=(const Command_line&);
-- 
1.9.3

Re: PATCH: PR gold/14675: No eh_frame info registered in exception_static_test

[H.J. Lu]

On Mon, Dec 22, 2014 at 09:37:38AM -0800, Cary Coutant wrote:
> > Here is an idea.  Does it look OK?
> 
> I don't like the idea of making a file named "crtbegin" magic, and
> with this patch, any link *without* such a file will no longer
> optimize eh data.
> 

Here is a different approach.  We always optimize eh data unless we
see crti followed by crtbeginT.  If it is true, we start to optimize eh
data only when we see crtbeginT.  It only leaves any eh data before
crtbeginT unoptimized.  This patch changes the output eh data only when
there is crti followed by crtbeginT.  OK for trunk and 2.25 branch?

Thanks.


H.J.
---
From 355d5e8db8f93fd2e5e73fc972bf3c3c818a4d66 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sat, 20 Dec 2014 05:45:51 -0800
Subject: [PATCH] Treat .eh_frame section before crtbeginT as normal input

Force the exception frame section from input files before the crtbeginT
file to be handled as an ordinary input section if we aren't creating
the exception frame header.  If we don't do this, we won't correctly
handle the special marker symbol in the exception frame section in the
crtbeginT file.

	PR gold/14675
	* ehframe.cc (Eh_frame::add_ehframe_input_section): Force the
	exception frame section from input files if it can't be
	optimized.
	(Eh_frame::add_ehframe_input_section<32, false>): Updated.
	(Eh_frame::add_ehframe_input_section<32, true>): Likewise.
	(Eh_frame::add_ehframe_input_section<64, false>): Likewise.
	(Eh_frame::add_ehframe_input_section<64, true>): Likewise.
	* ehframe.h (Eh_frame::add_ehframe_input_section): Add a
	bool parameter to indicate if the exception frame section
	can be optimized.
	* layout.cc (Layout::Layout): Initialize optimize_ehframe_ to
	!has_crtbeginT.
	(Layout::layout_eh_frame): Pass this->optimize_ehframe_ to
	Eh_frame::add_ehframe_input_section.
	(Layout::make_eh_frame_section): Set this->optimize_ehframe_
	to true when processing the crtbeginT file if it is on command
	line.
	(Layout::match_file_name (const char*, const char*)): New.
	(Layout::match_file_name(const Relobj*, const char*): Use it.
	* layout.h (Layout::Layout): Add has_crtbeginT.
	(Layout::match_file_name (const char*, const char*)): New.
	(Layout): Add an optimize_ehframe_ member.
	* main.cc (main): Pass command_line.has_crtbeginT() to layout.
	* options.cc: Include "layout.h".
	(Input_arguments::add_file): Set this->has_crtbeginT_ to true
	if there is a crtbeginT file and the last one is a crti file.
	* options.h (Input_arguments::Input_arguments): Initialize
	has_crtbeginT_ and last_is_crti_ to false.
	(Input_arguments::has_crtbeginT): New function.
	(Input_arguments::has_crtbeginT_): New bool member.
	(Input_arguments::last_is_crti_): Likewise.
	(Command_line::has_crtbeginT): New function.
---
 gold/ChangeLog  | 36 ++++++++++++++++++++++++++++++++++++
 gold/ehframe.cc | 31 +++++++++++++++++++++----------
 gold/ehframe.h  |  6 ++++--
 gold/layout.cc  | 37 +++++++++++++++++++++++++++++--------
 gold/layout.h   | 10 +++++++++-
 gold/main.cc    |  3 ++-
 gold/options.cc | 12 ++++++++++++
 gold/options.h  | 17 ++++++++++++++++-
 8 files changed, 129 insertions(+), 23 deletions(-)

diff --git a/gold/ChangeLog b/gold/ChangeLog
index 93529fe..d2ad829 100644
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,3 +1,39 @@
+2015-01-07  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR gold/14675
+	* ehframe.cc (Eh_frame::add_ehframe_input_section): Force the
+	exception frame section from input files if it can't be
+	optimized.
+	(Eh_frame::add_ehframe_input_section<32, false>): Updated.
+	(Eh_frame::add_ehframe_input_section<32, true>): Likewise.
+	(Eh_frame::add_ehframe_input_section<64, false>): Likewise.
+	(Eh_frame::add_ehframe_input_section<64, true>): Likewise.
+	* ehframe.h (Eh_frame::add_ehframe_input_section): Add a
+	bool parameter to indicate if the exception frame section
+	can be optimized.
+	* layout.cc (Layout::Layout): Initialize optimize_ehframe_ to
+	!has_crtbeginT.
+	(Layout::layout_eh_frame): Pass this->optimize_ehframe_ to
+	Eh_frame::add_ehframe_input_section.
+	(Layout::make_eh_frame_section): Set this->optimize_ehframe_
+	to true when processing the crtbeginT file if it is on command
+	line.
+	(Layout::match_file_name (const char*, const char*)): New.
+	(Layout::match_file_name(const Relobj*, const char*): Use it.
+	* layout.h (Layout::Layout): Add has_crtbeginT.
+	(Layout::match_file_name (const char*, const char*)): New.
+	(Layout): Add an optimize_ehframe_ member.
+	* main.cc (main): Pass command_line.has_crtbeginT() to layout.
+	* options.cc: Include "layout.h".
+	(Input_arguments::add_file): Set this->has_crtbeginT_ to true
+	if there is a crtbeginT file and the last one is a crti file.
+	* options.h (Input_arguments::Input_arguments): Initialize
+	has_crtbeginT_ and last_is_crti_ to false.
+	(Input_arguments::has_crtbeginT): New function.
+	(Input_arguments::has_crtbeginT_): New bool member.
+	(Input_arguments::last_is_crti_): Likewise.
+	(Command_line::has_crtbeginT): New function.
+
 2015-01-06  H.J. Lu  <hongjiu.lu@intel.com>
 	    Cary Coutant  <ccoutant@google.com>
 
diff --git a/gold/ehframe.cc b/gold/ehframe.cc
index faea1a8..2fee0ff 100644
--- a/gold/ehframe.cc
+++ b/gold/ehframe.cc
@@ -576,7 +576,8 @@ Eh_frame::add_ehframe_input_section(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type)
+    unsigned int reloc_type,
+    bool optimize_ehframe)
 {
   // Get the section contents.
   section_size_type contents_len;
@@ -595,15 +596,21 @@ Eh_frame::add_ehframe_input_section(
     return false;
 
   New_cies new_cies;
-  if (!this->do_add_ehframe_input_section(object, symbols, symbols_size,
+  bool recognized_eh_frame_section
+    = this->do_add_ehframe_input_section(object, symbols, symbols_size,
 					  symbol_names, symbol_names_size,
 					  shndx, reloc_shndx,
 					  reloc_type, pcontents,
-					  contents_len, &new_cies))
+					  contents_len, &new_cies);
+  if (!recognized_eh_frame_section && this->eh_frame_hdr_ != NULL)
+    this->eh_frame_hdr_->found_unrecognized_eh_frame_section();
+
+  // If we don't unrecognize the exception frame section or it can't
+  // be optimized, then return false to force it to be handled as an
+  // ordinary input section.
+  if (!recognized_eh_frame_section
+      || (this->eh_frame_hdr_ == NULL && !optimize_ehframe))
     {
-      if (this->eh_frame_hdr_ != NULL)
-	this->eh_frame_hdr_->found_unrecognized_eh_frame_section();
-
       for (New_cies::iterator p = new_cies.begin();
 	   p != new_cies.end();
 	   ++p)
@@ -1224,7 +1231,8 @@ Eh_frame::add_ehframe_input_section<32, false>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool optimize_ehframe);
 #endif
 
 #ifdef HAVE_TARGET_32_BIG
@@ -1238,7 +1246,8 @@ Eh_frame::add_ehframe_input_section<32, true>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool optimize_ehframe);
 #endif
 
 #ifdef HAVE_TARGET_64_LITTLE
@@ -1252,7 +1261,8 @@ Eh_frame::add_ehframe_input_section<64, false>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool optimize_ehframe);
 #endif
 
 #ifdef HAVE_TARGET_64_BIG
@@ -1266,7 +1276,8 @@ Eh_frame::add_ehframe_input_section<64, true>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool optimize_ehframe);
 #endif
 
 } // End namespace gold.
diff --git a/gold/ehframe.h b/gold/ehframe.h
index aa2bd31..73e0fd4 100644
--- a/gold/ehframe.h
+++ b/gold/ehframe.h
@@ -371,7 +371,8 @@ class Eh_frame : public Output_section_data
   // is the relocation section if any (0 for none, -1U for multiple).
   // RELOC_TYPE is the type of the relocation section if any.  This
   // returns whether the section was incorporated into the .eh_frame
-  // data.
+  // data.  OPTIMIZE_EHFRAME is true if the exception frame section
+  // can be optimized.
   template<int size, bool big_endian>
   bool
   add_ehframe_input_section(Sized_relobj_file<size, big_endian>* object,
@@ -380,7 +381,8 @@ class Eh_frame : public Output_section_data
 			    const unsigned char* symbol_names,
 			    section_size_type symbol_names_size,
 			    unsigned int shndx, unsigned int reloc_shndx,
-			    unsigned int reloc_type);
+			    unsigned int reloc_type,
+			    bool optimize_ehframe);
 
   // Add a CIE and an FDE for a PLT section, to permit unwinding
   // through a PLT.  The FDE data should start with 8 bytes of zero,
diff --git a/gold/layout.cc b/gold/layout.cc
index acc03b2..1ef4d18 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -418,7 +418,8 @@ Layout_task_runner::run(Workqueue* workqueue, const Task* task)
 
 // Layout methods.
 
-Layout::Layout(int number_of_input_files, Script_options* script_options)
+Layout::Layout(int number_of_input_files, Script_options* script_options,
+	       bool has_crtbeginT)
   : number_of_input_files_(number_of_input_files),
     script_options_(script_options),
     namepool_(),
@@ -469,6 +470,7 @@ Layout::Layout(int number_of_input_files, Script_options* script_options)
     unique_segment_for_sections_specified_(false),
     incremental_inputs_(NULL),
     record_output_section_data_from_script_(false),
+    optimize_ehframe_(!has_crtbeginT),
     script_output_section_data_list_(),
     segment_states_(NULL),
     relaxation_debug_check_(NULL),
@@ -1428,7 +1430,8 @@ Layout::layout_eh_frame(Sized_relobj_file<size, big_endian>* object,
 							 symbol_names_size,
 							 shndx,
 							 reloc_shndx,
-							 reloc_type))
+							 reloc_type,
+							 this->optimize_ehframe_))
     {
       os->update_flags_for_input_section(shdr.get_sh_flags());
 
@@ -1485,6 +1488,14 @@ Layout::make_eh_frame_section(const Relobj* object)
   if (os == NULL)
     return NULL;
 
+  // optimize_ehframe_ is false only if there is a crtbeginT file on
+  // command line.  We can't optimize the exception frame section
+  // until we have seen the crtbeginT file.
+  if (!this->optimize_ehframe_
+      && object != NULL
+      && Layout::match_file_name(object, "crtbeginT"))
+    this->optimize_ehframe_ = true;
+
   if (this->eh_frame_section_ == NULL)
     {
       this->eh_frame_section_ = os;
@@ -5092,19 +5103,18 @@ Layout::output_section_name(const Relobj* relobj, const char* name,
   return name;
 }
 
-// Return true if RELOBJ is an input file whose base name matches
-// FILE_NAME.  The base name must have an extension of ".o", and must
-// be exactly FILE_NAME.o or FILE_NAME, one character, ".o".  This is
+// Return true if FILE is an input file whose base name matches
+// MATCH.  The base name must have an extension of ".o", and must
+// be exactly MATCH.o or MATCH, one character, ".o".  This is
 // to match crtbegin.o as well as crtbeginS.o without getting confused
 // by other possibilities.  Overall matching the file name this way is
 // a dreadful hack, but the GNU linker does it in order to better
 // support gcc, and we need to be compatible.
 
 bool
-Layout::match_file_name(const Relobj* relobj, const char* match)
+Layout::match_file_name(const char* file, const char* match)
 {
-  const std::string& file_name(relobj->name());
-  const char* base_name = lbasename(file_name.c_str());
+  const char* base_name = lbasename(file);
   size_t match_len = strlen(match);
   if (strncmp(base_name, match, match_len) != 0)
     return false;
@@ -5114,6 +5124,17 @@ Layout::match_file_name(const Relobj* relobj, const char* match)
   return memcmp(base_name + base_len - 2, ".o", 2) == 0;
 }
 
+// Return true if FILE is an input file whose base name matches
+// MATCH.  The base name must have an extension of ".o", and must
+// be exactly MATCH.o or MATCH, one character, ".o".
+
+bool
+Layout::match_file_name(const Relobj* relobj, const char* match)
+{
+  const std::string& file_name(relobj->name());
+  return Layout::match_file_name(file_name.c_str(), match);
+}
+
 // Check if a comdat group or .gnu.linkonce section with the given
 // NAME is selected for the link.  If there is already a section,
 // *KEPT_SECTION is set to point to the existing section and the
diff --git a/gold/layout.h b/gold/layout.h
index aec0c53..bf4a44a 100644
--- a/gold/layout.h
+++ b/gold/layout.h
@@ -488,7 +488,7 @@ enum Output_section_order
 class Layout
 {
  public:
-  Layout(int number_of_input_files, Script_options*);
+  Layout(int number_of_input_files, Script_options*, bool has_crtbeginT);
 
   ~Layout()
   {
@@ -757,6 +757,12 @@ class Layout
 	    || strncmp(name, ".stab", sizeof(".stab") - 1) == 0);
   }
 
+  // Return true if FILE is an input file whose base name matches
+  // FILE_NAME.  The base name must have an extension of ".o", and
+  // must be exactly FILE_NAME.o or FILE_NAME, one character, ".o".
+  static bool
+  match_file_name(const char* file, const char* file_name);
+
   // Return true if RELOBJ is an input file whose base name matches
   // FILE_NAME.  The base name must have an extension of ".o", and
   // must be exactly FILE_NAME.o or FILE_NAME, one character, ".o".
@@ -1420,6 +1426,8 @@ class Layout
   Incremental_inputs* incremental_inputs_;
   // Whether we record output section data created in script
   bool record_output_section_data_from_script_;
+  // Whether to optimize the exception frame section.
+  bool optimize_ehframe_;
   // List of output data that needs to be removed at relaxation clean up.
   Output_section_data_list script_output_section_data_list_;
   // Structure to save segment states before entering the relaxation loop.
diff --git a/gold/main.cc b/gold/main.cc
index c5e50d6..a3aea4b 100644
--- a/gold/main.cc
+++ b/gold/main.cc
@@ -227,7 +227,8 @@ main(int argc, char** argv)
 
   // The layout object.
   Layout layout(command_line.number_of_input_files(),
-		&command_line.script_options());
+		&command_line.script_options(),
+		command_line.has_crtbeginT());
 
   if (layout.incremental_inputs() != NULL)
     layout.incremental_inputs()->report_command_line(argc, argv);
diff --git a/gold/options.cc b/gold/options.cc
index 7eb8f27..8db9a13 100644
--- a/gold/options.cc
+++ b/gold/options.cc
@@ -38,6 +38,7 @@
 #include "script.h"
 #include "target-select.h"
 #include "options.h"
+#include "layout.h"
 #include "plugin.h"
 
 namespace gold
@@ -1346,6 +1347,17 @@ Input_arguments::add_file(Input_file_argument& file)
       return this->input_argument_list_.back().lib()->add_file(file);
     }
   this->input_argument_list_.push_back(Input_argument(file));
+  if (Layout::match_file_name(file.name(), "crti"))
+    this->last_is_crti_ = true;
+  else
+    {
+      // Set has_crtbeginT_ to true only if the last one is a crti file.
+      if (!this->has_crtbeginT_
+	  && this->last_is_crti_
+	  && Layout::match_file_name(file.name(), "crtbeginT"))
+	this->has_crtbeginT_ = true;
+      this->last_is_crti_ = false;
+    }
   return this->input_argument_list_.back();
 }
 
diff --git a/gold/options.h b/gold/options.h
index 956a7f4..83bdd20 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -1970,7 +1970,8 @@ class Input_arguments
   typedef Input_argument_list::const_iterator const_iterator;
 
   Input_arguments()
-    : input_argument_list_(), in_group_(false), in_lib_(false), file_count_(0)
+    : input_argument_list_(), in_group_(false), in_lib_(false),
+      file_count_(0), has_crtbeginT_(false), last_is_crti_(false)
   { }
 
   // Add a file.
@@ -2030,11 +2031,20 @@ class Input_arguments
   number_of_input_files() const
   { return this->file_count_; }
 
+  // Return whether there is a crtbeginT file.
+  bool
+  has_crtbeginT() const
+  { return this->has_crtbeginT_; }
+
  private:
   Input_argument_list input_argument_list_;
   bool in_group_;
   bool in_lib_;
   unsigned int file_count_;
+  // Whether there is a crtbeginT file.
+  bool has_crtbeginT_;
+  // Whether the last one is a crti file.
+  bool last_is_crti_;
 };
 
 
@@ -2103,6 +2113,11 @@ class Command_line
   end() const
   { return this->inputs_.end(); }
 
+  // Whether there is a crtbeginT file.
+  bool
+  has_crtbeginT() const
+  { return this->inputs_.has_crtbeginT(); }
+
  private:
   Command_line(const Command_line&);
   Command_line& operator=(const Command_line&);
-- 
1.9.3

Re: PATCH: PR gold/14675: No eh_frame info registered in exception_static_test

[H.J. Lu]

On Wed, Jan 07, 2015 at 05:16:55AM -0800, H.J. Lu wrote:
> On Mon, Dec 22, 2014 at 09:37:38AM -0800, Cary Coutant wrote:
> > > Here is an idea.  Does it look OK?
> > 
> > I don't like the idea of making a file named "crtbegin" magic, and
> > with this patch, any link *without* such a file will no longer
> > optimize eh data.
> > 
> 
> Here is a different approach.  We always optimize eh data unless we
> see crti followed by crtbeginT.  If it is true, we start to optimize eh
> data only when we see crtbeginT.  It only leaves any eh data before
> crtbeginT unoptimized.  This patch changes the output eh data only when
> there is crti followed by crtbeginT.  OK for trunk and 2.25 branch?
> 
> Thanks.
> 
> 

Small update.  Set has_crtbeginT_ to true only if it is false.

H.J.
--
From 081ff0b8a64fa33a2fbd8b900f31228138400438 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sat, 20 Dec 2014 05:45:51 -0800
Subject: [PATCH] Treat .eh_frame section before crtbeginT as normal input

Force the exception frame section from input files before the crtbeginT
file to be handled as an ordinary input section if we aren't creating
the exception frame header.  If we don't do this, we won't correctly
handle the special marker symbol in the exception frame section in the
crtbeginT file.

	PR gold/14675
	* ehframe.cc (Eh_frame::add_ehframe_input_section): Force the
	exception frame section from input files if it can't be
	optimized.
	(Eh_frame::add_ehframe_input_section<32, false>): Updated.
	(Eh_frame::add_ehframe_input_section<32, true>): Likewise.
	(Eh_frame::add_ehframe_input_section<64, false>): Likewise.
	(Eh_frame::add_ehframe_input_section<64, true>): Likewise.
	* ehframe.h (Eh_frame::add_ehframe_input_section): Add a
	bool parameter to indicate if the exception frame section
	can be optimized.
	* layout.cc (Layout::Layout): Initialize optimize_ehframe_ to
	!has_crtbeginT.
	(Layout::layout_eh_frame): Pass this->optimize_ehframe_ to
	Eh_frame::add_ehframe_input_section.
	(Layout::make_eh_frame_section): Set this->optimize_ehframe_
	to true when processing the crtbeginT file if it is on command
	line.
	(Layout::match_file_name (const char*, const char*)): New.
	(Layout::match_file_name(const Relobj*, const char*): Use it.
	* layout.h (Layout::Layout): Add has_crtbeginT.
	(Layout::match_file_name (const char*, const char*)): New.
	(Layout): Add an optimize_ehframe_ member.
	* main.cc (main): Pass command_line.has_crtbeginT() to layout.
	* options.cc: Include "layout.h".
	(Input_arguments::add_file): Set this->has_crtbeginT_ to true
	if there is a crtbeginT file and the last one is a crti file.
	* options.h (Input_arguments::Input_arguments): Initialize
	has_crtbeginT_ and last_is_crti_ to false.
	(Input_arguments::has_crtbeginT): New function.
	(Input_arguments::has_crtbeginT_): New bool member.
	(Input_arguments::last_is_crti_): Likewise.
	(Command_line::has_crtbeginT): New function.
---
 gold/ChangeLog  | 36 ++++++++++++++++++++++++++++++++++++
 gold/ehframe.cc | 31 +++++++++++++++++++++----------
 gold/ehframe.h  |  6 ++++--
 gold/layout.cc  | 37 +++++++++++++++++++++++++++++--------
 gold/layout.h   | 10 +++++++++-
 gold/main.cc    |  3 ++-
 gold/options.cc | 14 ++++++++++++++
 gold/options.h  | 17 ++++++++++++++++-
 8 files changed, 131 insertions(+), 23 deletions(-)

diff --git a/gold/ChangeLog b/gold/ChangeLog
index 93529fe..d2ad829 100644
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,3 +1,39 @@
+2015-01-07  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR gold/14675
+	* ehframe.cc (Eh_frame::add_ehframe_input_section): Force the
+	exception frame section from input files if it can't be
+	optimized.
+	(Eh_frame::add_ehframe_input_section<32, false>): Updated.
+	(Eh_frame::add_ehframe_input_section<32, true>): Likewise.
+	(Eh_frame::add_ehframe_input_section<64, false>): Likewise.
+	(Eh_frame::add_ehframe_input_section<64, true>): Likewise.
+	* ehframe.h (Eh_frame::add_ehframe_input_section): Add a
+	bool parameter to indicate if the exception frame section
+	can be optimized.
+	* layout.cc (Layout::Layout): Initialize optimize_ehframe_ to
+	!has_crtbeginT.
+	(Layout::layout_eh_frame): Pass this->optimize_ehframe_ to
+	Eh_frame::add_ehframe_input_section.
+	(Layout::make_eh_frame_section): Set this->optimize_ehframe_
+	to true when processing the crtbeginT file if it is on command
+	line.
+	(Layout::match_file_name (const char*, const char*)): New.
+	(Layout::match_file_name(const Relobj*, const char*): Use it.
+	* layout.h (Layout::Layout): Add has_crtbeginT.
+	(Layout::match_file_name (const char*, const char*)): New.
+	(Layout): Add an optimize_ehframe_ member.
+	* main.cc (main): Pass command_line.has_crtbeginT() to layout.
+	* options.cc: Include "layout.h".
+	(Input_arguments::add_file): Set this->has_crtbeginT_ to true
+	if there is a crtbeginT file and the last one is a crti file.
+	* options.h (Input_arguments::Input_arguments): Initialize
+	has_crtbeginT_ and last_is_crti_ to false.
+	(Input_arguments::has_crtbeginT): New function.
+	(Input_arguments::has_crtbeginT_): New bool member.
+	(Input_arguments::last_is_crti_): Likewise.
+	(Command_line::has_crtbeginT): New function.
+
 2015-01-06  H.J. Lu  <hongjiu.lu@intel.com>
 	    Cary Coutant  <ccoutant@google.com>
 
diff --git a/gold/ehframe.cc b/gold/ehframe.cc
index faea1a8..2fee0ff 100644
--- a/gold/ehframe.cc
+++ b/gold/ehframe.cc
@@ -576,7 +576,8 @@ Eh_frame::add_ehframe_input_section(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type)
+    unsigned int reloc_type,
+    bool optimize_ehframe)
 {
   // Get the section contents.
   section_size_type contents_len;
@@ -595,15 +596,21 @@ Eh_frame::add_ehframe_input_section(
     return false;
 
   New_cies new_cies;
-  if (!this->do_add_ehframe_input_section(object, symbols, symbols_size,
+  bool recognized_eh_frame_section
+    = this->do_add_ehframe_input_section(object, symbols, symbols_size,
 					  symbol_names, symbol_names_size,
 					  shndx, reloc_shndx,
 					  reloc_type, pcontents,
-					  contents_len, &new_cies))
+					  contents_len, &new_cies);
+  if (!recognized_eh_frame_section && this->eh_frame_hdr_ != NULL)
+    this->eh_frame_hdr_->found_unrecognized_eh_frame_section();
+
+  // If we don't unrecognize the exception frame section or it can't
+  // be optimized, then return false to force it to be handled as an
+  // ordinary input section.
+  if (!recognized_eh_frame_section
+      || (this->eh_frame_hdr_ == NULL && !optimize_ehframe))
     {
-      if (this->eh_frame_hdr_ != NULL)
-	this->eh_frame_hdr_->found_unrecognized_eh_frame_section();
-
       for (New_cies::iterator p = new_cies.begin();
 	   p != new_cies.end();
 	   ++p)
@@ -1224,7 +1231,8 @@ Eh_frame::add_ehframe_input_section<32, false>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool optimize_ehframe);
 #endif
 
 #ifdef HAVE_TARGET_32_BIG
@@ -1238,7 +1246,8 @@ Eh_frame::add_ehframe_input_section<32, true>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool optimize_ehframe);
 #endif
 
 #ifdef HAVE_TARGET_64_LITTLE
@@ -1252,7 +1261,8 @@ Eh_frame::add_ehframe_input_section<64, false>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool optimize_ehframe);
 #endif
 
 #ifdef HAVE_TARGET_64_BIG
@@ -1266,7 +1276,8 @@ Eh_frame::add_ehframe_input_section<64, true>(
     section_size_type symbol_names_size,
     unsigned int shndx,
     unsigned int reloc_shndx,
-    unsigned int reloc_type);
+    unsigned int reloc_type,
+    bool optimize_ehframe);
 #endif
 
 } // End namespace gold.
diff --git a/gold/ehframe.h b/gold/ehframe.h
index aa2bd31..73e0fd4 100644
--- a/gold/ehframe.h
+++ b/gold/ehframe.h
@@ -371,7 +371,8 @@ class Eh_frame : public Output_section_data
   // is the relocation section if any (0 for none, -1U for multiple).
   // RELOC_TYPE is the type of the relocation section if any.  This
   // returns whether the section was incorporated into the .eh_frame
-  // data.
+  // data.  OPTIMIZE_EHFRAME is true if the exception frame section
+  // can be optimized.
   template<int size, bool big_endian>
   bool
   add_ehframe_input_section(Sized_relobj_file<size, big_endian>* object,
@@ -380,7 +381,8 @@ class Eh_frame : public Output_section_data
 			    const unsigned char* symbol_names,
 			    section_size_type symbol_names_size,
 			    unsigned int shndx, unsigned int reloc_shndx,
-			    unsigned int reloc_type);
+			    unsigned int reloc_type,
+			    bool optimize_ehframe);
 
   // Add a CIE and an FDE for a PLT section, to permit unwinding
   // through a PLT.  The FDE data should start with 8 bytes of zero,
diff --git a/gold/layout.cc b/gold/layout.cc
index acc03b2..1ef4d18 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -418,7 +418,8 @@ Layout_task_runner::run(Workqueue* workqueue, const Task* task)
 
 // Layout methods.
 
-Layout::Layout(int number_of_input_files, Script_options* script_options)
+Layout::Layout(int number_of_input_files, Script_options* script_options,
+	       bool has_crtbeginT)
   : number_of_input_files_(number_of_input_files),
     script_options_(script_options),
     namepool_(),
@@ -469,6 +470,7 @@ Layout::Layout(int number_of_input_files, Script_options* script_options)
     unique_segment_for_sections_specified_(false),
     incremental_inputs_(NULL),
     record_output_section_data_from_script_(false),
+    optimize_ehframe_(!has_crtbeginT),
     script_output_section_data_list_(),
     segment_states_(NULL),
     relaxation_debug_check_(NULL),
@@ -1428,7 +1430,8 @@ Layout::layout_eh_frame(Sized_relobj_file<size, big_endian>* object,
 							 symbol_names_size,
 							 shndx,
 							 reloc_shndx,
-							 reloc_type))
+							 reloc_type,
+							 this->optimize_ehframe_))
     {
       os->update_flags_for_input_section(shdr.get_sh_flags());
 
@@ -1485,6 +1488,14 @@ Layout::make_eh_frame_section(const Relobj* object)
   if (os == NULL)
     return NULL;
 
+  // optimize_ehframe_ is false only if there is a crtbeginT file on
+  // command line.  We can't optimize the exception frame section
+  // until we have seen the crtbeginT file.
+  if (!this->optimize_ehframe_
+      && object != NULL
+      && Layout::match_file_name(object, "crtbeginT"))
+    this->optimize_ehframe_ = true;
+
   if (this->eh_frame_section_ == NULL)
     {
       this->eh_frame_section_ = os;
@@ -5092,19 +5103,18 @@ Layout::output_section_name(const Relobj* relobj, const char* name,
   return name;
 }
 
-// Return true if RELOBJ is an input file whose base name matches
-// FILE_NAME.  The base name must have an extension of ".o", and must
-// be exactly FILE_NAME.o or FILE_NAME, one character, ".o".  This is
+// Return true if FILE is an input file whose base name matches
+// MATCH.  The base name must have an extension of ".o", and must
+// be exactly MATCH.o or MATCH, one character, ".o".  This is
 // to match crtbegin.o as well as crtbeginS.o without getting confused
 // by other possibilities.  Overall matching the file name this way is
 // a dreadful hack, but the GNU linker does it in order to better
 // support gcc, and we need to be compatible.
 
 bool
-Layout::match_file_name(const Relobj* relobj, const char* match)
+Layout::match_file_name(const char* file, const char* match)
 {
-  const std::string& file_name(relobj->name());
-  const char* base_name = lbasename(file_name.c_str());
+  const char* base_name = lbasename(file);
   size_t match_len = strlen(match);
   if (strncmp(base_name, match, match_len) != 0)
     return false;
@@ -5114,6 +5124,17 @@ Layout::match_file_name(const Relobj* relobj, const char* match)
   return memcmp(base_name + base_len - 2, ".o", 2) == 0;
 }
 
+// Return true if FILE is an input file whose base name matches
+// MATCH.  The base name must have an extension of ".o", and must
+// be exactly MATCH.o or MATCH, one character, ".o".
+
+bool
+Layout::match_file_name(const Relobj* relobj, const char* match)
+{
+  const std::string& file_name(relobj->name());
+  return Layout::match_file_name(file_name.c_str(), match);
+}
+
 // Check if a comdat group or .gnu.linkonce section with the given
 // NAME is selected for the link.  If there is already a section,
 // *KEPT_SECTION is set to point to the existing section and the
diff --git a/gold/layout.h b/gold/layout.h
index aec0c53..bf4a44a 100644
--- a/gold/layout.h
+++ b/gold/layout.h
@@ -488,7 +488,7 @@ enum Output_section_order
 class Layout
 {
  public:
-  Layout(int number_of_input_files, Script_options*);
+  Layout(int number_of_input_files, Script_options*, bool has_crtbeginT);
 
   ~Layout()
   {
@@ -757,6 +757,12 @@ class Layout
 	    || strncmp(name, ".stab", sizeof(".stab") - 1) == 0);
   }
 
+  // Return true if FILE is an input file whose base name matches
+  // FILE_NAME.  The base name must have an extension of ".o", and
+  // must be exactly FILE_NAME.o or FILE_NAME, one character, ".o".
+  static bool
+  match_file_name(const char* file, const char* file_name);
+
   // Return true if RELOBJ is an input file whose base name matches
   // FILE_NAME.  The base name must have an extension of ".o", and
   // must be exactly FILE_NAME.o or FILE_NAME, one character, ".o".
@@ -1420,6 +1426,8 @@ class Layout
   Incremental_inputs* incremental_inputs_;
   // Whether we record output section data created in script
   bool record_output_section_data_from_script_;
+  // Whether to optimize the exception frame section.
+  bool optimize_ehframe_;
   // List of output data that needs to be removed at relaxation clean up.
   Output_section_data_list script_output_section_data_list_;
   // Structure to save segment states before entering the relaxation loop.
diff --git a/gold/main.cc b/gold/main.cc
index c5e50d6..a3aea4b 100644
--- a/gold/main.cc
+++ b/gold/main.cc
@@ -227,7 +227,8 @@ main(int argc, char** argv)
 
   // The layout object.
   Layout layout(command_line.number_of_input_files(),
-		&command_line.script_options());
+		&command_line.script_options(),
+		command_line.has_crtbeginT());
 
   if (layout.incremental_inputs() != NULL)
     layout.incremental_inputs()->report_command_line(argc, argv);
diff --git a/gold/options.cc b/gold/options.cc
index 7eb8f27..12c1aaa 100644
--- a/gold/options.cc
+++ b/gold/options.cc
@@ -38,6 +38,7 @@
 #include "script.h"
 #include "target-select.h"
 #include "options.h"
+#include "layout.h"
 #include "plugin.h"
 
 namespace gold
@@ -1346,6 +1347,19 @@ Input_arguments::add_file(Input_file_argument& file)
       return this->input_argument_list_.back().lib()->add_file(file);
     }
   this->input_argument_list_.push_back(Input_argument(file));
+  if (!this->has_crtbeginT_)
+    {
+      // Set has_crtbeginT_ to true only if the last one is a crti file.
+      if (Layout::match_file_name(file.name(), "crti"))
+	this->last_is_crti_ = true;
+      else
+	{
+	  if (this->last_is_crti_
+	      && Layout::match_file_name(file.name(), "crtbeginT"))
+	    this->has_crtbeginT_ = true;
+	  this->last_is_crti_ = false;
+	}
+    }
   return this->input_argument_list_.back();
 }
 
diff --git a/gold/options.h b/gold/options.h
index 956a7f4..83bdd20 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -1970,7 +1970,8 @@ class Input_arguments
   typedef Input_argument_list::const_iterator const_iterator;
 
   Input_arguments()
-    : input_argument_list_(), in_group_(false), in_lib_(false), file_count_(0)
+    : input_argument_list_(), in_group_(false), in_lib_(false),
+      file_count_(0), has_crtbeginT_(false), last_is_crti_(false)
   { }
 
   // Add a file.
@@ -2030,11 +2031,20 @@ class Input_arguments
   number_of_input_files() const
   { return this->file_count_; }
 
+  // Return whether there is a crtbeginT file.
+  bool
+  has_crtbeginT() const
+  { return this->has_crtbeginT_; }
+
  private:
   Input_argument_list input_argument_list_;
   bool in_group_;
   bool in_lib_;
   unsigned int file_count_;
+  // Whether there is a crtbeginT file.
+  bool has_crtbeginT_;
+  // Whether the last one is a crti file.
+  bool last_is_crti_;
 };
 
 
@@ -2103,6 +2113,11 @@ class Command_line
   end() const
   { return this->inputs_.end(); }
 
+  // Whether there is a crtbeginT file.
+  bool
+  has_crtbeginT() const
+  { return this->inputs_.has_crtbeginT(); }
+
  private:
   Command_line(const Command_line&);
   Command_line& operator=(const Command_line&);
-- 
1.9.3

Re: PATCH: PR gold/14675: No eh_frame info registered in exception_static_test

[Cary Coutant]

Thanks for looking at this, but I find this a bit much for what it
does -- it's pretty intrusive, and still doesn't even do the right
thing with the eh info from crt1.o (which is still placed before the
__EH_FRAME_BEGIN__ symbol, and will be ignored).

I really dislike special-cases based on filename. At one point in the
discussion, Roland suggested recognizing crtbeginT.o by the fact that
it has a relocation to the .eh_frame section symbol. I think it may be
even simpler than that, though -- given that the endcap in crtend.o
has a non-zero length, I think it's safe to put all zero-length
.eh_frame sections in front of the optimized data. That should have
the desired affect for crtbeginT.o. Let me give that a try.

-cary


On Wed, Jan 7, 2015 at 6:43 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Jan 07, 2015 at 05:16:55AM -0800, H.J. Lu wrote:
>> On Mon, Dec 22, 2014 at 09:37:38AM -0800, Cary Coutant wrote:
>> > > Here is an idea.  Does it look OK?
>> >
>> > I don't like the idea of making a file named "crtbegin" magic, and
>> > with this patch, any link *without* such a file will no longer
>> > optimize eh data.
>> >
>>
>> Here is a different approach.  We always optimize eh data unless we
>> see crti followed by crtbeginT.  If it is true, we start to optimize eh
>> data only when we see crtbeginT.  It only leaves any eh data before
>> crtbeginT unoptimized.  This patch changes the output eh data only when
>> there is crti followed by crtbeginT.  OK for trunk and 2.25 branch?
>>
>> Thanks.
>>
>>
>
> Small update.  Set has_crtbeginT_ to true only if it is false.
>
> H.J.
> --
> From 081ff0b8a64fa33a2fbd8b900f31228138400438 Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Sat, 20 Dec 2014 05:45:51 -0800
> Subject: [PATCH] Treat .eh_frame section before crtbeginT as normal input
>
> Force the exception frame section from input files before the crtbeginT
> file to be handled as an ordinary input section if we aren't creating
> the exception frame header.  If we don't do this, we won't correctly
> handle the special marker symbol in the exception frame section in the
> crtbeginT file.
>
>         PR gold/14675
>         * ehframe.cc (Eh_frame::add_ehframe_input_section): Force the
>         exception frame section from input files if it can't be
>         optimized.
>         (Eh_frame::add_ehframe_input_section<32, false>): Updated.
>         (Eh_frame::add_ehframe_input_section<32, true>): Likewise.
>         (Eh_frame::add_ehframe_input_section<64, false>): Likewise.
>         (Eh_frame::add_ehframe_input_section<64, true>): Likewise.
>         * ehframe.h (Eh_frame::add_ehframe_input_section): Add a
>         bool parameter to indicate if the exception frame section
>         can be optimized.
>         * layout.cc (Layout::Layout): Initialize optimize_ehframe_ to
>         !has_crtbeginT.
>         (Layout::layout_eh_frame): Pass this->optimize_ehframe_ to
>         Eh_frame::add_ehframe_input_section.
>         (Layout::make_eh_frame_section): Set this->optimize_ehframe_
>         to true when processing the crtbeginT file if it is on command
>         line.
>         (Layout::match_file_name (const char*, const char*)): New.
>         (Layout::match_file_name(const Relobj*, const char*): Use it.
>         * layout.h (Layout::Layout): Add has_crtbeginT.
>         (Layout::match_file_name (const char*, const char*)): New.
>         (Layout): Add an optimize_ehframe_ member.
>         * main.cc (main): Pass command_line.has_crtbeginT() to layout.
>         * options.cc: Include "layout.h".
>         (Input_arguments::add_file): Set this->has_crtbeginT_ to true
>         if there is a crtbeginT file and the last one is a crti file.
>         * options.h (Input_arguments::Input_arguments): Initialize
>         has_crtbeginT_ and last_is_crti_ to false.
>         (Input_arguments::has_crtbeginT): New function.
>         (Input_arguments::has_crtbeginT_): New bool member.
>         (Input_arguments::last_is_crti_): Likewise.
>         (Command_line::has_crtbeginT): New function.
> ---
>  gold/ChangeLog  | 36 ++++++++++++++++++++++++++++++++++++
>  gold/ehframe.cc | 31 +++++++++++++++++++++----------
>  gold/ehframe.h  |  6 ++++--
>  gold/layout.cc  | 37 +++++++++++++++++++++++++++++--------
>  gold/layout.h   | 10 +++++++++-
>  gold/main.cc    |  3 ++-
>  gold/options.cc | 14 ++++++++++++++
>  gold/options.h  | 17 ++++++++++++++++-
>  8 files changed, 131 insertions(+), 23 deletions(-)
>
> diff --git a/gold/ChangeLog b/gold/ChangeLog
> index 93529fe..d2ad829 100644
> --- a/gold/ChangeLog
> +++ b/gold/ChangeLog
> @@ -1,3 +1,39 @@
> +2015-01-07  H.J. Lu  <hongjiu.lu@intel.com>
> +
> +       PR gold/14675
> +       * ehframe.cc (Eh_frame::add_ehframe_input_section): Force the
> +       exception frame section from input files if it can't be
> +       optimized.
> +       (Eh_frame::add_ehframe_input_section<32, false>): Updated.
> +       (Eh_frame::add_ehframe_input_section<32, true>): Likewise.
> +       (Eh_frame::add_ehframe_input_section<64, false>): Likewise.
> +       (Eh_frame::add_ehframe_input_section<64, true>): Likewise.
> +       * ehframe.h (Eh_frame::add_ehframe_input_section): Add a
> +       bool parameter to indicate if the exception frame section
> +       can be optimized.
> +       * layout.cc (Layout::Layout): Initialize optimize_ehframe_ to
> +       !has_crtbeginT.
> +       (Layout::layout_eh_frame): Pass this->optimize_ehframe_ to
> +       Eh_frame::add_ehframe_input_section.
> +       (Layout::make_eh_frame_section): Set this->optimize_ehframe_
> +       to true when processing the crtbeginT file if it is on command
> +       line.
> +       (Layout::match_file_name (const char*, const char*)): New.
> +       (Layout::match_file_name(const Relobj*, const char*): Use it.
> +       * layout.h (Layout::Layout): Add has_crtbeginT.
> +       (Layout::match_file_name (const char*, const char*)): New.
> +       (Layout): Add an optimize_ehframe_ member.
> +       * main.cc (main): Pass command_line.has_crtbeginT() to layout.
> +       * options.cc: Include "layout.h".
> +       (Input_arguments::add_file): Set this->has_crtbeginT_ to true
> +       if there is a crtbeginT file and the last one is a crti file.
> +       * options.h (Input_arguments::Input_arguments): Initialize
> +       has_crtbeginT_ and last_is_crti_ to false.
> +       (Input_arguments::has_crtbeginT): New function.
> +       (Input_arguments::has_crtbeginT_): New bool member.
> +       (Input_arguments::last_is_crti_): Likewise.
> +       (Command_line::has_crtbeginT): New function.
> +
>  2015-01-06  H.J. Lu  <hongjiu.lu@intel.com>
>             Cary Coutant  <ccoutant@google.com>
>
> diff --git a/gold/ehframe.cc b/gold/ehframe.cc
> index faea1a8..2fee0ff 100644
> --- a/gold/ehframe.cc
> +++ b/gold/ehframe.cc
> @@ -576,7 +576,8 @@ Eh_frame::add_ehframe_input_section(
>      section_size_type symbol_names_size,
>      unsigned int shndx,
>      unsigned int reloc_shndx,
> -    unsigned int reloc_type)
> +    unsigned int reloc_type,
> +    bool optimize_ehframe)
>  {
>    // Get the section contents.
>    section_size_type contents_len;
> @@ -595,15 +596,21 @@ Eh_frame::add_ehframe_input_section(
>      return false;
>
>    New_cies new_cies;
> -  if (!this->do_add_ehframe_input_section(object, symbols, symbols_size,
> +  bool recognized_eh_frame_section
> +    = this->do_add_ehframe_input_section(object, symbols, symbols_size,
>                                           symbol_names, symbol_names_size,
>                                           shndx, reloc_shndx,
>                                           reloc_type, pcontents,
> -                                         contents_len, &new_cies))
> +                                         contents_len, &new_cies);
> +  if (!recognized_eh_frame_section && this->eh_frame_hdr_ != NULL)
> +    this->eh_frame_hdr_->found_unrecognized_eh_frame_section();
> +
> +  // If we don't unrecognize the exception frame section or it can't
> +  // be optimized, then return false to force it to be handled as an
> +  // ordinary input section.
> +  if (!recognized_eh_frame_section
> +      || (this->eh_frame_hdr_ == NULL && !optimize_ehframe))
>      {
> -      if (this->eh_frame_hdr_ != NULL)
> -       this->eh_frame_hdr_->found_unrecognized_eh_frame_section();
> -
>        for (New_cies::iterator p = new_cies.begin();
>            p != new_cies.end();
>            ++p)
> @@ -1224,7 +1231,8 @@ Eh_frame::add_ehframe_input_section<32, false>(
>      section_size_type symbol_names_size,
>      unsigned int shndx,
>      unsigned int reloc_shndx,
> -    unsigned int reloc_type);
> +    unsigned int reloc_type,
> +    bool optimize_ehframe);
>  #endif
>
>  #ifdef HAVE_TARGET_32_BIG
> @@ -1238,7 +1246,8 @@ Eh_frame::add_ehframe_input_section<32, true>(
>      section_size_type symbol_names_size,
>      unsigned int shndx,
>      unsigned int reloc_shndx,
> -    unsigned int reloc_type);
> +    unsigned int reloc_type,
> +    bool optimize_ehframe);
>  #endif
>
>  #ifdef HAVE_TARGET_64_LITTLE
> @@ -1252,7 +1261,8 @@ Eh_frame::add_ehframe_input_section<64, false>(
>      section_size_type symbol_names_size,
>      unsigned int shndx,
>      unsigned int reloc_shndx,
> -    unsigned int reloc_type);
> +    unsigned int reloc_type,
> +    bool optimize_ehframe);
>  #endif
>
>  #ifdef HAVE_TARGET_64_BIG
> @@ -1266,7 +1276,8 @@ Eh_frame::add_ehframe_input_section<64, true>(
>      section_size_type symbol_names_size,
>      unsigned int shndx,
>      unsigned int reloc_shndx,
> -    unsigned int reloc_type);
> +    unsigned int reloc_type,
> +    bool optimize_ehframe);
>  #endif
>
>  } // End namespace gold.
> diff --git a/gold/ehframe.h b/gold/ehframe.h
> index aa2bd31..73e0fd4 100644
> --- a/gold/ehframe.h
> +++ b/gold/ehframe.h
> @@ -371,7 +371,8 @@ class Eh_frame : public Output_section_data
>    // is the relocation section if any (0 for none, -1U for multiple).
>    // RELOC_TYPE is the type of the relocation section if any.  This
>    // returns whether the section was incorporated into the .eh_frame
> -  // data.
> +  // data.  OPTIMIZE_EHFRAME is true if the exception frame section
> +  // can be optimized.
>    template<int size, bool big_endian>
>    bool
>    add_ehframe_input_section(Sized_relobj_file<size, big_endian>* object,
> @@ -380,7 +381,8 @@ class Eh_frame : public Output_section_data
>                             const unsigned char* symbol_names,
>                             section_size_type symbol_names_size,
>                             unsigned int shndx, unsigned int reloc_shndx,
> -                           unsigned int reloc_type);
> +                           unsigned int reloc_type,
> +                           bool optimize_ehframe);
>
>    // Add a CIE and an FDE for a PLT section, to permit unwinding
>    // through a PLT.  The FDE data should start with 8 bytes of zero,
> diff --git a/gold/layout.cc b/gold/layout.cc
> index acc03b2..1ef4d18 100644
> --- a/gold/layout.cc
> +++ b/gold/layout.cc
> @@ -418,7 +418,8 @@ Layout_task_runner::run(Workqueue* workqueue, const Task* task)
>
>  // Layout methods.
>
> -Layout::Layout(int number_of_input_files, Script_options* script_options)
> +Layout::Layout(int number_of_input_files, Script_options* script_options,
> +              bool has_crtbeginT)
>    : number_of_input_files_(number_of_input_files),
>      script_options_(script_options),
>      namepool_(),
> @@ -469,6 +470,7 @@ Layout::Layout(int number_of_input_files, Script_options* script_options)
>      unique_segment_for_sections_specified_(false),
>      incremental_inputs_(NULL),
>      record_output_section_data_from_script_(false),
> +    optimize_ehframe_(!has_crtbeginT),
>      script_output_section_data_list_(),
>      segment_states_(NULL),
>      relaxation_debug_check_(NULL),
> @@ -1428,7 +1430,8 @@ Layout::layout_eh_frame(Sized_relobj_file<size, big_endian>* object,
>                                                          symbol_names_size,
>                                                          shndx,
>                                                          reloc_shndx,
> -                                                        reloc_type))
> +                                                        reloc_type,
> +                                                        this->optimize_ehframe_))
>      {
>        os->update_flags_for_input_section(shdr.get_sh_flags());
>
> @@ -1485,6 +1488,14 @@ Layout::make_eh_frame_section(const Relobj* object)
>    if (os == NULL)
>      return NULL;
>
> +  // optimize_ehframe_ is false only if there is a crtbeginT file on
> +  // command line.  We can't optimize the exception frame section
> +  // until we have seen the crtbeginT file.
> +  if (!this->optimize_ehframe_
> +      && object != NULL
> +      && Layout::match_file_name(object, "crtbeginT"))
> +    this->optimize_ehframe_ = true;
> +
>    if (this->eh_frame_section_ == NULL)
>      {
>        this->eh_frame_section_ = os;
> @@ -5092,19 +5103,18 @@ Layout::output_section_name(const Relobj* relobj, const char* name,
>    return name;
>  }
>
> -// Return true if RELOBJ is an input file whose base name matches
> -// FILE_NAME.  The base name must have an extension of ".o", and must
> -// be exactly FILE_NAME.o or FILE_NAME, one character, ".o".  This is
> +// Return true if FILE is an input file whose base name matches
> +// MATCH.  The base name must have an extension of ".o", and must
> +// be exactly MATCH.o or MATCH, one character, ".o".  This is
>  // to match crtbegin.o as well as crtbeginS.o without getting confused
>  // by other possibilities.  Overall matching the file name this way is
>  // a dreadful hack, but the GNU linker does it in order to better
>  // support gcc, and we need to be compatible.
>
>  bool
> -Layout::match_file_name(const Relobj* relobj, const char* match)
> +Layout::match_file_name(const char* file, const char* match)
>  {
> -  const std::string& file_name(relobj->name());
> -  const char* base_name = lbasename(file_name.c_str());
> +  const char* base_name = lbasename(file);
>    size_t match_len = strlen(match);
>    if (strncmp(base_name, match, match_len) != 0)
>      return false;
> @@ -5114,6 +5124,17 @@ Layout::match_file_name(const Relobj* relobj, const char* match)
>    return memcmp(base_name + base_len - 2, ".o", 2) == 0;
>  }
>
> +// Return true if FILE is an input file whose base name matches
> +// MATCH.  The base name must have an extension of ".o", and must
> +// be exactly MATCH.o or MATCH, one character, ".o".
> +
> +bool
> +Layout::match_file_name(const Relobj* relobj, const char* match)
> +{
> +  const std::string& file_name(relobj->name());
> +  return Layout::match_file_name(file_name.c_str(), match);
> +}
> +
>  // Check if a comdat group or .gnu.linkonce section with the given
>  // NAME is selected for the link.  If there is already a section,
>  // *KEPT_SECTION is set to point to the existing section and the
> diff --git a/gold/layout.h b/gold/layout.h
> index aec0c53..bf4a44a 100644
> --- a/gold/layout.h
> +++ b/gold/layout.h
> @@ -488,7 +488,7 @@ enum Output_section_order
>  class Layout
>  {
>   public:
> -  Layout(int number_of_input_files, Script_options*);
> +  Layout(int number_of_input_files, Script_options*, bool has_crtbeginT);
>
>    ~Layout()
>    {
> @@ -757,6 +757,12 @@ class Layout
>             || strncmp(name, ".stab", sizeof(".stab") - 1) == 0);
>    }
>
> +  // Return true if FILE is an input file whose base name matches
> +  // FILE_NAME.  The base name must have an extension of ".o", and
> +  // must be exactly FILE_NAME.o or FILE_NAME, one character, ".o".
> +  static bool
> +  match_file_name(const char* file, const char* file_name);
> +
>    // Return true if RELOBJ is an input file whose base name matches
>    // FILE_NAME.  The base name must have an extension of ".o", and
>    // must be exactly FILE_NAME.o or FILE_NAME, one character, ".o".
> @@ -1420,6 +1426,8 @@ class Layout
>    Incremental_inputs* incremental_inputs_;
>    // Whether we record output section data created in script
>    bool record_output_section_data_from_script_;
> +  // Whether to optimize the exception frame section.
> +  bool optimize_ehframe_;
>    // List of output data that needs to be removed at relaxation clean up.
>    Output_section_data_list script_output_section_data_list_;
>    // Structure to save segment states before entering the relaxation loop.
> diff --git a/gold/main.cc b/gold/main.cc
> index c5e50d6..a3aea4b 100644
> --- a/gold/main.cc
> +++ b/gold/main.cc
> @@ -227,7 +227,8 @@ main(int argc, char** argv)
>
>    // The layout object.
>    Layout layout(command_line.number_of_input_files(),
> -               &command_line.script_options());
> +               &command_line.script_options(),
> +               command_line.has_crtbeginT());
>
>    if (layout.incremental_inputs() != NULL)
>      layout.incremental_inputs()->report_command_line(argc, argv);
> diff --git a/gold/options.cc b/gold/options.cc
> index 7eb8f27..12c1aaa 100644
> --- a/gold/options.cc
> +++ b/gold/options.cc
> @@ -38,6 +38,7 @@
>  #include "script.h"
>  #include "target-select.h"
>  #include "options.h"
> +#include "layout.h"
>  #include "plugin.h"
>
>  namespace gold
> @@ -1346,6 +1347,19 @@ Input_arguments::add_file(Input_file_argument& file)
>        return this->input_argument_list_.back().lib()->add_file(file);
>      }
>    this->input_argument_list_.push_back(Input_argument(file));
> +  if (!this->has_crtbeginT_)
> +    {
> +      // Set has_crtbeginT_ to true only if the last one is a crti file.
> +      if (Layout::match_file_name(file.name(), "crti"))
> +       this->last_is_crti_ = true;
> +      else
> +       {
> +         if (this->last_is_crti_
> +             && Layout::match_file_name(file.name(), "crtbeginT"))
> +           this->has_crtbeginT_ = true;
> +         this->last_is_crti_ = false;
> +       }
> +    }
>    return this->input_argument_list_.back();
>  }
>
> diff --git a/gold/options.h b/gold/options.h
> index 956a7f4..83bdd20 100644
> --- a/gold/options.h
> +++ b/gold/options.h
> @@ -1970,7 +1970,8 @@ class Input_arguments
>    typedef Input_argument_list::const_iterator const_iterator;
>
>    Input_arguments()
> -    : input_argument_list_(), in_group_(false), in_lib_(false), file_count_(0)
> +    : input_argument_list_(), in_group_(false), in_lib_(false),
> +      file_count_(0), has_crtbeginT_(false), last_is_crti_(false)
>    { }
>
>    // Add a file.
> @@ -2030,11 +2031,20 @@ class Input_arguments
>    number_of_input_files() const
>    { return this->file_count_; }
>
> +  // Return whether there is a crtbeginT file.
> +  bool
> +  has_crtbeginT() const
> +  { return this->has_crtbeginT_; }
> +
>   private:
>    Input_argument_list input_argument_list_;
>    bool in_group_;
>    bool in_lib_;
>    unsigned int file_count_;
> +  // Whether there is a crtbeginT file.
> +  bool has_crtbeginT_;
> +  // Whether the last one is a crti file.
> +  bool last_is_crti_;
>  };
>
>
> @@ -2103,6 +2113,11 @@ class Command_line
>    end() const
>    { return this->inputs_.end(); }
>
> +  // Whether there is a crtbeginT file.
> +  bool
> +  has_crtbeginT() const
> +  { return this->inputs_.has_crtbeginT(); }
> +
>   private:
>    Command_line(const Command_line&);
>    Command_line& operator=(const Command_line&);
> --
> 1.9.3
>

Re: PATCH: PR gold/14675: No eh_frame info registered in exception_static_test

[H.J. Lu]

On Wed, Jan 7, 2015 at 10:50 AM, Cary Coutant <ccoutant@google.com> wrote:
> Thanks for looking at this, but I find this a bit much for what it
> does -- it's pretty intrusive, and still doesn't even do the right
> thing with the eh info from crt1.o (which is still placed before the
> __EH_FRAME_BEGIN__ symbol, and will be ignored).
>
> I really dislike special-cases based on filename. At one point in the
> discussion, Roland suggested recognizing crtbeginT.o by the fact that
> it has a relocation to the .eh_frame section symbol. I think it may be
> even simpler than that, though -- given that the endcap in crtend.o
> has a non-zero length, I think it's safe to put all zero-length
> .eh_frame sections in front of the optimized data. That should have
> the desired affect for crtbeginT.o. Let me give that a try.
>

Hi Cary,

Do we have any progress on this?


-- 
H.J.

Re: PATCH: PR gold/14675: No eh_frame info registered in exception_static_test

[H.J. Lu]

On Mon, Jan 26, 2015 at 9:21 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Jan 7, 2015 at 10:50 AM, Cary Coutant <ccoutant@google.com> wrote:
>> Thanks for looking at this, but I find this a bit much for what it
>> does -- it's pretty intrusive, and still doesn't even do the right
>> thing with the eh info from crt1.o (which is still placed before the
>> __EH_FRAME_BEGIN__ symbol, and will be ignored).
>>
>> I really dislike special-cases based on filename. At one point in the
>> discussion, Roland suggested recognizing crtbeginT.o by the fact that
>> it has a relocation to the .eh_frame section symbol. I think it may be
>> even simpler than that, though -- given that the endcap in crtend.o
>> has a non-zero length, I think it's safe to put all zero-length
>> .eh_frame sections in front of the optimized data. That should have
>> the desired affect for crtbeginT.o. Let me give that a try.
>>
>
> Hi Cary,
>
> Do we have any progress on this?
>

Any progress?


-- 
H.J.

Re: PATCH: PR gold/14675: No eh_frame info registered in exception_static_test

[Cary Coutant]

[-- Attachment #1: Type: text/plain, Size: 1048 bytes --]

> Any progress?

I've committed the attached patch to fix this problem by delaying the
attachment of the optimized .eh_frame sections to the output section
until we see the end marker section (or to the end of pass 1 if we
never see an end marker).

-cary


2015-03-09  Cary Coutant  <ccoutant@google.com>

gold/
        PR gold/14675
        * ehframe.cc (Eh_frame::add_ehframe_input_section): Change return type;
        return enum indicating whether .eh_frame section is empty, optimizable,
        unrecognized, or an end marker. Adjust explicit instantiations.
        * ehframe.h (Eh_frame::Eh_frame_section_disposition): New enum type.
        (Eh_frame::add_ehframe_input_section): Change return type.
        * gold.cc (queue_middle_tasks): Call Layout::finalize_eh_frame_section.
        * layout.cc (Layout::layout_eh_frame): Don't add optimized sections
        to the .eh_frame output section until we see the end marker.
        (Layout::finalize_eh_frame_section): New.
        * layout.h: (Layout::finalize_eh_frame_section): New.

[-- Attachment #2: gold-ehframe-static.patch --]
[-- Type: application/octet-stream, Size: 9736 bytes --]

Fix failure in exception_static_test.

Because the __EH_FRAME_BEGIN__ symbol is provided in an empty .eh_frame
section in crtbeginT.o, if crt1.o has a non-empty .eh_frame section,
we place all optimized .eh_frame sections into the output section ahead
of the __EH_FRAME_BEGIN__ symbol, which breaks EH for statically-linked
binaries.

This patch fixes the problem by delaying the attachment of the optimized
.eh_frame sections to the output section until we see the end marker
section (or to the end of pass 1 if we never see an end marker).

2015-03-09  Cary Coutant  <ccoutant@google.com>

gold/
	PR gold/14675
	* ehframe.cc (Eh_frame::add_ehframe_input_section): Change return type;
	return enum indicating whether .eh_frame section is empty, optimizable,
	unrecognized, or an end marker. Adjust explicit instantiations.
	* ehframe.h (Eh_frame::Eh_frame_section_disposition): New enum type.
	(Eh_frame::add_ehframe_input_section): Change return type.
	* gold.cc (queue_middle_tasks): Call Layout::finalize_eh_frame_section.
	* layout.cc (Layout::layout_eh_frame): Don't add optimized sections
	to the .eh_frame output section until we see the end marker.
	(Layout::finalize_eh_frame_section): New.
	* layout.h: (Layout::finalize_eh_frame_section): New.


diff --git a/gold/ehframe.cc b/gold/ehframe.cc
index 711103b..262766e 100644
--- a/gold/ehframe.cc
+++ b/gold/ehframe.cc
@@ -567,7 +567,7 @@ Eh_frame::skip_leb128(const unsigned char** pp, const unsigned char* pend)
 // section.
 
 template<int size, bool big_endian>
-bool
+Eh_frame::Eh_frame_section_disposition
 Eh_frame::add_ehframe_input_section(
     Sized_relobj_file<size, big_endian>* object,
     const unsigned char* symbols,
@@ -584,7 +584,7 @@ Eh_frame::add_ehframe_input_section(
 							    &contents_len,
 							    false);
   if (contents_len == 0)
-    return false;
+    return EH_EMPTY_SECTION;
 
   // If this is the marker section for the end of the data, then
   // return false to force it to be handled as an ordinary input
@@ -592,7 +592,7 @@ Eh_frame::add_ehframe_input_section(
   // of unrecognized .eh_frame sections.
   if (contents_len == 4
       && elfcpp::Swap<32, big_endian>::readval(pcontents) == 0)
-    return false;
+    return EH_END_MARKER_SECTION;
 
   New_cies new_cies;
   if (!this->do_add_ehframe_input_section(object, symbols, symbols_size,
@@ -609,7 +609,7 @@ Eh_frame::add_ehframe_input_section(
 	   ++p)
 	delete p->first;
 
-      return false;
+      return EH_UNRECOGNIZED_SECTION;
     }
 
   // Now that we know we are using this section, record any new CIEs
@@ -624,7 +624,7 @@ Eh_frame::add_ehframe_input_section(
 	this->unmergeable_cie_offsets_.push_back(p->first);
     }
 
-  return true;
+  return EH_OPTIMIZABLE_SECTION;
 }
 
 // The bulk of the implementation of add_ehframe_input_section.
@@ -1206,7 +1206,7 @@ Eh_frame::do_sized_write(unsigned char* oview)
 
 #ifdef HAVE_TARGET_32_LITTLE
 template
-bool
+Eh_frame::Eh_frame_section_disposition
 Eh_frame::add_ehframe_input_section<32, false>(
     Sized_relobj_file<32, false>* object,
     const unsigned char* symbols,
@@ -1220,7 +1220,7 @@ Eh_frame::add_ehframe_input_section<32, false>(
 
 #ifdef HAVE_TARGET_32_BIG
 template
-bool
+Eh_frame::Eh_frame_section_disposition
 Eh_frame::add_ehframe_input_section<32, true>(
     Sized_relobj_file<32, true>* object,
     const unsigned char* symbols,
@@ -1234,7 +1234,7 @@ Eh_frame::add_ehframe_input_section<32, true>(
 
 #ifdef HAVE_TARGET_64_LITTLE
 template
-bool
+Eh_frame::Eh_frame_section_disposition
 Eh_frame::add_ehframe_input_section<64, false>(
     Sized_relobj_file<64, false>* object,
     const unsigned char* symbols,
@@ -1248,7 +1248,7 @@ Eh_frame::add_ehframe_input_section<64, false>(
 
 #ifdef HAVE_TARGET_64_BIG
 template
-bool
+Eh_frame::Eh_frame_section_disposition
 Eh_frame::add_ehframe_input_section<64, true>(
     Sized_relobj_file<64, true>* object,
     const unsigned char* symbols,
diff --git a/gold/ehframe.h b/gold/ehframe.h
index bb47381..e6e21b2 100644
--- a/gold/ehframe.h
+++ b/gold/ehframe.h
@@ -359,6 +359,14 @@ extern bool operator==(const Cie&, const Cie&);
 class Eh_frame : public Output_section_data
 {
  public:
+  enum Eh_frame_section_disposition
+  {
+    EH_EMPTY_SECTION,
+    EH_UNRECOGNIZED_SECTION,
+    EH_OPTIMIZABLE_SECTION,
+    EH_END_MARKER_SECTION
+  };
+
   Eh_frame();
 
   // Record the associated Eh_frame_hdr, if any.
@@ -374,7 +382,7 @@ class Eh_frame : public Output_section_data
   // returns whether the section was incorporated into the .eh_frame
   // data.
   template<int size, bool big_endian>
-  bool
+  Eh_frame_section_disposition
   add_ehframe_input_section(Sized_relobj_file<size, big_endian>* object,
 			    const unsigned char* symbols,
 			    section_size_type symbols_size,
diff --git a/gold/gold.cc b/gold/gold.cc
index ab15980..e345887 100644
--- a/gold/gold.cc
+++ b/gold/gold.cc
@@ -492,6 +492,9 @@ queue_middle_tasks(const General_options& options,
   if (timer != NULL)
     timer->stamp(0);
 
+  // Finalize the .eh_frame section.
+  layout->finalize_eh_frame_section();
+
   // Add any symbols named with -u options to the symbol table.
   symtab->add_undefined_symbols_from_command_line(layout);
 
diff --git a/gold/layout.cc b/gold/layout.cc
index 7836640..14bfda0 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -1420,15 +1420,21 @@ Layout::layout_eh_frame(Sized_relobj_file<size, big_endian>* object,
 
   elfcpp::Elf_Xword orig_flags = os->flags();
 
-  if (!parameters->incremental()
-      && this->eh_frame_data_->add_ehframe_input_section(object,
-							 symbols,
-							 symbols_size,
-							 symbol_names,
-							 symbol_names_size,
-							 shndx,
-							 reloc_shndx,
-							 reloc_type))
+  Eh_frame::Eh_frame_section_disposition disp =
+      Eh_frame::EH_UNRECOGNIZED_SECTION;
+  if (!parameters->incremental())
+    {
+      disp = this->eh_frame_data_->add_ehframe_input_section(object,
+							     symbols,
+							     symbols_size,
+							     symbol_names,
+							     symbol_names_size,
+							     shndx,
+							     reloc_shndx,
+							     reloc_type);
+    }
+
+  if (disp == Eh_frame::EH_OPTIMIZABLE_SECTION)
     {
       os->update_flags_for_input_section(shdr.get_sh_flags());
 
@@ -1440,35 +1446,49 @@ Layout::layout_eh_frame(Sized_relobj_file<size, big_endian>* object,
 	  os->set_order(ORDER_RELRO);
 	}
 
-      // We found a .eh_frame section we are going to optimize, so now
-      // we can add the set of optimized sections to the output
-      // section.  We need to postpone adding this until we've found a
-      // section we can optimize so that the .eh_frame section in
-      // crtbegin.o winds up at the start of the output section.
-      if (!this->added_eh_frame_data_)
-	{
-	  os->add_output_section_data(this->eh_frame_data_);
-	  this->added_eh_frame_data_ = true;
-	}
       *off = -1;
+      return os;
     }
-  else
+
+  if (disp == Eh_frame::EH_END_MARKER_SECTION && !this->added_eh_frame_data_)
     {
-      // We couldn't handle this .eh_frame section for some reason.
-      // Add it as a normal section.
-      bool saw_sections_clause = this->script_options_->saw_sections_clause();
-      *off = os->add_input_section(this, object, shndx, ".eh_frame", shdr,
-				   reloc_shndx, saw_sections_clause);
-      this->have_added_input_section_ = true;
+      // We found the end marker section, so now we can add the set of
+      // optimized sections to the output section.  We need to postpone
+      // adding this until we've found a section we can optimize so that
+      // the .eh_frame section in crtbeginT.o winds up at the start of
+      // the output section.
+      os->add_output_section_data(this->eh_frame_data_);
+      this->added_eh_frame_data_ = true;
+     }
 
-      if ((orig_flags & (elfcpp::SHF_WRITE | elfcpp::SHF_EXECINSTR))
-	  != (os->flags() & (elfcpp::SHF_WRITE | elfcpp::SHF_EXECINSTR)))
-	os->set_order(this->default_section_order(os, false));
-    }
+  // We couldn't handle this .eh_frame section for some reason.
+  // Add it as a normal section.
+  bool saw_sections_clause = this->script_options_->saw_sections_clause();
+  *off = os->add_input_section(this, object, shndx, ".eh_frame", shdr,
+			       reloc_shndx, saw_sections_clause);
+  this->have_added_input_section_ = true;
+
+  if ((orig_flags & (elfcpp::SHF_WRITE | elfcpp::SHF_EXECINSTR))
+      != (os->flags() & (elfcpp::SHF_WRITE | elfcpp::SHF_EXECINSTR)))
+    os->set_order(this->default_section_order(os, false));
 
   return os;
 }
 
+void
+Layout::finalize_eh_frame_section()
+{
+  // If we never found an end marker section, we need to add the
+  // optimized eh sections to the output section now.
+  if (!parameters->incremental()
+      && this->eh_frame_section_ != NULL
+      && !this->added_eh_frame_data_)
+    {
+      this->eh_frame_section_->add_output_section_data(this->eh_frame_data_);
+      this->added_eh_frame_data_ = true;
+    }
+}
+
 // Create and return the magic .eh_frame section.  Create
 // .eh_frame_hdr also if appropriate.  OBJECT is the object with the
 // input .eh_frame section; it may be NULL.
diff --git a/gold/layout.h b/gold/layout.h
index aec0c53..9039ee8 100644
--- a/gold/layout.h
+++ b/gold/layout.h
@@ -635,6 +635,12 @@ class Layout
 		  unsigned int reloc_shndx, unsigned int reloc_type,
 		  off_t* offset);
 
+  // After processing all input files, we call this to make sure that
+  // the optimized .eh_frame sections have been added to the output
+  // section.
+  void
+  finalize_eh_frame_section();
+
   // Add .eh_frame information for a PLT.  The FDE must start with a
   // 4-byte PC-relative reference to the start of the PLT, followed by
   // a 4-byte size of PLT.

Re: PATCH: PR gold/14675: No eh_frame info registered in exception_static_test

[H.J. Lu]

On Mon, Mar 9, 2015 at 10:16 AM, Cary Coutant <ccoutant@google.com> wrote:
>> Any progress?
>
> I've committed the attached patch to fix this problem by delaying the
> attachment of the optimized .eh_frame sections to the output section
> until we see the end marker section (or to the end of pass 1 if we
> never see an end marker).
>
> -cary
>
>
> 2015-03-09  Cary Coutant  <ccoutant@google.com>
>
> gold/
>         PR gold/14675
>         * ehframe.cc (Eh_frame::add_ehframe_input_section): Change return type;
>         return enum indicating whether .eh_frame section is empty, optimizable,
>         unrecognized, or an end marker. Adjust explicit instantiations.
>         * ehframe.h (Eh_frame::Eh_frame_section_disposition): New enum type.
>         (Eh_frame::add_ehframe_input_section): Change return type.
>         * gold.cc (queue_middle_tasks): Call Layout::finalize_eh_frame_section.
>         * layout.cc (Layout::layout_eh_frame): Don't add optimized sections
>         to the .eh_frame output section until we see the end marker.
>         (Layout::finalize_eh_frame_section): New.
>         * layout.h: (Layout::finalize_eh_frame_section): New.

Thanks.

-- 
H.J.

Re: PATCH: PR gold/14675: No eh_frame info registered in exception_static_test

[H.J. Lu]

On Mon, Mar 9, 2015 at 10:16 AM, Cary Coutant <ccoutant@google.com> wrote:
>> Any progress?
>
> I've committed the attached patch to fix this problem by delaying the
> attachment of the optimized .eh_frame sections to the output section
> until we see the end marker section (or to the end of pass 1 if we
> never see an end marker).
>
> -cary
>
>
> 2015-03-09  Cary Coutant  <ccoutant@google.com>
>
> gold/
>         PR gold/14675
>         * ehframe.cc (Eh_frame::add_ehframe_input_section): Change return type;
>         return enum indicating whether .eh_frame section is empty, optimizable,
>         unrecognized, or an end marker. Adjust explicit instantiations.
>         * ehframe.h (Eh_frame::Eh_frame_section_disposition): New enum type.
>         (Eh_frame::add_ehframe_input_section): Change return type.
>         * gold.cc (queue_middle_tasks): Call Layout::finalize_eh_frame_section.
>         * layout.cc (Layout::layout_eh_frame): Don't add optimized sections
>         to the .eh_frame output section until we see the end marker.
>         (Layout::finalize_eh_frame_section): New.
>         * layout.h: (Layout::finalize_eh_frame_section): New.

This caused:

https://sourceware.org/bugzilla/show_bug.cgi?id=18152

-- 
H.J.