VaList is buggy on Java 17?
Maurizio Cimadamore
maurizio.cimadamore at oracle.com
Thu Apr 20 14:01:14 UTC 2023
Hi Mark,
I’ve read more about the documentation of the library you have linked.
It seems that in this case |libevdev_device_log_func_t| is used by
clients to set some logging function that should be used by the libevdev
library. While it is possible for a client to plug in a 100% custom
function into this, I would expect that most C clients would, in reality
just write the logging function in a way that delegates to some other
va_list accepting function (e.g. vprintf). After all there’s a format
string to parse, so I don’t expect many clients to really want to do the
processing themselves.
So, if one wants to write a logging callback in Panama that just calls
vprintf with whatever format and va/list /parameter is passed, that is
absolutely possible, and you don’t need VaList for that. That is, if an
upcall takes a va_list parameter, you can still model it with the FFM
API, but the va_list parameter becomes just a regular pointer (e.g. a
memory segment, or memory address if you use JDK 17).
In this case (which is also the vastly common case), we say that the
va_list is being received and passed /opaquely/ (as a pointer) to some
other function.
The problem only arises in two cases:
* Java code wants to create a new va_list /from scratch/, and pass it
to some library function that accepts va_list
* A Java upcall receives a va/list parameter and wants to manually/
take it apart
(These were the use cases VaList was designed for).
While not ideal, these cases can still be served, by rolling in some
helper C library. Something like:
|#include <stdarg.h> void with_valist(void (*func)(va_list), int num,
...) { va_list arguments; va_start(arguments, num); func(arguments);
va_end (arguments); } int getInt(va_list arguments) { return
va_arg(arguments, int); } long getLong(va_list arguments) { return
va_arg(arguments, long); } long getDouble(va_list arguments) { return
va_arg(arguments, double); } |
Which can be used like this:
|#include <stdio.h> void someFunction(va_list arguments) { int x =
getInt(arguments); int y = getInt(arguments); printf("args = %d, %d\n",
x, y); } void main(void) { with_valist(someFunction, 2, 1, 2); } |
Note how this shim library allows creation of a va_list (from a variadic
function, which FFM supports) which is passed opaquely to a function
pointer (which can then pass it along, or process it directly). This
feature is crucial to correctness: a va_list might hold onto memory in
the current stack, so a va_list must only be accessed in the context of
the function in which the va_list was created. The library also adds
some basic functions to (opaquely) read different types from the
va_list. I believe such a simple library should work well with the FFM
API - and should be much simpler/portable than replicating the entire
VaList code we used to have. Of course I’m not saying you should do this
- I’m just thinking (aloud) about alternate approaches you might find
useful if you do decide to keep VaList support in SlinC.
(There is one limitation in the above approach: the shim library can
only expose getters for types it knows about - so it cannot e.g. provide
a getter for a user-defined struct).
Maurizio
On 20/04/2023 12:18, Mark Hammons wrote:
> Hi Maurizio,
>
> The main reason I was looking at VaList support is because I had a
> request from someone wanting to use my library that wants to bind to a
> function that takes this as a parameter:
>
> https://www.freedesktop.org/software/libevdev/doc/latest/group__logging.html#gab7eb997be2b701cc6f42e7b4c3478269
> <https://urldefense.com/v3/__https://www.freedesktop.org/software/libevdev/doc/latest/group__logging.html*gab7eb997be2b701cc6f42e7b4c3478269__;Iw!!ACWV5N9M2RV99hQ!LsJDJl_0sVJwkTjYLgu459mq_VPbzCOu-aM_qNvUDsC9JX9x1htJ_KOAUGBG77u5mWhLSq5fQ6Zdtm_pwYc34Mc8r5ElWw$>
> https://www.freedesktop.org/software/libevdev/doc/latest/group__logging.html#gaa60be86b83b3a6c82d8e536ba89ff955
> <https://urldefense.com/v3/__https://www.freedesktop.org/software/libevdev/doc/latest/group__logging.html*gaa60be86b83b3a6c82d8e536ba89ff955__;Iw!!ACWV5N9M2RV99hQ!LsJDJl_0sVJwkTjYLgu459mq_VPbzCOu-aM_qNvUDsC9JX9x1htJ_KOAUGBG77u5mWhLSq5fQ6Zdtm_pwYc34MfBqHQG3Q$>
>
> Considering VaList is being removed in Java 21, is there any way for
> an upcall to be defined that would be compatible with a va_list
> argument? I suppose it would be possible to add va_list support to my
> library manually using the methods you were using in java 19, but that
> adds some additional complexity to my library.
>
> Re int vs long, the test there doesn't attempt with Long, but changing
> `CInt` to `CLongLong` is how I do it. When that's done, the test
> starts passing without issues on java 17. It's very odd behavior.
>
> All in all, I'm a bit saddened to hear about VaList support being
> removed seeing as I just spent the better part of a week and a half
> adding support for it to my library, but I'll live should that be the
> final decision on VaList. That being said, it's one of the few things
> I've been asked for from people wanting to test my library, so I worry
> that it may be something that's wanted when the foreign api is no
> longer a preview feature. That being said, maybe I'm just one of the
> few users of the raw foreign API and everyone else will just use
> jextract ;)
>
> Thanks again for your hard work,
> Mark
>
> On Thu, Apr 20, 2023 at 11:19 AM Maurizio Cimadamore
> <maurizio.cimadamore at oracle.com> wrote:
>
> Hi Mark,
> the implementation of VaList has been ironed out a lot around 19
> (when
> the API became preview) - that said, I don't recall a specific fix
> for
> the issue you mention.
>
> Also, as mentioned here [1, 2], VaList is no longer part of the API
> since 21, as the interaction with a VaList is hopelessly platform
> dependent (since different platforms use different encoding for
> va_list), meaning that it is very problematic, from a javadoc
> perspective, to specify which exception will be issued and when.
>
> Regarding int vs. long, the C language defines a set of default
> argument
> promotion that should be applied when using variadic functions and
> va_lists. The builder methods and getters of the VaList class were
> designed with these constraints in mind, which means the calls to the
> valist builder have to "match" the calls to the getters (or undefined
> behavior will ensue). From the Scala code you paste below, it is not
> clear to me is such a mismatch occurs (I do no see `long`
> anywhere, but
> I do see `get[CInt]`).
>
> Maurizio
>
> [1] -
> https://mail.openjdk.org/pipermail/panama-dev/2022-November/018096.html
> [2] - https://git.openjdk.org/panama-foreign/pull/763
>
>
> On 20/04/2023 08:00, Mark Hammons wrote:
> > Hi all,
> >
> > I know that the panama project is pushing forward on java 20 and 21
> > right now, and java 17 is probably not a great concern for you
> all at
> > the moment, but I was testing VaList support in my library,
> Slinc, and
> > it seems to be flaky on Java 17.0.5. The test in question goes
> like so:
> >
> > property("varargs can be embedded in structs"):
> > forAll: (ints: Seq[CInt]) =>
> > val numTake = 8
> > Scope.confined:
> > val va = VarArgsBuilder.fromIterable
> > (
> > ints.map(a => a: Variadic).take(numTake)
> > )
> > .build
> >
> > val x = va.copy()
> >
> > val p = Ptr.copy(E(x))
> >
> > ints.take(numTake).foreach: value =>
> > assertEquals(va.get[CInt], value, "conversion test")
> >
> > ints.take(numTake).foreach: value =>
> > assertEquals((!p).list.get[CInt], value)
> > Originally in this test I was testing if I could write a VaList
> to a
> > struct and get it back out and pull the values out of it
> > appropriately. What I noticed is that if the number of integer
> values
> > in a row passed into VaList is 8 or greater, my test fails. 7 or
> less
> > ints and the test passes. On the Java 19 implementation, this
> does not
> > happen, nor does this happen when I pass a great deal more
> parameters
> > (but more varied in types, some longs, some pointers, some
> structs).
> > This test passes if I randomly generate Long values and pass those
> > into the VaList.
> >
> > This is not a critical bug by any means. I can sidestep the
> issue by
> > encoding all lower integral types into Long in the java 17
> runtime for
> > my library. I just wanted to bring this to your attention
> because it's
> > a weird issue that java 17 users of the foreign api may hit.
> >
> > Thanks for your time,
> > Mark Hammons
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/panama-dev/attachments/20230420/245bb9a0/attachment-0001.htm>
More information about the panama-dev
mailing list