Re: [PATCH] fix spelling of -linker-output-auto-nolto-rel

2021-12-01 Thread Rasmus Villemoes via Gcc-patches
On 01/12/2021 14.17, Richard Biener wrote:
> On Wed, Dec 1, 2021 at 12:08 PM Rasmus Villemoes  
> wrote:
>>
>> The transposition nolto -> notlo is confusing and it makes the long
>> name even harder to read than it already is - I kept reading it as
>> "not lo" until I realized it was a simply typo.
>>
>> Fixes: 5269b24605b1 (Silence warning in LTO mode on VxWorks)
> 
> Hmm, if we ever released GCC with that lto-plugin behavior you should preserve
> accepting -linker-output-auto-notlo-rel in lto-plugin because a newer 
> lto-plugin
> is required to work with older released GCC which continue to provide
> the option with the spelling mistake.

I thought a given version of gcc would only use the plugin it shipped
with(?).

And even if in theory one should keep the plugin compatible with older
gcc releases, in this case it is only relevant to vxworks ports, so if
Eric and Olivier are ok with this as-is, perhaps we can avoid having to
accept both spellings forever.

Rasmus


Re: [PATCH 3/4] libgcc: vxcrtstuff.c: make ctor/dtor functions static

2021-12-10 Thread Rasmus Villemoes via Gcc-patches
On 10/12/2021 15.20, Olivier Hainque wrote:
> Hi again Rasmus,
> 
>> On 1 Nov 2021, at 10:34, Rasmus Villemoes  wrote:
>>
>> When the translation unit itself creates pointers to the ctors/dtors
>> in a specific section handled by the linker (whether .init_array or
>> .ctors.*), there's no reason for the functions to have external
>> linkage. That ends up polluting the symbol table in the running
>> kernel.
>>
>> This makes vxcrtstuff.c on par with the generic crtstuff.c which also
>> defines e.g. frame_dummy and __do_global_dtors_aux static.
>>
>> libgcc/
>>  * config/vxcrtstuff.c: Make constructor and destructor
>>  functions static when possible.
> 
> 
> This is OK, can you please commit?

Well, yes, but it depends contextually (but not functionally) on patch
2/4. Should I redo this one, or can I get you to take a look at 2/4 first?

You've already ok'ed 1/4 and 4/4 (which were anyway independent of each
other and 2,3) and I've committed those

Rasmus


Re: [PATCH] Define _C99 in libstdc++ vxworks/os_defines.h

2021-12-10 Thread Rasmus Villemoes via Gcc-patches
On 10/12/2021 16.06, Olivier Hainque wrote:
> Hello,
> 
> The attached patch for libstdc++ / VxWorks helps building
> the library for old versions of the OS, as witnessed with
> VxWorks 6.9 in particular.
> 
> It explicitly requests C99 features from old system headers,
> on which libstc++ relies since at least c++98. The specific
> issue that exposed this was a failure to compile
> 
>   libstdc++-v3/src/c++17/floating_to_chars.cc
> 
> for VxWorks 6.9 with a batch of errors such as:
> 
>   error: 'FP_NAN' was not declared in this scope
> 
> The missing definitions are provided by the
> system headers with guards on _HAS_C9X, which gets
> internally defined when _C99 is.
> 
> Ok to commit?

Yes, we've observed that error as well. We have just patched our vxworks
5.5 headers to provide FP_NAN et al. There's no test of definedness or
other uses of a _C99 macro anywhere, so ok by me, as it shouldn't change
anything on our end.

Rasmus


Re: [PATCH] gcc: vx-common.h: fix test for VxWorks7

2021-11-05 Thread Rasmus Villemoes via Gcc-patches
On 05/11/2021 09.08, Olivier Hainque wrote:
> Hi Rasmus,
> 
>> On 3 Nov 2021, at 14:18, Rasmus Villemoes  wrote:
>>
>> The macro TARGET_VXWORKS7 is always defined (see vxworks-dummy.h).
>> Thus we need to test its value, not its definedness.
>>
>> Fixes aca124df (define NO_DOT_IN_LABEL only in vxworks6).
>>
>> gcc/ChangeLog:
>>
>>  * config/vx-common.h: Test value of TARGET_VXWORKS7 rather
>>  than definedness.
> 
> Indeed. Ok, thanks!

Applied to master and pushed - hope I've done it right.

How about the gcc-11 branch, can it be applied there as well, and if so,
should I do a "git cherry-pick -x" and push it to that branch? From
looking at the git history it seems to be the way things are done.

Thanks,
Rasmus


Re: [PATCH] gcc: vx-common.h: fix test for VxWorks7

2021-11-05 Thread Rasmus Villemoes via Gcc-patches
On 05/11/2021 15.08, Olivier Hainque wrote:
> 
> 
>> On 5 Nov 2021, at 09:48, Rasmus Villemoes  wrote:
>> Applied to master and pushed - hope I've done it right.
> 
> AFAICS, yes.
> 
>> How about the gcc-11 branch, can it be applied there as well,
> 
> Yes, I think so. The builds you do used to work before
> the change that introduced the ifdef, 

Well, apart from all the other fixups, some of which are not
upstreamable, that I also need to apply :)

IIUC, and the adjustment
> you propose is close to being in the "obvious" category.

Indeed. I'll cherry-pick it into releases/gcc-11.

Have you had a chance to look at the other four patches I've sent
recently? They are also vxworks-only, and shouldn't be very controversial.

Rasmus


Re: [PATCH] gcc: vx-common.h: fix test for VxWorks7

2021-11-05 Thread Rasmus Villemoes via Gcc-patches
On 05/11/2021 16.12, Olivier Hainque wrote:
> 
> 
>> On 5 Nov 2021, at 15:12, Rasmus Villemoes  wrote:

> We happen to also have a few fixincludes hunks around. Some of
> them have been there for years now and I thought would be nice to
> propagate at some point.
> 
> Do you use it?

Sort of, kind of. I have previously sent fixups for some of the vxworks
fixincludes, and I think I have a few more I could submit, but since I'm
able to modify our target headers directly that's often much easier.

For example, there's the mkdir fixup that adds

#define mkdir(dir, ...) ((void)0, ##__VA_ARGS__, (mkdir)(dir))

But that macro doesn't play well with code in libstdc++ that says

if (posix::mkdir(p.c_str(), mode))

because that expands to utter nonsense and breaks the build. A better
macro, also using the comma operator to have the mode argument evaluated
then ignored, would be something like

  #define mkdir(dir, mode) mkdir(((void)(mode), (dir)))

(or the variadic version to continue allowing a one-argument
invocation). But it was easier to just change the target header, making
that fixinclude pattern no longer apply - knowing that if we ever
compile the mkdir implementation [we have parts of the kernel sources,
but not all], we'll have to fixup the definition.

The one fixinclude-related problem I have is the stdint.h one I've
mentioned previously.

Rasmus


Re: [PATCH] Add VxWorks fixincludes hack, open posix API

2021-12-17 Thread Rasmus Villemoes via Gcc-patches
On 17/12/2021 13.01, Olivier Hainque wrote:
> Hello,
> 
> The attach patch adds a fixincludes "hack" for VxWorks
> to expose a more flexible (varargs) function prototype for 'open',
> able to accept calls with 2 or 3 arguments as we observe
> during libraries builds for powerpc vxworks 6.9.
> 
> We have been using this for a while in-house. I could
> still observe related failures with mainline sources without
> the change and get a complete successful with it (plus a couple
> of others).
> 
> Also bootstrapped and regression tested ok for x86_64-linux,
> just in case.
> 
> Ok to commit?

I had proposed more-or-less the same patch to my customer, just being
done to the vxworks 5.5 headers directly.

We then looked at the part of the kernel source we do have available
(and which gets rebuilt when they build their OS kernel). That happens
to include the open() implementation. We could modify the open()
implementation accordingly, fetching a mode argument iff there's O_CREAT
in flags, and otherwise setting it to 0. Unfortunately, looking down the
call tree from open(), it turns out that there are some weird uses of
the mode argument even when O_CREAT was not given (I don't remember the
details, but I think one case was looking at a non-mode bit, S_IFDIR. It
was somewhat hard to follow due to several levels of indirect calls, and
I don't even think we had the code for all the possible callbacks).
Unconditionally fetching the third argument [1] isn't really an option
either as that would be garbage for any two-argument call site.

In the end, we ended up adding a two-argument overload for C++ only, as
this is/was only relevant for getting libstdc++ (more specifically, the
new filesystem abstraction stuff) to build. That is, we added

+#if __GNUC__ > 6 && defined(__cplusplus)
+extern "C++" {
+extern int  open (const char *name, int flags);
+}
+#endif
+

to fcntl.h, and added a trivial definition of that (which calls the
tree-argument form with a 0 for mode) to the OS build. I guess it could
also have been an inline.

Another option we considered was inspired the fortified definition of
open() in glibc, which checks that when flags is compile-time known and
O_CREAT is there, there is a mode argument. We would not do that check,
but simply use __va_arg_pack_len() to decide whether the caller had
given a mode argument, and if not, pass a 0. That would also provide C
code with the ability to use both two- and three-arg open().

I'm not sure what to do. But this patch will definitely break our build
- primarily because we've done a private workaround.

Rasmus

[1] Instead of making the open() definition variadic, we could probably
also play a trick like renaming it at C level, and then re-renaming at
asm level, i.e. something like

-int open(const char *path, int flags, int mode) {
+int vxworks_open(const char *path, int flags, int mode) asm("open") {
 ...

but that would still make mode contain garbage when called with two
arguments.


Re: [PATCH] Add VxWorks fixincludes hack, kernel math.h FP_ constants

2021-12-17 Thread Rasmus Villemoes via Gcc-patches
On 17/12/2021 13.10, Olivier Hainque wrote:
> Hello,
> 
> The attached patch adds a fixincludes add for VxWorks
> to add missing FP_ constant definition to math.h, intended
> for old versions of the kernel math.h header.

Don't you also need to add an fpclassify() macro? There's a

checking for ISO C99 support in  for C++98

which checks whether math.h supplies (among others) fpclassify().

We've patched our math.h to supply those constants as well as an
fpclassify() macro. So I suppose the 'bypass = "FP_INFINITE"' would mean
that this fixinclude would just be skipped for our case(?).

Rasmus


Re: [PATCH] Add VxWworks fixincludes hack, prevent #include_next yvals.h

2021-12-17 Thread Rasmus Villemoes via Gcc-patches
On 17/12/2021 13.14, Olivier Hainque wrote:
> Hello,
> 
> yvals.h on VxWorks expects the toolchain to provide its own
> version of the header, which we don't do.
> 
> The attached patch adds a fixincludes hack to arrange to fallback
> on the common system definitions instead.
> 
> We were able to get a successful complete build with c++ and
> libstdc++ after this.
> 
> Also bootstrapped and regression tested ok for x86_64-linux,
> just in case.

There's no yvals.h header in VxWorks 5.5, so I'm not sure I need to have
an opinion on this one.

Rasmus


Re: [PATCH] Add VxWorks fixincludes hack, open posix API

2021-12-17 Thread Rasmus Villemoes via Gcc-patches
On 17/12/2021 15.12, Olivier Hainque wrote:
> Hi Rasmus
> 
>> On 17 Dec 2021, at 13:49, Rasmus Villemoes  
>> wrote:
> 
>> I'm not sure what to do. But this patch will definitely break our build
>> - primarily because we've done a private workaround.
> 
> I don't think we can reasonably cope with private changes
> to system headers.

Of course not. And I'm more than willing to undo that private change if
a suitable fix can be worked out.

> Can't you just undo your workaround and use this (very simple)
> "fix" instead?

No, because as I explained, the open() implementation on vxworks 5.5
must not be called as a two-arg function with garbage in r5. Do you have
access to any of the kernel source code for the other vxworks versions
with a three-arg-only open() in fcntl.h? If not, how can you know that
those kernels do not make use of the mode argument even if O_CREAT is
not in flags? (For example, I could actually imagine an implementation
where non-zero mode would imply O_CREAT. Then 'open("foo", O_RDWR)'
could result in foo being created with a more-or-less random mode, where
it should have resulted in ENOENT.)

I'm sure libstdc++ builds with this change, as I said I had the same
thing initially, but after looking at the open() implementation we were
worried about the implications.

> Otherwise, add a local patch to your tree to remove this fix?

Always an option, of course.

Rasmus


Re: [PATCH] Leverage sysroot for VxWorks

2021-12-20 Thread Rasmus Villemoes via Gcc-patches
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


vxworks libstdc++ locale

2021-12-21 Thread Rasmus Villemoes via Gcc-patches
Hi

While trying to upgrade our vxworks 5.5 compiler to gcc12, I've hit a
problem when loading the libstdc++ module on target. It manifests as

[00] tShell memPartFree: invalid block 8bf72c in partition 9605dc.
[00] tShell memPartFree: invalid block 8bf38c in partition 9605dc.
[00] tShell memPartFree: invalid block 8bf304 in partition 9605dc.
[00] tShell memPartFree: invalid block 8bf348 in partition 9605dc.
[00] tShell memPartFree: invalid block 8bf23c in partition 9605dc.
[00] tShell memPartFree: invalid block 8bf6c4 in partition 9605dc.
[00] tShell memPartFree: invalid block 8bf794 in partition 9605dc.
[00] tShell memPartFree: invalid block 8bf7a0 in partition 9605dc.
[00] tShell memPartFree: invalid block 8bf7bc in partition 9605dc.

being printed on the console. We didn't use to pass an explicit
--enable-clocale option to configure, but if I add
--enable-clocale=generic , thus reverting to the locale implementation
used for gcc11, the problem goes away.

The vxworks locale seems to be mostly identical to generic, just
differing in CCTYPE_CC. And comparing the .a files, it seems that that
TU ends up defining a constructor
_GLOBAL__sub_I__ZNSt12ctype_bynameIcEC2EPKcj , which calls
_ZNSt8ios_base4InitC1Ev . But then I'm lost.

Any ideas?

Rasmus


Re: vxworks libstdc++ locale

2021-12-22 Thread Rasmus Villemoes via Gcc-patches
On 21/12/2021 16.42, Rasmus Villemoes wrote:
> Hi
> 
> While trying to upgrade our vxworks 5.5 compiler to gcc12, I've hit a
> problem when loading the libstdc++ module on target. It manifests as
> 
> [00] tShell memPartFree: invalid block 8bf72c in partition 9605dc.
> [00] tShell memPartFree: invalid block 8bf38c in partition 9605dc.
> [00] tShell memPartFree: invalid block 8bf304 in partition 9605dc.
> [00] tShell memPartFree: invalid block 8bf348 in partition 9605dc.
> [00] tShell memPartFree: invalid block 8bf23c in partition 9605dc.
> [00] tShell memPartFree: invalid block 8bf6c4 in partition 9605dc.
> [00] tShell memPartFree: invalid block 8bf794 in partition 9605dc.
> [00] tShell memPartFree: invalid block 8bf7a0 in partition 9605dc.
> [00] tShell memPartFree: invalid block 8bf7bc in partition 9605dc.
> 
> being printed on the console. We didn't use to pass an explicit
> --enable-clocale option to configure, but if I add
> --enable-clocale=generic , thus reverting to the locale implementation
> used for gcc11, the problem goes away.
> 
> The vxworks locale seems to be mostly identical to generic, just
> differing in CCTYPE_CC. And comparing the .a files, it seems that that
> TU ends up defining a constructor
> _GLOBAL__sub_I__ZNSt12ctype_bynameIcEC2EPKcj , which calls
> _ZNSt8ios_base4InitC1Ev . But then I'm lost.
> 
> Any ideas?

So if I remove the

#include 

from libstdc++-v3/config/locale/vxworks/ctype_members.cc the problem
goes away, and I don't see the purpose of that #include anyway (a debug
leftover perhaps?).

Rasmus