RFR: JDK-8257604: JNI_ArgumentPusherVaArg leaks valist

Thomas Stuefe stuefe at openjdk.java.net
Thu Dec 3 07:53:54 UTC 2020


On Thu, 3 Dec 2020 04:57:42 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Hi David,
>> 
>> I think the thoughts are valid, I thought it odd too. The usage of va_args is tightly defined. 
>> 
>> https://pubs.opengroup.org/onlinepubs/009695399/basedefs/stdarg.h.html
>> Posix specifies:
>> `Each invocation of the va_start() and va_copy() macros shall be matched by a corresponding invocation of the va_end() macro in the same function.`
>> 
>> I also wondered why we copy in the first place, but as I wrote, I was not sure I missed some caller or some way the original ap got used.
>> 
>> "not rocking the boat" - should I just withdraw this patch? Its a small theoretical leak. But I did a quick test on my Ubuntu box, leaking an va_list, and could not see any memory loss. Of course it may still be leaking on other platforms.
>> 
>> Cheers, Thomas
>
> Hi Thomas,
> 
> IIUC (and at best that is a partial understanding) the allocation of memory by va_start only happens under specific conditions, and I don't know exactly what they are. I tried going to the source:
> 
> https://github.com/gcc-mirror/gcc/blob/master/gcc/builtins.c
> 
> but that wasn't exactly illuminating without understanding all the other gcc-isms in use. (You may understand better than I.)
> 
> I think your fix is harmless, and technically improves the existing code, even if there are potentially other flaws with how we do this. So if you and the other reviewers want to go ahead that seems fine to me.
> 
> Thanks,
> David

Thank you Coleen, David and Dan.

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

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


More information about the hotspot-dev mailing list