RFR: JDK-8257604: JNI_ArgumentPusherVaArg leaks valist

David Holmes dholmes at openjdk.java.net
Wed Dec 2 23:43:54 UTC 2020


On Wed, 2 Dec 2020 16:36:55 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> JNI_ArgumentPusherVaArg copies the given argument list pointer (va_copy) but never releases it. A call to va_end is missing.
>> 
>> AFAICS this coding is old. Interestingly, in jdk8 I find this:
>> 
>>    0:   inline void set_ap(va_list rap) {
>>    0: #ifdef va_copy
>>    0:     va_copy(_ap, rap);
>>    0: #elif defined (__va_copy)
>>    0:     __va_copy(_ap, rap);
>>    0: #else
>>    0:     _ap = rap;
>>    0: #endif
>>    0:   }
>> 
>> `_ap = rap` seems a strangely relaxed way of doing this. But I do not know the history behind that.
>> 
>> I could not find any usage of the original arg pointer, so maybe the `va_copy()` is not even needed. The jdk8 coding seems to indicate that too. But since I was not 100% sure I kept the `va_copy()` and added a `va_end()`.
>> 
>> I also made this private (removed the `protected`) since there are no child classes.
>
> 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.)

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

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


More information about the hotspot-dev mailing list