RFR: JDK-8257604: JNI_ArgumentPusherVaArg leaks valist

[David Holmes]

On Wed, 2 Dec 2020 23:40:50 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Looks good.
>
> You can read this for historical copying of va_list args:
> https://ftp.gnu.org/old-gnu/Manuals/glibc-2.2.3/html_chapter/libc_34.html#SEC674
> 
> But I'm not at all sure that our use of these macros in a RAII helper object is actually valid. From what I've read these macros have to be used in the same function in which the va_list was received.
> 
> I'm also not clear why we call va_copy in the first place, as we only need that if the caller also needs to access the va list of arguments. Or perhaps if we need to ensure we actually have something we can pass to another function ... but in that case the va_copy should be in the same function where the va_start is (and the corresponding va_end). (Imagine if args were passed in registers, you'd need to call va_copy to copy them to the stack in the function in which the register still holds the arg.)

E.g. I think this macro:

  va_list args; \
  va_start(args, methodID); \
  JavaValue jvalue(Tag); \
  JNI_ArgumentPusherVaArg ap(methodID, args); \
  jni_invoke_nonstatic(env, &jvalue, obj, JNI_VIRTUAL, methodID, &ap, CHECK_0); \
  va_end(args); \
should be:
  va_list args; \
+va_list args_copy; \
  va_start(args, methodID); \
+va_copy(args, args_copy); \
  JavaValue jvalue(Tag); \
! JNI_ArgumentPusherVaArg ap(methodID, args_copy); \
  jni_invoke_nonstatic(env, &jvalue, obj, JNI_VIRTUAL, methodID, &ap, CHECK_0); \
+  va_end(args_copy); \
  va_end(args); \

-------------

PR: https://git.openjdk.java.net/jdk/pull/1565