JDK-7030435 : Some oop_oop_iterate_m() methods iterate outside of specified memory bounds
  • Type: Bug
  • Component: hotspot
  • Sub-Component: gc
  • Affected Version: 7
  • Priority: P4
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2011-03-23
  • Updated: 2011-04-24
  • Resolved: 2011-04-24
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 7 Other
7Fixed hs21Fixed
Description
Examples are:-

arrayKlassKlass::oop_oop_iterate_m
constantPoolKlass::oop_oop_iterate_m
objArrayKlassKlass::oop_oop_iterate_m

These are all in the perm gen, so this will likely be
moot when the perm gen goes away.

Comments
EVALUATION http://hg.openjdk.java.net/jdk7/hotspot-gc/hotspot/rev/a0de1dfd1933
25-03-2011

EVALUATION Part of the changes in the suggested fix section were independently putback by Tom when he did the statics in class work for perm gen removal, so the only changes that will go out as part of this CR will be those for constantPoolKlass.
24-03-2011

EVALUATION Don't iterate outside specified interval.
24-03-2011

WORK AROUND The anomaly was discovered as a result of a somewhat paranoid assert I added for the changes for 7029036. The extra iteration of these perm gen objects is likely not fatal since they usually point to objects that are themselves in perm, so no secondary asserts will fire or code misbehave as a result of the extra work. However, avoiding the extra work and making the interfaces clean and uniform would be good.
23-03-2011

SUGGESTED FIX Check memory bounds before applying the closure to the refs:- diff --git a/src/share/vm/oops/arrayKlassKlass.cpp b/src/share/vm/oops/arrayKlassKlass.cpp --- a/src/share/vm/oops/arrayKlassKlass.cpp +++ b/src/share/vm/oops/arrayKlassKlass.cpp @@ -104,9 +104,13 @@ int arrayKlassKlass::oop_oop_iterate_m(oop obj, OopClosure* blk, MemRegion mr) { assert(obj->is_klass(), "must be klass"); arrayKlass* ak = arrayKlass::cast(klassOop(obj)); - blk->do_oop(ak->adr_component_mirror()); - blk->do_oop(ak->adr_lower_dimension()); - blk->do_oop(ak->adr_higher_dimension()); + oop* addr; + addr = ak->adr_component_mirror(); + if (mr.contains(addr)) { blk->do_oop(addr); } + addr = ak->adr_lower_dimension(); + if (mr.contains(addr)) { blk->do_oop(addr); } + addr = ak->adr_higher_dimension(); + if (mr.contains(addr)) { blk->do_oop(addr); } ak->vtable()->oop_oop_iterate_m(blk, mr); return klassKlass::oop_oop_iterate_m(obj, blk, mr); } diff --git a/src/share/vm/oops/constantPoolKlass.cpp b/src/share/vm/oops/constantPoolKlass.cpp --- a/src/share/vm/oops/constantPoolKlass.cpp +++ b/src/share/vm/oops/constantPoolKlass.cpp @@ -245,13 +245,13 @@ } oop* addr; addr = cp->tags_addr(); - blk->do_oop(addr); + if (mr.contains(addr)) blk->do_oop(addr); addr = cp->cache_addr(); - blk->do_oop(addr); + if (mr.contains(addr)) blk->do_oop(addr); addr = cp->operands_addr(); - blk->do_oop(addr); + if (mr.contains(addr)) blk->do_oop(addr); addr = cp->pool_holder_addr(); - blk->do_oop(addr); + if (mr.contains(addr)) blk->do_oop(addr); return size; } diff --git a/src/share/vm/oops/objArrayKlassKlass.cpp b/src/share/vm/oops/objArrayKlassKlass.cpp --- a/src/share/vm/oops/objArrayKlassKlass.cpp +++ b/src/share/vm/oops/objArrayKlassKlass.cpp @@ -236,7 +236,7 @@ addr = oak->bottom_klass_addr(); if (mr.contains(addr)) blk->do_oop(addr); - return arrayKlassKlass::oop_oop_iterate(obj, blk); + return arrayKlassKlass::oop_oop_iterate_m(obj, blk, mr); } #ifndef SERIALGC
23-03-2011