RFR (S) 8216308: StackTraceElement::fill_in can use injected Class source-file

[Aleksey Shipilev]

On 1/15/19 3:07 PM, Volker Simonis wrote:
>> Mmm. I *hope* so. But, since we are reading the source_file into local, NULL-checking it, and
>> only then using it, whatever happens with the class cache should not have immediate effect, and
>> current (racy) caller would use the non-NULL value even if cache is being concurrently cleared.
>> (There are silly C/C++ memory model issues that may still expose us to NULL even after
>> NULL-check, e.g. by re-reading the memory instead of using the local, but that would break lots
>> of other places too, I think)
> 
> But this scenario (i.e. re-reading a field from memory instead of using the value cached in a
> locale) is not related to the memory model. That depends on the compilers sole discretion and is
> perfectly legal C/C++. 

Technically, it is related to (deliberately underspecified) memory model, which C/C++ can exploit.

> We've faced such issues several time with XLC on AIX. I think the only
> thing that helps is to declare the field "volatile". This tells the compiler that the value of
> the field can change at any time so he won't try to re-read it a second time (and instead spill 
> its value to the stack if he runs out of registers).

That's a valid concern. However, here we are dealing with *Java* field in Class from the VM code,
and so it should definitely abide by Java semantics. If it does not, then javaClasses are broken too
(that's what I meant by "break lots of other places"). Putting "volatile" over Java field would not
help correctness here, because javaClass accesses ignore declaration-site volatility. We can model
it with use-site volatility by doing obj_field_acquire, but that does not give us C-style volatile
access, I think. Putting "volatile" in C++ is impossible, because there is nowhere to place it.

I say if we ever see compilers wreck up javaClasses ordering, we fix it there? In this case, this
wreckage would not be catastrophic, and would "only" result in transient "null" where source file
would be, under class redefinition race.

-Aleksey