RFR: 8265484: Fix up TRAPS usage in GenerateOopMap::compute_map and callers [v2]

[David Holmes]

Hi Dean,

Thanks for looking at this.

On 21/04/2021 1:23 pm, Dean Long wrote:
> On Tue, 20 Apr 2021 22:04:35 GMT, David Holmes <dholmes at openjdk.org> wrote:
> 
>>> If compute_map encounters malformed bytecode, or an OOM condition then it can raise a LinkageError to be thrown in regular JavaThreads, and this is reflected in the use of TRAPS in a couple of methods. But for non-JavaThreads, and compiler threads, the occurrence of such an error is treated as a fatal error and we abort the VM.
>>>
>>> In addition, most of the callers of compute_map actually use the CATCH macro, which just clears the exception and will abort a non-product VM.
>>>
>>> This use of TRAPS is a problem for the change to convert TRAPS to JavaThread, because these methods are called by non-JavaThreads. To address this we change compute_map to create, but not throw, the exception and return a bool indicating whether an error occurred, and the caller can then choose to throw the exception itself.
>>>
>>> Testing: tiers 1-3
>>>
>>> Thanks,
>>> David
>>
>> David Holmes has updated the pull request incrementally with one additional commit since the last revision:
>>
>>    Additional comments on failure modes
> 
> If this gets backported to an older jdk where CATCH called ShouldNotReachHere() instead of assert(), then it will be a change in behavior, right?

Right - though there is really no direct reason to backport such 
cleanups. Also Ioi and I were discussing that perhaps we should revert 
the current version of CATCH so it once again aborts in the product VM, 
and then fix up call-sites like these. But these call-sites would 
ideally properly deal with exceptions in regular JavaThreads - somehow.

> What's the reason for moving the exception throwing into the caller?  Is it necessary or just an optimization?

Necessary to get TRAPS out of the method signatures because I'm changing 
TRAPS to be JavaThread (in another issue) and compute_map is not always 
called on a JavaThread.

Thanks,
David

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