RFR [XS] : 8239224: libproc_impl.c previous_thr may be used uninitialized warning

[David Holmes]

On 17/02/2020 11:54 pm, Baesken, Matthias wrote:
> Hi David,  building  with gcc / -Os  sometimes gives strange (+different) warnings  when comparing to the other O-modes.
> 
>> I don't see the need for the NULL check however as previous_thr can't be
>> NULL if we reach that part of the code.
> 
> What about   current_thr != NULL,  but   current_thr == thr_to_be_removed   ?
> The we would not enter the body of the while-loop ,   would not return early in 283  , but we would  access  "previous_thr->next = current_thr->next;"   ?

If we reach the end of the loop, or we find the target thread in the 
loop, then previous_thr will be pointing to the last non-NULL element in 
the linked list.

David
-----

> 
>   277       while (current_thr && current_thr != thr_to_be_removed) {
>   278         previous_thr = current_thr;
>   279         current_thr = current_thr->next;
>   280       }
>   281       if (current_thr == NULL) {
>   282         print_error("Could not find the thread to be removed");
>   283         return;
>   284       }
>   285       if (previous_thr != NULL) {
>   286         previous_thr->next = current_thr->next;
>   287       }
> 
> 
> Best regards, Matthias
> 
> 
>>
>> On 17/02/2020 11:10 pm, Baesken, Matthias wrote:
>>> Hello, please review this small fix.
>>>
>>> When building a product build  with gcc7 on linux x86_64, and trying out
>> different optimization options, I run into this error when
>>> optimizing for space (-Os)  :
>>>
>>>
>>>
>> /open_jdk/jdk_5/jdk/src/jdk.hotspot.agent/linux/native/libsaproc/libproc_i
>> mpl.c: In function 'delete_thread_info':
>>>
>> /open_jdk/jdk_5/jdk/src/jdk.hotspot.agent/linux/native/libsaproc/libproc_i
>> mpl.c:285:26: error: 'previous_thr' may be used uninitialized in this function [-
>> Werror=maybe-uninitialized]
>>>          previous_thr->next = current_thr->next;
>>>          ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
>>>
>>> We should probably better initialize previous_thr to avoid this warning.
>>>
>>> (it does not show up however with the default optimization options ).
>>
>> Seems like a spurious warning to me but initializing to NULL is harmless.
>>
>> I don't see the need for the NULL check however as previous_thr can't be
>> NULL if we reach that part of the code.
>>
>> Thanks,
>> David
>>
>>>
>>> Bug/webrev :
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8239224
>>>
>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8239224.0/
>>>
>>>
>>> Thanks, Matthias
>>>