RFR: 8318982: Improve Exceptions::special_exception [v2]

David Holmes dholmes at openjdk.org
Wed Nov 1 02:24:04 UTC 2023


On Tue, 31 Oct 2023 19:31:56 GMT, Doug Simon <dnsimon at openjdk.org> wrote:

>> This PR consolidates the 2 almost identical versions of `Exceptions::special_exception` into a single method.
>> If a special exception is thrown and `-Xlog:exceptions` is enabled, a log message is emitted and it indicates the special handling.
>> 
>> Here's an example in the output from running `compiler/linkage/LinkageErrors.java` with `-Xlog:exceptions -Xcomp`:
>> 
>> [0.194s][info][exceptions] Exception <java/lang/IllegalAccessError: class java.lang.module.ModuleDescriptor$1 tried to access private method 'void java.lang.module.ModuleDescriptor$Exports.<init>(java.util.Set, java.lang.String, java.util.Set, boolean)' (java.lang.module.ModuleDescriptor$1 and java.lang.module.ModuleDescriptor$Exports are in module java.base of loader 'bootstrap')> (0x0000000000000000)
>> thrown [src/hotspot/share/interpreter/linkResolver.cpp, line 591]
>> for thread 0x000000011e18c600
>> thread cannot call Java, throwing pre-allocated exception: a 'java/lang/VirtualMachineError'{0x0000000772e06f00}
>> 
>> 
>> The motivation for this change was work on [JDK-8318694](https://bugs.openjdk.org/browse/JDK-8318694) where it's useful to know when exceptions are thrown on a CompilerThread.
>
> Doug Simon has updated the pull request incrementally with one additional commit since the last revision:
> 
>   add missing ResourceMark

So to review this I had to mentally try to untangle the new combined version, to see if each potential path resulted in the same behaviour as before - that was/is extremely painful. I'm really not seeing the benefit of combining these as it just makes the code much more complicated. Sure there is a little duplication but each method was used in different circumstances. I think someone is going to look at this in the future and consider it an obvious candidate for splitting into two methods!

src/hotspot/share/utilities/exceptions.cpp line 85:

> 83: // Implementation of Exceptions
> 84: 
> 85: bool Exceptions::special_exception(JavaThread* thread, const char* file, int line, Handle h_exception, Symbol* h_name, const char* message) {

So IIUC now we either have a non-null exception and a null symbol and msg; or vice-versa. Can we assert that so someone doesn't mistakenly pass in non-null values for all three.

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

PR Review: https://git.openjdk.org/jdk/pull/16401#pullrequestreview-1707482098
PR Review Comment: https://git.openjdk.org/jdk/pull/16401#discussion_r1378328110


More information about the hotspot-dev mailing list