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

[Volker Simonis]

On Tue, Jan 15, 2019 at 3:42 PM Aleksey Shipilev <shade at redhat.com> wrote:
>
> 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 see. You're right.

> 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.
>

Agreed!

> -Aleksey
>