RFR (S) 8219459: oopDesc::is_valid() is broken

Thomas Stüfe thomas.stuefe at gmail.com
Mon May 13 19:29:15 UTC 2019


Hi Coleen,

On Mon, May 13, 2019, 21:15 <coleen.phillimore at oracle.com> wrote:

>
> Hi Thomas,
>
> On 5/13/19 2:07 PM, Thomas Stüfe wrote:
>
> Hi Coleen, Martin,
>
> I'm also fine with this patch. I agree this function is a bit overkillish.
>
> Also, in Klass::is_valid(), we do:
>
> if (!MetaspaceUtils::is_range_in_committed(k, k + 1)) return false;
>
> which is strict but then again not really correct, since we only do test
> range of the base Klass structure, excluding v/itable. We should probably
> check with k->size() range instead.
>
>
> I had a tested version that looked like this:
>
>   // Should only call unlocked Metaspace::contains when error is reported
> or Debugging
>   // but Shenandoah calls this before asserting.
>   // Lots of verifications call Metaspace::contains without the
> MetaspaceExpand_lock.
>   if (!Metaspace::contains(k) || !Metaspace::contains(k+(k->size()-1)))
> return false;
>
>
Looks fine to me (may need to cast k to address though and size is in words
I believe)


>
> Or, just do a Metaspace::contains() with the Klass pointer. I think the
> cases where it points to valid Metaspace but its end does not are probably
> rare.
>
>
> In the end, I wanted to go with the minimal change to get is_valid()
> working again, so that was what I changed.
>
> I could just take out is_range_in_committed() and have the line be if
> (!Metaspace::contains(k)) return false;
>

I would be fine with that but would like to hear Martins opinion first.


> for extra simplicity.  This oopDesc::oop_or_null (vs. is_oop_or_null) is
> only called during error reporting, so it doesn't need the
> MetaspaceExpand_lock, unless very rarely during crashing we're doing Full
> GC and shrinking the metaspace.
>
> Thanks,
> Coleen
>

Thanks, Thomas


>
>
> Coleen
>
>
> Cheers, Thomas
>
>
> On Wed, May 8, 2019 at 11:45 PM <coleen.phillimore at oracle.com> wrote:
>
>> Summary: Use Metaspace::contains() to test address ranges.
>>
>> Tested with hs-tier1-3, and all tier1 on linux-x64-debug.  Tested
>> manually in gdb.
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/2019/8219459.01/webrev
>> bug link https://bugs.openjdk.java.net/browse/JDK-8219459
>>
>> Thanks,
>> Coleen
>>
>
>


More information about the hotspot-runtime-dev mailing list