| Message ID | 20200817080834.357678-1-ibuclaw@gdcproject.org |
|---|---|
| State | New |
| Headers | show |
| Series | vxworks: Fix GCC selftests for *-wrs-vxworks7-* targets | expand |
Hello Iain, > On 17 Aug 2020, at 10:08, Iain Buclaw <ibuclaw@gdcproject.org> wrote: > > Hi, > > Currently when building a cross-compiler targeting arm-wrs-vxworks7, the > selftests fail unless the VSB_DIR environment variable is set. > The same !nostdinc condition is used for VXWORKS_ADDITIONAL_CPP_SPEC. > > OK for mainline? Thanks for proposing this patch. I'd rather remove the self-tests -> -nostdinc thing, apparently introduced explicitly for VxWorks and this dependency on environment variables in specs. Can you please just test for fself-test instead, in both cases, with a comment like "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." ? Thanks in advance, Best Regards, Olivier > Iain. > > --- > gcc/ChangeLog: > > * config/vxworks.h (STARTFILE_PREFIX_SPEC): Avoid using VSB_DIR if > -nostdinc is used. > --- > gcc/config/vxworks.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/config/vxworks.h b/gcc/config/vxworks.h > index d648d2f23cb..065c9e12b88 100644 > --- a/gcc/config/vxworks.h > +++ b/gcc/config/vxworks.h > @@ -108,7 +108,7 @@ along with GCC; see the file COPYING3. If not see > > #if TARGET_VXWORKS7 > #undef STARTFILE_PREFIX_SPEC > -#define STARTFILE_PREFIX_SPEC "%:getenv(VSB_DIR /usr/lib/common)" > +#define STARTFILE_PREFIX_SPEC "%{!nostdinc:%:getenv(VSB_DIR /usr/lib/common)}" > #define TLS_SYM "-u __tls__" > #else > #define TLS_SYM "" > -- > 2.20.1 >
Excerpts from Olivier Hainque's message of August 18, 2020 10:01 am: > Hello Iain, > >> On 17 Aug 2020, at 10:08, Iain Buclaw <ibuclaw@gdcproject.org> wrote: >> >> Hi, >> >> Currently when building a cross-compiler targeting arm-wrs-vxworks7, the >> selftests fail unless the VSB_DIR environment variable is set. > >> The same !nostdinc condition is used for VXWORKS_ADDITIONAL_CPP_SPEC. >> >> OK for mainline? > > Thanks for proposing this patch. > > I'd rather remove the self-tests -> -nostdinc thing, apparently > introduced explicitly for VxWorks and this dependency on environment > variables in specs. > > Can you please just test for fself-test instead, in both cases, with > a comment like > > "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." > Hi Oliver, Attached is the change as per your proposal. Iain. --- gcc/ChangeLog: * Makefile.in (SELFTEST_FLAGS): Remove -nostdinc. * config/vxworks.h (VXWORKS_ADDITIONAL_CPP_SPEC): Replace -nostdinc with -fself-tests. (STARTFILE_PREFIX_SPEC): Avoid using VSB_DIR if -fself-tests is used. --- diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 79e854aa938..a71585239da 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -1995,8 +1995,6 @@ rest.cross: specs # GCC's selftests. # Specify a dummy input file to placate the driver. -# Specify -nostdinc to work around missing WIND_BASE environment variable -# required for *-wrs-vxworks-* targets. # Specify -o /dev/null so the output of -S is discarded. More importantly # It does not try to create a file with the name "null.s" on POSIX and # "nul.s" on Windows. Because on Windows "nul" is a reserved file name. @@ -2005,7 +2003,7 @@ rest.cross: specs # Specify the path to gcc/testsuite/selftests within the srcdir # as an argument to -fself-test. DEVNULL=$(if $(findstring mingw,$(build)),nul,/dev/null) -SELFTEST_FLAGS = -nostdinc $(DEVNULL) -S -o $(DEVNULL) \ +SELFTEST_FLAGS = $(DEVNULL) -S -o $(DEVNULL) \ -fself-test=$(srcdir)/testsuite/selftests # Run the selftests during the build once we have a driver and the frontend, diff --git a/gcc/config/vxworks.h b/gcc/config/vxworks.h index d648d2f23cb..4aa1e320743 100644 --- a/gcc/config/vxworks.h +++ b/gcc/config/vxworks.h @@ -36,11 +36,16 @@ along with GCC; see the file COPYING3. If not see /* Since we provide a default -isystem, expand -isystem on the command line early. */ + +/* 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 #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) \ @@ -55,7 +60,7 @@ along with GCC; see the file COPYING3. If not see #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) \ @@ -108,7 +113,8 @@ along with GCC; see the file COPYING3. If not see #if TARGET_VXWORKS7 #undef STARTFILE_PREFIX_SPEC -#define STARTFILE_PREFIX_SPEC "%:getenv(VSB_DIR /usr/lib/common)" +#define STARTFILE_PREFIX_SPEC \ + "%{!fself-test=*:%:getenv(VSB_DIR /usr/lib/common)}" #define TLS_SYM "-u __tls__" #else #define TLS_SYM ""
Hi Iain, > On 18 Aug 2020, at 13:45, Iain Buclaw <ibuclaw@gdcproject.org> wrote: > > Attached is the change as per your proposal. > > * config/vxworks.h (VXWORKS_ADDITIONAL_CPP_SPEC): Replace -nostdinc > with -fself-tests. > #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) \ > @@ -55,7 +60,7 @@ along with GCC; see the file COPYING3. If not see > > #undef VXWORKS_ADDITIONAL_CPP_SPEC > #define VXWORKS_ADDITIONAL_CPP_SPEC \ > - "%{!nostdinc: \ > + "%{!fself-test=*: \ Thanks for the updated proposal. Sorry, I have been unclear: If I'm reading the spec of -nostdinc correctly, I think we should still prevent those CPP switches from being added if the option is provided. Can you please amend just this part to prevent the addition of the following switches if either -nostdinc or -fself-test is provided ? Ok, for me with this change, assuming -nostdinc in SELFTEST_FLAGS didn't have other uses than the one documented in the attached comment (I'm not familiar enough with the self-tests to know for sure). Thanks! Olivier
Excerpts from Olivier Hainque's message of August 18, 2020 2:25 pm: > Hi Iain, > >> On 18 Aug 2020, at 13:45, Iain Buclaw <ibuclaw@gdcproject.org> wrote: >> >> Attached is the change as per your proposal. >> >> * config/vxworks.h (VXWORKS_ADDITIONAL_CPP_SPEC): Replace -nostdinc >> with -fself-tests. >> #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) \ >> @@ -55,7 +60,7 @@ along with GCC; see the file COPYING3. If not see >> >> #undef VXWORKS_ADDITIONAL_CPP_SPEC >> #define VXWORKS_ADDITIONAL_CPP_SPEC \ >> - "%{!nostdinc: \ >> + "%{!fself-test=*: \ > > Thanks for the updated proposal. > > Sorry, I have been unclear: If I'm reading the spec of > -nostdinc correctly, I think we should still prevent those > CPP switches from being added if the option is provided. > Ah, no worries, attached updated patch. > Can you please amend just this part to prevent the addition > of the following switches if either -nostdinc or -fself-test > is provided ? > > Ok, for me with this change, assuming -nostdinc in > SELFTEST_FLAGS didn't have other uses than the one documented > in the attached comment (I'm not familiar enough with the > self-tests to know for sure). > The introducing commit is cf7bb33f4d9532dc7ea2551acbf888341b8f12ce, the reliance on -nostdinc might have changed in the meantime though. [Update] As we have discussed this off the lists though, we agreed to compromise and leave -nostdinc as it is in SELFTEST_FLAGS. Iain. --- gcc/ChangeLog: * config/vxworks.h (VXWORKS_ADDITIONAL_CPP_SPEC): Don't include VxWorks header files if -fself-tests is used. (STARTFILE_PREFIX_SPEC): Avoid using VSB_DIR if -fself-tests is used. --- diff --git a/gcc/config/vxworks.h b/gcc/config/vxworks.h index d648d2f23cb..e50260b08e4 100644 --- a/gcc/config/vxworks.h +++ b/gcc/config/vxworks.h @@ -36,11 +36,16 @@ along with GCC; see the file COPYING3. If not see /* Since we provide a default -isystem, expand -isystem on the command line early. */ + +/* 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 #undef VXWORKS_ADDITIONAL_CPP_SPEC #define VXWORKS_ADDITIONAL_CPP_SPEC \ - "%{!nostdinc: \ + "%{!nostdinc:%{!fself-test=*: \ %{isystem*} \ %{mrtp: -idirafter %:getenv(VSB_DIR /h) \ -idirafter %:getenv(VSB_DIR /share/h) \ @@ -49,19 +54,19 @@ along with GCC; see the file COPYING3. If not see ;: -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 /krnl/h/public)}}}" #else /* TARGET_VXWORKS7 */ #undef VXWORKS_ADDITIONAL_CPP_SPEC #define VXWORKS_ADDITIONAL_CPP_SPEC \ - "%{!nostdinc: \ + "%{!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) \ -}}" +}}}" #endif @@ -108,7 +113,8 @@ along with GCC; see the file COPYING3. If not see #if TARGET_VXWORKS7 #undef STARTFILE_PREFIX_SPEC -#define STARTFILE_PREFIX_SPEC "%:getenv(VSB_DIR /usr/lib/common)" +#define STARTFILE_PREFIX_SPEC \ + "%{!fself-test=*:%:getenv(VSB_DIR /usr/lib/common)}" #define TLS_SYM "-u __tls__" #else #define TLS_SYM ""
Hello Iain, > On 19 Aug 2020, at 14:17, Iain Buclaw <ibuclaw@gdcproject.org> wrote: > Ah, no worries, attached updated patch. > As we have discussed this off the lists though, we agreed to compromise > and leave -nostdinc as it is in SELFTEST_FLAGS. > Iain. > > --- > gcc/ChangeLog: > > * config/vxworks.h (VXWORKS_ADDITIONAL_CPP_SPEC): Don't include > VxWorks header files if -fself-tests is used. > (STARTFILE_PREFIX_SPEC): Avoid using VSB_DIR if -fself-tests is used. OK, nit on the ChangeLog: this is -fself-test (no trailing s). We have a batch of vxworks changes queued that we will be submitting soon, and we might get to rationalize this with other places along the way. Thanks! Olivier
Excerpts from Olivier Hainque's message of August 20, 2020 11:01 am: > Hello Iain, > >> On 19 Aug 2020, at 14:17, Iain Buclaw <ibuclaw@gdcproject.org> wrote: > >> Ah, no worries, attached updated patch. > >> As we have discussed this off the lists though, we agreed to compromise >> and leave -nostdinc as it is in SELFTEST_FLAGS. > >> Iain. >> >> --- >> gcc/ChangeLog: >> >> * config/vxworks.h (VXWORKS_ADDITIONAL_CPP_SPEC): Don't include >> VxWorks header files if -fself-tests is used. >> (STARTFILE_PREFIX_SPEC): Avoid using VSB_DIR if -fself-tests is used. > > OK, nit on the ChangeLog: this is -fself-test (no trailing s). > > We have a batch of vxworks changes queued that we will be submitting soon, > and we might get to rationalize this with other places along the way. > Running the build through one more time, and I've noticed that the make recipe macro_list also triggers a VSB_DIR not defined error. However unlike selftests, it does not result in a failed build, so probably only a minor concern. Iain.
Hello Iain, > On 20 Aug 2020, at 14:54, Iain Buclaw <ibuclaw@gdcproject.org> wrote: > >> We have a batch of vxworks changes queued that we will be submitting soon, >> and we might get to rationalize this with other places along the way. >> > > Running the build through one more time, and I've noticed that the make > recipe macro_list also triggers a VSB_DIR not defined error. > > However unlike selftests, it does not result in a failed build, so > probably only a minor concern. Thanks for the note. macro_list is essentially for fixincludes, a notorious source of headaches for VxWorks. IIRC, there is code to inhibit macro-lists somewhere down the line so it doesn't seem problematic. It seems ok for the kind of sanity-check build you are doing anyway, and not the kind of thing we want to make special efforts hiding, as we want to catch such missing definitions in builds intended for real environments, when we do need visibility on the system headers. Cheers, Olivier
diff --git a/gcc/config/vxworks.h b/gcc/config/vxworks.h index d648d2f23cb..065c9e12b88 100644 --- a/gcc/config/vxworks.h +++ b/gcc/config/vxworks.h @@ -108,7 +108,7 @@ along with GCC; see the file COPYING3. If not see #if TARGET_VXWORKS7 #undef STARTFILE_PREFIX_SPEC -#define STARTFILE_PREFIX_SPEC "%:getenv(VSB_DIR /usr/lib/common)" +#define STARTFILE_PREFIX_SPEC "%{!nostdinc:%:getenv(VSB_DIR /usr/lib/common)}" #define TLS_SYM "-u __tls__" #else #define TLS_SYM ""