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

[Langer, Christoph]

Hi,

while I agree with David and I think we can't reach line 285 with "previous_thr" uninitialized, I think your code change is benign. And when it quiesces the compiler, it should probably be good to go.

Cheers
Christoph


> -----Original Message-----
> From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net> On Behalf Of
> David Holmes
> Sent: Montag, 17. Februar 2020 22:55
> To: Baesken, Matthias <matthias.baesken at sap.com>; 'hotspot-
> dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
> Subject: Re: RFR [XS] : 8239224: libproc_impl.c previous_thr may be used
> uninitialized warning
> 
> 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
> >>>