[Bug lto/63409] [5 Regression] FAIL: g++.dg/lto/pr63270 cp_lto_pr63270_0.o-cp_lto_pr63270_1.o link, -flto -O2 -Wno-odr -fno-linker-plugin

["mliska at suse dot cz" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63409

--- Comment #7 from Martin Liška <mliska at suse dot cz> ---
Yeah, sorry for wrong dg argument. There's new version that should work
correctly. If not regression will be seen, I will commit the patch.
>From gcc-bugs-return-463383-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Mon Oct 06 16:19:15 2014
Return-Path: <gcc-bugs-return-463383-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 11458 invoked by alias); 6 Oct 2014 16:19:15 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 11406 invoked by uid 48); 6 Oct 2014 16:19:11 -0000
From: "mliska at suse dot cz" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug lto/63409] [5 Regression] FAIL: g++.dg/lto/pr63270 cp_lto_pr63270_0.o-cp_lto_pr63270_1.o link, -flto -O2 -Wno-odr -fno-linker-plugin
Date: Mon, 06 Oct 2014 16:19:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: lto
X-Bugzilla-Version: 5.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: mliska at suse dot cz
X-Bugzilla-Status: NEW
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: 5.0
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: attachments.created
Message-ID: <bug-63409-4-1ZIFLvlVVO@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-63409-4@http.gcc.gnu.org/bugzilla/>
References: <bug-63409-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2014-10/txt/msg00404.txt.bz2
Content-length: 231

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63409

--- Comment #8 from Martin Liška <mliska at suse dot cz> ---
Created attachment 33653
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33653&action=edit
Fix patch2
>From gcc-bugs-return-463384-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Mon Oct 06 16:27:22 2014
Return-Path: <gcc-bugs-return-463384-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 15256 invoked by alias); 6 Oct 2014 16:27:22 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 15199 invoked by uid 48); 6 Oct 2014 16:27:18 -0000
From: "ian at airs dot com" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug go/60406] recover.go: test13reflect2 test failure
Date: Mon, 06 Oct 2014 16:27:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: go
X-Bugzilla-Version: 4.9.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: ian at airs dot com
X-Bugzilla-Status: NEW
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: ian at airs dot com
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-60406-4-ECeKU5RHeK@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-60406-4@http.gcc.gnu.org/bugzilla/>
References: <bug-60406-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2014-10/txt/msg00405.txt.bz2
Content-length: 3798

https://gcc.gnu.org/bugzilla/show_bug.cgi?id`406

--- Comment #16 from Ian Lance Taylor <ian at airs dot com> ---
>> I'm not really happy with Dominik's patch because 1) it doesn't work when
>> configuring with --enable-sjlj-exceptions;
>
> Why is that important?

It's not very important but it's still a point to consider.  Some targets
default to SJLJ exceptions, albeit not very important ones.


>> 2) the current code almost always works, even on S/390, but the patch
>> forces us to do a lookup in the FDE table every time we call recover.
>
> The current code works unreliably as s390 uses memcopy to copy call
> arguments to the stack.  The control flow introduced by the function
> call triggers basic block reordering that may result in anything.

Sure, I understand, and it can obviously cause a false negative: some cases
that should recover will fail to do so.  However, I don't see any way that it
can ever cause a false positive: I don't see any way that it can cause recover
to succeed when it should not.


> * On systems that "use a leading underscore on symbol names", the test
> for functions beginning with "__go_" or "_go_" would yield "true" from user
> functions named "_go_..." (because the system adds one '_' and the patch
> strips it).

Yes.  We are already going to have trouble on such systems.  Really the library
needs to learn which systems use a leading underscore and which do not.  This
is actually available as __USER_LABEL_PREFIX__, and we should use that.


> * Wouldn't the new patch re-introduce the bug that
>
>   func foo(n int) {
>     if (n == 0) { recover(); } else { foo(0); }
>   }
>   func main() {
>     defer foo(1)
>     panic("...")
>   }
>
>   would recover although it should not?

Hmmm, I hadn't fully internalized that issue.  Your new withoutRecoverRecursive
test doesn't fail for me on x86_64.  I'll have to figure out why.


> * The code is even more expensive than the approach I had chosen because
> now it needs to fetch a two level backtrace instead of just one level
> (and probably each level is more expensive than the one
> _Unwind_FindEnclosingFunc()).

Yes, but the expensive case only happens in the rare cases where either recover
should not work or when the existing code has a false negative.  In the normal
case, where recover is permitted and the existing code works, we save the FDE
lookup.


> 2) The current checks for "return address + 16" may point into a
> different function, allowing recover() in weird situations.

It's a potential problem but I'm not too worried about it.


> "The return value of recover is nil if any of the following conditions holds:
>  ...
>  *recover was not called directly by a deferred function."
>
> According to the spec, the following code should recover the panic but
> does not:
>
>   func main() { defer foo(); panic("..."); }
>   func foo() { defer bar(); }
>   func bar() { recover(); }
>
> Note that this is also also "broken" in Golang (well, at least in the old
> version that comes with Ubuntu).  This may be an effect of imprecise
> wording of the spec.

In this case, the call to recover in bar is supposed to return nil; it should
not recover the panic.  If you read the paragraph before the one you quote, you
will see that recover only returns non-nil if it was called by a function that
was deferred before the call to panic.  In your example, the defer of bar
happens after the call to panic.  The reason Go works this way is to that the
deferred function foo can itself call a function that panics and recovers
without that function being confused by the earlier panic, one that it may not
know anything about.


> 4) __go_can_recover assumes that any call through libffi is allowed
> to recover.

Thanks for the example.  Does your patch fix this problem?