Results for last successful build are here:
http://gcc.gnu.org/ml/gcc-testresults/2006-08/msg00530.html

Don't recall any backend changes since then.

I'm thinking this may have been caused by the emutls patch
which was subsequently reverted.

This problem was introduced by this change:

Author: rguenth
Date: Tue Oct 10 08:27:02 2006
New Revision: 117598

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=117598
Log:
2006-10-10  Richard Guenther  <rguenther@suse.de>

        PR rtl-optimization/29323
        * except.c (set_nothrow_function_flags): For functions
        that do not bind local bail out early.

        * decl.c (finish_function): Set TREE_NOTHROW only for
        functions that bind local.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/decl.c
    trunk/gcc/except.c

(In reply to comment #3)
> This problem was introduced by this change:
That makes less sense really, because this just changes how to deal with TREE_NOTHROW.  This sounds like a latent bug really.

Subject: Re:  Shared libstdc++ fails to link

> > This problem was introduced by this change:
> That makes less sense really, because this just changes how to deal with
> TREE_NOTHROW.  This sounds like a latent bug really.

The question is where.  The linker error message indicates that
we aren't generating a PLABEL for a function entry.  Unfortunately,
this entry point is a local symbol that has been reduced to a text
section offset.  That's what "$CODE$" indicates.  There are quite
a few in the stdexcept.o and so far I haven't been able to figure
out which symbol is causing the error.

Dave

Subject: Re:  Shared libstdc++ fails to link

On Mon, Nov 13, 2006 at 02:37:02AM -0000, pinskia at gcc dot gnu dot org wrote:

> > This problem was introduced by this change:
> That makes less sense really, because this just changes how to deal with
> TREE_NOTHROW.  This sounds like a latent bug really.

The change does affect the EH table (diff attached).  Hacking the
assembler output for stdexcept, I find that it's the following
reference that causes the error:

	.word   L$FB0940

This symbol is a local alias for _ZNSt12domain_errorD0Ev.  This
symbol also occurs in functexcept.o which occurs earlier in the
link list.  So, it looks as if the linker can't handle absolute
references to local code symbols in the second and subsequent
instances of a function.

Dave

Created attachment 12647 [details]
stdexcept.s.diff

Subject: Re:  Shared libstdc++ fails to link

> > > This problem was introduced by this change:
> > That makes less sense really, because this just changes how to deal with
> > TREE_NOTHROW.  This sounds like a latent bug really.
> 
> The change does affect the EH table (diff attached).  Hacking the
> assembler output for stdexcept, I find that it's the following
> reference that causes the error:
> 
>         .word   L$FB0940

The TREE_NOTHROW change exposes a problem in HP ld that only can be fixed
by HP and that's not going to happen.  The TREE_NOTHROW change has been
applied to 4.1, so we have a regression.  However, it probably would be
possible to trigger this problem even without the TREE_NOTHROW change
(e.g., setting flag_non_call_exceptions).  So, I think the only fix
is to revert DWARF2 EH support on this target on 4.1 and later ;)

I've debugged HP ld sufficiently to determine the nature of the problem.
Weak symbol support is implemented on this target using "COMDAT" subspaces.
When duplicates occur in a link, the HP linker nullifies symbol references
to the second and latter occurences of symbols in the "COMDAT" subspace.
It does this by changing the symbol type to ST_NULL.  As a result, the
references to omitted subspaces change from ST_DATA to ST_NULL.  This
triggers the DATA_ONE_SYM_FOR_PLAB error from HP ld when we hit a reference
in the DWARF2 EH table to an omitted subspace.

The message is actually somewhat misleading as we nominally started with
a ST_DATA symbol and not a ST_CODE code symbol.  Well, actually the symbol
is a code symbol but the default is to treat code labels as data symbols!

I tried changing the absolute reference to a plabel reference but this
results in another problem.

Attached is a simplified bit of assembler output from stdexcept.cc
and a small hack to HP ld which might work around the problem.  To
trigger the problem, the assembled output from pr29487.s needs to included
twice in a shared link.

Dave

Created attachment 12737 [details]
pr29487.s

Created attachment 12738 [details]
fixups.c.d

The patch mentioned in comment #3 was applied to the 4.1 branch and
introduces a regression in 4.1.2 on hppa1.1-hp-hpux10.20.

As a result, it's no longer possible to use EH exception support on
this target.  This is the default.  Unfortunately, I haven't had a
chance to change config.gcc to disable use of EH exceptions or to
document this problem.

The change installed to fix PR rtl-optimization/29323 causes EH
data to emitted for all functions which don't bind locally.  This
exposed a latent bug in the HP linker; it can't handle the relocations
relating to COMDAT sections that have been nullified.

On the one hand, this is likely to cause problems for weak (COMDAT)
functions which can throw.  On the other hand, there aren't any in
libstdc++ and the problem didn't expose itself in building at least
one large C++ application (lyx).

Personally, I believe that the fix for PR 29323 was wrong and has
bloated the EH data emitted by GCC.  The EH data for a module are
only relevant to the functions in the module itself.  If a function
in a module can't throw, then we don't need EH exception data for
it.

I think we need to decide how TREE_NOTHROW is to be used.  If
it's to be used to control the emission of EH data, then it should
be set based on analysis of the module being compiled and not whether
a function binds locally or not. 


 

>  If a function in a module can't throw, then we don't need EH exception data
> for it.
Only if the use specifically marked it as such.  Really you can replace the weak function with any other function which then just throws.  I don't see how that patch is wrong, it makes the throw optimization the same as the inliner and const/pure optimizations.  Now emitting EH info for the function itself is a different story.  Right now the use TREE_NOTHROW is two fold, it is used in dwarf2out to ask the question does the function compiling actually throw, and every where else does this function decl throw.

At one point, the meaning was only the last one and then Paolo Bonzini changed it to mean the first one too (though in a sense Richard's patch needs to be updated to just not set TREE_NOTHROW for the binds local case).

Patch which added the extra meaning of TREE_NOTHROW:
http://gcc.gnu.org/ml/gcc-patches/2004-07/msg00559.html

Subject: Re:  Shared libstdc++ fails to link

> ------- Comment #12 from pinskia at gcc dot gnu dot org  2007-02-03 02:08 -------
> >  If a function in a module can't throw, then we don't need EH exception data
> > for it.
> Only if the use specifically marked it as such.  Really you can replace the
> weak function with any other function which then just throws.  I don't see how
> that patch is wrong, it makes the throw optimization the same as the inliner
> and const/pure optimizations.  Now emitting EH info for the function itself is
> a different story.  Right now the use TREE_NOTHROW is two fold, it is used in
> dwarf2out to ask the question does the function compiling actually throw, and
> every where else does this function decl throw.
> 
> At one point, the meaning was only the last one and then Paolo Bonzini changed
> it to mean the first one too (though in a sense Richard's patch needs to be
> updated to just not set TREE_NOTHROW for the binds local case).

Still stand by my comment.  If a weak function is replaced, the EH data
used in handling any exception will be that for the replacement.  So if
a function is marked as nothrow or analysis indicates it can't throw,
then we don't need to output EH data for it.

While the GNU linker may be able to eliminate unnecessary EH data, this
isn't the case for the HP linker.  In HP-UX 11, we don't use SDEf symbols
instead of COMDAT sections (subspaces).  I'm fairly certain in this case
that we end up with multiple function copies and EH data.

The two fold use of TREE_NOTHROW is the problem.  I haven't looked at
this in detail but I would have thought that a combination of the first
meaning together with knowing whether a function can be overloaded/replaced
would be sufficient to determine whether this function might throw.

Dave

Subject: Re:  Shared libstdc++ fails to link

Sorry, typo:

> isn't the case for the HP linker.  In HP-UX 11, we don't use SDEf symbols
                                                     use SDEF
Dave

Subject: Re:  Shared libstdc++ fails to link

danglin at gcc dot gnu dot org wrote:

> Personally, I believe that the fix for PR 29323 was wrong and has
> bloated the EH data emitted by GCC.  The EH data for a module are
> only relevant to the functions in the module itself.  If a function
> in a module can't throw, then we don't need EH exception data for
> it.

I'm not sure what "EH data" is being described here.  Certainly, it
makes no sense at all to emit EH unwind information for functions which
are not part of the current object file; their unwind information will
be emitted with those function.

What sort of data is being emitted?

Subject: Re:  Shared libstdc++ fails to link

> I'm not sure what "EH data" is being described here.  Certainly, it
> makes no sense at all to emit EH unwind information for functions which
> are not part of the current object file; their unwind information will
> be emitted with those function.
> 
> What sort of data is being emitted?

Unwind data.  We're talking about functions compiled in the
current object.

On the HP-UX PA-RISC targets,  unwind data is placed in the data
subspace.  In principle, there's no problem with other EH data as
the HP linker can handle the relocations in "noload" subspaces.

In HP-UX 10, one-only was implemented using COMDAT subspaces.
When the linker sees the second or later occurence of a COMDAT
subspace with the same name as the first one, it nullifies these
subspaces and doesn't emit them in the final object.  However,
it turns out that that the linker can't handle relocations referring
to symbols in that are in a nullified subspace if these references
occur in a subspace that is loadable.  Didn't see this when
developing the DWARF2 unwind support for the 4.1 branch because
of the following issue.

There are numerous "one-only/weak" functions which don't bind locally
in libstdc++.  Previously, *ALL* these functions were determined to be
nothrow and no unwind data was emitted for them.  Now, unwind
data is being emitted because these functions don't bind locally.
This breaks the HP-UX 10 DWARF2 unwind implementation (obviously a
latent bug) because of the relocation issue mentioned above.  It also
increases the amount of DWARF2 unwind data emitted on all targets
using this exception mechanism and might have an impact on search
performance when unwinding.

It's my belief that DWARF2 unwind support present in GCC 4.1.0
and 4.1.1 for HP-UX 10 was usable in spite of the linker limitation.

Dave

Subject: Re:  Shared libstdc++ fails to link

dave at hiauly1 dot hia dot nrc dot ca wrote:

> Unwind data.  We're talking about functions compiled in the
> current object.

OK.

I'm not sure it matters, but if these functions don't throw exceptions,
I don't understand why we're not marking them TREE_NOTHROW.  I suspect
there's something that I'm just not following.

The fact that linker semantics allow you to replace a function at the
object level do not make it valid at the language level.

So, for example, I would expect:

  void f () throw () {}

and:

  void g() {}

to both be TREE_NOTHROW, independent of linkage, weakness, etc.
Certainly, not marking such functions as TREE_NOTHROW will inhibit
optimization of their callers, which seems bad.

Why aren't the functions being marked TREE_NOTHROW?

> There are numerous "one-only/weak" functions which don't bind locally
> in libstdc++.  Previously, *ALL* these functions were determined to be
> nothrow and no unwind data was emitted for them.  Now, unwind
> data is being emitted because these functions don't bind locally.
> This breaks the HP-UX 10 DWARF2 unwind implementation (obviously a
> latent bug) because of the relocation issue mentioned above.

I understand that this is a latent bug in the HP-UX linker.

To be honest, I'm less concerned about HP-UX 10.10 per se than about the
possible bloat on other targets.  (HP-UX 10 is not a primary or
secondary target.)  At the same time, I certainly don't want to
gratuitously break any target.

Assuming that the changes in TREE_NOTHROW and emission of exception
information make sense, what solution would you implement for HP-UX 10?
 Use SJLJ exception handling?

Thanks,

Subject: Re:  Shared libstdc++ fails to link

> I'm not sure it matters, but if these functions don't throw exceptions,
> I don't understand why we're not marking them TREE_NOTHROW.  I suspect
> there's something that I'm just not following.

See the testcase for PR 29323 and the initial problem description.

> The fact that linker semantics allow you to replace a function at the
> object level do not make it valid at the language level.
> 
> So, for example, I would expect:
> 
>   void f () throw () {}

This is essentially identical to the example in weakthrow.C except
that the function there is annotated with __attribute__ ((weak)).

It was argued in PR 29323 that it was incorrect to mark functions
that don't bind locally with TREE_NOTHROW.

I'm not sure whether it's valid at the language level to replace
a function that can't throw with one that can.  However, as far
as unwind data goes, the only thing that matters is whether the
function being compiled needs unwind data or not.

> Why aren't the functions being marked TREE_NOTHROW?

When a function doesn't bind locally, it may be overloaded/replaced
by one that does throw.  So, we no longer mark such functions with
TREE_NOTHROW.  This results in unwind data being emitted for functions
that would have been marked TREE_NOTHROW if they were local.

> Assuming that the changes in TREE_NOTHROW and emission of exception
> information make sense, what solution would you implement for HP-UX 10?
>  Use SJLJ exception handling?

Yes, config.gcc could be modified to force SJLJ exception handling.
Of course, I'm hoping for a better fix to PR 29323.

Dave

Subject: Re:  Shared libstdc++ fails to link

dave at hiauly1 dot hia dot nrc dot ca wrote:
> ------- Comment #18 from dave at hiauly1 dot hia dot nrc dot ca  2007-02-05 04:02 -------
> Subject: Re:  Shared libstdc++ fails to link
> 
>> I'm not sure it matters, but if these functions don't throw exceptions,
>> I don't understand why we're not marking them TREE_NOTHROW.  I suspect
>> there's something that I'm just not following.
> 
> See the testcase for PR 29323 and the initial problem description.

Thanks.

The fact that the declaration is explicitly declare weak seems somewhat
persuasive.  C++ has no notion of weak symbols, so once you declare a
function weak, you're outside the standard anyhow, and it's reasonable
to say that you can replace such a function with one of the same type.

However, that doesn't mean that the check for binds_local makes sense.
In particular, a COMDAT template instantiation may not bind locally
(IIUC), but it's invalid to replace the template instantiation with one
that behaves differently.  So, it sounds like the change that's been
made unduly pessimizes C++ template instantiations, which would
plausibly result in the behavior you're seeing.

And, as you say, even for explicitly weak functions, it doesn't make
sense to emit EH unwind information if the functions never throw.
That's just a pessimization.  So, as has been said elsewhere, that
suggests that -- for the weak function case -- we need to distinguish
"body of this function needs unwind information" from "callers of this
function must assume it throws exceptions".

Frankly, my inclination would be to just declare that in GNU C++,
replacing a "weak" function with an implementation that throws more
exceptions than the version in the current translation unit is invalid,
no diagnostic required.  This doesn't seem like a case worth twisting
our internal representations around to support.

>> Assuming that the changes in TREE_NOTHROW and emission of exception
>> information make sense, what solution would you implement for HP-UX 10?
>>  Use SJLJ exception handling?
> 
> Yes, config.gcc could be modified to force SJLJ exception handling.
> Of course, I'm hoping for a better fix to PR 29323.

Right.

I've CC'd Richard G on the PR.  Richard, what are your thoughts?

What we want to prevent with the patch for PR29323 is the TREE_NOTHROW flag
propagating to a locally binding function.  Consider

void foo() nothrow __attribute__((weak)) {}

void bar()
{
  foo();
}

we need EH unwind data emitted for bar() even if foo() is marked or
analyzed as nothrow as at run time bar() might call a foo() that throws.

At least if using C and -fexceptions this is a valid use (we of course
can declare this invalid for C++, but this can be a runtime error only
which will then be hard to diagnose).

I'm in no way expert enough to say if we can omit EH data for foo() in
the above case (we probably can), but if so then splitting the TREE_NOTHROW
flag into two is probably the right thing to go.

I suppose PR29323 was found by inspection of GCC code rather than a real-world
testcase so the option to revert that patch on the 4.1 branch looks appealing.

(CCed Joern to clarify)

CCint Paolo who changed the meaning of TREE_NOTHROW.

If you refer to http://gcc.gnu.org/ml/gcc-patches/2004-07/msg00559.html this little patch would undo any semantic changes introduced there.  I didn't follow the discussion however and I don't know if this is the correct fix.

Index: except.c
===================================================================
--- except.c    (revision 121287)
+++ except.c    (working copy)
@@ -2785,6 +2785,9 @@ set_nothrow_function_flags (void)
 {
   rtx insn;
 
+  if (TREE_NOTHROW (current_function_decl))
+    return 0;
+
   if (!targetm.binds_local_p (current_function_decl))
     return 0;
 

(In reply to comment #20)
> I suppose PR29323 was found by inspection of GCC code rather than a real-world
> testcase so the option to revert that patch on the 4.1 branch looks appealing.
> 
> (CCed Joern to clarify)

It wasn't as clear-cut as this.  I was investigating a bug which later turned out to be a duplicate of PR27781.  I was searching for the point where the
invalid analysis was made, and TREE_NOTHROW was more easily visible than
constness (as in ECF_CONST).  So, I was working on a real testcase that was
affected manifestly by PR27781, and latently by PR29323, i.e. the function
was incorrectly tagged as nothrow, although that incorrect piece of analysis
had no effect in the original testcase.  (Omitting the function call
altogether, as was prompted by the invalid constness analysis, had an
oversable effect, though - which was the bug I was really working on).

(In reply to comment #18)
> It was argued in PR 29323 that it was incorrect to mark functions
> that don't bind locally with TREE_NOTHROW.
> 
> I'm not sure whether it's valid at the language level to replace
> a function that can't throw with one that can.

Remember that the determination if the function can throw or not is made by
analyzing the rtl of the function.  I.e. there might have been a throw
statement in the source, which was optimized away, so that the compiler
can then determine that the function does not actually throw.
Thus, TREE_NOTHROW can depend on how good the compiler is at solving specific
instances of the halting problem.  These might involve mathematical theorems
that are yet unproven, but might be in the future so that a funture compiler
can make use of it.
Thus, if you require that a function that can't throw can't be replaced by one that can, you won't be able to decide if the program is valid till the mathematical theorem is either proven or disproven.

Moreover, if, as the languguage level, you consider the C or C++ standard, we can't actually replace functions in first place.
If that is what you want, you can use the appropriate visibility attribute
in the delacation, and save a lot of overhead for each function call.
If you want the function to be replacable, but don;t want the overhead of it
being consider potentially throwing, you can again express this with the
appropriate attribute in the declaration.
 
> 
> > Assuming that the changes in TREE_NOTHROW and emission of exception
> > information make sense, what solution would you implement for HP-UX 10?

If the affected library functions in fact are not supposed to be replaced by
functions that can throw, adding declarations with nothrow attribute will
get you the performance back for the callers.
It would also solve the immediate problem of the excess EH information for
the function definitions, although I agree that we generally shouldn't emit
it when we can reasonably determine that the actual function definition
doesn't throw, even when the declaration says that it may.

Subject: Re:  Shared libstdc++ fails to link

rguenth at gcc dot gnu dot org wrote:

[Paolo, see below for question.]

> ------- Comment #20 from rguenth at gcc dot gnu dot org  2007-02-05 09:06 -------
> What we want to prevent with the patch for PR29323 is the TREE_NOTHROW flag
> propagating to a locally binding function.  Consider
> 
> void foo() nothrow __attribute__((weak)) {}
> 
> void bar()
> {
>   foo();
> }
> 
> we need EH unwind data emitted for bar() even if foo() is marked or
> analyzed as nothrow as at run time bar() might call a foo() that throws.
> 
> At least if using C and -fexceptions this is a valid use (we of course
> can declare this invalid for C++, but this can be a runtime error only
> which will then be hard to diagnose).

I think I agree that the C -fexceptions example is compelling, and I
find the same example in C++ reasonably compelling too, the ODR
notwithstanding, since that's clearly the intent of __attribute__((weak)).

> I'm in no way expert enough to say if we can omit EH data for foo() in
> the above case (we probably can), but if so then splitting the TREE_NOTHROW
> flag into two is probably the right thing to go.

I don't think there's any reason we should need EH data for foo.  I
would think that it is in fact reasonable to mark "foo" TREE_NOTHROW.
However, when analyzing bar() (which we do by walking its body, looking
for calls, to work out whether anything that bar calls can throw an
exception), we should avoid marking "bar" TREE_NOTHROW in this case.

I think the analysis should be deeper than just "binds_local_p", though;
I think it should be based on whether "attribute ((weak))" (or
equivalent) explicitly appears, so that we don't pessimize bar if foo is
in a template function, out-of-line inline, or other such function.  The
test ought to be based on the demonstrable intent of the user to replace
the function, not on whether or not the function happens to be weak.

> I suppose PR29323 was found by inspection of GCC code rather than a real-world
> testcase so the option to revert that patch on the 4.1 branch looks appealing.

I agree.  If my suggestion above seems correct, then the first step is
to undo the change that make "foo" not marked TREE_NOTHROW.  Since that
restores the 4.1.1 status quo, will presumably fix HP-UX 10.10, and
eliminate any pessimization in 4.1.2, I think we should do that.  The
same reversion should be applied to mainline and 4.2, since the same
considerations apply there.  If we get excited about fixing the
pre-existing problem with weak functions, we can do that later.

Paolo, would you be able to undo the change to make "foo" not marked
TREE_NOTHROW?  IIUC, that would be different than the patch you posted
in Comment #22, which appears to affect "bar".  Also, I didn't quite
understand your patch, in that it would appear to result in fewer
functions being marked TREE_NOTHROW, whereas we want more functions to
be so marked to restore status quo?  I would think we want to remove the
check for binds_local_p at the top of set_nothrow_function_flags?

Thanks,

(In reply to comment #22)
> If you refer to http://gcc.gnu.org/ml/gcc-patches/2004-07/msg00559.html this
> little patch would undo any semantic changes introduced there.  I didn't follow
> the discussion however and I don't know if this is the correct fix.

No.  What is needed is a way to again remember when the current function
doesn't throw (without altering its declaration), so that exception handling
information generation can be suppressed.

(In reply to comment #25)
> I think the analysis should be deeper than just "binds_local_p", though;
> I think it should be based on whether "attribute ((weak))" (or
> equivalent) explicitly appears, so that we don't pessimize bar if foo is
> in a template function, out-of-line inline, or other such function.  The
> test ought to be based on the demonstrable intent of the user to replace
> the function, not on whether or not the function happens to be weak.

What about an exported definition in a shared library?  Users expect to be
able to replace such definitions.  Should we constrain them in what they
can replace these definitions with by requiring them not to extend the
throw specification beyond what gcc could figure out about it using data
and control flow analysis on the original definition?
That defeats the purpose of defining an interface with prototypes.
 > 
> I agree.  If my suggestion above seems correct, then the first step is
> to undo the change that make "foo" not marked TREE_NOTHROW.

Sorry, but at the moment we have no way of making "foo" marked TREE_NOTHROW.
We can only mark its declaration TREE_NOTHROW, which then causes all the
callers to assume that the function of that name doesn't throw.

Subject: Re:  Shared libstdc++ fails to link

amylaar at gcc dot gnu dot org wrote:
> ------- Comment #27 from amylaar at gcc dot gnu dot org  2007-02-05 19:52 -------
> (In reply to comment #25)
>> I think the analysis should be deeper than just "binds_local_p", though;
>> I think it should be based on whether "attribute ((weak))" (or
>> equivalent) explicitly appears, so that we don't pessimize bar if foo is
>> in a template function, out-of-line inline, or other such function.  The
>> test ought to be based on the demonstrable intent of the user to replace
>> the function, not on whether or not the function happens to be weak.
> 
> What about an exported definition in a shared library?  Users expect to be
> able to replace such definitions.  Should we constrain them in what they
> can replace these definitions with by requiring them not to extend the
> throw specification beyond what gcc could figure out about it using data
> and control flow analysis on the original definition?

Yes, I believe that we should forbid this kind of replacement.  We can,
if necessary, make GCC not be more conservative than the original
exception-specification if present, so that:

  void foo() throw (X) {}

still results in the compiler thinking that "foo" may throw "X", but we
should not extent that to thinking that:

  void foo() {}

can throw anything.  That's making the compiler generate inferior code,
for a corner-case situation, outside the realm of the language.  Making
the user use an attribute to declare what they mean is not unreasonable.

> That defeats the purpose of defining an interface with prototypes.
>  > 
>> I agree.  If my suggestion above seems correct, then the first step is
>> to undo the change that make "foo" not marked TREE_NOTHROW.
> 
> Sorry, but at the moment we have no way of making "foo" marked TREE_NOTHROW.
> We can only mark its declaration TREE_NOTHROW, which then causes all the
> callers to assume that the function of that name doesn't throw.

Yes, which was the status quo before your patch.  I don't see any good
argument for exchanging the relatively obscure problem fixed by the
patch for a different set of problems: broken HP-UX 10.10, and inferior
code for C++ applications that are not using either weak functions or
shared libraries.  Therefore, I think we should go back to marking "foo"
TREE_NOTHROW, and look for a more a complete fix to both your original
problem and the other cases.

Subject: Re:  Shared libstdc++ fails to link


> Paolo, would you be able to undo the change to make "foo" not marked
> TREE_NOTHROW?  IIUC, that would be different than the patch you posted
> in Comment #22, which appears to affect "bar".

My patch was related to Richi's comment when CCing me; the only patch I 
found from me that was related to TREE_NOTHROW subtly changed the 
semantics, and that patch sort of undone that change (I say "sort of" 
because it takes a little more care actually).

> Also, I didn't quite
> understand your patch, in that it would appear to result in fewer
> functions being marked TREE_NOTHROW

It would result in more functions being marked TREE_NOTHROW, since those 
functions will not go through the insn loop in 
set_nothrow_function_flags if the front-end declared them nothrow.

I'm attaching an updated version of the patch, which passes some 
internal (i.e. my brain and a small testcase using weak and nothrow) 
sanity checks, but I've not tried in any real world situation.  It's 
actually an alternate fix for PR29323 which doesn't trigger this bug. 
I'll let other people consider if it makes any sense since I'm not at 
all expert in this area.

> I would think we want to remove the
> check for binds_local_p at the top of set_nothrow_function_flags?

Agreed, and we have to move it elsewhere.  (See the upcoming patch).

Paolo

Created attachment 13011 [details]
proposed, untested patch

The patch proposed makes sense, Dave can you verify it fixes this PR for you?  I'll spin some testing on the trunk in a moment.

(In reply to comment #30)
> Created an attachment (id=13011) [edit]
> proposed, untested patch
> 

As far as I can tell, this patch takes care of the correctness issues,
however at the expense of optimization.  Declaring a function as nothrow
will no longer enable optimizations in the callers that depend on the
the callee to be nothrow.  Including analysis that finds the caller
to be nothrow, so again, exception handling information of libraries
is likely to increase.

I don't think it makes sense that you hijack the tree flag of the declaration
to be not about the declaration at all.

Andrew Pinski's observation in #12 that the root problem is to try to squish
two bits int one still stands.  IMO the bit formerly known as
current_function_nothrow should be put into struct function, and then found
via cfun.

It does not work either:

/abuild/rguenther/obj-29487/./gcc/xgcc -shared-libgcc -B/abuild/rguenther/obj-29487/./gcc -nostdinc++ -L/abuild/rguenther/obj-29487/x86_64-unknown-linux-gnu/libstdc++-v3/src -L/abuild/rguenther/obj-29487/x86_64-unknown-linux-gnu/libstdc++-v3/src/.libs -B/usr/local/x86_64-unknown-linux-gnu/bin/ -B/usr/local/x86_64-unknown-linux-gnu/lib/ -isystem /usr/local/x86_64-unknown-linux-gnu/include -isystem /usr/local/x86_64-unknown-linux-gnu/sys-include -Winvalid-pch -Wno-deprecated -x c++-header -g -O2  -D_GNU_SOURCE -I/abuild/rguenther/obj-29487/x86_64-unknown-linux-gnu/libstdc++-v3/include/x86_64-unknown-linux-gnu -I/abuild/rguenther/obj-29487/x86_64-unknown-linux-gnu/libstdc++-v3/include -I/abuild/rguenther/gcc/libstdc++-v3/libsupc++ -O2 -g /abuild/rguenther/gcc/libstdc++-v3/include/precompiled/extc++.h -o x86_64-unknown-linux-gnu/bits/extc++.h.gch/O2g.gch
In file included from /abuild/rguenther/gcc/libstdc++-v3/include/precompiled/extc++.h:65:
/abuild/rguenther/obj-29487/x86_64-unknown-linux-gnu/libstdc++-v3/include/ext/pb_ds/exception.hpp: In function 'void pb_ds::__throw_insert_error()':
/abuild/rguenther/obj-29487/x86_64-unknown-linux-gnu/libstdc++-v3/include/ext/pb_ds/exception.hpp:79: error: statement marked for throw, but doesn't
D.155211_27 = D.155210_26;

/abuild/rguenther/obj-29487/x86_64-unknown-linux-gnu/libstdc++-v3/include/ext/pb_ds/exception.hpp:79: error: statement marked for throw, but doesn't
D.155275_49 = D.155274_48;

/abuild/rguenther/obj-29487/x86_64-unknown-linux-gnu/libstdc++-v3/include/ext/pb_ds/exception.hpp:79: internal compiler error: verify_stmts failed
Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.
make[4]: *** [x86_64-unknown-linux-gnu/bits/extc++.h.gch/O2g.gch] Error 1

The two bits were actually the same, since passes.c was doing this exactly after calling set_nothrow_function_flags

  if (current_function_nothrow)
    /* Now we know that this can't throw; set the flag for the benefit
       of other functions later in this translation unit.  */
    TREE_NOTHROW (current_function_decl) = 1;

The patch I proposed does not hinder optimization "that much". Declaring a function as nothrow will still enable optimizations in the callers that depend on the the callee to be nothrow, as far as the callee binds locally; this includes static functions, non-default visibility functions, and locally declared non-weak functions.

Richard, another necessary hunk for correctness would be this:

Index: cvs/gcc/gcc/tree-eh.c
===================================================================
--- cvs/gcc/gcc/tree-eh.c       (revision 120669)
+++ cvs/gcc/gcc/tree-eh.c       (working copy)
@@ -1979,8 +1979,7 @@ tree_could_trap_p (tree expr)
 
     case CALL_EXPR:
       t = get_callee_fndecl (expr);
-      /* Assume that calls to weak functions may trap.  */
-      if (!t || !DECL_P (t) || DECL_WEAK (t))
+      if (!t || !DECL_P (t) || !targetm.binds_local_p (t))
        return true;
       return false;
 


Finally, let me remark again that I'm *no expert in this area*.  I'd be pretty nervous if this patch ended up in 4.1.2 without huge scrutiny from experts.

I'll prepare a patch to revert my 2004 change too.

 (In reply to comment #25)
> I think the analysis should be deeper than just "binds_local_p", though;
> I think it should be based on whether "attribute ((weak))" (or
> equivalent) explicitly appears, so that we don't pessimize bar if foo is
> in a template function, out-of-line inline, or other such function.  The
> test ought to be based on the demonstrable intent of the user to replace
> the function, not on whether or not the function happens to be weak.

Thinking a bit more about this, I agree.  If the function is declared
in the same translation unit, and no attribute or pragma prevents the
function from being inlined, it is already fair game for inlining, irrespective
of wether it binds locally or not.  Thus, if we decide for pragmatic reasons
(code size, snafus in analysis) not to inline such a function, we should still
be able to use analysis results about nothrow, const and pure characteristics
of the function.

When we get to the point where we can do cross-module optimizations (LTO or
otherwise), some other issues arise:
The situation is a bit different if the function is defined in a different
translation unit.  For shared libraries, putting functions into separate
translation unit is a common technique to show that the function should
not be inlined into a function in a different translation unit, or any
analysis results applied to a caller/callee in a different translation unit.
But it would make sense to have an option to enable such inlining / analysis,
and even to turn it on by default, since for any ordinary conformant program
the one-definition rule applies.

(In reply to comment #34)
> I'll prepare a patch to revert my 2004 change too.

I suspect that a 100% literal reversion will run into problems where the
use of a global variable will result in the the analysis of one function
being applied to another function.

It will probably work (modulo minor merge conflicts) if instead of
bringing the global variable current_function_nothrow back, you define
a nothrow bit in struct function and
#define current_function_nothrow cfun->nothrow
.

Subject: Re:  Shared libstdc++ fails to link

> The patch proposed makes sense, Dave can you verify it fixes this PR for you? 
> I'll spin some testing on the trunk in a moment.

Yes.  I'll try when an updated patch is ready.  I understand the first
candidate doesn't work.

My hpux 10 system is just finishing a build and check.  I'll also tweak
the backend to use COMDAT support on hpux 11 and do a build on it.  That's
much faster.

Dave

Subject: Re:  Shared libstdc++ fails to link

> Created an attachment (id=13011)
>  --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=13011&action=view)
> proposed, untested patch

I did a build and check of c and c++ on 4.1.2 using the above patch
modified for the 4.1 branch on HP-UX 11.11 (attached).  I modified
the pa-hpux11.h file to disable TARGET_SOM_SDEF so that the COMDAT
support used with HP-UX 10 was used for one-only support.

I had to include target.h in one file but I didn't bother updating
the Makefile dependencies.

The build was successful and libstdc++-v3 linked successfully.  There
was one minor regression.  nothrow1.C failed in the g++ testsuite.
I've attached the tree dump.

Dave

Created attachment 13016 [details]
nothrow1.C.t93.optimized

Created attachment 13017 [details]
pr29487_patch-4.1.2.txt

Subject: Bug 29487

Author: mmitchel
Date: Fri Feb  9 02:52:53 2007
New Revision: 121738

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=121738
Log:
	PR target/29487
	* tree.h (DECL_REPLACEABLE_P): New macro.
	* except.c (set_nothrow_function_flags): Likewise.

	PR target/29487
	* decl.c (finish_function): Use DECL_REPLACEABLE.
	* tree.c (cp_cannot_inline_tree_fn): Likewise.

	PR c++/29487
	* g++.dg/eh/weak1-C: New test.
	* g++.dg/eh/weak1-a.cc: Likewise.
	* g++.dg/eh/comdat1.C: Likewise.

Added:
    branches/gcc-4_1-branch/gcc/testsuite/g++.dg/eh/comdat1.C
    branches/gcc-4_1-branch/gcc/testsuite/g++.dg/eh/weak1-a.cc
    branches/gcc-4_1-branch/gcc/testsuite/g++.dg/eh/weak1.C
Modified:
    branches/gcc-4_1-branch/gcc/ChangeLog
    branches/gcc-4_1-branch/gcc/cp/ChangeLog
    branches/gcc-4_1-branch/gcc/cp/decl.c
    branches/gcc-4_1-branch/gcc/cp/tree.c
    branches/gcc-4_1-branch/gcc/except.c
    branches/gcc-4_1-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_1-branch/gcc/tree.h

Subject: Bug 29487

Author: mmitchel
Date: Sat Feb 10 01:02:30 2007
New Revision: 121788

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=121788
Log:
2007-02-06  Mark Mitchell  <mark@codesourcery.com>

	PR target/29487
	* tree.h (DECL_REPLACEABLE_P): New macro.
	* except.c (set_nothrow_function_flags): Likewise.

2007-02-06  Mark Mitchell  <mark@codesourcery.com>

	PR target/29487
	* decl.c (finish_function): Use DECL_REPLACEABLE.
	* tree.c (cp_cannot_inline_tree_fn): Likewise.

2007-02-06  Mark Mitchell  <mark@codesourcery.com>

	PR c++/29487
	* g++.dg/eh/weak1-C: New test.
	* g++.dg/eh/weak1-a.cc: Likewise.
	* g++.dg/eh/comdat1.C: Likewise.

Added:
    branches/gcc-4_2-branch/gcc/testsuite/g++.dg/eh/comdat1.C
    branches/gcc-4_2-branch/gcc/testsuite/g++.dg/eh/weak1-a.cc
    branches/gcc-4_2-branch/gcc/testsuite/g++.dg/eh/weak1.C
Modified:
    branches/gcc-4_2-branch/gcc/ChangeLog
    branches/gcc-4_2-branch/gcc/cp/ChangeLog
    branches/gcc-4_2-branch/gcc/cp/decl.c
    branches/gcc-4_2-branch/gcc/cp/tree.c
    branches/gcc-4_2-branch/gcc/except.c
    branches/gcc-4_2-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_2-branch/gcc/tree.h

Fixed in 4.1.2, 4.2.0.

Subject: Bug 29487

Author: mmitchel
Date: Sun Feb 11 18:58:05 2007
New Revision: 121819

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=121819
Log:
	PR target/29487
	* tree.h (DECL_REPLACEABLE_P): New macro.
	* except.c (set_nothrow_function_flags): Likewise.

	PR target/29487
	* decl.c (finish_function): Use DECL_REPLACEABLE.
	* tree.c (cp_cannot_inline_tree_fn): Likewise.

	PR c++/29487
	* g++.dg/eh/weak1-C: New test.
	* g++.dg/eh/weak1-a.cc: Likewise.
	* g++.dg/eh/comdat1.C: Likewise.

Added:
    trunk/gcc/testsuite/g++.dg/eh/comdat1.C
    trunk/gcc/testsuite/g++.dg/eh/weak1-a.cc
    trunk/gcc/testsuite/g++.dg/eh/weak1.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/decl.c
    trunk/gcc/cp/tree.c
    trunk/gcc/except.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree.h

Fixed in 4.2.0, mainline.