RFR [XS]: 8239462: jdk.hotspot.agent misses some ReleaseStringUTFChars calls in case of early returns

Alex Menkov alexey.menkov at oracle.com
Tue Feb 25 19:30:28 UTC 2020


Hi Matthias,

LGTM

--alex

On 02/25/2020 08:20, Baesken, Matthias wrote:
> 
> New webrev :
> 
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8239462.3/
> 
> 
> Best regards, Matthias
> 
> 
>> IMO the solution with goto makes it even worse.
>> If you don't want to introduce the wrapper, could you please restore
>> changes in LinuxDebuggerLocal_attach0 from webrev.1
>>
>> --alex
>>
>> On 02/21/2020 00:32, Baesken, Matthias wrote:
>>> Hi Alex ,
>>>
>>> new webrev :
>>>
>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8239462.2/
>>>
>>> Best Regards, Matthias
>>>
>>>
>>>>
>>>> Hi Matthias,
>>>>
>>>> Looks good in general, but I think it makes sense to fix #2 cases (at
>>>> least I see them in LinuxDebuggerLocal). If GetStringUTFChars fails, the
>>>> code will crash.
>>>> Also I see GetStringUTFChars(str, JNI_FALSE). This look bad as well -
>>>> 2nd arg is a pointer, so it should be NULL or nullptr.
>>>>
>>>> As for #1 and #3 - AFAIU they are both right ways.
>>>> If GetStringUTFChars fails, it throws OOM and return NULL.
>>>>
>>>> And one more thing to consider.
>>>> LinuxDebuggerLocal_attach0 function looks terrible - 7
>>>> ReleaseStringUTFChars calls for 2 GetStringUTFChars.
>>>> Maybe it make sense to introduce simple wrapper like AutoJavaString in
>>>> src/jdk.hotspot.agent/windows/native/libsaproc/sawindbg.cpp
>>>> It would make the code simpler and less error prone.
>>>>
>>>> --alex
>>>>
>>>


More information about the hotspot-dev mailing list