Re: [PATCH v2, libbacktrace]: Compile with -funwind-tables

2012-10-02 Thread Ian Lance Taylor
On Tue, Oct 2, 2012 at 10:48 AM, Uros Bizjak  wrote:
> On Tue, Oct 2, 2012 at 7:44 PM, Gabriel Dos Reis
>  wrote:
>
 > On a related issue, it looks to me that the compiler itself should be
 > compiled with -funwind-tables, otherwise there are no backtraces
 > generated, even if libbacktrace is linked in and operational. Again,
 > x86_64-linux-gnu host defaults to this flag, but other hosts are left
 > behind.

 Compiling with C++ should always give us -funwind-tables.
>>>
>>> It doesn't give that, because the compiler is compiled with
>>> -fno-exceptions -fno-rtti.
>>
>> I believe in the long term we would to drop either of those.
>
> For the short term, I am bootstrapping attached patch, that adds
> -funwind-tables to other noexcept flags.

I think you should use -fasynchronous-unwind-tables here.  That way we
can get a backtrace if the compiler gets a segmentation violation.

I'll approve this patch with that change.  But you might want to check
whether you can see any change in bootstrap time or compiler size
(sorry).

Thanks.

Ian


Re: [PATCH v2, libbacktrace]: Compile with -funwind-tables

2012-10-02 Thread Uros Bizjak
On Tue, Oct 2, 2012 at 7:44 PM, Gabriel Dos Reis
 wrote:

>>> > On a related issue, it looks to me that the compiler itself should be
>>> > compiled with -funwind-tables, otherwise there are no backtraces
>>> > generated, even if libbacktrace is linked in and operational. Again,
>>> > x86_64-linux-gnu host defaults to this flag, but other hosts are left
>>> > behind.
>>>
>>> Compiling with C++ should always give us -funwind-tables.
>>
>> It doesn't give that, because the compiler is compiled with
>> -fno-exceptions -fno-rtti.
>
> I believe in the long term we would to drop either of those.

For the short term, I am bootstrapping attached patch, that adds
-funwind-tables to other noexcept flags.

Uros.
Index: configure
===
--- configure   (revision 191991)
+++ configure   (working copy)
@@ -6636,7 +6636,7 @@
 # Disable exceptions and RTTI if building with g++
 noexception_flags=
 save_CFLAGS="$CFLAGS"
-for real_option in -fno-exceptions -fno-rtti; do
+for real_option in -fno-exceptions -fno-rtti -funwind-tables; do
   # Do the check with the no- prefix removed since gcc silently
   # accepts any -Wno-* option on purpose
   case $real_option in
Index: configure.ac
===
--- configure.ac(revision 191991)
+++ configure.ac(working copy)
@@ -365,7 +365,8 @@
 
 # Disable exceptions and RTTI if building with g++
 ACX_PROG_CC_WARNING_OPTS(
-   m4_quote(m4_do([-fno-exceptions -fno-rtti])), [noexception_flags])
+   m4_quote(m4_do([-fno-exceptions -fno-rtti -funwind-tables])),
+  [noexception_flags])

 # Enable expensive internal checks
 is_release=


Re: [PATCH v2, libbacktrace]: Compile with -funwind-tables

2012-10-02 Thread Gabriel Dos Reis
On Tue, Oct 2, 2012 at 12:14 PM, Jakub Jelinek  wrote:
> On Tue, Oct 02, 2012 at 10:12:38AM -0700, Ian Lance Taylor wrote:
>> On Tue, Oct 2, 2012 at 8:22 AM, Uros Bizjak  wrote:
>> >
>> > On a related issue, it looks to me that the compiler itself should be
>> > compiled with -funwind-tables, otherwise there are no backtraces
>> > generated, even if libbacktrace is linked in and operational. Again,
>> > x86_64-linux-gnu host defaults to this flag, but other hosts are left
>> > behind.
>>
>> Compiling with C++ should always give us -funwind-tables.
>
> It doesn't give that, because the compiler is compiled with
> -fno-exceptions -fno-rtti.

I believe in the long term we would to drop either of those.

-- Gaby


Re: [PATCH v2, libbacktrace]: Compile with -funwind-tables

2012-10-02 Thread Jakub Jelinek
On Tue, Oct 02, 2012 at 10:12:38AM -0700, Ian Lance Taylor wrote:
> On Tue, Oct 2, 2012 at 8:22 AM, Uros Bizjak  wrote:
> >
> > On a related issue, it looks to me that the compiler itself should be
> > compiled with -funwind-tables, otherwise there are no backtraces
> > generated, even if libbacktrace is linked in and operational. Again,
> > x86_64-linux-gnu host defaults to this flag, but other hosts are left
> > behind.
> 
> Compiling with C++ should always give us -funwind-tables.

It doesn't give that, because the compiler is compiled with
-fno-exceptions -fno-rtti.

Jakub


Re: [PATCH v2, libbacktrace]: Compile with -funwind-tables

2012-10-02 Thread Ian Lance Taylor
On Tue, Oct 2, 2012 at 8:22 AM, Uros Bizjak  wrote:
>
> On a related issue, it looks to me that the compiler itself should be
> compiled with -funwind-tables, otherwise there are no backtraces
> generated, even if libbacktrace is linked in and operational. Again,
> x86_64-linux-gnu host defaults to this flag, but other hosts are left
> behind.

Compiling with C++ should always give us -funwind-tables.

If it doesn't for some reason, then I agree.  We might even want
-fasynchronous-unwind-tables for the compiler.

Ian


Re: [PATCH v2, libbacktrace]: Compile with -funwind-tables

2012-10-02 Thread Uros Bizjak
On Tue, Oct 2, 2012 at 3:08 PM, Ian Lance Taylor  wrote:
>> 2012-10-02  Uros Bizjak  
>>
>> PR other/54761
>> * configure.ac (EXTRA_FLAGS): New.
>> * Makefile.am (AM_FLAGS): Add $(EXTRA_FLAGS).
>> * configure, Makefile.in: Regenerate.
>
> This is OK.

Thanks, committed.

On a related issue, it looks to me that the compiler itself should be
compiled with -funwind-tables, otherwise there are no backtraces
generated, even if libbacktrace is linked in and operational. Again,
x86_64-linux-gnu host defaults to this flag, but other hosts are left
behind.

Uros.


Re: [PATCH v2, libbacktrace]: Compile with -funwind-tables

2012-10-02 Thread Ian Lance Taylor
On Tue, Oct 2, 2012 at 2:01 AM, Uros Bizjak  wrote:
>
> 2012-10-02  Uros Bizjak  
>
> PR other/54761
> * configure.ac (EXTRA_FLAGS): New.
> * Makefile.am (AM_FLAGS): Add $(EXTRA_FLAGS).
> * configure, Makefile.in: Regenerate.

This is OK.

Thanks.

Ian


Re: [PATCH v2, libbacktrace]: Compile with -funwind-tables

2012-10-02 Thread Uros Bizjak
On Tue, Oct 2, 2012 at 10:08 AM, Andreas Schwab  wrote:

>> +if test "x$GCC" = "xyes"; then
>> +  CFLAGS="$CFLAGS -funwind-tables"
>> +fi
>> +
>
> Don't modify CFLAGS, instead you should substitute a new variable that
> is added to AM_CFLAGS.  CFLAGS is reserved for the user to override.

Thanks, attached is a version that introduces EXTRA_FLAGS instead.

2012-10-02  Uros Bizjak  

PR other/54761
* configure.ac (EXTRA_FLAGS): New.
* Makefile.am (AM_FLAGS): Add $(EXTRA_FLAGS).
* configure, Makefile.in: Regenerate.

Testing on {x86_64,alphaev68}-linux-gnu in progress.

Uros.
Index: configure
===
--- configure   (revision 191955)
+++ configure   (working copy)
@@ -612,6 +612,7 @@
 BACKTRACE_SUPPORTS_THREADS
 PIC_FLAG
 WARN_FLAGS
+EXTRA_FLAGS
 BACKTRACE_FILE
 multi_basedir
 OTOOL64
@@ -11080,7 +11081,7 @@
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11083 "configure"
+#line 11084 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -11186,7 +11187,7 @@
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11189 "configure"
+#line 11190 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -11488,6 +11489,12 @@
 fi
 
 
+EXTRA_FLAGS=
+if test "x$GCC" = "xyes"; then
+  EXTRA_FLAGS=-funwind-tables
+fi
+
+
 WARN_FLAGS=
 save_CFLAGS="$CFLAGS"
 for real_option in -W -Wall -Wwrite-strings -Wstrict-prototypes \
Index: Makefile.in
===
--- Makefile.in (revision 191955)
+++ Makefile.in (working copy)
@@ -152,6 +152,7 @@
 ECHO_T = @ECHO_T@
 EGREP = @EGREP@
 EXEEXT = @EXEEXT@
+EXTRA_FLAGS = @EXTRA_FLAGS@
 FGREP = @FGREP@
 FORMAT_FILE = @FORMAT_FILE@
 GREP = @GREP@
@@ -253,7 +254,7 @@
 AM_CPPFLAGS = -I $(top_srcdir)/../include -I $(top_srcdir)/../libgcc \
-I ../libgcc -I ../gcc/include -I $(MULTIBUILDTOP)../../gcc/include
 
-AM_CFLAGS = $(WARN_FLAGS) $(PIC_FLAG)
+AM_CFLAGS = $(EXTRA_FLAGS) $(WARN_FLAGS) $(PIC_FLAG)
 noinst_LTLIBRARIES = libbacktrace.la
 libbacktrace_la_SOURCES = \
backtrace.h \
Index: configure.ac
===
--- configure.ac(revision 191955)
+++ configure.ac(working copy)
@@ -96,6 +96,12 @@
 fi
 AC_SUBST(BACKTRACE_FILE)
 
+EXTRA_FLAGS=
+if test "x$GCC" = "xyes"; then
+  EXTRA_FLAGS=-funwind-tables
+fi
+AC_SUBST(EXTRA_FLAGS)
+
 ACX_PROG_CC_WARNING_OPTS([-W -Wall -Wwrite-strings -Wstrict-prototypes \
  -Wmissing-prototypes -Wold-style-definition \
  -Wmissing-format-attribute -Wcast-qual],
Index: Makefile.am
===
--- Makefile.am (revision 191955)
+++ Makefile.am (working copy)
@@ -34,7 +34,7 @@
 AM_CPPFLAGS = -I $(top_srcdir)/../include -I $(top_srcdir)/../libgcc \
-I ../libgcc -I ../gcc/include -I $(MULTIBUILDTOP)../../gcc/include
 
-AM_CFLAGS = $(WARN_FLAGS) $(PIC_FLAG)
+AM_CFLAGS = $(EXTRA_FLAGS) $(WARN_FLAGS) $(PIC_FLAG)
 
 noinst_LTLIBRARIES = libbacktrace.la
 


Re: [PATCH v2, libbacktrace]: Compile with -funwind-tables

2012-10-02 Thread Andreas Schwab
Uros Bizjak  writes:

> Index: configure.ac
> ===
> --- configure.ac  (revision 191953)
> +++ configure.ac  (working copy)
> @@ -66,6 +66,10 @@
>  AC_PROG_CC
>  m4_rename_force([backtrace_PRECIOUS],[_AC_ARG_VAR_PRECIOUS])
>  
> +if test "x$GCC" = "xyes"; then
> +  CFLAGS="$CFLAGS -funwind-tables"
> +fi
> +

Don't modify CFLAGS, instead you should substitute a new variable that
is added to AM_CFLAGS.  CFLAGS is reserved for the user to override.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."


[PATCH v2, libbacktrace]: Compile with -funwind-tables

2012-10-02 Thread Uros Bizjak
On Tue, Oct 2, 2012 at 12:58 AM, Ian Lance Taylor  wrote:

>> Without -fasynchronous-unwind-tables, FDE is not generated for
>> backtrace_full and backtrace_simple wrappers. Without FDE, unwinding
>> terminates at these functions.
>
> I'm not opposed to -fasynchronous-unwind-tables, but now that you
> bring it up I'm fairly certain that it would suffice to use
> -funwind-tables.  I've been testing mainly on x86_64, and I forgot
> that on x86_64 -funwind-tables is the default.  Sorry about that.  And
> -fasynchronous-unwind-tables is the default also, so I could be wrong
> that -funwind-tables is all that is needed.

Yes, you are correct. -funwind-tables works as well.

>> Attached patch fixes this problem by adding
>> -fasynchronous-unwind-tables, and this way forcing FDEs for all
>> functions. With this change, btest passes OK, failing log and
>> runtime/pprof from libgo testsuite also pass OK.
>
> This is basically fine but libbacktrace may be compiled by the host
> compiler and that may not be GCC, so please add a configure test to
> see if the compiler accepts the -fasynchronous-unwind-tables option.

I have simplified the check for -funwind-tables to just look if the
library is compiled with gcc. This option is supported by gcc-2.96
(and probably earlier versions too).

2012-10-02  Uros Bizjak  

PR other/54761
* configure.ac (CFLAGS): Add -funwind-tables when compiling with GCC.
* configure: Regenerate.

The patch is re-tested on x86_64-linux-gnu and alphaev68-linux-gnu.

OK for mainline?

Uros.
Index: configure
===
--- configure   (revision 191953)
+++ configure   (working copy)
@@ -4872,8 +4872,12 @@
 
 
 
+if test "x$GCC" = "xyes"; then
+  CFLAGS="$CFLAGS -funwind-tables"
+fi
 
 
+
 if test -n "$ac_tool_prefix"; then
   # Extract the first word of "${ac_tool_prefix}ranlib", so it can be a 
program name with args.
 set dummy ${ac_tool_prefix}ranlib; ac_word=$2
@@ -11080,7 +11084,7 @@
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11083 "configure"
+#line 11087 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -11186,7 +11190,7 @@
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11189 "configure"
+#line 11193 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
Index: configure.ac
===
--- configure.ac(revision 191953)
+++ configure.ac(working copy)
@@ -66,6 +66,10 @@
 AC_PROG_CC
 m4_rename_force([backtrace_PRECIOUS],[_AC_ARG_VAR_PRECIOUS])
 
+if test "x$GCC" = "xyes"; then
+  CFLAGS="$CFLAGS -funwind-tables"
+fi
+
 AC_SUBST(CFLAGS)
 
 AC_PROG_RANLIB