gcc/gcc.c: add outfiles spec

Message ID 20200403233714.9971-1-rv@rasmusvillemoes.dk
State New
Headers show
Series
  • gcc/gcc.c: add outfiles spec
Related show

Commit Message

Rasmus Villemoes April 3, 2020, 11:37 p.m.
Commit 0b7fb27b698d (Fix and document -r option) broke building of
VxWorks modules, at least for our version of VxWorks. A VxWorks module
is a relocatable ELF file - the final link is done when the file is
linked in to the running kernel. So building with -r is
necessary. However, the module also needs to be wrapped in
crtbegin.o/crtend.o in order to have the exception frame info
registered with the run-time.

We used to achieve that with a custom spec file that specified
crtbegin.o and crtend.o as start/endfile, but after the above commit,
%S and %E no longer get substituted.

In order to allow custom spec files to add arbitrary objects at the
beginning/end, add a level of indirection that puts "%o" in a new
outfiles spec - then the custom spec file can override that with the

%rename outfiles old_outfiles

*outfiles:
  ...

idiom.

Of course, the custom spec file could also override all of the
link_command, but that's a lot of logic to copy-paste and keep in
sync.

gcc/ChangeLog:

2020-04-04  Rasmus Villemoes  <rv@rasmusvillemoes.dk>

	* gcc.c (OUTFILES_SPEC): New spec, hook for wrapping or
          overriding %o.
	
---
I'm not quite up to speed on how the gcc repo works after the switch
to git. Please advise.

 gcc/gcc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

-- 
2.23.0

Comments

Joseph Myers April 6, 2020, 9:18 p.m. | #1
On Sat, 4 Apr 2020, Rasmus Villemoes wrote:

> +#ifndef OUTFILES_SPEC

> +#define OUTFILES_SPEC "%o"

> +#endif


A new target macro needs to be documented in tm.texi.in, with tm.texi then 
being regenerated.

-- 
Joseph S. Myers
joseph@codesourcery.com
Rasmus Villemoes April 14, 2020, 12:32 p.m. | #2
On 06/04/2020 23.18, Joseph Myers wrote:
> On Sat, 4 Apr 2020, Rasmus Villemoes wrote:

> 

>> +#ifndef OUTFILES_SPEC

>> +#define OUTFILES_SPEC "%o"

>> +#endif

> 

> A new target macro needs to be documented in tm.texi.in,


Will do.

> with tm.texi then being regenerated.


Please include information on how I would go about doing that, or point
at a README/wiki that explains it. Is it just copying the generated
tm.texi from the build/gcc/ dir back to the src/gcc/doc/ dir?

Also, does my write-after-approval permission still apply after ~1.5
years of non-activity, and the switch to git? If so, do I just make sure
my changes (after approval, of course) apply cleanly on top of master
and then push that?

Thanks,
Rasmus
Joseph Myers April 14, 2020, 11:57 p.m. | #3
On Tue, 14 Apr 2020, Rasmus Villemoes wrote:

> On 06/04/2020 23.18, Joseph Myers wrote:

> > On Sat, 4 Apr 2020, Rasmus Villemoes wrote:

> > 

> >> +#ifndef OUTFILES_SPEC

> >> +#define OUTFILES_SPEC "%o"

> >> +#endif

> > 

> > A new target macro needs to be documented in tm.texi.in,

> 

> Will do.

> 

> > with tm.texi then being regenerated.

> 

> Please include information on how I would go about doing that, or point

> at a README/wiki that explains it. Is it just copying the generated

> tm.texi from the build/gcc/ dir back to the src/gcc/doc/ dir?


Yes, copy from the build directory to the source directory.

> Also, does my write-after-approval permission still apply after ~1.5

> years of non-activity, and the switch to git? If so, do I just make sure


It's not time-limited.  (However, when someone's employer changes they may 
need to get a new employer assignment / disclaimer before continuing to 
contribute changes that are significant for copyright purposes.)

> my changes (after approval, of course) apply cleanly on top of master

> and then push that?


Yes, once changes are approved you should push them.

-- 
Joseph S. Myers
joseph@codesourcery.com
Olivier Hainque April 15, 2020, 8:52 a.m. | #4
Hi Rasmus,

Thanks for your proposal.

This is an issue we discussed a bit very recently
https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543073.html

and it's nice to see a proposal not requiring
the replication of all the link spec logic.


I had mixed feelings about a change to get gcc -r behave
differently on VxWorks, but on second thoughts ISTM there
is sufficient variability in the linkage possibilities on
that OS to justify some differences. 

As a side question to your proposal, have you considered
the alternative which consists in using gcc -Wl,-r instead
of gcc -r to build your modules ?

It would be interesting to know if that somehow wouldn't
work for you.

Regards,

Olivier
Rasmus Villemoes April 15, 2020, 9:41 a.m. | #5
On 15/04/2020 10.52, Olivier Hainque wrote:
> Hi Rasmus,

> 

> Thanks for your proposal.

> 

> This is an issue we discussed a bit very recently

> https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543073.html

> 

> and it's nice to see a proposal not requiring

> the replication of all the link spec logic.

> 

> 

> I had mixed feelings about a change to get gcc -r behave

> differently on VxWorks, but on second thoughts ISTM there

> is sufficient variability in the linkage possibilities on

> that OS to justify some differences. 

> 

> As a side question to your proposal, have you considered

> the alternative which consists in using gcc -Wl,-r instead

> of gcc -r to build your modules ?

> 

> It would be interesting to know if that somehow wouldn't

> work for you.


I'll have to try it, but it sounds like it might work. We still need to
use a custom spec file to specify crtbegin.o/crtend.o as
startfile/endfile, so in that sense it was a bit easier to just also
include the extra logic for sandwiching the objects in that spec file as
well. As you also mention in the thread above, we really want to avoid
duplicating all of LINK_COMMAND_SPEC. Also, using the linker directly is
quite cumbersome, as one would first have to ask gcc via 'gcc
-print-file-name=crtbegin.o' where that file is - it's much neater when
the startfile spec just does its magic.


On a somewhat related note, we're currently carrying this locally in
order to build (link) the vxworks kernel image itself using gcc:

diff --git a/gcc/config/vxworks.h b/gcc/config/vxworks.h
index 0f604f1bcc4..06b4e2e6870 100644
--- a/gcc/config/vxworks.h
+++ b/gcc/config/vxworks.h
@@ -112,7 +112,7 @@ along with GCC; see the file COPYING3.  If not see
    will treat it as an unrecognized option.  */
 #undef VXWORKS_LINK_SPEC
 #define VXWORKS_LINK_SPEC                              \
-"%{!mrtp:-r}                                           \
+"                                                      \
  %{!shared:                                            \
    %{mrtp:-q %{h*}                                     \
           %{R*} %{!T*: %(link_start) }                 \

and then module builds must pass -r [or perhaps -Wl,-r depending on how
the above gets solved] explicitly. From the link above, it sounds like
you are already passing -r explicitly and not relying on lack of -mrtp
implying it, so would the above be acceptable for upstream (with proper
changelog and whitespace cleanup of course)?

Rasmus
Olivier Hainque April 17, 2020, 7:51 a.m. | #6
> On 15 Apr 2020, at 11:41, Rasmus Villemoes <rv@rasmusvillemoes.dk> wrote:

> On a somewhat related note, we're currently carrying this locally in

> order to build (link) the vxworks kernel image itself using gcc:

> 

> diff --git a/gcc/config/vxworks.h b/gcc/config/vxworks.h

> index 0f604f1bcc4..06b4e2e6870 100644

> --- a/gcc/config/vxworks.h

> +++ b/gcc/config/vxworks.h

> @@ -112,7 +112,7 @@ along with GCC; see the file COPYING3.  If not see

>    will treat it as an unrecognized option.  */

> #undef VXWORKS_LINK_SPEC

> #define VXWORKS_LINK_SPEC                              \

> -"%{!mrtp:-r}                                           \

> +"                                                      \

>  %{!shared:                                            \

>    %{mrtp:-q %{h*}                                     \

>           %{R*} %{!T*: %(link_start) }                 \

> 

> and then module builds must pass -r [or perhaps -Wl,-r depending on how

> the above gets solved] explicitly. From the link above, it sounds like

> you are already passing -r explicitly and not relying on lack of -mrtp

> implying it, so would the above be acceptable for upstream (with proper

> changelog and whitespace cleanup of course)?


This is a sensitive area and, while I understand the rationale,
I think we'd first need to experiment quite a bit in-house to go
with such a change.

As I mentioned in the thread referenced a couple of messages up,
the issue is not so much our own uses of the toolchain but assumptions
by possible invocations through IDE and Makefiles part of the
VxWorks environment.

I'll discuss with the team here.

Olivier

Patch

diff --git a/gcc/gcc.c b/gcc/gcc.c
index 9f790db0daf..7ea7e2a3ae0 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -804,6 +804,10 @@  proper position among the other output files.  */
 #define ENDFILE_SPEC ""
 #endif
 
+#ifndef OUTFILES_SPEC
+#define OUTFILES_SPEC "%o"
+#endif
+
 #ifndef LINKER_NAME
 #define LINKER_NAME "collect2"
 #endif
@@ -1042,7 +1046,7 @@  proper position among the other output files.  */
    "%X %{o*} %{e*} %{N} %{n} %{r}\
     %{s} %{t} %{u*} %{z} %{Z} %{!nostdlib:%{!r:%{!nostartfiles:%S}}} \
     %{static|no-pie|static-pie:} %@{L*} %(mfwrap) %(link_libgcc) " \
-    VTABLE_VERIFICATION_SPEC " " SANITIZER_EARLY_SPEC " %o "" \
+    VTABLE_VERIFICATION_SPEC " " SANITIZER_EARLY_SPEC " %(outfiles) "" \
     %{fopenacc|fopenmp|%:gt(%{ftree-parallelize-loops=*:%*} 1):\
 	%:include(libgomp.spec)%(link_gomp)}\
     %{fgnu-tm:%:include(libitm.spec)%(link_itm)}\
@@ -1087,6 +1091,7 @@  static const char *link_gomp_spec = "";
 static const char *libgcc_spec = LIBGCC_SPEC;
 static const char *endfile_spec = ENDFILE_SPEC;
 static const char *startfile_spec = STARTFILE_SPEC;
+static const char *outfiles_spec = OUTFILES_SPEC;
 static const char *linker_name_spec = LINKER_NAME;
 static const char *linker_plugin_file_spec = "";
 static const char *lto_wrapper_spec = "";
@@ -1596,6 +1601,7 @@  static struct spec_list static_specs[] =
   INIT_STATIC_SPEC ("linker_plugin_file",	&linker_plugin_file_spec),
   INIT_STATIC_SPEC ("lto_wrapper",		&lto_wrapper_spec),
   INIT_STATIC_SPEC ("lto_gcc",			&lto_gcc_spec),
+  INIT_STATIC_SPEC ("outfiles",			&outfiles_spec),
   INIT_STATIC_SPEC ("post_link",		&post_link_spec),
   INIT_STATIC_SPEC ("link_libgcc",		&link_libgcc_spec),
   INIT_STATIC_SPEC ("md_exec_prefix",		&md_exec_prefix),