RFR: 8260341: CDS dump VM init code does not check exceptions

[David Holmes]

On 11/02/2021 4:13 am, Ioi Lam wrote:
> On Wed, 10 Feb 2021 14:33:50 GMT, Harold Seigel <hseigel at openjdk.org> wrote:
> 
>> Hi Ioi,
>> Do you avoid using CHECK if it's the last line of a function? For example, why is THREAD used instead of CHECK at line 1506?
>> Thanks, Harold
>>
>> 1503 void ClassLoader::initialize_module_path(TRAPS) {
>> 1504 if (Arguments::is_dumping_archive()) {
>> 1505 ClassLoaderExt::setup_module_paths(CHECK);
>> 1506 FileMapInfo::allocate_shared_path_table(THREAD);
>> 1507 }
>> 1508 }
> 
> I thought it was a commonly used coding convention, but I could only find a few cases where the code wasn't written by me :-(

It is something we have been gradually fixing. I'm made a number of 
requests to remove CHECK from code that will return anyway.

> - https://github.com/openjdk/jdk/blame/b9d4211bc1aa92e257ddfe86c7a2b4e4e60598a0/src/hotspot/share/prims/jvm.cpp#L311
> - https://github.com/openjdk/jdk/blame/f03e839e481f905358ce7d95a5d1f5179e7f46fe/src/hotspot/share/classfile/javaClasses.cpp#L2415
> 
> I will go back to `CHECK`, since the C++ compiler will elide the last `CHECK` anyway: in both cases, gcc compiles the last call to a direct branch to FileMapInfo::allocate_shared_path_table (i.e., a tail call).

Do all our C++ compilers do this? If they do that is great, but if they 
only may perhaps sometimes do then not so great.

> Using `CHECK` makes the code easier to maintain (if you add new code below it).

I prefer not to rely on the C++ compiler to remove the exception 
checking logic and use THREAD in such cases.

David
-----

> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/2494
>