[RFR] Fix clobbered address passed to TSAN

Man Cao manc at google.com
Wed Jun 26 23:19:57 UTC 2019


Offline chat with Arthur, for passing parameter to
SharedRuntime::verify_oop_index, it might be better to do the following
instead, so it accounts for field._disp:
__ leaq(c_rarg0, field);
__ subq(c_rarg0, field.base());

And why does it need "__ get_method(c_rarg2)", as c_rarg2 seems unused?

-Man


On Wed, Jun 26, 2019 at 4:07 PM Jean Christophe Beyler <jcbeyler at google.com>
wrote:

> Hi Arthur,
>
> LGTM,  I was curious if the DEBUG code you added made sense to go upstream
> to help diagnose issues or was this a failure that is only provoked by
> TSAN's instrumentation before this patch and can never happen otherwise?
>
> Thanks,
> Jc
>
> On Wed, Jun 26, 2019 at 8:47 AM Arthur Eubanks <aeubanks at google.com>
> wrote:
>
> > webrev:
> >
> http://cr.openjdk.java.net/~aeubanks/tsanarrayclobber/webrev.00/index.html
> > (ignore the wrong description, webrev.ksh is weird, it's good locally)
> >
> > Fix clobbered address passed to TSAN
> >
> > access_load_at() sometimes clobbered the address we passed to TSAN in
> > tsan_observe_load_or_store().
> >
> > Do the TSAN instrumentation first, then do the load. Initially the order
> > was access_load_at(), then TSAN instrumentation. Field accesses can
> behave
> > as volatile, and I wanted arrays to match up wi
> > th the field TSAN instrumentation/actual load order. But it doesn't
> really
> > matter for array loads since they can never be volatile.
> > Added some asserts for fastdebug that the oop is valid and that the
> > field/element offset is within the oop size. The size check fails without
> > the TSAN instrumentation/actual load reordering becau
> > se the offset register was clobbered.
> >
>
>
> --
>
> Thanks,
> Jc
>


More information about the tsan-dev mailing list