RFR: 8230797: ARM32-softfp: assertion in InterpreterRuntime::resolve_ldc

[Christoph Göttschkes]

Hi David

On 7/1/21 3:44 AM, David Holmes wrote:
> Hi Christoph,
> 
> On 24/06/2021 10:06 pm, Christoph Göttschkes wrote:
>> Hi,
>>
>> please review the following change, which was way too long on my chest. It fixes an assertion in the template interpreter for ARM32-softfp.
>>
>> For ARM32-softfp, the template interpreter calls into the runtime to load a double constant using the ldc bytecode. After the interpreter loaded the constants, the assert block does some sanity checks on the cached constants. But if the double constant is the first constant to be loaded, the cache is not yet initialized and the check results in a SIGSEGV.
>>
>> I guarded the usage of `ConstantPool::cp_to_object_index` by another check, which tests if there are any resolved references and if that's the case, the cache has already been initialized and the sanity checks can be performed.
> 
> Why isn't this something that should be fixed in the ARM code rather than the shared code? I admit I'm struggling to understand exactly how the problem arises - ldc is not to be used for long/double values.

Sorry for confusing you. Hopefully the following explanation clears things up.

The runtime executes the ldc2_w bytecode to load the double constant. For ARM32-softfp this uses the TemplateTable::condy_helper method, which calls InterpreterRuntime::resolve_ldc to load the constant. resolve_ldc is used for all kinds of bytecodes. I probably wrote "calls into the runtime to load a double constant using the ldc bytecode" because of the name resolve_ldc.

I guess ARM32-softfp is the only platform which uses resolve_ldc for double constants. From the way resolve_ldc is written, it also looks like it should be able to handle ldc2_w bytecodes. There are multiple assertions which check for correct bytecodes etc. and ldc2_w is explicitly among the supported bytecodes.

We could still think about not using the condy_helper and resolve_ldc for loading double constants on ARM32-softfp, but after that change we would still have a resolve_ldc implementation, which works for ldc2_w, but fails in debug VMs because of the ASSERT check. So I think we have to address both things even if we change the ARM interpreter behavior for ldc2_w.

> 
> Thanks,
> David
> -----
> 
-- Christoph