RFR: 8318462: [GenShen] Prevent unsafe access to displaced mark-word [v3]

[Y. Srinivas Ramakrishna]

On Tue, 31 Oct 2023 16:17:01 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

>> I think the safe thing to do is treat max_age+1 as if it were 0, insofar as promotion semantics is concerned.  We may delay promotion that way.
>
> 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.

-------------

PR Review Comment: https://git.openjdk.org/shenandoah/pull/343#discussion_r1378223807