RFR: 8283725: Launching java with "-Xlog:gc*=trace,safepoint*=trace,class*=trace" crashes the JVM

[Ioi Lam]

On Mon, 28 Mar 2022 01:33:51 GMT, David Holmes <dholmes at openjdk.org> wrote:

> I think the loop termination condition in `LogOutput::update_config_string` is not quite correct. We process "deviating tagsets" until there are no more - tracking `n_deviants` and `n_selections`. However, we can reach the case where `n_deviants == 1` and there are no further selections possible - `add_selections` finds no more subsets for the given tagset. This causes the guarantee to fire as it expects to see selections as long as we still have deviations. We can fix this by adding a new check at the bottom of the loop:
> 
>     if (n_deviates == 1 && n_selections == 0) {
>       // we're done as we couldn't reduce things any further
>       break;
>     }
> 
> I do not know what a "deviating tagset" means so it is unclear whether the bug is not checking for this "1 and 0" case, or whether the bug is that we got that final 0. Hopefully someone else may be able to expand on that.
> 
> Testing:
> - tiers 1-3

Marked as reviewed by iklam (Reviewer).

The fix looks harmless. Since it fixes the immediate problem, I think we can integrate it.

However, it seems to me that the code is hard to understand. More comments (and perhaps refactoring/rewrite) would be beneficial in a separate RFE.

For example, there are many access to arrays without checking/asserting the range. These are quite worrying:


  size_t deviating_tagsets = LogTagSet::ntagsets() - max;
  ...
  const LogTagSet** deviates = NEW_C_HEAP_ARRAY(const LogTagSet*, deviating_tagsets, mtLogging);
  for (LogTagSet* ts = LogTagSet::first(); ts != NULL; ts = ts->next()) {
    LogLevelType level = ts->level_for(this);
    if (level == mcl) {
      continue;
    }
    deviates[n_deviates++] = ts;   //<<<<<< here

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

PR: https://git.openjdk.java.net/jdk/pull/7978