RFR (XS): 8218937: Make mlvmJvmtiUtils strncpy uses GCC 8.x friendly

[Mikael Vidstedt]

> On Feb 13, 2019, at 4:48 PM, Igor Ignatyev <igor.ignatyev at oracle.com> wrote:
> 
> > One could imagine wrapping the resource in a C++ class and have the destructor do the deallocation.
> that sounds nice.
> 
>>      strncpy(mn->methodName, szName, sizeof(mn->methodName));
>>      strncpy(mn->classSig, szSignature, sizeof(mn->classSig));
> as we are doing this just to make gcc happier, wouldn't it be better to add -1 to sizeof?

While that does silence the warning, it (still) has the issue of potentially leaving the resulting strings unterminated. We may know today that the strings in question are always shorter than the respective MethodName buffers, but it seems prudent to actually perform the checks to ensure that.

Cheers,
Mikael

> 
> Thanks,
> -- Igor
> 
> 
>> On Feb 13, 2019, at 4:38 PM, Mikael Vidstedt <mikael.vidstedt at oracle.com <mailto:mikael.vidstedt at oracle.com>> wrote:
>> 
>> 
>> 
>>> On Feb 13, 2019, at 4:12 PM, Igor Ignatyev <igor.ignatyev at oracle.com <mailto:igor.ignatyev at oracle.com>> wrote:
>>> 
>>> Hi Mikael,
>>> 
>>> I really don't like us adding goto in cpp code, can we avoid that?
>> 
>> One alternative is to make sure the Deallocate calls are injected in every if statement. I don’t think that’s any better.
>> 
>> Another alternative is to nest the if statements. I’m not a fan of that code style personally.
>> 
>> One could imagine wrapping the resource in a C++ class and have the destructor do the deallocation.
>> 
>> Let me know if you have other suggestions, and/or what you’d prefer.
>> 
>>> I also doubt that any of mlvm tests are run in tier1, could you please also run :vmTestbase_vm_mlvm to test your fix?
>> 
>> Will do.
>> 
>> Cheers,
>> Mikael
>> 
>>> 
>>> Thanks,
>>> -- Igor
>>> 
>>> 
>>>> On Feb 13, 2019, at 3:45 PM, Mikael Vidstedt <mikael.vidstedt at oracle.com <mailto:mikael.vidstedt at oracle.com>> wrote:
>>>> 
>>>> 
>>>> Please review this small fix which updates some uses of strncpy to address some GCC 8.x warnings.
>>>> 
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8218937 <https://bugs.openjdk.java.net/browse/JDK-8218937>
>>>> Webrev: http://cr.openjdk.java.net/~mikael/webrevs/8218937/webrev.00/open/webrev/ <http://cr.openjdk.java.net/~mikael/webrevs/8218937/webrev.00/open/webrev/>
>>>> 
>>>> GCC 8.2 is producing a warning for mlvmJvmtiUtils.cpp: 
>>>> 
>>>> In file included from test/hotspot/jtreg/vmTestbase/vm/mlvm/indy/func/jvmti/share/libIndyRedefineClass.cpp:31: 
>>>> test/hotspot/jtreg/vmTestbase/vm/mlvm/share/mlvmJvmtiUtils.cpp: In function 'MethodName* getMethodName(jvmtiEnv*, jmethodID)': 
>>>> test/hotspot/jtreg/vmTestbase/vm/mlvm/share/mlvmJvmtiUtils.cpp:80:12: error: 'char* strncpy(char*, const char*, size_t)' specified bound 256 equals destination size [-Werror=stringop-truncation] 
>>>>      strncpy(mn->methodName, szName, sizeof(mn->methodName)); 
>>>>      ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>> 
>>>> Basically, gcc is pointing out that the resulting string is not necessarily going to be terminated. Explicitly checking the length of the source strings provides the information needed to guarantee that the strings will be terminated, and silences the warning.
>>>> 
>>>> Passes tier1.
>>>> 
>>>> Cheers,
>>>> Mikael
>>>> 
>>> 
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20190213/4fa95ea9/attachment.html>