RFR (S): 8211394: CHECK_ must be used in the rhs of an assignment statement within a block

[Thomas Stüfe]

Hi David,

looks good to me now. I'm surprised compilers do not warn about this though.

Best Regards, Thomas

On Mon, Oct 8, 2018 at 9:03 AM David Holmes <david.holmes at oracle.com> wrote:
>
> Hi Thomas,
>
> On 8/10/2018 4:48 PM, Thomas Stüfe wrote:
> > Looks good.
>
> Thanks for looking at it.
>
> > You missed:
> >
> > classfile/verificationType.cpp line 113
> >
> >        return comp_this.is_component_assignable_from(comp_from, context,
> >                                            from_field_is_protected, CHECK_false);
>
> Thank you - webrev updated in place with new file.
>
> > and possibly more, did not check further. Difficult to grep for when multi line.
>
> Yes. I don't know how to exhaustively detect this problem. :(
>
> Cheers,
> David
>
> > Cheers, Thomas
> >
> >
> >
> > On Mon, Oct 8, 2018 at 12:08 AM David Holmes <david.holmes at oracle.com> wrote:
> >>
> >> Bug: https://bugs.openjdk.java.net/browse/JDK-8211394
> >> webrev: http://cr.openjdk.java.net/~dholmes/8211394/webrev/
> >>
> >> If a CHECK_ macro is used on a function call that is part of a return
> >> statement i.e.
> >>
> >> return foo(CHECK_NULL);
> >>
> >> then it expands into an unreachable if-statement that checks the
> >> exception state:
> >>
> >> return foo();
> >> if (EXCEPTION_OCCURRED)
> >>     return NULL;
> >>
> >> This is obviously a programming error, but unfortunately not something
> >> our often pedantic compilers complain about.
> >>
> >> There are two ways to fix:
> >>
> >> 1. Convert to assignment:
> >>
> >> T* t = foo(CHECK_NULL);
> >> return t;
> >>
> >> 2. If the method is local and its exception behaviour easily discernible
> >> and matches the expected behaviour, then change CHECK_ to THREAD
> >>
> >> return foo(THREAD);
> >>
> >> Both fixes are applied as appropriate. As per the bug report I also
> >> revisited an earlier fix in this area - JDK-8062808 - and made adjustments.
> >>
> >> Thanks,
> >> David