On Sat, 3 Sep 2022 07:48:50 GMT, Dean Long <dl...@openjdk.org> wrote:
>> John R Rose has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains two additional >> commits since the last revision: >> >> - Merge branch 'master' of https://git.openjdk.org/jdk into >> compressed-stream >> - 8292758: put support for UNSIGNED5 format into its own header file > > Wouldn't the SA stack walk fail when attaching to a core dump from an earlier > JVM that does not exclude nulls? @dean-long and @coleenp Thank you for the reviews. The public functions in `UNSIGNED5` are not internal but are designed to be useful by themselves. That is why the gtest tests them separately. For example, `check_length` is used inside the `Reader` but if you are building your own reader-like logic, you might want to use it to avoid buffer overflows. Likewise `fits_in_limit` is used by the sizing logic of `write_uint_grow` but if you are rolling your own auto-grow logic, you would want to call that function on its own, or maybe `encoded_length`. The fancy API points like `Reader` and `Sizer` and `write_uint_grow` are not intended to be primitives, but rather best practices for using the static functions, which are the real primitives. Testing notes: This change passes a targeted test that runs `test/hotspot/jtreg/compiler/c2/Test6*.java` (about 50 tests) with `CONF=fastdebug` and `JTREG="JAVA_OPTIONS=-Xcomp -XX:+TraceDeoptimization"`. That run is chosen to deoptimize many times (about 39k), so as to exercise the pre-existing stack walk logic, which relies on compressed streams. Fault injection confirms that any JVM run crashes immediately if there is a bug in the encoding. It also passes tiers 1/2/3 (in a slightly earlier version) and all gtests (in the latest version). ------------- PR: https://git.openjdk.org/jdk/pull/10067
