RFR: 8252148: vmError::controlled_crash should be #ifdef ASSERT [v2]
Coleen Phillimore
coleenp at openjdk.java.net
Fri Dec 11 16:17:58 UTC 2020
On Fri, 11 Dec 2020 08:29:11 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:
>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Fixed va_copy problem, and added changes from pull request.
>
> Hi Coleen,
>
> this is a good cleanup, I like this a lot.
>
> About moving ErrorHandlerTest.java to gtest: this is a slick way of doing these tests. However, some things to consider:
> - Strictly speaking we now do not test the real error handler output but the artificial output `ExecutingUnitTests` gives us. The real printing code is now untested. This may have bitten you right here, since I believe in `print_error_for_unit_test` you miss a `va_copy` which should have messed up detail_args and garbled the real output later on.
> - moving these tests out of jtreg means we give up flexibility (excluding them or moving them to another tier). Do they impact the gtest run time noticeably?
>
> There are also remaining problems with gtest death tests, which I hope we can fix at some point. I don't hold my breath though since we decided to not own the gtest code and cannot make modifications:
> - Death tests are easily confused by stderr output. See: https://bugs.openjdk.java.net/browse/JDK-8257229. So whenever we have unconditional stderr output in the VM (eg if some large page alloc failed) these death tests fail too.
> - Death tests don't tell you the assert message which happened if it did not match their expectations. Thats plain annoying. See https://bugs.openjdk.java.net/browse/JDK-8257227.
I posted a comment on https://bugs.openjdk.java.net/browse/JDK-8257227. I don't know how to deal with https://bugs.openjdk.java.net/browse/JDK-8257229. These tests would fail because of that also. The assert message tests are nice and imo better than having crashing jtreg tests, but they have these limitations.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1723
More information about the hotspot-dev
mailing list