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.
