RFR: 8318462: [GenShen] Prevent unsafe access to displaced mark-word [v3]
Kelvin Nilsen
kdnilsen at openjdk.org
Tue Oct 31 23:24:33 UTC 2023
On Tue, 31 Oct 2023 22:33:16 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:
>> Thanks for change. Code might read easier with parens around (age == markWord::max_age + 1), but ok as is.
>
> The add method of ShenandoahAgeCensus explicitly recognizes and uses the sentinel value, and explicitly skips sentinels that are used to signal that the age could not be read:
>
>
> CENSUS_NOISE(void ShenandoahAgeCensus::add(uint obj_age, uint region_age, uint region_youth, size_t size, uint worker_id) {)
> NO_CENSUS_NOISE(void ShenandoahAgeCensus::add(uint obj_age, uint region_age, size_t size, uint worker_id) {)
> if (obj_age <= markWord::max_age) {
> assert(obj_age < MAX_COHORTS && region_age < MAX_COHORTS, "Should have been tenured");
> #ifdef SHENANDOAH_CENSUS_NOISE
> // Region ageing is stochastic and non-monotonic; this vitiates mortality
> // demographics in ways that might defeat our algorithms. Marking may be a
> // time when we might be able to correct this, but we currently do not do
> // this. Like skipped statistics further below, we want to track the
> // impact of this noise to see if this may be worthwhile. JDK-<TBD>.
> uint age = obj_age;
> if (region_age > 0) {
> add_aged(size, worker_id); // this tracking is coarse for now
> age += region_age;
> if (age >= MAX_COHORTS) {
> age = (uint)(MAX_COHORTS - 1); // clamp
> add_clamped(size, worker_id);
> }
> }
> if (region_youth > 0) { // track object volume with retrograde age
> add_young(size, worker_id);
> }
> #else // SHENANDOAH_CENSUS_NOISE
> uint age = MIN2(obj_age + region_age, (uint)(MAX_COHORTS - 1)); // clamp
> #endif // SHENANDOAH_CENSUS_NOISE
> get_local_age_table(worker_id)->add(age, size);
> } else {
> // update skipped statistics
> CENSUS_NOISE(add_skipped(size, worker_id);)
> }
> }
>
>
> I think we should leave in place the original code, where all of the special handling of the sentinel values is relegated to the ShenandoahAgeCensus's add method, with no special handling needed at the caller.
Thanks. That looks good. I concur.
-------------
PR Review Comment: https://git.openjdk.org/shenandoah/pull/343#discussion_r1378252443
More information about the shenandoah-dev
mailing list