RFR: 8262368: wrong verifier message for bogus return type [v2]
David Holmes
david.holmes at oracle.com
Tue Mar 2 22:53:12 UTC 2021
On 3/03/2021 1:26 am, Harold Seigel wrote:
> On Tue, 2 Mar 2021 05:37:52 GMT, David Holmes <dholmes at openjdk.org> wrote:
>
>>> Harold Seigel has updated the pull request incrementally with one additional commit since the last revision:
>>>
>>> changed error message text
>>
>> src/hotspot/share/classfile/verifier.cpp line 3152:
>>
>>> 3150: verify_error(ErrorContext::bad_type(bci,
>>> 3151: current_frame->stack_top_ctx(), TypeOrigin::signature(return_type)),
>>> 3152: "Method does not expect a return value");
>>
>> You've changed the wrong line. This one does expect a return value
>
> Hi David,
> If a method's descriptor has a void return type, but the method contains a bytecode such as areturn, then I think the message "Method does not expect a return type" is correct. Perhaps the word 'expect' is ambiguous? If so, then I think a more explicit message, similar to my original suggestion, should be used.
First the bug report does not concern the line you changed! The bug
report is about:
"There is a typo in an error message in verifier.cpp on line 1678"
1675: case Bytecodes::_return :
1676: if (return_type != VerificationType::bogus_type()) {
1677: verify_error(ErrorContext::bad_code(bci),
1678: "Method expects a return value");
1679: return;
1680: }
Here we have a plain _return bytecode, but the return-type is not void
as expected. So we have a return value where we do not expect one. Hence
the message on line 1678 is wrong, the method does not expect a return
value.
Now looking at the code you changed:
if (return_type == VerificationType::bogus_type()) {
verify_error(ErrorContext::bad_type(bci,
current_frame->stack_top_ctx(),
TypeOrigin::signature(return_type)),
"Method does not expect a return value");
return;
}
We have found a void return and this is an error. That means a void
return was not expected i.e. the method DOES expect a return value. And
the original message is correct.
Is there some weird double-negative logic being applied in all this that
is somehow inverting the sense of everything ???
Cheers,
David
-----
> Harold
>
> -------------
>
> PR: https://git.openjdk.java.net/jdk/pull/2757
>
More information about the hotspot-runtime-dev
mailing list