RFR (S): 8186787: clang-4.0 SIGSEGV in Unsafe_PutByte

Kim Barrett kim.barrett at oracle.com
Tue Nov 28 20:30:11 UTC 2017


> On Nov 27, 2017, at 7:36 AM, Erik Österlund <erik.osterlund at oracle.com> wrote:
> 
> Hi,
> 
> There is currently a bug when using unsafe accesses off-heap:
> 
> 1) We write into a thread that we enable crash protection (using GuardUnsafeAccess):
> 2) We perform the access
> 3) We write into a thread that we disable crash protection (using ~GuardUnsafeAccess)
> 
> The problem is that the crash protection stores are volatile, but the actual access is non-volatile. Compilers have different interpretation whether volatile - non-volatile accesses are allowed to reorder. MSVC is known to interpret such interactions as-if the volatile accesses have acquire/release semantics from the compiler point of view, and others such as GCC are known to reorder away freely.
> 
> To prevent any issues, the accesses involved when using GuardUnsafeAccess should be at least volatile.
> This change makes the few remaining ones volatile. The JMM-volatile (SEQ_CST) accesses with crash protection already have stronger ordering than volatile and hence do not need changing.
> 
> By making the address passed in to the Access API volatile, the MO_VOLATILE decorator is automatically set, which not surprisingly makes the access volatile. Therefore, the solution is to simply make the address passed in to Access volatile in this case.
> 
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8186787
> 
> Webrev:
> http://cr.openjdk.java.net/~eosterlund/8186787/webrev.00/
> 
> Thanks,
> /Erik

Not my first choice for a fix (you know how I feel about casts), but
it works, and is currently much simpler than my preferred solution.

Should there be a comment justifying the cast to volatile?  Perhaps
something like "volatile to keep access within guarded scope."  I
don't need a new webrev if you add such a comment.

Looks good.



More information about the hotspot-runtime-dev mailing list