[PATCH] PR libstdc++/59392: Fix ARM EABI uncaught throw from unexpected exception handler
[This patch is on the git-only branch roland/pr59392.]
As described in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59392, this
bug looks to have been present since 4.2 originally introduced support
for ARM EABI-based C++ exception handling. I'd like to put this fix on
trunk and 4.8, and don't personally care about older versions but the
same fix should apply to all versions still being maintained.
The nature of the bug is quite straightforward: it's an unconditional
null pointer dereference in the code path for an unexpected throw done
inside a user-supplied handler for unexpected exceptions. I'm not
really sure if there are other ways to make it manifest.
Mark Seaborn is responsible for identifying the fix, which mimics the
similar code for the non-EABI implementation (and copies its comment).
I filled it out with a regression test. (We're both covered by Google's
blanket copyright assignment.)
No regressions in 'make check-c++' on arm-linux-gnueabihf.
Ok for trunk and 4.8?
Thanks,
Roland
libstdc++-v3/
2013-12-06 Roland McGrath
Mark Seaborn
PR libstdc++/59392
* libsupc++/eh_call.cc (__cxa_call_unexpected): Call __do_catch with
the address of a null pointer, not with a null pointer to pointer.
Copy comment for this case from eh_personality.cc:__cxa_call_unexpected.
* testsuite/18_support/bad_exception/59392.cc: New file.
--- a/libstdc++-v3/libsupc++/eh_call.cc
+++ b/libstdc++-v3/libsupc++/eh_call.cc
@@ -140,7 +140,11 @@ __cxa_call_unexpected(void* exc_obj_in)
&new_ptr) != ctm_failed)
__throw_exception_again;
- if (catch_type->__do_catch(&bad_exc, 0, 1))
+ // If the exception spec allows std::bad_exception, throw that.
+ // We don't have a thrown object to compare against, but since
+ // bad_exception doesn't have virtual bases, that's OK; just pass
NULL.
+ void* obj = NULL;
+ if (catch_type->__do_catch(&bad_exc, &obj, 1))
bad_exception_allowed = true;
}
--- /dev/null
+++ b/libstdc++-v3/testsuite/18_support/bad_exception/59392.cc
@@ -0,0 +1,51 @@
+// Copyright (C) 2013 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+#include
+#include
+
+class expected {};
+class unexpected {};
+class from_handler {};
+
+static void func_with_exception_spec() throw(expected)
+{
+ throw unexpected();
+}
+
+static void unexpected_handler()
+{
+ throw from_handler();
+}
+
+static void terminate_handler()
+{
+ exit(0);
+}
+
+// libstdc++/59392
+int main()
+{
+ std::set_unexpected(unexpected_handler);
+ std::set_terminate(terminate_handler);
+ try {
+func_with_exception_spec();
+ } catch (expected&) {
+abort();
+ }
+ abort();
+}
[PATCH PR 63758] fix liblto_plugin.so undefined _environ reference on OSX host
As I described in the bug, OSX (as documented) does not permit shared
libraries to refer directly to the environ global variable. Instead, they
must call the function _NSGetEnviron to get the address of environ. This
becomes a problem when some libiberty code that uses environ directly gets
linked into the liblto_plugin.so shared library.
On an OSX 10.7.5 system, any attempt to run gcc/g++ for a linking step
fails like this:
../bin/../lib/gcc/.../4.9.2/../../../../.../bin/ld:
.../bin/../libexec/gcc/.../4.9.2/liblto_plugin.so: error loading
plugin: dlopen(.../bin/../libexec/gcc/.../4.9.2/liblto_plugin.so, 2):
Symbol not found: _environ
Referenced from: .../bin/../libexec/gcc/.../4.9.2/liblto_plugin.so
Expected in: flat namespace
in .../bin/../libexec/gcc/.../4.9.2/liblto_plugin.so
collect2:error: ld returned 1 exit status
On some difference OSX systems, it works fine. I don't know why. But it's
documented in OSX's environ(7) man page that shared libraries cannot refer
to environ directly.
This patch fixed it for me. It's not very autoconfy, but it's simple and
it works.
OK for trunk and 4.9 branch?
Thanks,
Roland
libiberty/
2014-11-05 Roland McGrath
PR other/63758
* environ.h: New file.
* xmalloc.c: Include it.
(xmalloc_failed): Don't declare environ directly here.
* pex-unix.c: Likewise.
--- /dev/null
+++ b/libiberty/environ.h
@@ -0,0 +1,30 @@
+/* Declare the environ system variable.
+ Copyright (C) 2014 Free Software Foundation, Inc.
+
+This file is part of the libiberty library.
+Libiberty is free software; you can redistribute it and/or
+modify it under the terms of the GNU Library General Public
+License as published by the Free Software Foundation; either
+version 2 of the License, or (at your option) any later version.
+
+Libiberty is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+Library General Public License for more details.
+
+You should have received a copy of the GNU Library General Public
+License along with libiberty; see the file COPYING.LIB. If not,
+write to the Free Software Foundation, Inc., 51 Franklin Street - Fifth Floor,
+Boston, MA 02110-1301, USA. */
+
+/* On OSX, the environ variable can be used directly in the code of an
+ executable, but cannot be used in the code of a shared library (such as
+ GCC's liblto_plugin, which links in libiberty code). Instead, the
+ function _NSGetEnviron can be called to get the address of environ. */
+
+#ifdef __APPLE__
+# include
+# define environ (*_NSGetEnviron ())
+#else
+extern char **environ;
+#endif
--- a/libiberty/pex-unix.c
+++ b/libiberty/pex-unix.c
@@ -23,6 +23,7 @@ Boston, MA 02110-1301, USA. */
#include "config.h"
#include "libiberty.h"
#include "pex-common.h"
+#include "environ.h"
#include
#include
@@ -389,8 +390,6 @@ pex_child_error (struct pex_obj *obj, const char
*executable,
/* Execute a child. */
-extern char **environ;
-
#if defined(HAVE_SPAWNVE) && defined(HAVE_SPAWNVPE)
/* Implementation of pex->exec_child using the Cygwin spawn operation. */
--- a/libiberty/xmalloc.c
+++ b/libiberty/xmalloc.c
@@ -65,6 +65,7 @@ function will be called to print an error message
and terminate execution.
#endif
#include "ansidecl.h"
#include "libiberty.h"
+#include "environ.h"
#include
@@ -117,7 +118,6 @@ void
xmalloc_failed (size_t size)
{
#ifdef HAVE_SBRK
- extern char **environ;
size_t allocated;
if (first_break != NULL)
[PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections
This fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77609 (which I've just filed). OK for trunk? I'm not sure if this kind of fix is appropriate for gcc-6-branch or not, but I'd like to backport it there too if it is acceptable. Thanks, Roland gcc/ 2016-09-15 Roland McGrath < PR other/77609 * varasm.c (default_section_type_flags): Set SECTION_NOTYPE for any section for which we don't know a specific type it should have, regardless of name. Previously this was done only for the exact names ".init_array", ".fini_array", and ".preinit_array". diff --git a/gcc/varasm.c b/gcc/varasm.c index 00a9b30..0d7ea38 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -6210,15 +6210,20 @@ default_section_type_flags (tree decl, const char *name, int reloc) || strncmp (name, ".gnu.linkonce.tb.", 17) == 0) flags |= SECTION_TLS | SECTION_BSS; - /* These three sections have special ELF types. They are neither - SHT_PROGBITS nor SHT_NOBITS, so when changing sections we don't - want to print a section type (@progbits or @nobits). If someone - is silly enough to emit code or TLS variables to one of these - sections, then don't handle them specially. */ - if (!(flags & (SECTION_CODE | SECTION_BSS | SECTION_TLS)) - && (strcmp (name, ".init_array") == 0 - || strcmp (name, ".fini_array") == 0 - || strcmp (name, ".preinit_array") == 0)) + /* Various sections have special ELF types that the assembler will + assign by default based on the name. They are neither SHT_PROGBITS + nor SHT_NOBITS, so when changing sections we don't want to print a + section type (@progbits or @nobits). Rather than duplicating the + assembler's knowledge of what those special name patterns are, just + let the assembler choose the type if we don't know a specific + reason to set it to something other than the default. SHT_PROGBITS + is the default for sections whose name is not specially known to + the assembler, so it does no harm to leave the choice to the + assembler when @progbits is the best thing we know to use. If + someone is silly enough to emit code or TLS variables to one of + these sections, then don't handle them specially. */ + if (!(flags & (SECTION_CODE | SECTION_BSS | SECTION_TLS | SECTION_ENTSIZE)) + && !(HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE))) flags |= SECTION_NOTYPE; return flags;
Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections
ping? On Thu, Sep 15, 2016 at 4:09 PM, Roland McGrath wrote: > This fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77609 (which I've > just filed). > > OK for trunk? > > I'm not sure if this kind of fix is appropriate for gcc-6-branch or not, > but I'd like to backport it there too if it is acceptable. > > > Thanks, > Roland > > > gcc/ > 2016-09-15 Roland McGrath < > > PR other/77609 > * varasm.c (default_section_type_flags): Set SECTION_NOTYPE for > any section for which we don't know a specific type it should have, > regardless of name. Previously this was done only for the exact > names ".init_array", ".fini_array", and ".preinit_array". > > diff --git a/gcc/varasm.c b/gcc/varasm.c > index 00a9b30..0d7ea38 100644 > --- a/gcc/varasm.c > +++ b/gcc/varasm.c > @@ -6210,15 +6210,20 @@ default_section_type_flags (tree decl, const > char *name, int reloc) >|| strncmp (name, ".gnu.linkonce.tb.", 17) == 0) > flags |= SECTION_TLS | SECTION_BSS; > > - /* These three sections have special ELF types. They are neither > - SHT_PROGBITS nor SHT_NOBITS, so when changing sections we don't > - want to print a section type (@progbits or @nobits). If someone > - is silly enough to emit code or TLS variables to one of these > - sections, then don't handle them specially. */ > - if (!(flags & (SECTION_CODE | SECTION_BSS | SECTION_TLS)) > - && (strcmp (name, ".init_array") == 0 > - || strcmp (name, ".fini_array") == 0 > - || strcmp (name, ".preinit_array") == 0)) > + /* Various sections have special ELF types that the assembler will > + assign by default based on the name. They are neither SHT_PROGBITS > + nor SHT_NOBITS, so when changing sections we don't want to print a > + section type (@progbits or @nobits). Rather than duplicating the > + assembler's knowledge of what those special name patterns are, just > + let the assembler choose the type if we don't know a specific > + reason to set it to something other than the default. SHT_PROGBITS > + is the default for sections whose name is not specially known to > + the assembler, so it does no harm to leave the choice to the > + assembler when @progbits is the best thing we know to use. If > + someone is silly enough to emit code or TLS variables to one of > + these sections, then don't handle them specially. */ > + if (!(flags & (SECTION_CODE | SECTION_BSS | SECTION_TLS | SECTION_ENTSIZE)) > + && !(HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE))) > flags |= SECTION_NOTYPE; > >return flags;
[COMMITTED PATCH] add myself to write after approval list
2012-11-15 Roland McGrath * MAINTAINERS (Write After Approval): Add myself. --- a/MAINTAINERS +++ b/MAINTAINERS @@ -457,6 +457,7 @@ Simon Martin simar...@users.sourceforge.net Ranjit Mathew rmat...@hotmail.com Michael Matz m...@suse.de Greg McGaryg...@gnu.org +Roland McGrath rol...@hack.frob.com Adam Megacza...@xwt.org Bingfeng Mei b...@broadcom.com Jim Meyering j...@meyering.net
[PATCH] Fix -Wformat-security warning in arm.c
This fixes a gratuitous warning. Thanks, Roland gcc/ 2013-03-25 Roland McGrath * config/arm/arm.c (arm_print_operand: case 'w'): Use fputs rather than fprintf with a non-constant, non-format string. --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -17997,7 +17997,7 @@ arm_print_operand (FILE *stream, rtx x, int code) "wC12", "wC13", "wC14", "wC15" }; - fprintf (stream, wc_reg_names [INTVAL (x)]); + fputs (wc_reg_names [INTVAL (x)], stream); } return;
Re: [PATCH] Fix -Wformat-security warning in arm.c
Committed to trunk. Thanks, Roland
Re: [PATCH] Fix -Wformat-security warning in arm.c
Backport committed to gcc-4_8-branch.
[PATCH] x86: Use ud2 assembly mnemonic when available.
Non-ancient assemblers support the "ud2" mnemonic, so there is no need
to emit the literal opcode as data.
OK for trunk and 4.8?
Thanks,
Roland
gcc/
2014-02-13 Roland McGrath
* configure.ac (HAVE_AS_IX86_UD2): New test for 'ud2' mnemonic.
* configure: Regenerated.
* config.in: Regenerated.
* config/i386/i386.md (trap) [HAVE_AS_IX86_UD2]: Use the mnemonic
instead of ASM_SHORT.
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -375,6 +375,12 @@
#endif
+/* Define if your assembler supports the 'ud2' mnemonic. */
+#ifndef USED_FOR_TARGET
+#undef HAVE_AS_IX86_UD2
+#endif
+
+
/* Define if your assembler supports the lituse_jsrdirect relocation. */
#ifndef USED_FOR_TARGET
#undef HAVE_AS_JSRDIRECT_RELOCS
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -17843,7 +17843,13 @@
(define_insn "trap"
[(trap_if (const_int 1) (const_int 6))]
""
- { return ASM_SHORT "0x0b0f"; }
+{
+#ifdef HAVE_AS_IX86_UD2
+ return "ud2";
+#else
+ return ASM_SHORT "0x0b0f";
+#endif
+}
[(set_attr "length" "2")])
(define_expand "prefetch"
--- a/gcc/configure
+++ b/gcc/configure
@@ -25109,6 +25109,37 @@ $as_echo "#define
HAVE_AS_IX86_REP_LOCK_PREFIX 1" >>confdefs.h
fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for
ud2 mnemonic" >&5
+$as_echo_n "checking assembler for ud2 mnemonic... " >&6; }
+if test "${gcc_cv_as_ix86_ud2+set}" = set; then :
+ $as_echo_n "(cached) " >&6
+else
+ gcc_cv_as_ix86_ud2=no
+ if test x$gcc_cv_as != x; then
+$as_echo 'ud2' > conftest.s
+if { ac_try='$gcc_cv_as $gcc_cv_as_flags -o conftest.o conftest.s >&5'
+ { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+ (eval $ac_try) 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; }
+then
+ gcc_cv_as_ix86_ud2=yes
+else
+ echo "configure: failed program was" >&5
+ cat conftest.s >&5
+fi
+rm -f conftest.o conftest.s
+ fi
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_as_ix86_ud2" >&5
+$as_echo "$gcc_cv_as_ix86_ud2" >&6; }
+if test $gcc_cv_as_ix86_ud2 = yes; then
+
+$as_echo "#define HAVE_AS_IX86_UD2 1" >>confdefs.h
+
+fi
+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for
R_386_TLS_GD_PLT reloc" >&5
$as_echo_n "checking assembler for R_386_TLS_GD_PLT reloc... " >&6; }
if test "${gcc_cv_as_ix86_tlsgdplt+set}" = set; then :
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -3895,6 +3895,12 @@ foo: nop
[AC_DEFINE(HAVE_AS_IX86_REP_LOCK_PREFIX, 1,
[Define if the assembler supports 'rep , lock '.])])
+gcc_GAS_CHECK_FEATURE([ud2 mnemonic],
+ gcc_cv_as_ix86_ud2,,,
+ [ud2],,
+ [AC_DEFINE(HAVE_AS_IX86_UD2, 1,
+ [Define if your assembler supports the 'ud2' mnemonic.])])
+
gcc_GAS_CHECK_FEATURE([R_386_TLS_GD_PLT reloc],
gcc_cv_as_ix86_tlsgdplt,,,
[calltls_gd@tlsgdplt],
Re: [PATCH] x86: Use ud2 assembly mnemonic when available.
Did you read the patch? It uses an empirical configure check to discover if the assembler does in fact support ud2.
Re: [PATCH] x86: Use ud2 assembly mnemonic when available.
On Thu, Feb 13, 2014 at 10:58 PM, Uros Bizjak wrote: > You forgot to tell us how the patch tested... Right. It's a pretty obviously harmless change. I tested that the configure check passes with binutils-2.22, and eyeball'd a -S compile of a trivial function calling __builtin_trap() to see it uses the mnemonic. I don't have ready access to an assembler that does not support the mnemonic, so I simulated the negative case by momentarily hacking configure to try 'ud2x' instead of 'ud2' and verified that this configure check failed and that for the same trivial function it then emits '.value 0x0b0f' as before. > OK for mainline and release branches. Committed to trunk and 4.8. Thanks, Roland
Re: [PATCH] PR libstdc++/59392: Fix ARM EABI uncaught throw from unexpected exception handler
Committed (r208519 on trunk and r208520 on 4.8) after approval posted
in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59392.
Only the dates are changed from what I posted originally.
Thanks,
Roland
On Fri, Dec 6, 2013 at 3:24 PM, Roland McGrath wrote:
> [This patch is on the git-only branch roland/pr59392.]
>
> As described in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59392, this
> bug looks to have been present since 4.2 originally introduced support
> for ARM EABI-based C++ exception handling. I'd like to put this fix on
> trunk and 4.8, and don't personally care about older versions but the
> same fix should apply to all versions still being maintained.
>
> The nature of the bug is quite straightforward: it's an unconditional
> null pointer dereference in the code path for an unexpected throw done
> inside a user-supplied handler for unexpected exceptions. I'm not
> really sure if there are other ways to make it manifest.
>
> Mark Seaborn is responsible for identifying the fix, which mimics the
> similar code for the non-EABI implementation (and copies its comment).
> I filled it out with a regression test. (We're both covered by Google's
> blanket copyright assignment.)
>
> No regressions in 'make check-c++' on arm-linux-gnueabihf.
>
> Ok for trunk and 4.8?
>
>
> Thanks,
> Roland
>
>
> libstdc++-v3/
> 2013-12-06 Roland McGrath
> Mark Seaborn
>
> PR libstdc++/59392
> * libsupc++/eh_call.cc (__cxa_call_unexpected): Call __do_catch with
> the address of a null pointer, not with a null pointer to pointer.
> Copy comment for this case from
> eh_personality.cc:__cxa_call_unexpected.
> * testsuite/18_support/bad_exception/59392.cc: New file.
>
> --- a/libstdc++-v3/libsupc++/eh_call.cc
> +++ b/libstdc++-v3/libsupc++/eh_call.cc
> @@ -140,7 +140,11 @@ __cxa_call_unexpected(void* exc_obj_in)
>&new_ptr) != ctm_failed)
> __throw_exception_again;
>
> - if (catch_type->__do_catch(&bad_exc, 0, 1))
> + // If the exception spec allows std::bad_exception, throw that.
> + // We don't have a thrown object to compare against, but since
> + // bad_exception doesn't have virtual bases, that's OK; just pass
> NULL.
> + void* obj = NULL;
> + if (catch_type->__do_catch(&bad_exc, &obj, 1))
> bad_exception_allowed = true;
> }
>
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/18_support/bad_exception/59392.cc
> @@ -0,0 +1,51 @@
> +// Copyright (C) 2013 Free Software Foundation, Inc.
> +//
> +// This file is part of the GNU ISO C++ Library. This library is free
> +// software; you can redistribute it and/or modify it under the
> +// terms of the GNU General Public License as published by the
> +// Free Software Foundation; either version 3, or (at your option)
> +// any later version.
> +
> +// This library is distributed in the hope that it will be useful,
> +// but WITHOUT ANY WARRANTY; without even the implied warranty of
> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +// GNU General Public License for more details.
> +
> +// You should have received a copy of the GNU General Public License along
> +// with this library; see the file COPYING3. If not see
> +// <http://www.gnu.org/licenses/>.
> +
> +#include
> +#include
> +
> +class expected {};
> +class unexpected {};
> +class from_handler {};
> +
> +static void func_with_exception_spec() throw(expected)
> +{
> + throw unexpected();
> +}
> +
> +static void unexpected_handler()
> +{
> + throw from_handler();
> +}
> +
> +static void terminate_handler()
> +{
> + exit(0);
> +}
> +
> +// libstdc++/59392
> +int main()
> +{
> + std::set_unexpected(unexpected_handler);
> + std::set_terminate(terminate_handler);
> + try {
> +func_with_exception_spec();
> + } catch (expected&) {
> +abort();
> + }
> + abort();
> +}
[PATCH] libiberty: fix psignal parameter type
When libiberty defines psignal, it doesn't use the canonical signature.
This came up as a problem in a configuration where libiberty wants to
define psignal itself, but the build environment's declares
it too.
This was a --with-newlib configuration using the newlib trunk, where newlib
does define psignal (and strsignal, among other things), but libiberty's
configure thinks it knows exactly what newlib does and doesn't define
without checking. This hard-coding seems unwise to me, exactly because of
the potential for cases like this, where newlib's set of available
functions changes over time.
I think the fragility of that hard-coding ought to be addressed somehow.
But regardless, this change alleviates the immediate problem, and is
otherwise harmless.
Thanks,
Roland
libiberty/
2011-10-18 Roland McGrath
* strsignal.c (psignal): Use const second in parameter type.
* functions.texi: Updated.
diff --git a/libiberty/functions.texi b/libiberty/functions.texi
index c9df186..2945c61 100644
--- a/libiberty/functions.texi
+++ b/libiberty/functions.texi
@@ -1097,7 +1097,7 @@ documented.
@end deftypefn
@c strsignal.c:541
-@deftypefn Supplemental void psignal (int @var{signo}, char *@var{message})
+@deftypefn Supplemental void psignal (int @var{signo}, const char
*@var{message})
Print @var{message} to the standard error, followed by a colon,
followed by the description of the signal specified by @var{signo},
diff --git a/libiberty/strsignal.c b/libiberty/strsignal.c
index 666b1b4..3b56d16 100644
--- a/libiberty/strsignal.c
+++ b/libiberty/strsignal.c
@@ -538,7 +538,7 @@ strtosigno (const char *name)
/*
-@deftypefn Supplemental void psignal (int @var{signo}, char *@var{message})
+@deftypefn Supplemental void psignal (int @var{signo}, const char
*@var{message})
Print @var{message} to the standard error, followed by a colon,
followed by the description of the signal specified by @var{signo},
@@ -551,7 +551,7 @@ followed by a newline.
#ifndef HAVE_PSIGNAL
void
-psignal (int signo, char *message)
+psignal (int signo, const char *message)
{
if (signal_names == NULL)
{
Re: [PATCH] libiberty: fix psignal parameter type
On Tue, Oct 18, 2011 at 9:50 AM, Andrew Pinski wrote: > libiberty is no longer compiled for the target so this should never > happen really. I see. I was indeed using an older source base, and just noticed that all the offending configure logic was still the same. Perhaps all the --with-newlib logic in libiberty's configure should be removed now? Regardless, the psignal change is an appropriate cleanup in its own right. Thanks, Roland
Re: [PATCH] libiberty: fix psignal parameter type
On Tue, Oct 18, 2011 at 10:18 AM, Ian Lance Taylor wrote: > On Tue, Oct 18, 2011 at 9:49 AM, Roland McGrath wrote: >> >> libiberty/ >> 2011-10-18 Roland McGrath >> >> * strsignal.c (psignal): Use const second in parameter type. >> * functions.texi: Updated. > > This is OK. Thanks. But last I checked, I'm not a GCC committer. So somebody has to do it for me. Thanks, Roland
[top-level patch] Do proper target tool checks for readelf
newlib has a configure check that works by running readelf on a target
binary. While a native readelf is generally pretty generic and
cross-friendly, it's possible for a build host not to have any readelf at
all. It's clearly the right thing that configure use a target readelf tool
if there is one. Right now, this seems to happen for the "main" newlib
build but doesn't happen for a multilib variant newlib build (e.g. in an
x86_64-foo config, build/x86_64-foo/newlib/... uses a target readelf but
build/x86_64-foo/32/newlib/... doesn't find one and so uses unadorned
"readelf", which might not exist on the build host in a cross situation).
Frankly I couldn't entirely tell which of all these magic spots were the
ones necessary to get the multilib newlib build to do the right thing. But
it makes sense that the treatment of readelf be uniformly analogous to the
treatment of objdump, so I did that everywhere I found it.
MAINTAINERS doesn't make it entirely clear what the procedure really is for
touching these files. Looking at src/ChangeLog I see both what appear to
be instances where changes were first committed to GCC and then merged over
to src/, and instances where changes were committed directly to src/. I'm
able to commit (after approval) to src/ CVS, but not to GCC.
Ok for src/ trunk, or should somebody be putting this into GCC first?
I've omitted the generated files from the patch, but of course would
include them in a commit.
Thanks,
Roland
2011-11-02 Roland McGrath
* configure.ac: Add tool checks for READELF and READELF_FOR_TARGET.
* configure: Rebuild.
* Makefile.def (flags_to_pass): Add READELF_FOR_TARGET.
* Makefile.tpl (READELF, READELF_FOR_TARGET): New variables.
(HOST_EXPORTS): Add READELF, READELF_FOR_TARGET.
(BASE_FLAGS_TO_PASS): Add READELF_FOR_TARGET.
(BASE_TARGET_EXPORTS, EXTRA_HOST_FLAGS, EXTRA_TARGET_FLAGS):
Add READELF.
* Makefile.in: Rebuild.
diff --git a/Makefile.def b/Makefile.def
index 5116341..27434d3 100644
--- a/Makefile.def
+++ b/Makefile.def
@@ -259,6 +259,7 @@ flags_to_pass = { flag= LIBCXXFLAGS_FOR_TARGET ; };
flags_to_pass = { flag= NM_FOR_TARGET ; };
flags_to_pass = { flag= OBJDUMP_FOR_TARGET ; };
flags_to_pass = { flag= RANLIB_FOR_TARGET ; };
+flags_to_pass = { flag= READELF_FOR_TARGET ; };
flags_to_pass = { flag= STRIP_FOR_TARGET ; };
flags_to_pass = { flag= WINDRES_FOR_TARGET ; };
flags_to_pass = { flag= WINDMC_FOR_TARGET ; };
diff --git a/Makefile.tpl b/Makefile.tpl
index 4dd2391..a42d154 100644
--- a/Makefile.tpl
+++ b/Makefile.tpl
@@ -6,7 +6,7 @@ in
#
# Makefile for directory with subdirs to build.
# Copyright (C) 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1997, 1998,
-# 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010
+# 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010,
2011
# Free Software Foundation
#
# This file is free software; you can redistribute it and/or modify
@@ -209,6 +209,7 @@ HOST_EXPORTS = \
WINDMC="$(WINDMC)"; export WINDMC; \
OBJCOPY="$(OBJCOPY)"; export OBJCOPY; \
OBJDUMP="$(OBJDUMP)"; export OBJDUMP; \
+ READELF="$(READELF)"; export READELF; \
AR_FOR_TARGET="$(AR_FOR_TARGET)"; export AR_FOR_TARGET; \
AS_FOR_TARGET="$(AS_FOR_TARGET)"; export AS_FOR_TARGET; \
GCC_FOR_TARGET="$(GCC_FOR_TARGET)"; export GCC_FOR_TARGET; \
@@ -216,6 +217,7 @@ HOST_EXPORTS = \
NM_FOR_TARGET="$(NM_FOR_TARGET)"; export NM_FOR_TARGET; \
OBJDUMP_FOR_TARGET="$(OBJDUMP_FOR_TARGET)"; export OBJDUMP_FOR_TARGET; \
RANLIB_FOR_TARGET="$(RANLIB_FOR_TARGET)"; export RANLIB_FOR_TARGET; \
+ READELF_FOR_TARGET="$(READELF_FOR_TARGET)"; export READELF_FOR_TARGET; \
TOPLEVEL_CONFIGURE_ARGUMENTS="$(TOPLEVEL_CONFIGURE_ARGUMENTS)"; export
TOPLEVEL_CONFIGURE_ARGUMENTS; \
HOST_LIBS="$(STAGE1_LIBS)"; export HOST_LIBS; \
GMPLIBS="$(HOST_GMPLIBS)"; export GMPLIBS; \
@@ -288,6 +290,7 @@ BASE_TARGET_EXPORTS = \
NM="$(COMPILER_NM_FOR_TARGET)"; export NM; \
OBJDUMP="$(OBJDUMP_FOR_TARGET)"; export OBJDUMP; \
RANLIB="$(RANLIB_FOR_TARGET)"; export RANLIB; \
+ READELF="$(READELF_FOR_TARGET)"; export READELF; \
STRIP="$(STRIP_FOR_TARGET)"; export STRIP; \
WINDRES="$(WINDRES_FOR_TARGET)"; export WINDRES; \
WINDMC="$(WINDMC_FOR_TARGET)"; export WINDMC; \
@@ -400,6 +403,7 @@ LIPO = @LIPO@
NM = @NM@
OBJDUMP = @OBJDUMP@
RANLIB = @RANLIB@
+READELF = @READELF@
STRIP = @STRIP@
WINDRES = @WINDRES@
WINDMC = @WINDMC@
@@ -493,6 +497,7 @@ LIPO_FOR_TARGET=@LIPO_FOR_TARGET@
NM_FOR_TARGET=@NM_FOR_TARGET@
OBJDUMP_FOR_TARGET=@OBJD
Re: [top-level patch] Do proper target tool checks for readelf
On Wed, Nov 2, 2011 at 8:32 PM, Ian Lance Taylor wrote: > The top level is not automatically merged between gcc and src. However, > people are expected to merge manually. Normally gcc is considered to be > the master. Ok. src/MAINTAINERS could be clearer about that. >> Ok for src/ trunk, or should somebody be putting this into GCC first? > > So, yes, should probably go into gcc first. Ok. Would you like to commit it for me, then? I can take care of the src/ merge myself if you prefer. Thanks, Roland
Re: [top-level patch] Do proper target tool checks for readelf
On Thu, Nov 3, 2011 at 10:39 AM, Ian Lance Taylor wrote: > Ideally I'd like to see a build maintainer approve it. Perhaps I missed > that. Ah. The src/MAINTAINERS file says, "Any global maintainer can approve changes to these files, ...", which in that context I think includes you. But since "..." is "but they should be aware that they need to be kept in sync with their counterparts in the GCC repository," I've now looked at gcc/MAINTAINERS and see that it has a category "build machinery (*.in)" that you are not in. (You are in its "global reviewers" category, which the wording there suggests is in fact sufficient. But I certainly can't argue with wanting to hear from one of the build maintainers, though I do hope they respond soon. That said, it seems hard to imagine that my change would be objectionable.) Thanks, Roland
Re: [top-level patch] Do proper target tool checks for readelf
On Thu, Nov 3, 2011 at 10:55 AM, DJ Delorie wrote: > The patch looks OK to me. Thanks! As I'm still not a GCC committer, someone please check it in for me. If people would like me to handle the merge over to src/ myself, I'd be glad to do that step. Thanks, Roland
Re: [top-level patch] Do proper target tool checks for readelf
On Thu, Nov 3, 2011 at 11:05 AM, Roland McGrath wrote: > On Thu, Nov 3, 2011 at 10:55 AM, DJ Delorie wrote: >> The patch looks OK to me. > > Thanks! As I'm still not a GCC committer, someone please check it in for me. > If people would like me to handle the merge over to src/ myself, I'd be > glad to do that step. Ping. Anybody going to do this commit for me? (If insteaed someone would like to add me to the gcc group so I can do "write after approval", that would be fine too.) Thanks, Roland
Re: [PATCH] Fix ARM constant-pool layout calculations under -falign-labels
Hi Richard, You never responded to this. Is there something wrong with this fix? Can you address whether it's sufficient for align_loops > align_labels and such cases that Julian Brown raised? A patch against the current trunk is below. Thanks, Roland On Wed, Aug 1, 2012 at 2:43 PM, Roland McGrath wrote: > Using e.g. -falign-labels=16 on ARM can confuse the constant-pool layout > code such that it places pool entries too far away from their referring > instructions. This change seems to fix it. > > I don't have a small test case, only a large one, which I haven't actually > tried to get to reproduce on any vanilla ARM target. But the logic of the > change seems straightforward and sound. gcc/ 2012-08-22 Roland McGrath * config/arm/arm.c (get_label_padding): Use align_labels as minimum. diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index 2805b7c..586d094 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -12409,6 +12409,7 @@ get_label_padding (rtx label) HOST_WIDE_INT align, min_insn_size; align = 1 << label_to_alignment (label); + align = MAX (align, align_labels); min_insn_size = TARGET_THUMB ? 2 : 4; return align > min_insn_size ? align - min_insn_size : 0; }
[PATCH] ARM: exclude fixed_regs for stack-alignment save/restore
When the ARM compiler needs to ensure the stack pointer stays aligned
and it's already doing a multi-register push/pop in the prologue and
epilogue, it chooses some arbitrary register to add to the register set
in that push and pop just to increase the size of the stack used by 4
bytes. This is presumed to be harmless, since some register that is
either call-clobbered or not touched by the function at all is just
getting pushed and then the same value popped into it.
But if e.g. I use -ffixed-r9 then I think it's a reasonable expectation
that no code is generated that touches r9 in any way, shape, or form.
(My actual concern is a variant target port still in progress, where
the ABI specifies that r9 is reserved, and the system enforces that no
instruction may modify r9.)
I haven't managed to come up with an isolated test case to demonstrate
the bug. Apparently I just don't understand the stack and register
pressure requirements that make the compiler get into the situation
where it wants to add a random register for alignment padding purposes.
I don't have a setup where I can do a proper regression test for ARM.
(My system has a /usr/arm-linux-gnueabi/include/ but configuring with
--target=arm-linux-gnueabi --with-headers=/usr/arm-linux-gnueabi/include
did not succeed in building libgcc.)
But the change seems pretty obviously correct IMHO.
Thanks,
Roland
gcc/
2012-06-14 Roland McGrath
* config/arm/arm.c (arm_get_frame_offsets): Never use a fixed register
as the extra register to save/restore for stack-alignment padding.
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 092e202..bcfef3e 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -16752,7 +16752,12 @@ arm_get_frame_offsets (void)
else
for (i = 4; i <= (TARGET_THUMB1 ? LAST_LO_REGNUM : 11); i++)
{
- if ((offsets->saved_regs_mask & (1 << i)) == 0)
+ /* While the gratuitous register save/restore is ordinarily
+ harmless, if a register is marked as fixed it may be
+ entirely forbidden by the system ABI to touch it, so we
+ should avoid those registers. */
+ if (!fixed_regs[i]
+ && (offsets->saved_regs_mask & (1 << i)) == 0)
{
reg = i;
break;
Re: [PATCH] ARM: exclude fixed_regs for stack-alignment save/restore
On Thu, Jun 14, 2012 at 1:13 PM, Mike Stump wrote: > On Jun 14, 2012, at 10:16 AM, Roland McGrath wrote: >> But if e.g. I use -ffixed-r9 then I think it's a reasonable expectation >> that no code is generated that touches r9 in any way, shape, or form. > > Also, I can't help but wonder if global_regs is respected. It's clearly not. Making the added condition !fixed_regs[i] && !global_regs[i] seems sensible to me. Thanks, Roland
Re: [PATCH] ARM: exclude fixed_regs for stack-alignment save/restore
Here's the version of the change that incorporates Mike's suggestion.
Thanks,
Roland
gcc/
2012-06-14 Roland McGrath
* config/arm/arm.c (arm_get_frame_offsets): Never use a fixed register
as the extra register to save/restore for stack-alignment padding.
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 092e202..13771d9 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -16752,7 +16752,12 @@ arm_get_frame_offsets (void)
else
for (i = 4; i <= (TARGET_THUMB1 ? LAST_LO_REGNUM : 11); i++)
{
- if ((offsets->saved_regs_mask & (1 << i)) == 0)
+ /* While the gratuitous register save/restore is ordinarily
+ harmless, if a register is marked as fixed or global it
+ may be entirely forbidden by the system ABI to touch it,
+ so we should avoid those registers. */
+ if (!fixed_regs[i] && !global_regs[i]
+ && (offsets->saved_regs_mask & (1 << i)) == 0)
{
reg = i;
break;
Re: [PATCH] ARM: exclude fixed_regs for stack-alignment save/restore
OK then. If you like the original patch, would you like to commit it for me? Thanks, Roland
Re: [PATCH] ARM: exclude fixed_regs for stack-alignment save/restore
On Mon, Jun 18, 2012 at 9:34 AM, Roland McGrath wrote: > OK then. If you like the original patch, would you like to commit it for me? ping?
[PATCH] x86: use 'rep bsf' syntax when assembler supports it
The GNU assembler now (just as of today) accepts 'rep bsf ...' or 'rep bsr ...'
syntax. It's always better to put a prefix on the instruction itself
rather than to write 'rep; ...'. This changes 'rep; bsf ...' to 'rep bsf ...'
when the assembler accepts the latter.
Thanks,
Roland
gcc/
2012-06-22 Roland McGrath
* configure.ac (HAVE_AS_IX86_REP_BSFBSR): New test.
* configure: Regenerate.
* config.in: Regenerate.
* config/i386/i386.h (REP_BEFORE_BSF): New macro.
* config/i386/i386.md (ctz2): Use it.
diff --git a/gcc/config.in b/gcc/config.in
index 7b12b4b..be7271f 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -345,6 +345,12 @@
#endif
+/* Define if the assembler supports 'rep bsf' and 'rep bsr'. */
+#ifndef USED_FOR_TARGET
+#undef HAVE_AS_IX86_REP_BSFBSR
+#endif
+
+
/* Define if the assembler supports 'rep , lock '. */
#ifndef USED_FOR_TARGET
#undef HAVE_AS_IX86_REP_LOCK_PREFIX
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index f7f13d2..90eaf95 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -1,6 +1,6 @@
/* Definitions of target machine for GCC for IA-32.
Copyright (C) 1988, 1992, 1994, 1995, 1996, 1997, 1998, 1999, 2000,
- 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011
+ 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012
Free Software Foundation, Inc.
This file is part of GCC.
@@ -2030,6 +2030,12 @@ do {
\
asm (SECTION_OP "\n\t" \
"call " CRT_MKSTR(__USER_LABEL_PREFIX__) #FUNC "\n" \
TEXT_SECTION_ASM_OP);
+
+#ifdef HAVE_AS_IX86_REP_BSFBSR
+#define REP_BEFORE_BSF "rep "
+#else
+#define REP_BEFORE_BSF "rep; "
+#endif
/* Which processor to tune code generation for. */
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 879b87b..4a1cbbe 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -12235,7 +12235,7 @@
;
else if (TARGET_GENERIC)
/* tzcnt expands to rep;bsf and we can use it even if !TARGET_BMI. */
-return "rep; bsf{}\t{%1, %0|%0, %1}";
+return REP_BEFORE_BSF "bsf{}\t{%1, %0|%0, %1}";
return "bsf{}\t{%1, %0|%0, %1}";
}
diff --git a/gcc/configure b/gcc/configure
index 1fdf0af..c5039d0 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -24815,6 +24815,38 @@ $as_echo "#define HAVE_AS_IX86_REP_LOCK_PREFIX 1"
>>confdefs.h
fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for rep prefix
on bsf/bsr" >&5
+$as_echo_n "checking assembler for rep prefix on bsf/bsr... " >&6; }
+if test "${gcc_cv_as_ix86_rep_bsf+set}" = set; then :
+ $as_echo_n "(cached) " >&6
+else
+ gcc_cv_as_ix86_rep_bsf=no
+ if test x$gcc_cv_as != x; then
+$as_echo 'rep bsf %ecx, %eax
+rep bsr %ecx, %eax' > conftest.s
+if { ac_try='$gcc_cv_as $gcc_cv_as_flags -o conftest.o conftest.s >&5'
+ { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+ (eval $ac_try) 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; }
+then
+ gcc_cv_as_ix86_rep_bsf=yes
+else
+ echo "configure: failed program was" >&5
+ cat conftest.s >&5
+fi
+rm -f conftest.o conftest.s
+ fi
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_as_ix86_rep_bsf" >&5
+$as_echo "$gcc_cv_as_ix86_rep_bsf" >&6; }
+if test $gcc_cv_as_ix86_rep_bsf = yes; then
+
+$as_echo "#define HAVE_AS_IX86_REP_BSFBSR 1" >>confdefs.h
+
+fi
+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for
R_386_TLS_GD_PLT reloc" >&5
$as_echo_n "checking assembler for R_386_TLS_GD_PLT reloc... " >&6; }
if test "${gcc_cv_as_ix86_tlsgdplt+set}" = set; then :
@@ -28698,4 +28730,3 @@ if test -n "$ac_unrecognized_opts" && test
"$enable_option_checking" != no; then
$as_echo "$as_me: WARNING: unrecognized options: $ac_unrecognized_opts" >&2;}
fi
-
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 22dab55..c366181 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -3646,6 +3646,13 @@ foo: nop
[AC_DEFINE(HAVE_AS_IX86_REP_LOCK_PREFIX, 1,
[Define if the assembler supports 'rep , lock '.])])
+gcc_GAS_CHECK_FEATURE([rep prefix on bsf/bsr],
+gcc_cv_as_ix86_rep_bsf,,,
+ [rep bsf %ecx, %eax
+rep bsr %ecx, %eax],,
+[AC_DEFINE(HAVE_AS_IX86_REP_BSFBSR, 1,
+ [Define if the assembler supports 'rep bsf' and 'rep bsr'.])])
+
gcc_GAS_CHECK_FEATURE([R_386_TLS_GD_PLT reloc],
gcc_cv_as_ix86_tlsgdplt,,,
[calltls_gd@tlsgdplt],
Re: [PATCH] x86: use 'rep bsf' syntax when assembler supports it
On Fri, Jun 22, 2012 at 11:57 AM, Uros Bizjak wrote: > Please do not introduce new macro, just use #ifdef > HAVE_AS_IX86_REP_BSFBSR directly in i386.md. Would you want the same #ifdef in two places if I extend this to handle 'rep ret' too, or would a macro then be preferable? To me, the macro is nicer because it avoids the temptation to repeat nontrivial bits on both sides of an #ifdef, as your example does. > OK for mainline SVN with this change. I'm not a GCC committer. > On a related note, gcc is also emitting "rep; ret". Do we need the > same treatment here too? I noticed that but ignored it because the variant target I'm working on avoids that case anyway (for unrelated reasons). I haven't extended the assembler to handle 'rep ret', but it would be trivial to do so and I think it would make sense to do that and then make GCC emit 'rep ret' when supported too, just on general principles. I'll see if I can make the assembler change today and follow up with a new version of this patch that handles the 'ret' case too. Thanks, Roland
Re: [off list] Re: [PATCH] x86: use 'rep bsf' syntax when assembler supports it
Here is an alternative patch that just changes the configure test
controlling %; so it will elide the ; only for an assembler that
also accepts 'rep bsf', 'rep bsr', and 'rep ret', and just uses
%; for these cases too. You'll need to have built binutils from its trunk
within the last five minutes or so to have an assembler that passes the
test now.
Thanks,
Roland
gcc/
2012-06-22 Roland McGrath
* configure.ac (HAVE_AS_IX86_REP_LOCK_PREFIX): Also require that the
assembler accept 'rep bsf ...', 'rep bsr ...', and 'rep ret'.
* configure: Regenerated.
* config/i386/i386.md (simple_return_internal_long): Use %;
(ctz2): Likewise.
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 879b87b..16056c1 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -11902,7 +11902,7 @@
[(simple_return)
(unspec [(const_int 0)] UNSPEC_REP)]
"reload_completed"
- "rep\;ret"
+ "rep%; ret"
[(set_attr "length" "2")
(set_attr "atom_unit" "jeu")
(set_attr "length_immediate" "0")
@@ -12234,8 +12234,8 @@
else if (optimize_function_for_size_p (cfun))
;
else if (TARGET_GENERIC)
-/* tzcnt expands to rep;bsf and we can use it even if !TARGET_BMI. */
-return "rep; bsf{}\t{%1, %0|%0, %1}";
+/* tzcnt expands to 'rep bsf' and we can use it even if !TARGET_BMI. */
+return "rep%; bsf{}\t{%1, %0|%0, %1}";
return "bsf{}\t{%1, %0|%0, %1}";
}
diff --git a/gcc/configure b/gcc/configure
index 1fdf0af..5e8107f 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -24791,7 +24791,10 @@ else
if test x$gcc_cv_as != x; then
$as_echo 'rep movsl
lock addl %edi, (%eax,%esi)
-lock orl $0, (%esp)' > conftest.s
+lock orl $0, (%esp)
+rep bsf %ecx, %eax
+rep bsr %ecx, %eax
+rep ret' > conftest.s
if { ac_try='$gcc_cv_as $gcc_cv_as_flags -o conftest.o conftest.s >&5'
{ { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
(eval $ac_try) 2>&5
@@ -28698,4 +28701,3 @@ if test -n "$ac_unrecognized_opts" && test
"$enable_option_checking" != no; then
$as_echo "$as_me: WARNING: unrecognized options: $ac_unrecognized_opts" >&2;}
fi
-
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 22dab55..428a21a 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -3642,7 +3642,10 @@ foo: nop
gcc_cv_as_ix86_rep_lock_prefix,,,
[rep movsl
lock addl %edi, (%eax,%esi)
-lock orl $0, (%esp)],,
+lock orl $0, (%esp)
+rep bsf %ecx, %eax
+rep bsr %ecx, %eax
+rep ret],,
[AC_DEFINE(HAVE_AS_IX86_REP_LOCK_PREFIX, 1,
[Define if the assembler supports 'rep , lock '.])])
@@ -5163,4 +5166,3 @@ done
],
[subdirs='$subdirs'])
AC_OUTPUT
-
Re: [off list] Re: [PATCH] x86: use 'rep bsf' syntax when assembler supports it
On Sun, Jul 1, 2012 at 1:08 AM, Uros Bizjak wrote:
> Based on the observation above, the patch is OK for mainline, but
> please also handle "rep nop" case.
Here's the new version of the patch that does that. Note that someone
needs to commit this for me, since I am not empowered to do it myself.
Thanks,
Roland
gcc/
2012-07-02 Roland McGrath
* configure.ac (HAVE_AS_IX86_REP_LOCK_PREFIX): Also require that the
assembler accept 'rep bsf ...', 'rep bsr ...', 'rep ret', and 'rep nop'.
* configure: Regenerated.
* config/i386/i386.md (simple_return_internal_long): Use %;
(ctz2): Likewise.
(*pause): Likewise.
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 60aa65a..f23c13c 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -11904,7 +11904,7 @@
[(simple_return)
(unspec [(const_int 0)] UNSPEC_REP)]
"reload_completed"
- "rep\;ret"
+ "rep%; ret"
[(set_attr "length" "2")
(set_attr "atom_unit" "jeu")
(set_attr "length_immediate" "0")
@@ -12236,8 +12236,8 @@
else if (optimize_function_for_size_p (cfun))
;
else if (TARGET_GENERIC)
-/* tzcnt expands to rep;bsf and we can use it even if !TARGET_BMI. */
-return "rep; bsf{}\t{%1, %0|%0, %1}";
+/* tzcnt expands to 'rep bsf' and we can use it even if !TARGET_BMI. */
+return "rep%; bsf{}\t{%1, %0|%0, %1}";
return "bsf{}\t{%1, %0|%0, %1}";
}
@@ -18131,7 +18131,7 @@
[(set (match_operand:BLK 0)
(unspec:BLK [(match_dup 0)] UNSPEC_PAUSE))]
""
- "rep; nop"
+ "rep%; nop"
[(set_attr "length" "2")
(set_attr "memory" "unknown")])
diff --git a/gcc/configure b/gcc/configure
index fd3be52..db81441 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -24791,7 +24791,11 @@ else
if test x$gcc_cv_as != x; then
$as_echo 'rep movsl
lock addl %edi, (%eax,%esi)
-lock orl $0, (%esp)' > conftest.s
+lock orl $0, (%esp)
+rep bsf %ecx, %eax
+rep bsr %ecx, %eax
+rep ret
+rep nop' > conftest.s
if { ac_try='$gcc_cv_as $gcc_cv_as_flags -o conftest.o conftest.s >&5'
{ { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
(eval $ac_try) 2>&5
@@ -28693,4 +28697,3 @@ if test -n "$ac_unrecognized_opts" && test
"$enable_option_checking" != no; then
$as_echo "$as_me: WARNING: unrecognized options: $ac_unrecognized_opts" >&2;}
fi
-
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 89644e2..3bc523d 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -3642,7 +3642,11 @@ foo: nop
gcc_cv_as_ix86_rep_lock_prefix,,,
[rep movsl
lock addl %edi, (%eax,%esi)
-lock orl $0, (%esp)],,
+lock orl $0, (%esp)
+rep bsf %ecx, %eax
+rep bsr %ecx, %eax
+rep ret
+rep nop],,
[AC_DEFINE(HAVE_AS_IX86_REP_LOCK_PREFIX, 1,
[Define if the assembler supports 'rep , lock '.])])
@@ -5158,4 +5162,3 @@ done
],
[subdirs='$subdirs'])
AC_OUTPUT
-
Re: [PATCH] ARM: exclude fixed_regs for stack-alignment save/restore
Richard, here is the patch against the current trunk, as I promised
last week in Prague. Please apply.
Thanks,
Roland
gcc/
2012-07-17 Roland McGrath
* config/arm/arm.c (arm_get_frame_offsets): Never use a fixed register
as the extra register to save/restore for stack-alignment padding.
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index e2f625c..189f71e 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -16121,7 +16121,12 @@ arm_get_frame_offsets (void)
else
for (i = 4; i <= (TARGET_THUMB1 ? LAST_LO_REGNUM : 11); i++)
{
- if ((offsets->saved_regs_mask & (1 << i)) == 0)
+ /* While the gratuitous register save/restore is ordinarily
+ harmless, if a register is marked as fixed or global it
+ may be entirely forbidden by the system ABI to touch it,
+ so we should avoid those registers. */
+ if (!fixed_regs[i]
+ && (offsets->saved_regs_mask & (1 << i)) == 0)
{
reg = i;
break;
Re: [PATCH] ARM: exclude fixed_regs for stack-alignment save/restore
ping?
Re: [PATCH] ARM: exclude fixed_regs for stack-alignment save/restore
Thanks muchly!
[PATCH] Fix ARM constant-pool layout calculations under -falign-labels
Using e.g. -falign-labels=16 on ARM can confuse the constant-pool layout code such that it places pool entries too far away from their referring instructions. This change seems to fix it. I don't have a small test case, only a large one, which I haven't actually tried to get to reproduce on any vanilla ARM target. But the logic of the change seems straightforward and sound. Thanks, Roland 2012-08-01 Roland McGrath * config/arm/arm.c (get_label_padding): Use align_labels as minimum. diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index 701ab4c..9bdb52c 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -12384,6 +12384,7 @@ get_label_padding (rtx label) HOST_WIDE_INT align, min_insn_size; align = 1 << label_to_alignment (label); + align = MAX (align, align_labels); min_insn_size = TARGET_THUMB ? 2 : 4; return align > min_insn_size ? align - min_insn_size : 0; }
Re: [PATCH] Fix ARM constant-pool layout calculations under -falign-labels
On Thu, Aug 2, 2012 at 1:57 AM, Julian Brown wrote: > FWIW, I've hit this issue in the past, and used a patch as follows to > fix it: > > @@ -12015,7 +12025,10 @@ create_fix_barrier (Mfix *fix, HOST_WIDE >gcc_assert (GET_CODE (from) != BARRIER); > >/* Count the length of this insn. */ > - count += get_attr_length (from); > + if (LABEL_P (from) && (align_jumps > 0 || align_loops > 0)) > +count += MAX (align_jumps, align_loops); > + else > +count += get_attr_length (from); > That looks like it's from a version of arm.c before get_label_padding was added. The existing trunk code uses get_label_padding at that spot, and my change was to get_label_padding. > Not sure if that's equivalent, but you might want to check > [-falign-loops] too while you're at it. That seems like it could overestimate, if the -falign-loops value is larger than the -falign-labels value. Not every label is for a loop. Thanks, Roland
[PATCH] don't presume undelegitimized UNSPEC_TLS SYMBOL_REF is a decl
cf this change:
2010-11-19 Jakub Jelinek
PR target/45870
* dwarf2out.c (const_ok_for_output_1): Don't complain about
non-delegitimized TLS UNSPECs.
This case hit me where the rtx was:
(unspec:SI [
(symbol_ref:SI ("*.LANCHOR0") [flags 0x1aa])
(const_int 4 [0x4])
] UNSPEC_TLS)
Note:
1. The UNSPEC has two operands, not one.
2. The SYMBOL_REF does not correspond to any decl.
This corresponds to this ARM code:
ldr r3, .L10+4
...
.L10:
.word .LANCHOR0(tpoff)
...
.section.tdata,"awT",%progbits
.align 4
.LANCHOR0 = . + 0
.type tdata1, %object
.size tdata1, 4
tdata1:
.word 1
The only way I know to reproduce this is using a variant ARM target that
I'm still developing and is not yet ready to be submitted, so I don't have
a proper test case to offer. But I think the principle of the following
change is fairly sound.
What do you think? (Recall that I am not a GCC committer, so if you like
the change, please commit it for me.)
Thanks,
Roland
2012-06-06 Roland McGrath
* dwarf2out.c (const_ok_for_output_1): Detect a TLS UNSPEC using
SYMBOL_REF_TLS_MODEL rather than DECL_THREAD_LOCAL_P, in case it's
not a VAR_DECL. Also don't limit it to UNSPECs with exactly one
operand.
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 6e4ab76..bc68205 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -10129,12 +10129,12 @@ const_ok_for_output_1 (rtx *rtlp, void *data
ATTRIBUTE_UNUSED)
we can't express it in the debug info. */
#ifdef ENABLE_CHECKING
/* Don't complain about TLS UNSPECs, those are just too hard to
-delegitimize. */
- if (XVECLEN (rtl, 0) != 1
+delegitimize. Note this could be a non-decl SYMBOL_REF such as
+one in a constant pool entry, so testing SYMBOL_REF_TLS_MODEL
+rather than DECL_THREAD_LOCAL_P is not just an optimization. */
+ if (XVECLEN (rtl, 0) == 0
|| GET_CODE (XVECEXP (rtl, 0, 0)) != SYMBOL_REF
- || SYMBOL_REF_DECL (XVECEXP (rtl, 0, 0)) == NULL
- || TREE_CODE (SYMBOL_REF_DECL (XVECEXP (rtl, 0, 0))) != VAR_DECL
- || !DECL_THREAD_LOCAL_P (SYMBOL_REF_DECL (XVECEXP (rtl, 0, 0
+ || SYMBOL_REF_TLS_MODEL (XVECEXP (rtl, 0, 0)) == TLS_MODEL_NONE)
inform (current_function_decl
? DECL_SOURCE_LOCATION (current_function_decl)
: UNKNOWN_LOCATION,
[PATCH] libgcc: If __GLIBC__ is defined, don't refer to pthread_cancel.
See http://gcc.gnu.org/ml/gcc-patches/2012-01/msg01427.html for the
previous discussion about the motivation for this patch and the choice
of __pthread_key_create (also the comment added in the code by this patch).
This had no effect on testsuite results on x86_64-linux-gnu
(Ubuntu EGLIBC 2.11.1-0ubuntu7.10).
Thanks,
Roland
libgcc/
2012-06-08 Roland McGrath
* gthr-posix.h [neither FreeBSD nor Solaris] (__gthread_active_p):
If __GLIBC__ is defined, refer to __pthread_key_create instead of
pthread_cancel.
diff --git a/libgcc/gthr-posix.h b/libgcc/gthr-posix.h
index b5b1611..cc4e518 100644
--- a/libgcc/gthr-posix.h
+++ b/libgcc/gthr-posix.h
@@ -212,18 +212,43 @@ __gthread_active_p (void)
#else /* neither FreeBSD nor Solaris */
+/* For a program to be multi-threaded the only thing that it certainly must
+ be using is pthread_create. However, there may be other libraries that
+ intercept pthread_create with their own definitions to wrap pthreads
+ functionality for some purpose. In those cases, pthread_create being
+ defined might not necessarily mean that libpthread is actually linked
+ in.
+
+ For the GNU C library, we can use a known internal name. This is always
+ available in the ABI, but no other library would define it. That is
+ ideal, since any public pthread function might be intercepted just as
+ pthread_create might be. __pthread_key_create is an "internal"
+ implementation symbol, but it is part of the public exported ABI. Also,
+ it's among the symbols that the static libpthread.a always links in
+ whenever pthread_create is used, so there is no danger of a false
+ negative result in any statically-linked, multi-threaded program.
+
+ For others, we choose pthread_cancel as a function that seems unlikely
+ to be redefined by an interceptor library. The bionic (Android) C
+ library does not provide pthread_cancel, so we do use pthread_create
+ there (and interceptor libraries lose). */
+
+#ifdef __GLIBC__
+__gthrw2(__gthrw_(__pthread_key_create),
+__pthread_key_create,
+pthread_key_create)
+# define GTHR_ACTIVE_PROXY __gthrw_(__pthread_key_create)
+#elif defined (__BIONIC__)
+# define GTHR_ACTIVE_PROXY __gthrw_(pthread_create)
+#else
+# define GTHR_ACTIVE_PROXY __gthrw_(pthread_cancel)
+#endif
+
static inline int
__gthread_active_p (void)
{
-/* Android's C library does not provide pthread_cancel, check for
- `pthread_create' instead. */
-#ifndef __BIONIC__
static void *const __gthread_active_ptr
-= __extension__ (void *) &__gthrw_(pthread_cancel);
-#else
- static void *const __gthread_active_ptr
-= __extension__ (void *) &__gthrw_(pthread_create);
-#endif
+= __extension__ (void *) >HR_ACTIVE_PROXY;
return __gthread_active_ptr != 0;
}
Re: [PATCH] don't presume undelegitimized UNSPEC_TLS SYMBOL_REF is a decl
ping?
On Wed, Jun 6, 2012 at 2:04 PM, Roland McGrath wrote:
> cf this change:
>
> 2010-11-19 Jakub Jelinek
>
> PR target/45870
> * dwarf2out.c (const_ok_for_output_1): Don't complain about
> non-delegitimized TLS UNSPECs.
>
> This case hit me where the rtx was:
>
> (unspec:SI [
> (symbol_ref:SI ("*.LANCHOR0") [flags 0x1aa])
> (const_int 4 [0x4])
> ] UNSPEC_TLS)
>
> Note:
> 1. The UNSPEC has two operands, not one.
> 2. The SYMBOL_REF does not correspond to any decl.
>
> This corresponds to this ARM code:
>
> ldr r3, .L10+4
> ...
> .L10:
>
> .word .LANCHOR0(tpoff)
> ...
> .section .tdata,"awT",%progbits
> .align 4
> .LANCHOR0 = . + 0
> .type tdata1, %object
> .size tdata1, 4
> tdata1:
> .word 1
>
> The only way I know to reproduce this is using a variant ARM target that
> I'm still developing and is not yet ready to be submitted, so I don't have
> a proper test case to offer. But I think the principle of the following
> change is fairly sound.
>
> What do you think? (Recall that I am not a GCC committer, so if you like
> the change, please commit it for me.)
>
>
> Thanks,
> Roland
>
>
> 2012-06-06 Roland McGrath
>
> * dwarf2out.c (const_ok_for_output_1): Detect a TLS UNSPEC using
> SYMBOL_REF_TLS_MODEL rather than DECL_THREAD_LOCAL_P, in case it's
> not a VAR_DECL. Also don't limit it to UNSPECs with exactly one
> operand.
>
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index 6e4ab76..bc68205 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -10129,12 +10129,12 @@ const_ok_for_output_1 (rtx *rtlp, void *data
> ATTRIBUTE_UNUSED)
> we can't express it in the debug info. */
> #ifdef ENABLE_CHECKING
> /* Don't complain about TLS UNSPECs, those are just too hard to
> - delegitimize. */
> - if (XVECLEN (rtl, 0) != 1
> + delegitimize. Note this could be a non-decl SYMBOL_REF such as
> + one in a constant pool entry, so testing SYMBOL_REF_TLS_MODEL
> + rather than DECL_THREAD_LOCAL_P is not just an optimization. */
> + if (XVECLEN (rtl, 0) == 0
> || GET_CODE (XVECEXP (rtl, 0, 0)) != SYMBOL_REF
> - || SYMBOL_REF_DECL (XVECEXP (rtl, 0, 0)) == NULL
> - || TREE_CODE (SYMBOL_REF_DECL (XVECEXP (rtl, 0, 0))) != VAR_DECL
> - || !DECL_THREAD_LOCAL_P (SYMBOL_REF_DECL (XVECEXP (rtl, 0, 0
> + || SYMBOL_REF_TLS_MODEL (XVECEXP (rtl, 0, 0)) == TLS_MODEL_NONE)
> inform (current_function_decl
> ? DECL_SOURCE_LOCATION (current_function_decl)
> : UNKNOWN_LOCATION,
Re: [PATCH] don't presume undelegitimized UNSPEC_TLS SYMBOL_REF is a decl
On Mon, Jun 11, 2012 at 1:42 PM, Richard Henderson wrote: > Applied. Thanks!
Re: [PATCH] libgcc: If __GLIBC__ is defined, don't refer to pthread_cancel.
On Mon, Jun 11, 2012 at 2:16 PM, Richard Henderson wrote: > Applied. Thanks!
[PATCH] libgcc: refer to pthread_create, not pthread_cancel
For GNU systems, the fact that libgcc/libstdc++ refers to pthread_cancel is
utterly bizarre. I don't know of any rationale for this that has ever
applied to glibc. AFAICT the choice was made based on the foibles of
various non-glibc systems.
In practice, things are working fine on GNU systems now, of course. But
they'd work just as well for all extant glibc versions if the reference
were to pthread_create instead. That's what makes clear sense on its face:
if the program is multithreaded, pthread_create is the one function that
certainly must have been linked in.
It is actively troublesome for the case of static linking. It necessitates
linking in a lot of dead code in a program that uses pthreads but doesn't
use pthread_cancel. It only actually works right at all because glibc
added a special hack to cope with this bizarre reference to pthread_cancel.
It would be nice if we could remove that someday. (I don't intend to start
any general discussion here about the level of support for static linking.
It's just an example of what's nonsensical about this usage.)
With any kind of linking, it has whatever side effects linking in
pthread_cancel has. There is at least one port of glibc (to a non-GNU
system) where pthread_cancel has a link-time warning because it's not fully
supported. Getting this warning on every link that uses C++ at all is
rather bewildering, especially to application developers who don't use
pthreads directly at all or don't have any idea what pthread_cancel is.
What do folks think about this change? It obviously should have no effect
whatsoever on builds not using glibc. I'm pretty confident that it won't
have any bad effect on a build using any extant version of glibc that had
pthreads at all.
Thanks,
Roland
2012-01-26 Roland McGrath
* gthr-posix.h [neither FreeBSD nor Solaris] (__gthread_active_p):
If __GLIBC__ is defined, refer to pthread_create, not pthread_cancel.
diff --git a/libgcc/gthr-posix.h b/libgcc/gthr-posix.h
index 46054f6..688253d 100644
--- a/libgcc/gthr-posix.h
+++ b/libgcc/gthr-posix.h
@@ -241,18 +241,24 @@ __gthread_active_p (void)
#else /* neither FreeBSD nor Solaris */
+/* The bionic (Android) C library does not provide pthread_cancel.
+ The GNU C library provides pthread_cancel, but it is a poor
+ choice there. For static linking, referring to pthread_cancel
+ causes it to be linked in even when it's never actually used.
+ For a program to be multi-threaded the only thing that it
+ certainly must be using is pthread_create. */
+
+#if defined(__GLIBC__) || defined(__BIONIC__)
+# define GTHR_ACTIVE_PROXY __gthrw_(pthread_create)
+#else
+# define GTHR_ACTIVE_PROXY __gthrw_(pthread_cancel)
+#endif
+
static inline int
__gthread_active_p (void)
{
-/* Android's C library does not provide pthread_cancel, check for
- `pthread_create' instead. */
-#ifndef __BIONIC__
static void *const __gthread_active_ptr
-= __extension__ (void *) &__gthrw_(pthread_cancel);
-#else
- static void *const __gthread_active_ptr
-= __extension__ (void *) &__gthrw_(pthread_create);
-#endif
+= __extension__ (void *) >HR_ACTIVE_PROXY;
return __gthread_active_ptr != 0;
}
Re: [PATCH] libgcc: refer to pthread_create, not pthread_cancel
I see. There certainly should have been a comment in the code about why pthread_cancel was chosen. I still think it's a particularly poor choice. For glibc, I think pthread_getspecific or pthread_key_create are better choices. Those are much smaller functions that don't bring very much dead code into the link, and they are things actually used by libstdc++ et al. For supporting the interception cases, it's really not entirely safe to use any of the public API functions. Someone might have intercepted any of them in the same way as was cited for pthread_create. In glibc, we have exported __ names for various things, including __pthread_getspecific and __pthread_key_create. IMHO one of those is the best choice. They are part of the permanent public ABI and so won't ever go away, but they are not part of the public API that anyone writing an interceptor library would ever want to touch. Thanks, Roland
Re: [RFC PATCH] Typed DWARF stack
It's been a while since I read Cary's proposal, and I am no longer
likely to do much work of my own in this area. So I'll just respond at
the high level.
I like very much the essential notion of the stack being of typed
entities with no specification of how the consumer actually implements
it. In particular, this also brings implicit-pointer into the fold as a
new type of a stack element, i.e. "imaginary pointer". Obviously only
the operations that amount to addition/subtraction with an integer
actually make sense on such a stack element. This makes the second
operand of DW_OP(GNU_)implicit_pointer superfluous (and hence the common
case of zero a byte smaller), since nonzero cases can just be followed
by an appropriate DW_OP_plus_uconst or suchlike.
To make clear that this should be supported, the specification wording
would have to be somewhat more abstract than suggesting that a union of
target bit-pattern types of various sizes would suffice. But it does
not seem much of a stretch to me, and IMHO it's most appropriate that
DWARF not say very much about consumer implementation.
This is more of a stretch, but IMHO it would also make sense to exploit
this "typed" concept to roll in the distinction between values and
locations. This notion is not very well-formed, but perhaps worth
investigating. That is, one could consider DW_OP_reg* a stack operation
that pushes the location in a register. Implicitly, traditional
operations push the location in an address. But DW_OP_stack_value
"pops" the address location and "pushes" the address literal as a value.
If the TOS at the end of an expression is a location, then it's a
mutable location, otherwise it's a value. When an expression is used in
a value-only context (DW_AT_frame_base, etc.) then TOS addresses are
always (identity-)converted integer values and TOS registers are always
fetched values. This way of thinking is natural to me, and it makes the
DW_AT_frame_base specification a natural and straightforward instance of
a general thing rather than being a one-off special case. But perhaps
it is too convoluted for other people.
As to encoding, I have a fancy idea that I discussed off the cuff at the
Summit with Jakub and Richard, and still quite like, though I haven't
fleshed it out any more than I did then. I hope to inspire someone else
to actually want to implement it. It's rather more ambitious than the
things that Jakub will just add while the rest of us sleep, so I
wouldn't suggest that such DW_OP_GNU_* extensions be delayed for this
plan. But perhaps it can become coherent enough and get done enough to
seriously propose it for DWARF 5.
The basic notion is extreme extensibility for DWARF operations, done in
a way that could yield in practice even smaller encodings that we get
today, while supporting an arbitrarily larger vocabulary of operations.
It's modelled on the DIE encoding, using abbrevs.
Add a section ".debug_opabbrev". A chunk of this would be pointed to by
a CU's DW_AT_op_list attribute--or it could be a CU header field, but
there's not much reason to go that route, except perhaps if such a CU
would always be version=5 anyway so as to make old consumers know that
they don't know how to inerpret expressions therein. (It seems very
unlikely that any .debug_frame/.eh_frame DW_CFA_*expression would ever
need to go beyond the DWARF 4 operation vocabuluary. But if they do,
that header could be likewise extended with a .{debug,eh}_opabbrev
pointer. Though that's pretty clearly beyond the pale for introspective
in-process .eh_frame decoding.) The presence of this attribute signals
that this opabbrev table controls the interpretation of all expressions
found in that CU (i.e. in its DW_FORM_exprloc attributes and in any
.debug_loc lists it points to).
The opabbrev table is much like the DIE abbrev table. It maps an opcode
to an operation and a list of operand encodings. Each operand is
indicated by a DW_FORM_* encoding and the list terminated with zero, as
with DIE abbrevs. For generality, opcodes would be ULEB128 and that
seems fine since any one CU using more than 127 seems like an outlier.
But if real-world CU's might not too infrequently use 128-255 different
opcodes, then a table header byte could give the encoding of opcodes
too, so it might be DW_FORM_data1 instead to optimize the packing of
such a case.
I'm somewhat undecided on how best to encode operations in an opabbrev
table. My first instinct is towards easy extensibility, by encoding
each as a pair of DW_FORM_strp, being "family" and "operation".
(Perhaps for compactness a table would contain several runs of
subtables, each subtable using a single family string with only the
operation string in each subtable element that defines a particular
opcode.) Then family is either unspecified and conventional (perhaps
instead called "vendor", but I prefer "family" so as to indicate side
standards on common families), or perhaps a domain name, reverse domain
name, or
[PATCH] libcpp: Add missing configure check for setlocale.
The libcpp code uses `#ifdef HAVE_SETLOCALE` but its configure doesn't have the corresponding check. Ok for trunk and 14 branch? (Sorry for the attachment, my MUA is difficult.) Thanks, Roland libcpp/ 2025-03-27 Roland McGrath * configure.ac: Check for setlocale. * configure, config.in: Regenerated. diff --git a/libcpp/config.in b/libcpp/config.in index b2e2f4e842c..c81ee08c253 100644 --- a/libcpp/config.in +++ b/libcpp/config.in @@ -210,6 +210,9 @@ /* Define to 1 if you have the `putc_unlocked' function. */ #undef HAVE_PUTC_UNLOCKED +/* Define to 1 if you have the `setlocale' function. */ +#undef HAVE_SETLOCALE + /* Define to 1 if you can assemble SSSE3 insns. */ #undef HAVE_SSSE3 diff --git a/libcpp/configure b/libcpp/configure index 1391081ba09..2d517397c02 100755 --- a/libcpp/configure +++ b/libcpp/configure @@ -4654,7 +4654,7 @@ else We can't simply define LARGE_OFF_T to be 9223372036854775807, since some C++ compilers masquerading as C compilers incorrectly reject 9223372036854775807. */ -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62)) +#define LARGE_OFF_T off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31)) int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721 && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; @@ -4700,7 +4700,7 @@ else We can't simply define LARGE_OFF_T to be 9223372036854775807, since some C++ compilers masquerading as C compilers incorrectly reject 9223372036854775807. */ -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62)) +#define LARGE_OFF_T off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31)) int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721 && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; @@ -4724,7 +4724,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext We can't simply define LARGE_OFF_T to be 9223372036854775807, since some C++ compilers masquerading as C compilers incorrectly reject 9223372036854775807. */ -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62)) +#define LARGE_OFF_T off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31)) int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721 && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; @@ -4769,7 +4769,7 @@ else We can't simply define LARGE_OFF_T to be 9223372036854775807, since some C++ compilers masquerading as C compilers incorrectly reject 9223372036854775807. */ -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62)) +#define LARGE_OFF_T off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31)) int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721 && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; @@ -4793,7 +4793,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext We can't simply define LARGE_OFF_T to be 9223372036854775807, since some C++ compilers masquerading as C compilers incorrectly reject 9223372036854775807. */ -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62)) +#define LARGE_OFF_T off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31)) int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721 && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; @@ -6034,7 +6034,7 @@ _ACEOF -for ac_func in clearerr_unlocked feof_unlocked ferror_unlocked fflush_unlocked fgetc_unlocked fgets_unlocked fileno_unlocked fprintf_unlocked fputc_unlocked fputs_unlocked fread_unlocked fwrite_unlocked getchar_unlocked getc_unlocked putchar_unlocked putc_unlocked +for ac_func in clearerr_unlocked feof_unlocked ferror_unlocked fflush_unlocked fgetc_unlocked fgets_unlocked fileno_unlocked fprintf_unlocked fputc_unlocked fputs_unlocked fread_unlocked fwrite_unlocked getchar_unlocked getc_unlocked putchar_unlocked putc_unlocked setlocale do : as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh` ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var" diff --git a/libcpp/configure.ac b/libcpp/configure.ac index 981f97c4abd..845dd644948 100644 --- a/libcpp/configure.ac +++ b/libcpp/configure.ac @@ -81,7 +81,7 @@ define(libcpp_UNLOCKED_FUNCS, clearerr_unlocked feof_unlocked dnl fileno_unlocked fprintf_unlocked fputc_unlocked fputs_unlocked dnl fread_unlocked fwrite_unlocked getchar_unlocked getc_unlocked dnl putchar_unlocked putc_unlocked) -AC_CHECK_FUNCS(libcpp_UNLOCKED_FUNCS) +AC_CHECK_FUNCS(libcpp_UNLOCKED_FUNCS setlocale) AC_CHECK_DECLS([abort, asprintf, basename(char *), errno, getopt, vasprintf]) AC_CHECK_DECLS(m4_split(m4_normalize(libcpp_UNLOCKED_FUNCS))) @@ -172,7 +172,7 @@ do esac done IFS="$ac_save_IFS" - + if test x$ac_checking != x ; then AC_DEFINE(CHECKING_P, 1, [Define to 1 if you want more run-time sanity checks.])
Re: [PATCH] libcpp: Add missing configure check for setlocale.
I've spent more than my fair share of my life fiddling with autoconf installations, so I just hand-editted the patches to elide the unwanted changes. Now committed on both trunk and releases/14. Thanks, Roland
Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections
A direct cherry-pick of the trunk change (db7548a2771bbf34cf7430712af7ac670b429958 / r259969) applies fine to today's 8 branch and has no check-gcc regressions on x86_64-linux-gnu. OK to commit to 8 branch now? Thanks, Roland
Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections
Committed. Thanks, Roland
Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections
Anybody want to look at this? It rebases identically on today's trunk. I'd like to commit it to trunk and gcc-7-branch and gcc-6-branch ideally. Thanks, Roland On Thu, Sep 22, 2016 at 1:15 PM, Roland McGrath wrote: > ping? > > On Thu, Sep 15, 2016 at 4:09 PM, Roland McGrath wrote: >> This fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77609 (which I've >> just filed). >> >> OK for trunk? >> >> I'm not sure if this kind of fix is appropriate for gcc-6-branch or not, >> but I'd like to backport it there too if it is acceptable. >> >> >> Thanks, >> Roland >> >> >> gcc/ >> 2016-09-15 Roland McGrath < >> >> PR other/77609 >> * varasm.c (default_section_type_flags): Set SECTION_NOTYPE for >> any section for which we don't know a specific type it should have, >> regardless of name. Previously this was done only for the exact >> names ".init_array", ".fini_array", and ".preinit_array". >> >> diff --git a/gcc/varasm.c b/gcc/varasm.c >> index 00a9b30..0d7ea38 100644 >> --- a/gcc/varasm.c >> +++ b/gcc/varasm.c >> @@ -6210,15 +6210,20 @@ default_section_type_flags (tree decl, const >> char *name, int reloc) >>|| strncmp (name, ".gnu.linkonce.tb.", 17) == 0) >> flags |= SECTION_TLS | SECTION_BSS; >> >> - /* These three sections have special ELF types. They are neither >> - SHT_PROGBITS nor SHT_NOBITS, so when changing sections we don't >> - want to print a section type (@progbits or @nobits). If someone >> - is silly enough to emit code or TLS variables to one of these >> - sections, then don't handle them specially. */ >> - if (!(flags & (SECTION_CODE | SECTION_BSS | SECTION_TLS)) >> - && (strcmp (name, ".init_array") == 0 >> - || strcmp (name, ".fini_array") == 0 >> - || strcmp (name, ".preinit_array") == 0)) >> + /* Various sections have special ELF types that the assembler will >> + assign by default based on the name. They are neither SHT_PROGBITS >> + nor SHT_NOBITS, so when changing sections we don't want to print a >> + section type (@progbits or @nobits). Rather than duplicating the >> + assembler's knowledge of what those special name patterns are, just >> + let the assembler choose the type if we don't know a specific >> + reason to set it to something other than the default. SHT_PROGBITS >> + is the default for sections whose name is not specially known to >> + the assembler, so it does no harm to leave the choice to the >> + assembler when @progbits is the best thing we know to use. If >> + someone is silly enough to emit code or TLS variables to one of >> + these sections, then don't handle them specially. */ >> + if (!(flags & (SECTION_CODE | SECTION_BSS | SECTION_TLS | >> SECTION_ENTSIZE)) >> + && !(HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE))) >> flags |= SECTION_NOTYPE; >> >>return flags;
Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections
On Mon, Feb 26, 2018 at 8:11 PM, Ian Lance Taylor wrote:
> You are recreating the conditions used in
> default_elf_asm_named_section, so I think you ought to have comments
> referring back and forth between them.
>
> This is OK with the two additional comments.
Thanks for the review. I've added those comments.
However, in testing on x86_64-linux-gnu it caused a regression in:
gcc/testsuite/gcc.target/i386/pr25254.c
which got the "section type conflict" error.
This is because x86_64_elf_select_section for that case calls:
get_section (".lrodata", SECTION_LARGE, NULL)
but something else had previously instantiated the section via
the section_type_flags logic that now adds in SECTION_NOTYPE.
I addressed this by making get_section accept having SECTION_NOTYPE and not
as a non-conflict if none of SECTION_BSS et al is present. That seemed
like a better bet than finding every get_section caller and making sure
they use SECTION_NOTYPE when appropriate. But I'm not sure if there might
be some downside to that logic or if there is a third way to resolve this
that's better than either of those two.
Here's the new patch I'd like to commit. It has no regressions on
x86_64-linux-gnu, but I'm not set up to test other configurations.
gcc/
2018-02-27 Roland McGrath
PR other/77609
* varasm.c (default_section_type_flags): Set SECTION_NOTYPE for
any section for which we don't know a specific type it should have,
regardless of name. Previously this was done only for the exact
names ".init_array", ".fini_array", and ".preinit_array".
(default_elf_asm_named_section): Add comment about
relationship with default_section_type_flags and SECTION_NOTYPE.
(get_section): Don't consider it a type conflict if one side has
SECTION_NOTYPE and the other doesn't, as long as neither has the
SECTION_BSS et al used in the default_section_type_flags logic.
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 6e345d39d31..e488f866011 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -296,6 +296,17 @@ get_section (const char *name, unsigned int
flags, tree decl)
else
{
sect = *slot;
+ /* It is fine if one of the sections has SECTION_NOTYPE as long as
+ the other has none of the contrary flags (see the logic at the end
+ of default_section_type_flags, below). */
+ if (((sect->common.flags ^ flags) & SECTION_NOTYPE)
+ && !((sect->common.flags | flags)
+ & (SECTION_CODE | SECTION_BSS | SECTION_TLS | SECTION_ENTSIZE
+ | (HAVE_COMDAT_GROUP ? SECTION_LINKONCE : 0
+{
+ sect->common.flags |= SECTION_NOTYPE;
+ flags |= SECTION_NOTYPE;
+}
if ((sect->common.flags & ~SECTION_DECLARED) != flags
&& ((sect->common.flags | flags) & SECTION_OVERRIDE) == 0)
{
@@ -6361,15 +6372,23 @@ default_section_type_flags (tree decl, const
char *name, int reloc)
|| strncmp (name, ".gnu.linkonce.tb.", 17) == 0)
flags |= SECTION_TLS | SECTION_BSS;
- /* These three sections have special ELF types. They are neither
- SHT_PROGBITS nor SHT_NOBITS, so when changing sections we don't
- want to print a section type (@progbits or @nobits). If someone
- is silly enough to emit code or TLS variables to one of these
- sections, then don't handle them specially. */
- if (!(flags & (SECTION_CODE | SECTION_BSS | SECTION_TLS))
- && (strcmp (name, ".init_array") == 0
- || strcmp (name, ".fini_array") == 0
- || strcmp (name, ".preinit_array") == 0))
+ /* Various sections have special ELF types that the assembler will
+ assign by default based on the name. They are neither SHT_PROGBITS
+ nor SHT_NOBITS, so when changing sections we don't want to print a
+ section type (@progbits or @nobits). Rather than duplicating the
+ assembler's knowledge of what those special name patterns are, just
+ let the assembler choose the type if we don't know a specific
+ reason to set it to something other than the default. SHT_PROGBITS
+ is the default for sections whose name is not specially known to
+ the assembler, so it does no harm to leave the choice to the
+ assembler when @progbits is the best thing we know to use. If
+ someone is silly enough to emit code or TLS variables to one of
+ these sections, then don't handle them specially.
+
+ default_elf_asm_named_section (below) handles the BSS, TLS, ENTSIZE, and
+ LINKONCE cases when NOTYPE is not set, so leave those to its logic. */
+ if (!(flags & (SECTION_CODE | SECTION_BSS | SECTION_TLS | SECTION_ENTSIZE))
+ && !(HAVE_C
Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections
On Tue, Feb 27, 2018 at 8:14 PM, Ian Lance Taylor wrote: > Still OK, but it should wait until after the tree is back in stage 1. I've been hoping to get this fixed on stable branches (6, 7). Are you saying that this bug can only be fixed in 9? Or will it be OK to backport to 6, 7, and 8 once stage 1 opens for 9? Thanks, Roland
Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections
OK. I'll come back in stage 1. Thanks, Roland
Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections
I'm back for stage 1! The same patch from https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01549.html rebases cleanly and I didn't change anything but the date on the log entry since what I posted there. The fresh rebase is on the roland/pr77609 git branch for your convenience. It has no check-gcc failures on x86_64-linux-gnu. OK to commit to trunk now? When will be the right time to raise the question of backporting it, perhaps shortly after the 8 release? Thanks, Roland
Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections
ping On Sat, Apr 28, 2018 at 2:42 AM Roland McGrath wrote: > I'm back for stage 1! > The same patch from https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01549.html > rebases cleanly and I didn't change anything but the date on the log entry > since what I posted there. The fresh rebase is on the roland/pr77609 git > branch for your convenience. > It has no check-gcc failures on x86_64-linux-gnu. > OK to commit to trunk now? > When will be the right time to raise the question of backporting it, > perhaps shortly after the 8 release? > Thanks, > Roland
Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections
Committed. Thanks, Roland
