RFR: 8293660: Fix frame::sender_for_compiled_frame frame size assert

Dean Long dlong at openjdk.org
Tue Sep 13 03:14:44 UTC 2022


On Mon, 12 Sep 2022 17:56:17 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

> The condition and assert messages are contradicting each other here:
> 
> 
> frame frame::sender_for_compiled_frame(RegisterMap* map) const {
>   ...
>   assert(_cb->frame_size() >= 0, "must have non-zero frame size");
>   intptr_t* sender_sp = unextended_sp() + _cb->frame_size();
> 
> 
> I found this in x86_32 Loom port, where a entry generator bug caused zero-sized frames. I believe the assert message is correct, and the condition is not: sender SP should be different from (unextended) SP. In fact, if allowed to proceed, the tests can then fail the later assert, assuming `sp() == unextended_sp()`:
> 
> 
>   assert(sender_sp != sp(), "must have changed");
> 
> 
> This code predates OpenJDK history, and assert was copy-pasted in this form to many arches. 
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug `tier1`

This looks fine.  You could probably even use >= 2.  More correct would probably be >= metadata_words, but some ports are setting that value to 0, which will probably break things with loom.

There are also some places under hotspot/agent/src/share/classes that are doing the equivalent in Java:

  Assert.that(cb.getFrameSize() > 0, "CodeBlob must have non-zero frame size");

which would probably be low-risk to fix in this PR if you wanted.

-------------

Marked as reviewed by dlong (Reviewer).

PR: https://git.openjdk.org/jdk/pull/10242


More information about the hotspot-dev mailing list