[PATCH] Leverage sysroot for VxWorks

[PATCH] Leverage sysroot for VxWorks

[Olivier Hainque]

[-- Attachment #1: Type: text/plain, Size: 2953 bytes --]

Hello,

The build of a VxWorks toolchain relies a lot on system headers
and VxWorks has a few very specific features that require special
processing. For example, different sets of headers for the kernel
vs the rtp modes, which the compiler knows about by way of -mrtp
on the command line.

If we manage to avoid the need for fixincludes on recent versions
of VxWorks (>= 7), we still need to handle at least VxWorks 6.9 at
this stage.

We sort of get away with locating the correct headers at
run-time thanks to environment variables and various tests for
-mrtp in cpp specs, but getting fixincludes to work for old
configurations has always been tricky and getting a toolchain
to build with c++/libstdc++ support gets trickier with every
move to a more recent release.

sysroot_headers_suffix_spec is a pretty powerful device to help
address such issues, and this patch introduces changes that let
us get advantage of it.

The general idea is to leverage the assumption that compilations
occur with --sysroot=$VSB_DIR on vx7 or --sysroot=$WIND_BASE/target
prior to that.

For the toolchains we build, this is achieved with a few
configure options like:

  --with-sysroot
  --with-build-sysroot=${WIND_BASE}/target
  --with-specs=%{!sysroot=*:--sysroot=%:getenv(WIND_BASE /target)}

This allows simplifying the libgcc compilation flags control
and we take the opportunity to merge t-vxworks7 into t-vxworks as
the two files were differing only on the libgcc2 flags part.

This also makes sure that we don't set inhibit_libc true
during the build, which is sane since the build really relies a
lot on system headers.

Preliminary tests showed that a build from mainline sources
now actually fail if the build configuration somehow gets to
inhibit_libc=true.

I have had very good results with this for several ports on a
gcc-11 branch, for both VxWorks 6.9 and two variants of 7.2,
for both kernel and rtp modes, including with shared libs on
two targets for 7.2 (powerpc64 and x86_64).

I was also able to get a successful build of c, c++
and libstdc++ on mainline for VxWorks 6.9, with a few fixincludes
adjustments as expected.

This touches only VxWorks related items and, all in all, I
believe this robustifies the family of ports and helps avoid
build failure with mainline sources so remains applicable
to the current stage. 

Olivier

---

2021-12-09  Olivier Hainque  <hainque@adacore.com>

gcc/
        * config/t-vxworks: Clear NATIVE_SYSTEM_HEADER_DIR.
        * config/vxworks.h (SYSROOT_HEADERS_SUFFIX_SPEC): Define, for
        VxWorks 7 and earlier.
        (VXWORKS_ADDITIONAL_CPP_SPEC): Simplify accordingly.
        (STARTFILE_PREFIX_SPEC): Adjust accordingly.
        * config/rs6000/vxworks.h (STARTFILE_PREFIX_SPEC): Adjust.

libgcc/
        * config/t-vxworks (LIBGCC2_INCLUDES): Simplify and handle
        both VxWorks7 and earlier.
        * config/t-vxworks7: Remove.
        * config.host: Remove special case for vxworks7.


[-- Attachment #2: 0001-Leverage-sysroot-for-VxWorks.patch --]
[-- Type: application/octet-stream, Size: 10888 bytes --]

From 5976a53ec96c7835504adb0808a31888440ee6a2 Mon Sep 17 00:00:00 2001
From: Olivier Hainque <hainque@adacore.com>
Date: Fri, 26 Nov 2021 21:37:46 +0000
Subject: [PATCH] Leverage sysroot for VxWorks

The build of a VxWorks toolchain relies a lot on system headers
and VxWorks has a few very specific features that require special
processing. For example, different sets of headers for the kernel
vs the rtp modes, which the compiler knows about by way of -mrtp
on the command line.

If we manage to avoid the need for fixincludes on recent versions
of VxWorks (>= 7), we still need to handle at least VxWorks 6.9 at
this stage.

We sort of get away with locating the correct headers at
run-time thanks to environment variables and various tests for
-mrtp in cpp specs, but getting fixincludes to work for old
configurations has always been tricky and getting a toolchain
to build with c++/libstdc++ support gets trickier with every
move to a more recent release.

sysroot_headers_suffix_spec is a pretty powerful device to help
address such issues, and this patch introduces changes that let
us get advantage of it.

The general idea is to leverage the assumption that compilations
occur with --sysroot=$VSB_DIR on vx7 or --sysroot=$WIND_BASE/target
prior to that.

For the toolchains we build, this is achieved with a few
configure options like:

  --with-sysroot
  --with-build-sysroot=${WIND_BASE}/target
  --with-specs=%{!sysroot=*:--sysroot=%:getenv(WIND_BASE /target)}

This also allows simplifying the libgcc compilation flags control
and we take the opportunity to merge t-vxworks7 into t-vxworks as
the two files were differing only on the libgcc2 flags part.

2021-12-09  Olivier Hainque  <hainque@adacore.com>

gcc/
	* config/t-vxworks: Clear NATIVE_SYSTEM_HEADER_DIR.
	* config/vxworks.h (SYSROOT_HEADERS_SUFFIX_SPEC): Define, for
	VxWorks 7 and earlier.
	(VXWORKS_ADDITIONAL_CPP_SPEC): Simplify accordingly.
	(STARTFILE_PREFIX_SPEC): Adjust accordingly.
	* config/rs6000/vxworks.h (STARTFILE_PREFIX_SPEC): Adjust.

libgcc/
	* config/t-vxworks (LIBGCC2_INCLUDES): Simplify and handle
	both VxWorks7 and earlier.
	* config/t-vxworks7: Remove.
	* config.host: Remove special case for vxworks7.
---
 gcc/config/rs6000/vxworks.h |  2 +-
 gcc/config/t-vxworks        |  8 ++++
 gcc/config/vxworks.h        | 74 ++++++++++++++++++++++++-------------
 libgcc/config.host          |  3 --
 libgcc/config/t-vxworks     |  7 ++--
 libgcc/config/t-vxworks7    | 22 -----------
 6 files changed, 61 insertions(+), 55 deletions(-)
 delete mode 100644 libgcc/config/t-vxworks7

diff --git a/gcc/config/rs6000/vxworks.h b/gcc/config/rs6000/vxworks.h
index ca21a3a7bd3..fbe09027163 100644
--- a/gcc/config/rs6000/vxworks.h
+++ b/gcc/config/rs6000/vxworks.h
@@ -206,7 +206,7 @@ along with GCC; see the file COPYING3.  If not see
 
 #undef  STARTFILE_PREFIX_SPEC
 #define STARTFILE_PREFIX_SPEC						\
- "%{mrtp:%{!shared:%:getenv(WIND_BASE /target/lib/usr/lib/ppc/PPC32/common)}}"
+ "%{mrtp:%{!shared:/lib/usr/lib/ppc/PPC32/common}}"
 
 /* For aggregates passing, use the same, consistent ABI as Linux.  */
 #define AGGREGATE_PADDING_FIXED 0
diff --git a/gcc/config/t-vxworks b/gcc/config/t-vxworks
index 0175a5f8ce5..689de141644 100644
--- a/gcc/config/t-vxworks
+++ b/gcc/config/t-vxworks
@@ -24,6 +24,14 @@ vxworks-c.o: $(srcdir)/config/vxworks-c.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
 
+# We leverage $sysroot to find target system headers only, distributed
+# in a VxWorks (a)typical fashion with a different set of headers for
+# rtp vs kernel mode.  We setup SYSROOT_HEADERS_SUFFIX_SPEC to handle
+# this, and need to clear NATIVE_SYSTEM_HEADER_DIR to prevent it from
+# interfering.
+
+NATIVE_SYSTEM_HEADER_DIR =
+
 # Both the kernel and RTP headers provide limits.h.  They embed VxWorks
 # specificities and are dated on some configurations so we both need to
 # provide our own version and make sure the system one gets exposed.
diff --git a/gcc/config/vxworks.h b/gcc/config/vxworks.h
index 8210de467dd..a89e3d68c62 100644
--- a/gcc/config/vxworks.h
+++ b/gcc/config/vxworks.h
@@ -28,45 +28,70 @@ along with GCC; see the file COPYING3.  If not see
    like a traditional Unix, with more external files.  Most of our specs
    must be aware of the difference.  */
 
-/* We look for the VxWorks header files using the environment
-   variables that are set in VxWorks to indicate the location of the
-   system header files.  We use -idirafter so that the GCC's own
-   header-file directories (containing <stddef.h>, etc.) come before
-   the VxWorks system header directories.  */
+/* Help locate system headers, assuming $sysroot set to $VSB_DIR on vx7 and
+   $WIND_BASE/target prior to that.  Specs allow tailoring for RTP vs kernel,
+   and -idirafter allows putting system directories after GCC's own directories
+   for standard headers such as <stddef.h> or fixed include.
+
+   Regarding fixed includes, note the effect of sysroot_headers_suffix_spec:
+
+   For the case of VxWorks prior to 7 below, we have:
+
+     #define SYSROOT_HEADERS_SUFFIX_SPEC "%{mrtp:/usr/h;:/h}"
+
+   This results in
+
+     $build_sysroot/h     ---> $prefix/include-fixed
+     $build_sysroot/usr/h ---> $prefix/include-fixed/mrtp for -mrtp
+
+   This is very different from what we'd get without a headers_suffix,
+   which would be:
+
+     $build_sysroot     ---> $prefix/include-fixed/h
+                                                  /usr/h
+
+   From (say) #include <assert.h>, we would find the fixed version
+   in the first case, not in the second.  */
 
 /* Since we provide a default -isystem, expand -isystem on the command
-   line early.  */
+   line early.  Then restrict the amount of references we add when compiling
+   self-tests, as these may be run in contexts where the VxWorks environment
+   isn't available.  */
 
-/* Self-tests may be run in contexts where the VxWorks environment isn't
-   available.  Prevent attempts at designating the location of runtime header
-   files, libraries or startfiles, which would fail on unset environment
-   variables and aren't needed for such tests.  */
 #if TARGET_VXWORKS7
 
+/* We arrange not rely on fixed includes for vx7 and the headers spread over
+   common kernel/rtp directories in addition to specific ones for each mode.
+   Setup sysroot_headers_suffix_spec to deal with kernel/rtp distinction.  */
+
+#undef SYSROOT_HEADERS_SUFFIX_SPEC
+#define SYSROOT_HEADERS_SUFFIX_SPEC "%{mrtp:/usr/h;:/krnl/h}"
+
 #undef VXWORKS_ADDITIONAL_CPP_SPEC
 #define VXWORKS_ADDITIONAL_CPP_SPEC                     \
  "%{!nostdinc:%{!fself-test=*:                          \
     %{isystem*}                                         \
-    %{mrtp: -idirafter %:getenv(VSB_DIR /h)             \
-            -idirafter %:getenv(VSB_DIR /share/h)       \
-            -idirafter %:getenv(VSB_DIR /usr/h/public)  \
-            -idirafter %:getenv(VSB_DIR /usr/h)         \
-      ;:    -idirafter %:getenv(VSB_DIR /h)             \
-            -idirafter %:getenv(VSB_DIR /share/h)       \
-            -idirafter %:getenv(VSB_DIR /krnl/h/system) \
-            -idirafter %:getenv(VSB_DIR /krnl/h/public)}}}"
+    -idirafter %:getenv(VSB_DIR /h)  \
+    -idirafter %:getenv(VSB_DIR /share/h)  \
+    -idirafter =/system \
+    -idirafter =/public \
+  }}"
 
 #else /* TARGET_VXWORKS7 */
 
+/* Prior to vx7, rtp and kernel headers are fairly segregated and fixincludes
+   is needed on each set of headers to cope with expectations of not so old
+   libstdc++.  A perfect use case for sysroot_headers_suffix.  */
+
+#undef SYSROOT_HEADERS_SUFFIX_SPEC
+#define SYSROOT_HEADERS_SUFFIX_SPEC "%{mrtp:/usr/h;:/h}"
+
 #undef VXWORKS_ADDITIONAL_CPP_SPEC
 #define VXWORKS_ADDITIONAL_CPP_SPEC		\
  "%{!nostdinc:%{!fself-test=*:			\
     %{isystem*}					\
-    %{mrtp: -idirafter %:getenv(WIND_USR /h)	\
-	    -idirafter %:getenv(WIND_USR /h/wrn/coreip) \
-      ;:    -idirafter %:getenv(WIND_BASE /target/h) \
-	    -idirafter %:getenv(WIND_BASE /target/h/wrn/coreip) \
-}}}"
+    -idirafter =/wrn/coreip \
+  }}"
 
 #endif
 
@@ -119,8 +144,7 @@ along with GCC; see the file COPYING3.  If not see
 
 #if TARGET_VXWORKS7
 #undef  STARTFILE_PREFIX_SPEC
-#define STARTFILE_PREFIX_SPEC \
-  "%{!fself-test=*:%:getenv(VSB_DIR /usr/lib/common)}"
+#define STARTFILE_PREFIX_SPEC "/usr/lib/common"
 #define TLS_SYM "-u __tls__"
 #else
 #define TLS_SYM ""
diff --git a/libgcc/config.host b/libgcc/config.host
index bd44f1bbf42..1c1b60cac0b 100644
--- a/libgcc/config.host
+++ b/libgcc/config.host
@@ -325,9 +325,6 @@ case ${host} in
   tmake_file="vms/t-vms"
   extra_parts="crt0.o crtbegin.o crtbeginS.o crtend.o crtendS.o"
   ;;
-*-*-vxworks7*)
-  tmake_file=t-vxworks7
-  ;;
 *-*-vxworksae*)
   tmake_file=t-vxworksae
   ;;
diff --git a/libgcc/config/t-vxworks b/libgcc/config/t-vxworks
index 5f7ced8b561..aa794744976 100644
--- a/libgcc/config/t-vxworks
+++ b/libgcc/config/t-vxworks
@@ -13,9 +13,8 @@ LIB2FUNCS_EXCLUDE += _clear_cache
 LIBGCC2_INCLUDES = -nostdinc -I. \
   -I$(MULTIBUILDTOP)../../gcc/include-fixed$(MULTISUBDIR) \
   -I$(MULTIBUILDTOP)../../gcc/include \
-  `case "/$(MULTIDIR)" in \
-     */mrtp*) echo -I$(WIND_USR)/h -I$(WIND_USR)/h/wrn/coreip ;; \
-     *) echo -I$(WIND_BASE)/target/h -I$(WIND_BASE)/target/h/wrn/coreip ;; \
-   esac`
+  $(if $(findstring vxworks7, $(target_noncanonical)), \
+    -I$(VSB_DIR)/h -I$(VSB_DIR)/share/h -I=/system -I=/public, \
+    -I=/ -I=/wrn/coreip)
 
 CRTSTUFF_T_CFLAGS = $(LIBGCC2_INCLUDES)
diff --git a/libgcc/config/t-vxworks7 b/libgcc/config/t-vxworks7
deleted file mode 100644
index 180784bf3a1..00000000000
--- a/libgcc/config/t-vxworks7
+++ /dev/null
@@ -1,22 +0,0 @@
-# Don't build libgcc.a with debug info
-LIBGCC2_DEBUG_CFLAGS =
-
-# We provide our own implementation for __clear_cache, using a
-# VxWorks specific entry point.
-LIB2FUNCS_EXCLUDE += _clear_cache
-
-# This ensures that the correct target headers are used; some VxWorks
-# system headers have names that collide with GCC's internal (host)
-# headers, e.g. regs.h. Make sure the local libgcc headers still
-# prevail (e.g. unwind.h), and that gcc provided header files intended
-# to be user visible eventually are visible as well.
-LIBGCC2_INCLUDES = -nostdinc -I. \
-  -I$(MULTIBUILDTOP)../../gcc/include-fixed$(MULTISUBDIR) \
-  -I$(VSB_DIR)/h -I$(VSB_DIR)/share/h \
-  -I$(MULTIBUILDTOP)../../gcc/include \
-  `case "/$(MULTIDIR)" in \
-      */mrtp*) echo -I$(VSB_DIR)/usr/h/public -I$(VSB_DIR)/usr/h ;; \
-      *) echo -I$(VSB_DIR)/krnl/h/system -I$(VSB_DIR)/krnl/h/public ;; \
-   esac`
-
-CRTSTUFF_T_CFLAGS = $(LIBGCC2_INCLUDES)
-- 
2.25.1


[-- Attachment #3: Type: text/plain, Size: 3 bytes --]

Re: [PATCH] Leverage sysroot for VxWorks

[Rasmus Villemoes]

On 10/12/2021 19.24, Olivier Hainque wrote:

> For the toolchains we build, this is achieved with a few
> configure options like:
> 
>   --with-sysroot
>   --with-build-sysroot=${WIND_BASE}/target

So forward-porting our private patches up until

7bf710b5116 - libstdc++: Add support for '?' in linker script globs

(i.e. the commit before this one) went without problems, with no changes
required in our build scripts. Then when rebasing to this one, now known as

f3f923e5139 - Leverage sysroot for VxWorks

the build broke as expected because I'd been using somewhat different
values of --with(-build)-sysroot. However, changing those configure
options as indicated above again produced a working toolchain, so this
is all good.

>   --with-specs=%{!sysroot=*:--sysroot=%:getenv(WIND_BASE /target)}

However, I'm a little confused about the purpose of this one. First,
shouldn't this be '%{!-sysroot=*....}', i.e. with a leading dash, since
the option is --sysroot ? But whether I use one or the other, it seems
that the resulting compiler ignores a --sysroot argument; if I
explicitly unexport WIND_BASE and manually add a --sysroot argument that
should have the same effect as above, gcc fails with WIND_BASE not defined:

$ echo $WIND_BASE
/usr/powerpc-wrs-vxworks/wind_base
$ export -n WIND_BASE
$ powerpc-wrs-vxworks-gcc --sysroot=${WIND_BASE}/target -v -E -  < /dev/null
Using built-in specs.
powerpc-wrs-vxworks-gcc: fatal error: environment variable 'WIND_BASE'
not defined
compilation terminated.

The specs syntax doesn't explicitly list the negative form of %{S*:X},
and I can't find any in-tree examples (though it's rather hard to grep
for), so I don't even know if this is supposed to work.

It's not a big deal, we can just continue to define and export
WIND_BASE, but it's probably best for long-term maintenance if our build
scripts and configure options are aligned with what you do on your end,
so I would just like to understand this fully.

Rasmus

Re: [PATCH] Leverage sysroot for VxWorks

[Olivier Hainque]

Hi Rasmus,

> On 20 Dec 2021, at 12:40, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote:
> 
> On 10/12/2021 19.24, Olivier Hainque wrote:
> 
>> For the toolchains we build, this is achieved with a few
>> configure options like:
>> 
>>   --with-sysroot
>>   --with-build-sysroot=${WIND_BASE}/target
> 
> So forward-porting our private patches up until
> 
> 7bf710b5116 - libstdc++: Add support for '?' in linker script globs
> 
> (i.e. the commit before this one) went without problems, with no changes
> required in our build scripts. Then when rebasing to this one, now known as
> 
> f3f923e5139 - Leverage sysroot for VxWorks
> 
> the build broke as expected because I'd been using somewhat different
> values of --with(-build)-sysroot. However, changing those configure
> options as indicated above again produced a working toolchain, so this
> is all good.

Great !

>>  --with-specs=%{!sysroot=*:--sysroot=%:getenv(WIND_BASE /target)}
> 
> However, I'm a little confused about the purpose of this one.

> First,
> shouldn't this be '%{!-sysroot=*....}', i.e. with a leading dash, since
> the option is --sysroot ? But whether I use one or the other, it seems
> that the resulting compiler ignores a --sysroot argument; if I
> explicitly unexport WIND_BASE and manually add a --sysroot argument that
> should have the same effect as above, gcc fails with WIND_BASE not defined:

> The specs syntax doesn't explicitly list the negative form of %{S*:X},
> and I can't find any in-tree examples (though it's rather hard to grep
> for), so I don't even know if this is supposed to work.

I think it is, and it is used for example in:

  config/i386/i386.h:  {"cpu", "%{!mtune=*:%{!mcpu=*:%{!march=*:-mtune=%(VALUE)}}}" },  \

> It's not a big deal, we can just continue to define and export
> WIND_BASE, but it's probably best for long-term maintenance if our build
> scripts and configure options are aligned with what you do on your end,
> so I would just like to understand this fully.

Sure, appreciated.

The purpose was indeed to also prevent producing the default option
if one is there already, and you are right that the form above doesn't
work as expected on this account.

I had made a couple of basic tests and thought it did, but must have
made a mistake. Might have been skewed by an implicit conviction.

The reason for which it doesn't work is the do_save = false in
driver_handle_option for --sysroot:

   case OPT__sysroot_:
      target_system_root = arg;
      target_system_root_changed = 1;
      do_save = false;
      break;


Changing that has the intended effect on our use case,
with an additional dash indeed.

I'll test further and propose the change if it doesn't raise
unexpected problems. We could also even consider using DRIVER_SELF_SPECS
then.

Thanks for your feedback,

Olivier