Re: [google/integration] Support arm-grtev2-linux-*eabi (issue4628043)
On Mon, Sep 12, 2011 at 15:24, Gerald Pfeifer wrote: > Should this be added to the GCC 4.7 release notes at > http://gcc.gnu.org/gcc-4.7/changes.html ? If you need help doing > so, propose a text and I'll be happy to care of it. I absolutely need help (just ask Diego)... but not with that... 8-) This was a change that was made on the google/integration branch, and is not appropriate for trunk. So, no NEWS (or changes.html) here. Thanks, though. 8-) -c
Re: [google][RFA] add extra text to stack frame warnings (issue4479046)
[resending plain text. Sorry for the noise.] [sorry, i got stuck doing a bunch of other things.] On Fri, May 6, 2011 at 10:05, Andrew Pinski wrote: > > On Fri, May 6, 2011 at 1:52 AM, Chris Demetriou wrote: > > In theory, a more general warning-text-addition mechanism could be useful. > > e.g. a flag that said "when outputting a warning about flag 'foo', > > output this additional text" could be useful. > > However, we haven't felt the need to do this for other warnings. > > > > IMO, a general solution along these lines would be solving a problem > > that ~nobody has. 8-) > > We already output the option which enables the warning that seems like > a general solution. Yeah. I was going to look into using something similar to implement a generic text-addition mechanism for warnings... but got stuck in previously-mentioned bunch of other things. 8-( I ran out of time, ended up checking this in as-is (after updating the changelog date and retesting). I may get back to doing it the better way... but it won't be for a while. chris
[google/gcc-4_6] merge google/main r174890 to google/gcc-4_6 branch (issue4576055)
testing with a native bootstrap, not quite done yet but since there were
no conflicts (except for the ChangeLog) it'll be fine.
OK for google/gcc-4_6 assuming tests pass?
(Note that the properties changes were generated by svnmerge.py, and
TBH I have *no idea* what some of them are about.)
thanks,
chris
-
[gcc/ChangeLog.google-4_6]
2011-06-09 Chris Demetriou
Backport from google/main r174890:
2011-06-09 Chris Demetriou
* doc/install.texi (--with-warn-frame-larger-than-extra-text): New.
* configure.ac (--with-warn-frame-larger-than-extra-text): New.
(WARN_FRAME_LARGER_THAN_EXTRA_TEXT): Define.
* final.c (final_start_function): Use
WARN_FRAME_LARGER_THAN_EXTRA_TEXT.
* configure: Regenerate.
* config.in: Regenerate.
Property changes on: .
___
Modified: svnmerge-integrated
- /branches/google/main:1-174706,174789
/branches/google/integration:1-170988,173923,173959
/branches/gcc-4_6-branch:1-174748
+ /branches/google/main:1-174706,174789,174890
/branches/google/integration:1-170988,173923,173959
/branches/gcc-4_6-branch:1-174748
Modified: svn:mergeinfo
Merged /branches/google/main:r174890
Property changes on: libjava/classpath
___
Modified: svn:mergeinfo
Merged /branches/google/main/libjava/classpath:r174890
Index: gcc/doc/install.texi
===
--- gcc/doc/install.texi(revision 174926)
+++ gcc/doc/install.texi(working copy)
@@ -1717,6 +1717,10 @@
See @option{-canonical-prefixes} or @option{-no-canonical-prefixes} for
more details, including how to override this configuration option when
compiling.
+
+@item --with-warn-frame-larger-than-extra-text=@var{text}
+Append @samp{@var{text}} to frame size warnings generated by
+the @option{-Wframe-larger-than} warning flag.
@end table
@subheading Cross-Compiler-Specific Options
Index: gcc/configure
===
--- gcc/configure (revision 174926)
+++ gcc/configure (working copy)
@@ -919,6 +919,7 @@
enable_canonical_prefixes
enable_plugin
enable_libquadmath_support
+with_warn_frame_larger_than_extra_text
'
ac_precious_vars='build_alias
host_alias
@@ -1678,6 +1679,8 @@
with the compiler
--with-system-zlib use installed libz
--with-slibdir=DIR shared libraries in DIR [LIBDIR]
+ --with-warn-frame-larger-than-extra-text=TEXT
+ specifies extra text for frame size warnings
Some influential environment variables:
CC C compiler command
@@ -17578,7 +17581,7 @@
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
-#line 17581 "configure"
+#line 17584 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
@@ -17684,7 +17687,7 @@
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
-#line 17687 "configure"
+#line 17690 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
@@ -26510,6 +26513,24 @@
fi
+warn_frame_larger_than_extra_text=
+
+# Check whether --with-warn-frame-larger-than-extra-text was given.
+if test "${with_warn_frame_larger_than_extra_text+set}" = set; then :
+ withval=$with_warn_frame_larger_than_extra_text; case "${withval}" in
+yes) as_fn_error "bad value ${withval} given for frame size warning text"
"$LINENO" 5 ;;
+no);;
+*) warn_frame_larger_than_extra_text="$withval" ;;
+esac
+fi
+
+
+cat >>confdefs.h <<_ACEOF
+#define WARN_FRAME_LARGER_THAN_EXTRA_TEXT "$warn_frame_larger_than_extra_text"
+_ACEOF
+
+
+
# Configure the subdirectories
# AC_CONFIG_SUBDIRS($subdirs)
Index: gcc/final.c
===
--- gcc/final.c (revision 174926)
+++ gcc/final.c (working copy)
@@ -1576,9 +1576,13 @@
if (warn_frame_larger_than
&& get_frame_size () > frame_larger_than_size)
{
- /* Issue a warning */
+ /* Issue a warning. (WARN_FRAME_LARGER_THAN_EXTRA_TEXT is
+ provided by configuration. The way extra text is added
+ here may prevent localization from working properly.
+ It's totally broken.) */
warning (OPT_Wframe_larger_than_,
- "the frame size of %wd bytes is larger than %wd bytes",
+ "the frame size of %wd bytes is larger than %wd bytes"
+ WARN_FRAME_LARGER_THAN_EXTRA_TEXT,
get_frame_size (), frame_larger_than_size);
}
Property changes on: gcc/testsuite/gcc.target/powerpc/ppc-round.
[google/integration] Support arm-grtev2-linux-*eabi (issue4628043)
Diego and/or Ollie,
This patch adds support for the arm-grtev2-linux-gnueabi configuration.
This is like the x86 linux config of similar 'vendor':
* it plays some spec games to support easier use of static NSS
configuration, and
* it adds RUNTIME_ROOT_PREFIX support to the dynamic loader path.
(technically, linux-elf.h didn't need the RUNTIME_ROOT_PREFIX change,
but i would have felt ... odd making it in linux-eabi.h and not
linux-elf.h)
tested by building a arm-linux-gnueabi compiler (i.e., *not* -grtev2-)
to sanity check that I didn't break that config. And also building
a -grtev2- compiler as well.
thanks,
chris
[gcc/ChangeLog.google-integration]
2011-06-16 Chris Demetriou
* config/arm/linux-grtev2.h: New file.
* config/arm/linux-elf.h (GLIBC_DYNAMIC_LINKER): Prefix with
RUNTIME_ROOT_PREFIX.
* config/arm/linux-eabi.h (GLIBC_DYNAMIC_LINKER): Likewise.
* config/linux-grtev2.h (LIB_SPEC): Update comment about what
this definition overrides.
* config.gcc: Use linux-grtev2.h for arm-grtev2-linux-*eabi targets.
Index: config.gcc
===
--- config.gcc (revision 175095)
+++ config.gcc (working copy)
@@ -832,6 +832,12 @@
tmake_file="$tmake_file arm/t-linux-androideabi"
;;
esac
+ # Pull in spec changes for GRTEv2 configurations.
+ case ${target} in
+ *-grtev2-*)
+ tm_file="${tm_file} linux-grtev2.h arm/linux-grtev2.h"
+ ;;
+ esac
# The BPABI long long divmod functions return a 128-bit value in
# registers r0-r3. Correctly modeling that requires the use of
# TImode.
Index: config/arm/linux-elf.h
===
--- config/arm/linux-elf.h (revision 175095)
+++ config/arm/linux-elf.h (working copy)
@@ -62,7 +62,7 @@
#define LIBGCC_SPEC "%{msoft-float:-lfloat} %{mfloat-abi=soft*:-lfloat} -lgcc"
-#define GLIBC_DYNAMIC_LINKER "/lib/ld-linux.so.2"
+#define GLIBC_DYNAMIC_LINKER RUNTIME_ROOT_PREFIX "/lib/ld-linux.so.2"
#define LINUX_TARGET_LINK_SPEC "%{h*} \
%{static:-Bstatic} \
Index: config/arm/linux-eabi.h
===
--- config/arm/linux-eabi.h (revision 175095)
+++ config/arm/linux-eabi.h (working copy)
@@ -62,7 +62,7 @@
/* Use ld-linux.so.3 so that it will be possible to run "classic"
GNU/Linux binaries on an EABI system. */
#undef GLIBC_DYNAMIC_LINKER
-#define GLIBC_DYNAMIC_LINKER "/lib/ld-linux.so.3"
+#define GLIBC_DYNAMIC_LINKER RUNTIME_ROOT_PREFIX "/lib/ld-linux.so.3"
/* At this point, bpabi.h will have clobbered LINK_SPEC. We want to
use the GNU/Linux version, not the generic BPABI version. */
Index: config/arm/linux-grtev2.h
===
--- config/arm/linux-grtev2.h (revision 0)
+++ config/arm/linux-grtev2.h (revision 0)
@@ -0,0 +1,27 @@
+/* Definitions for ARM Linux-based GRTE (Google RunTime Environment) version 2.
+ Copyright (C) 2011 Free Software Foundation, Inc.
+ Contributed by Chris Demetriou.
+
+This file is part of GCC.
+
+GCC 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.
+
+GCC 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.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
+
+You should have received a copy of the GNU General Public License and
+a copy of the GCC Runtime Library Exception along with this program;
+see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
+<http://www.gnu.org/licenses/>. */
+
+#undef SUBSUBTARGET_EXTRA_SPECS
+#define SUBSUBTARGET_EXTRA_SPECS LINUX_GRTE_EXTRA_SPECS
Index: config/linux-grtev2.h
===
--- config/linux-grtev2.h (revision 175095)
+++ config/linux-grtev2.h (working copy)
@@ -23,7 +23,7 @@
see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
<http://www.gnu.org/licenses/>. */
-/* Overrides LIB_SPEC from linux.h. */
+/* Overrides LIB_SPEC from gnu-user.h. */
#undef LIB_SPEC
#define LIB_SPEC \
"%{pthread:-lpthread} \
--
This patch is available for review at http://codereview.appspot.com/4628043
[trunk] RFA: translate built-in include paths for sysroot (issue4641076)
The setup:
Configuring a toolchain targeting x86-64 GNU Linux (Ubuntu Lucid), as a
cross-compiler. Using a sysroot to provide the Lucid headers+libraries,
with the sysroot path being within the GCC install tree. Want to use the
Lucid system libstdc++ and headers, which means that I'm not
building/installing libstdc++-v3.
So, configuring with:
--with-sysroot="$SYSROOT"
--disable-libstdc++-v3 \
--with-gxx-include-dir="$SYSROOT/usr/include/c++/4.4" \
(among other options).
Hoping to support two usage models with this configuration, w.r.t. use of
the sysroot:
(1) somebody installs the sysroot in the normal location relative to the
GCC install, and relocates the whole bundle (sysroot+GCC). This works
great AFAICT, GCC finds its includes (including the C++ includes) thanks
to the add_standard_paths iprefix handling.
(2) somebody installs the sysroot in a non-standard location, and uses
--sysroot to try to access it. This works fine for the C headers, but
doesn't work.
For the C headers, add_standard_paths prepends the sysroot location to
the /usr/include path (since that's what's specified in cppdefault.c for
that path). It doesn't do the same for the C++ include path, though
(again, as specified in cppdefault.c).
add_standard_paths doesn't attempt to relocate built-in include paths that
start with the compiled-in sysroot location (e.g., the g++ include dir, in
this case). This isn't surprising really: normally you either prepend the
sysroot location or you don't (as specified by cppdefault.c); none of the
built-in paths normally *start* with the sysroot location and need to be
relocated. However, in this odd-ball case of trying to use the C++ headers
from the sysroot, one of the paths *does* need to be relocated in this way.
The following patch adds code to add_standard_paths to do the sysroot
translation (for oddball configurations like mine).
Tested before/after on a x86_64-unknown-linux-gnu system, with
--enable-languages=all, RUNTESTFLAGS='--target_board=unix\{-m32,-m64\}'.
No changes in test results. (!! usually there are some, only dates this time!)
Also built my special cross configuration, that worked as expected.
OK for trunk?
chris
---
[gcc/ChangeLog]
2011-06-25 Chris Demetriou
* cppdefault.h (cpp_TARGET_SYSTEM_ROOT): New variable.
(cpp_TARGET_SYSTEM_ROOT_len): Likewise.
* cppdefault.c (cpp_TARGET_SYSTEM_ROOT): Likewise.
(cpp_TARGET_SYSTEM_ROOT_len): Likewise.
* incpath.c (add_standard_paths): Relocate paths that start
with the compiled-in sysroot path, if a sysroot is given.
Index: incpath.c
===
--- incpath.c (revision 175395)
+++ incpath.c (working copy)
@@ -133,6 +133,30 @@ add_standard_paths (const char *sysroot, const cha
int relocated = cpp_relocated();
size_t len;
+ if (sysroot && (len = cpp_TARGET_SYSTEM_ROOT_len) != 0)
+{
+ /* Look for directories that start with the compiled-in sysroot prefix.
+"Translate" them, i.e. replace /usr/local/target/sys-root... with
+SYSROOT and search them first. */
+ for (p = cpp_include_defaults; p->fname; p++)
+ {
+ if (!p->cplusplus || cxx_stdinc)
+ {
+ /* If we're going to add the sysroot to a path, then it
+ is not expected to start with the path to the sysroot. */
+ if (p->add_sysroot)
+ continue;
+ if (!filename_ncmp (p->fname, cpp_TARGET_SYSTEM_ROOT, len))
+ {
+ char *str = concat (sysroot, p->fname + len, NULL);
+ if (p->multilib && imultilib)
+ str = concat (str, dir_separator_str, imultilib, NULL);
+ add_path (str, SYSTEM, p->cxx_aware, false);
+ }
+ }
+ }
+}
+
if (iprefix && (len = cpp_GCC_INCLUDE_DIR_len) != 0)
{
/* Look for directories that start with the standard prefix.
Index: cppdefault.c
===
--- cppdefault.c(revision 175395)
+++ cppdefault.c(working copy)
@@ -117,6 +117,15 @@ const char cpp_EXEC_PREFIX[] = STANDARD_EXEC_PREFI
/* This value is set by cpp_relocated at runtime */
const char *gcc_exec_prefix;
+/* The configured target system root (sysroot). */
+#ifdef TARGET_SYSTEM_ROOT
+const char cpp_TARGET_SYSTEM_ROOT[] = TARGET_SYSTEM_ROOT;
+const size_t cpp_TARGET_SYSTEM_ROOT_len = sizeof TARGET_SYSTEM_ROOT - 1;
+#else
+const char cpp_TARGET_SYSTEM_ROOT[] = "";
+const size_t cpp_TARGET_SYSTEM_ROOT_len = 0;
+#endif
+
/* Return true if the toolchain is relocated. */
bool
cpp_relocated (void)
Index: cppdefault.h
===
--- cppdefault.h(revision 175
Re: [trunk] RFA: translate built-in include paths for sysroot (issue4641076)
On Sun, Jun 26, 2011 at 07:28, Joseph S. Myers wrote: > It seems to me that what's really wanted here is to change the add_sysroot > flag for the C++ path (all of the C++ paths?), rather than adding special > code to detect paths starting with the sysroot and reinterpret them. I considered doing that, I wasn't sure what (if any) implications that would have on the way gcc normally builds + configures / how it works when other people use flags like --with-gxx-include-dir. I couldn't think of *harm*, but it seemed to me that my change was least likely to cause harm to existing working paths. (That doesn't mean that it's the right change, just the one that I thought I could understand best. 8-) > And > so making configure detect when the --with-gxx-include-dir setting starts > with the sysroot and adjusting the flag accordingly in that case would be > a cleaner solution - that way it would be obvious that the semantics of > the relocation are exactly the same as for other sysrooted paths, whereas > it isn't when the relocation goes through different code paths in the > compiler. yeah, i can do that. thanks for the quick look. chris
[google/main][RFA] change i386 pc_thunk prefix to be "__x86"
Makes -S output more easily preprocessable -- otherwise, the __i686 in
__i686.get_pc_think.reg chokes things.
bootstrapped x86_64-linux for C/C++ native on Ubunutu Lucid (x86-64),
no diff in testsuite output before/after.
(c, c++, libgomp, libmudflap, libstdc++ tested.)
Also manually tested:
char *foo() {
return "foo";
}
with "cc -fPIC -S test.c -o - -m32" to make sure that the new name was
used as expected.
OK for google/main branch?
(I don't *think* there's a need to push this to google/integration,
it's not *that* critical. And frankly while I think the standard
naming is stupid, it's long-standing and I *assume* that this issue
has come up before and thought not-important-enough-to-change -- even
though AFAIK there's ~no down-side to changing since these thunks
aren't exposed. So, I'm not really proposing this for trunk, but if
somebody wants it... great! 8-)
chris
---
[gcc/ChangeLog.google-main]
2011-04-30 Chris Demetriou
* config/i386/i386.c (get_pc_thunk_name): Make 32-bit
thunk prefix be __x86.get_pc_thunk.
[gcc/ChangeLog.google-main]
2011-04-30 Chris Demetriou
* config/i386/i386.c (get_pc_thunk_name): Make 32-bit
thunk prefix be __x86.get_pc_thunk.
Index: config/i386/i386.c
===
--- config/i386/i386.c (revision 173204)
+++ config/i386/i386.c (working copy)
@@ -8751,7 +8751,7 @@
gcc_assert (!TARGET_64BIT);
if (USE_HIDDEN_LINKONCE)
-sprintf (name, "__i686.get_pc_thunk.%s", reg_names[regno]);
+sprintf (name, "__x86.get_pc_thunk.%s", reg_names[regno]);
else
ASM_GENERATE_INTERNAL_LABEL (name, "LPR", regno);
}
Re: [google/main][RFA] change i386 pc_thunk prefix to be "__x86"
On Mon, May 2, 2011 at 11:49, Andrew Pinski wrote: > On Sat, Apr 30, 2011 at 2:14 PM, Chris Demetriou wrote: >> Makes -S output more easily preprocessable -- otherwise, the __i686 in >> __i686.get_pc_think.reg chokes things. > > IIRC the reason why it uses __i686 is because 586 and before does not > need to worry about the return stack addresses. > > And really I think having a name which is not able to be produced by a > C code is the best. the "." prevents in either case, of course. And of course, either name involvinv __i686 or __x86 is reserved for implementation AFAIK. "__i686" is a preprocessor token, and when compiling with certain flags (-mcpu=i686 and perhaps others, IIRC -- I forget), it's a macro defined to 1. So, when preprocessing -S output (for whatever reason) with cpu set to i686, that beautiful: __i686.get_pc_thunk. gets turned into... 1.get_pc_thunk.. My favorite old annoyance from this was the glibc source that (at least used to) include (inline IIRC) asm code with calls to __i686.get_pc_thunk.. You couldn't compile them with -mcpu=i686 or with an i686-configured toolchain. And, there (at the time at least) IIRC the response was "not a bug." (I've not looked at that code for a long time -- dunno if it's been fixed in stock glibc.) anyway, that's one specific example where (historically) this would have been useful. i've seen others -- enough that we made this change in our compiler a few of years ago. chris
[trunk][RFA] Add __i686.get_pc_thunk.bx to libgcc i386 morestack.S
After I submitted http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02422.html, Rong Xu noted that the resulting sources, if configured --with-pic, would not actually build properly due to missing __i686.get_pc_thunk.bx (due to my renaming that to __x86...). While that issue was exposed by my change, in that configuration, in summary my change exposed a latent issue in morestack.S. (Callers of __i686.get_pc_thunk.bx (etc.) are also supposed to define those functions -- because who knows whether the other objects they're linked against will provide them.) So, I've added the __i686.get_pc_thunk.bx function (as generated by current GCC) to libgcc/config/i386/morestack.S. (I also added __ELF__ #ifdefs and a .size directive for the function, following the apparent conventions in the file.) bootstrapped (C only) on Ubuntu Lucid x86-64, and ran -m32/-m64 tests for compilers configured as normal, and also --with-pic. No regressions before/after. Ian, OK for trunk? chris [libgcc/ChangeLog] 2011-05-03 Chris Demetriou * libgcc/config/i386/morestack.S (__i686.get_pc_thunk.bx): New. [libgcc/ChangeLog] 2011-05-03 Chris Demetriou * libgcc/config/i386/morestack.S (__i686.get_pc_thunk.bx): New. Index: libgcc/config/i386/morestack.S === --- libgcc/config/i386/morestack.S (revision 173287) +++ libgcc/config/i386/morestack.S (working copy) @@ -449,6 +449,24 @@ .size __morestack, . - __morestack #endif +#if !defined(__x86_64__) && defined(__PIC__) +# Output the thunk to get PC into bx, since we use it above. +# (__i686 was already undef'd above; don't need to worry about it here.) + .section .text.__i686.get_pc_thunk.bx,"axG",@progbits,__i686.get_pc_thunk.bx,comdat + .globl __i686.get_pc_thunk.bx + .hidden __i686.get_pc_thunk.bx +#ifdef __ELF__ + .type __i686.get_pc_thunk.bx, @function +#endif +__i686.get_pc_thunk.bx: + .cfi_startproc + movl (%esp), %ebx + ret + .cfi_endproc +#ifdef __ELF__ + .size __i686.get_pc_thunk.bx, . - __i686.get_pc_thunk.bx +#endif +#endif # The exception table. This tells the personality routine to execute # the exception handler.
Re: [trunk][RFA] Add __i686.get_pc_thunk.bx to libgcc i386 morestack.S
On Tue, May 3, 2011 at 15:05, Ian Lance Taylor wrote: > > > 2011-05-03 Chris Demetriou > > > > * libgcc/config/i386/morestack.S (__i686.get_pc_thunk.bx): New. > > No "libgcc" in libgcc/ChangeLog. Fixed, sorry. (That's what I get for pasting from svn status output. 8-) > It is also OK if you s/__i686/__x86/ to correspond to your earlier > change. Either way is OK. Well, that change is not in trunk. Should that change move to trunk, yes, it's appropriate to change this as well (though strictly not *necessary*). (On the google branches, my plan was to rename, yes.) chris
Re: [trunk][RFA] Add __i686.get_pc_thunk.bx to libgcc i386 morestack.S
On Tue, May 3, 2011 at 16:39, Chris Demetriou wrote: > > It is also OK if you s/__i686/__x86/ to correspond to your earlier > > change. Either way is OK. > > Well, that change is not in trunk. > Should that change move to trunk, yes, it's appropriate to change this > as well (though strictly not *necessary*). *sigh* As it turns out, adding __i686.get_pc_thunk.bx to this assembly code -- *cough* this _preprocessed_ assembly code -- chokes in certain configurations. For instance, if you configure with: --with-arch-32=pentium3 then libgcc will build with -march=pentium3 The way the symbol-visibility files are built and processed, morestack.vis is generated from morestack.S, and contains: .hidden __i686.get_pc_thunk.bx morestack.vis is included via -include when compiling morestack.S. ./morestack.vis:4: Error: junk at end of line, first unrecognized character is `1' As someone smarter than me might have guessed... in this configuration, __i686 is defined to be '1'. So, even if the trunk get_thunk code continues to use __i686.get_pc_thunk., it's necessary to use something different here. Kinda feels like I got smacked in the face by walking into my own rake... *sigh* Ian, per your previous comment (which I read as pre-approval for this change 8-), I'm planning to commit the attached as soon as it's done testing. chris - [libgcc/ChangeLog] 2011-05-04 Chris Demetriou * config/i386/morestack.S (__i686.get_pc_thunk.bx): Rename... (__x86.get_pc_thunk.bx): To this. (__morestack): Adjust for rename, remove undef of __i686. [libgcc/ChangeLog] 2011-05-04 Chris Demetriou * config/i386/morestack.S (__i686.get_pc_thunk.bx): Rename... (__x86.get_pc_thunk.bx): To this. (__morestack): Adjust for rename, remove undef of __i686. Index: libgcc/config/i386/morestack.S === --- libgcc/config/i386/morestack.S (revision 173355) +++ libgcc/config/i386/morestack.S (working copy) @@ -278,8 +278,7 @@ movl 4(%esp),%eax # Function argument. movl %eax,(%esp) #ifdef __PIC__ -#undef __i686 - call __i686.get_pc_thunk.bx # %ebx may not be set up for us. + call __x86.get_pc_thunk.bx # %ebx may not be set up for us. addl $_GLOBAL_OFFSET_TABLE_, %ebx call _Unwind_Resume@PLT # Resume unwinding. #else @@ -451,20 +450,19 @@ #if !defined(__x86_64__) && defined(__PIC__) # Output the thunk to get PC into bx, since we use it above. -# (__i686 was already undef'd above; don't need to worry about it here.) - .section .text.__i686.get_pc_thunk.bx,"axG",@progbits,__i686.get_pc_thunk.bx,comdat - .globl __i686.get_pc_thunk.bx - .hidden __i686.get_pc_thunk.bx + .section .text.__x86.get_pc_thunk.bx,"axG",@progbits,__x86.get_pc_thunk.bx,comdat + .globl __x86.get_pc_thunk.bx + .hidden __x86.get_pc_thunk.bx #ifdef __ELF__ - .type __i686.get_pc_thunk.bx, @function + .type __x86.get_pc_thunk.bx, @function #endif -__i686.get_pc_thunk.bx: +__x86.get_pc_thunk.bx: .cfi_startproc movl (%esp), %ebx ret .cfi_endproc #ifdef __ELF__ - .size __i686.get_pc_thunk.bx, . - __i686.get_pc_thunk.bx + .size __x86.get_pc_thunk.bx, . - __x86.get_pc_thunk.bx #endif #endif
Re: [trunk][RFA] Add __i686.get_pc_thunk.bx to libgcc i386 morestack.S
On Wed, May 4, 2011 at 00:52, Chris Demetriou wrote: > Ian, per your previous comment (which I read as pre-approval for this > change 8-), I'm planning to commit the attached as soon as it's done > testing. > > > chris > - > [libgcc/ChangeLog] > 2011-05-04 Chris Demetriou > > * config/i386/morestack.S (__i686.get_pc_thunk.bx): Rename... > (__x86.get_pc_thunk.bx): To this. > (__morestack): Adjust for rename, remove undef of __i686. > Passed my tests (bootstrap C + test C on ubuntu lucid x86-64, with and w/o --with-pic), so submitted @ r173391. Slightly different changelog message: 2011-05-04 Chris Demetriou * config/i386/morestack.S (__i686.get_pc_thunk.bx): Rename to... (__x86.get_pc_thunk.bx): ...this. (__morestack): Adjust for rename, remove undef of __i686. (more consistent with other renames in the same file. 8-) sorry for the churn. chris
[google/main][RFA] backport trunk morestack changes (issue4465045)
Diego, Testing w/ ubuntu lucid native bootstrap + check (C/C++), with --with-pic and numerous other flags, looking good. OK for google/main (assuming tests succeed)? chris -- [libgcc/ChangeLog.google-main] 2011-05-04 Chris Demetriou Backport from trunk r173391: 2011-05-04 Chris Demetriou * config/i386/morestack.S (__i686.get_pc_thunk.bx): Rename to... (__x86.get_pc_thunk.bx): ...this. (__morestack): Adjust for rename, remove undef of __i686. Backport from trunk r173345: 2011-05-03 Chris Demetriou * config/i386/morestack.S (__i686.get_pc_thunk.bx): New. Index: libgcc/config/i386/morestack.S === --- libgcc/config/i386/morestack.S (revision 173353) +++ libgcc/config/i386/morestack.S (working copy) @@ -278,8 +278,7 @@ movl4(%esp),%eax# Function argument. movl%eax,(%esp) #ifdef __PIC__ -#undef __i686 - call__i686.get_pc_thunk.bx # %ebx may not be set up for us. + call__x86.get_pc_thunk.bx # %ebx may not be set up for us. addl$_GLOBAL_OFFSET_TABLE_, %ebx call_Unwind_Resume@PLT # Resume unwinding. #else @@ -449,6 +448,23 @@ .size __morestack, . - __morestack #endif +#if !defined(__x86_64__) && defined(__PIC__) +# Output the thunk to get PC into bx, since we use it above. + .section .text.__x86.get_pc_thunk.bx,"axG",@progbits,__x86.get_pc_thunk.bx,comdat + .globl __x86.get_pc_thunk.bx + .hidden __x86.get_pc_thunk.bx +#ifdef __ELF__ + .type __x86.get_pc_thunk.bx, @function +#endif +__x86.get_pc_thunk.bx: + .cfi_startproc + movl(%esp), %ebx + ret + .cfi_endproc +#ifdef __ELF__ + .size __x86.get_pc_thunk.bx, . - __x86.get_pc_thunk.bx +#endif +#endif # The exception table. This tells the personality routine to execute # the exception handler. -- This patch is available for review at http://codereview.appspot.com/4465045
[google][RFA] add extra text to stack frame warnings (issue4479046)
Diego,
I know this is a truly horrible and broken way to do this, but alternatives
(e.g., NLS) don't really work for us.
bootstrapped without new configuration (x86-64 ubuntu lucid, didn't bother
to run tests), bootstrapped with option (x86-64 ubuntu lucid, with full
tests, no regressions). Manually tested that the resulting warning
looks right, too.
OK for google/main?
chris
---
[gcc/ChangeLog.google-main]
2011-05-05 Chris Demetriou
* doc/install.texi (--with-warn-frame-larger-than-extra-text): New.
* configure.ac (--with-warn-frame-larger-than-extra-text): New.
(WARN_FRAME_LARGER_THAN_EXTRA_TEXT): Define.
* final.c (final_start_function): Use
WARN_FRAME_LARGER_THAN_EXTRA_TEXT.
* configure: Regenerate.
* config.in: Regenerate.
Index: gcc/doc/install.texi
===
--- gcc/doc/install.texi(revision 173353)
+++ gcc/doc/install.texi(working copy)
@@ -1707,6 +1707,10 @@
See @option{-canonical-prefixes} or @option{-no-canonical-prefixes} for
more details, including how to override this configuration option when
compiling.
+
+@item --with-warn-frame-larger-than-extra-text=@var{text}
+Append @samp{@var{text}} to frame size warnings generated by
+the @option{-Wframe-larger-than} warning flag.
@end table
@subheading Cross-Compiler-Specific Options
Index: gcc/final.c
===
--- gcc/final.c (revision 173353)
+++ gcc/final.c (working copy)
@@ -1576,9 +1576,13 @@
if (warn_frame_larger_than
&& get_frame_size () > frame_larger_than_size)
{
- /* Issue a warning */
+ /* Issue a warning. (WARN_FRAME_LARGER_THAN_EXTRA_TEXT is
+ provided by configuration. The way extra text is added
+ here may prevent localization from working properly.
+ It's totally broken.) */
warning (OPT_Wframe_larger_than_,
- "the frame size of %wd bytes is larger than %wd bytes",
+ "the frame size of %wd bytes is larger than %wd bytes"
+ WARN_FRAME_LARGER_THAN_EXTRA_TEXT,
get_frame_size (), frame_larger_than_size);
}
Index: gcc/configure.ac
===
--- gcc/configure.ac(revision 173353)
+++ gcc/configure.ac(working copy)
@@ -4951,6 +4951,20 @@
fi
+warn_frame_larger_than_extra_text=
+AC_ARG_WITH(warn-frame-larger-than-extra-text,
+[ --with-warn-frame-larger-than-extra-text=TEXT
+ specifies extra text for frame size warnings],
+[case "${withval}" in
+yes) AC_MSG_ERROR(bad value ${withval} given for frame size warning text) ;;
+no);;
+*) warn_frame_larger_than_extra_text="$withval" ;;
+esac])
+AC_DEFINE_UNQUOTED(WARN_FRAME_LARGER_THAN_EXTRA_TEXT,
+ "$warn_frame_larger_than_extra_text",
+ [Define to be extra text for frame size warnings.])
+
+
# Configure the subdirectories
# AC_CONFIG_SUBDIRS($subdirs)
--
This patch is available for review at http://codereview.appspot.com/4479046
Re: [google][RFA] add extra text to stack frame warnings (issue4479046)
On Thu, May 5, 2011 at 12:19, Andrew Pinski wrote: > Is there a reason why this cannot be an option that someone passes on > the command line of GCC instead of a configure option? I don't think we ever considered that approach. That's actually a great idea, I think better for our purposes than a configuration option. (Previously, it didn't much matter, since in our tree this was a small local patch directly to final.c.) Thank you, I'm going to do over taking the approach you suggested. > Also can you > show an example of why this message would be changed? We use the stack frame size warning on some of our internal code. (Obvious, I guess -- otherwise, why would I be messing with it. 8-) In summary, -Wframe-larger-than does not always produce obvious results. 8-) There are common questions, e.g.: * why we care about this warning at all (i.e., "why does stack frame size matter?!"). * how to identify the cause of the warning (since it's not necessarily obvious what's causing stack growth, and because the warning is somewhat ... finicky thanks to inlining and thanks to sometimes-less-than-great reuse of stack space from dead variables in optimized and especially unoptimized code). * how to work around, or if absolutely necessary disable the warning. So, to help, when we output the frame-size warning, we also provide a link to an internal documentation page to help with the stuff mentioned above. Of necessity, the doc link we provide explains our internal circumstances and workarounds. (Generic documentation wouldn't help with a number of the questions.) In theory, a more general warning-text-addition mechanism could be useful. e.g. a flag that said "when outputting a warning about flag 'foo', output this additional text" could be useful. However, we haven't felt the need to do this for other warnings. IMO, a general solution along these lines would be solving a problem that ~nobody has. 8-) If one wanted to dive into warning message changes, there are other, more substantial changes IMO that would be generally useful and would enable this type of functionality via external tools. E.g., structured warnings with fixed identifiers (numbers, words, whatever), blah blah blah. If there were support for *that*, then people could write wrapper tools that automatically annotate warnings with additional information as necessary. (it would also make parsing errors/warnings a lot easier. 8-) Anyway, thanks for the suggestion. 8-) chris
[PATCH RFA]: clean up -fdiagnostics-show-option and -Werror= docs slightly
-fdiagnostics-show-option is now the default, and it looks like
neither its description or the -Werror= description was updated to
match.
This patch attempts to do that.
tested by building, running nroff -man gcc.1, and reading the result.
OK for trunk (i.e., pre-4.6)?
(I feel a tiny bit bad about not attempting to address the issues
raised in PRs 40989 and 48088 where -Werror= and -Wno-error= don't
work as expected... but I'm at a loss for a concise set of words to
describe the problems and it's late. 8-S)
chris
---
2011-03-12 Chris Demetriou
* doc/invoke.texi (-fdiagnostics-show-option): Replace with...
(-fno-diagnostics-show-option): this, to reflect current default.
(-Werror=): Update text about -fno-diagnostics-show-option.
[gcc/ChangeLog]
2011-03-12 Chris Demetriou
* doc/invoke.texi (-fdiagnostics-show-option): Replace with...
(-fno-diagnostics-show-option): this, to reflect current default.
(-Werror=): Update text about -fno-diagnostics-show-option.
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 170864)
+++ doc/invoke.texi (working copy)
@@ -227,7 +227,7 @@
@xref{Language Independent Options,,Options to Control Diagnostic Messages Formatting}.
@gccoptlist{-fmessage-length=@var{n} @gol
-fdiagnostics-show-location=@r{[}once@r{|}every-line@r{]} @gol
--fdiagnostics-show-option}
+-fno-diagnostics-show-option}
@item Warning Options
@xref{Warning Options,,Options to Request or Suppress Warnings}.
@@ -2771,12 +2771,13 @@
prefix) for physical lines that result from the process of breaking
a message which is too long to fit on a single line.
-@item -fdiagnostics-show-option
+@item -fno-diagnostics-show-option
+@opindex fno-diagnostics-show-option
@opindex fdiagnostics-show-option
-This option instructs the diagnostic machinery to add text to each
-diagnostic emitted, which indicates which command line option directly
-controls that diagnostic, when such an option is known to the
-diagnostic machinery.
+By default, each diagnostic emitted includes text which indicates the
+command line option that directly controls the diagnostic (if such an
+option is known to the diagnostic machinery). Specifying the
+@option{-fno-diagnostics-show-option} flag suppresses that behavior.
@item -Wcoverage-mismatch
@opindex Wcoverage-mismatch
@@ -2842,10 +2843,14 @@
negative form, to be used to negate @option{-Werror} for specific
warnings, for example @option{-Wno-error=switch} makes
@option{-Wswitch} warnings not be errors, even when @option{-Werror}
-is in effect. You can use the @option{-fdiagnostics-show-option}
-option to have each controllable warning amended with the option which
-controls it, to determine what to use with this option.
+is in effect.
+The warning message for each controllable warning includes the
+option which controls the warning. That option can then be used with
+@option{-Werror=} and @option{-Wno-error=} as described above.
+(Printing of the option in the warning message can be disabled using the
+@option{-fno-diagnostics-show-option} flag.)
+
Note that specifying @option{-Werror=}@var{foo} automatically implies
@option{-W}@var{foo}. However, @option{-Wno-error=}@var{foo} does not
imply anything.
