[aarch64-port-dev ] RFR: Fix SEGV with volatile field accesses

[Andrew Dinn]

On 20/02/14 09:57, Edward Nevill wrote:
> This is caused by the following in aarch64.ad
> 
> enc_class aarch64_enc_ldar(iRegL dst, memory mem) %{ 
> MOV_VOLATILE(as_Register($dst$$reg), $mem$$base, $mem$$index,
> $mem$$scale, $mem$$disp, rscratch1, ldar); %}
> . . .
> The above code may generate 1 or more instructions before the actual
> load. However there is an implicit assumption in
> src/share/vm/opto/output.cpp that the instruction encoding will
> generate precisely 1 instruction only.

Hmm, well, this same problem will apply to the encodign of a
non-volatile load when an lea is required before the ldr (see function
loadStore).

> This assumption is contained in the code.
> 
> // If this is a null check, then add the start of the previous
> instruction to the list else if( mach->is_MachNullCheck() ) { 
> inct_starts[inct_cnt++] = previous_offset;
> 
> Where 'previous_offset' is the offset of the last instruction
> encoding which in this case points to the add rather than the ldar.

I think I have seen this problem before and I thought I came up with a
way of fixing it. I have a vague memory that the code included an
opportunity for the offset to the potentially excepting instruction to
be included in the calculation if you supplied the node with a method
that declared its offset. If this is not just a  wild hallucination then
anything I provided was there before Andrew Haley refactored the load
rules and introduced laodStore and MOV_VOLATILE. I cannot see any
evidence for that in the latest code though.

> The solution I have adopted is (in the case of aarch64) to use
> 'current_offset - NativeInstruction::instruction_size' rather than
> 'previous_offset'. This is safe on aarch64 because

Well, that sounds good but I am concerned that we might be missing an
existing mechanism. I'll take a look just to be sure.

regards,


Andrew Dinn
-----------