Crash on finalizer in tsanOopMap

[Man Cao]

Yes, any valid oop pointer should be in the reserved space. Here the code
is not allocating anything on the heap, and pointer_adjuster->do_oop()
typically just loads from the header word and updates the holder (
target_oop).

So this assert just checks if target_oop looks like a valid pointer after
adjusting. It is a quite relaxed check, but better than nothing.

-Man

On Thu, Jul 11, 2019, 19:22 Jean Christophe Beyler <jcbeyler at google.com>
wrote:

> Just by curiosity, isn't any memory allocated in the heap in the reserved
> space? So essentially, this assert is no longer really checking for
> anything?
>
> Or does this actually do what we want? :-)
> Jc
>
> On Thu, Jul 11, 2019 at 6:09 PM Man Cao <manc at google.com> wrote:
>
>> Can I have reviews for this bug fix?
>> https://cr.openjdk.java.net/~manc/tsan20190711/webrev.00/
>> The assertion check is too strong for target_oop after calling
>> pointer_adjuster->do_oop().
>>
>>
>> -Man
>>
>>
>> On Thu, Jul 11, 2019 at 10:52 AM Arthur Eubanks <aeubanks at google.com>
>> wrote:
>>
>> > Here it is:
>> >
>> http://cr.openjdk.java.net/~aeubanks/tsanfinalizercrash/NonRacyFinalizerLoopTest.java
>> >
>> > On Wed, Jul 10, 2019 at 4:59 PM Man Cao <manc at google.com> wrote:
>> >
>> >> I don't see the attached reproducer. I don't think you could add
>> >> attachment on the mailing list though.
>> >> Could you inline it in the email or put it on webrev?
>> >>
>> >> -Man
>> >>
>> >>
>> >> On Mon, Jul 8, 2019 at 1:23 PM Arthur Eubanks <aeubanks at google.com>
>> >> wrote:
>> >>
>> >>> I have a simpler reproducer attached.
>> >>>
>> >>> Some logging:
>> >>>            // The object survived GC, add its updated oop to the new
>> oops
>> >>> map.
>> >>>            oop target_oop = cast_to_oop((intptr_t)source_obj);
>> >>>            pointer_adjuster->do_oop(&target_oop);
>> >>> +          oopDesc *target_obj = target_oop;
>> >>> +          if (!heap->is_in(target_oop)) {
>> >>> +            fprintf(stderr, "%p\n", source_obj);
>> >>> +            fprintf(stderr, "%p\n", target_obj);
>> >>> +          }
>> >>>            assert(heap->is_in(target_oop), "Adjustment failed");
>> >>> -          oopDesc *target_obj = target_oop;
>> >>>            new_map->put(target_obj, obj_size);
>> >>>
>> >>> 0xffea5570
>> >>> 0xf8238138
>> >>>
>> >>> On Wed, Jul 3, 2019 at 10:44 AM Arthur Eubanks <aeubanks at google.com>
>> >>> wrote:
>> >>>
>> >>> > Trying to implement finalizer support, I created a test (attached),
>> and
>> >>> > tsanOopMap.cpp seems buggy.
>> >>> >
>> >>> > # after -XX: or in .hotspotrc:  SuppressErrorAt=/tsanOopMap.cpp:294
>> >>> > #
>> >>> > # A fatal error has been detected by the Java Runtime Environment:
>> >>> > #
>> >>> > #  Internal Error
>> >>> >
>> >>>
>> (/usr/local/google/home/aeubanks/jdk/tsan/src/hotspot/share/tsan/tsanOopMap.cpp:294),
>> >>> > pid=16182, tid=16188
>> >>> > #  assert(heap->is_in(target_oop)) failed: Adjustment failed
>> >>> > #
>> >>> > # JRE version: OpenJDK Runtime Environment (13.0) (fastdebug build
>> >>> > 13-internal+0-adhoc.aeubanks.tsan)
>> >>> > # Java VM: OpenJDK 64-Bit Server VM (fastdebug
>> >>> > 13-internal+0-adhoc.aeubanks.tsan, interpreted mode, tiered,
>> compressed
>> >>> > oops, g1 gc, linux-amd64)
>> >>> > # Problematic frame:
>> >>> > # V  [libjvm.so+0x164e63b]
>> >>> >  TsanOopMapImpl::TsanOopSizeMap::collect_oops(BoolObjectClosure*,
>> >>> > OopClosure*, GrowableArray<TsanOopMapImpl::PendingMove>*, int*,
>> char**,
>> >>> > char**)+0x24b
>> >>> > #
>> >>> > # No core dump will be written. Core dumps have been disabled. To
>> >>> enable
>> >>> > core dumping, try "ulimit -c unlimited" before starting Java again
>> >>> > #
>> >>> > # An error report file with more information is saved as:
>> >>> > #
>> >>> >
>> >>>
>> /usr/local/google/home/aeubanks/jdk/tsan/build/test-support/jtreg_test_hotspot_jtreg_tsan_NonRacyFinalizerLoopTest_java/scratch/0/hs_err_pid16182.log
>> >>> > #
>> >>> > # If you would like to submit a bug report, please visit:
>> >>> > #   http://bugreport.java.com/bugreport/crash.jsp
>> >>> > #
>> >>> >
>> >>>
>> >>
>>
>
>
> --
>
> Thanks,
> Jc
>