Crash on finalizer in tsanOopMap

[Arthur Eubanks]

LGTM

On Fri, Jul 12, 2019 at 9:19 AM Man Cao <manc at google.com> wrote:

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