[14] RFR(S): 8225653: Provide more information when hitting SIGILL from HaltNode

[Christian Hagedorn]

Hi Vladimir

I updated the webrev choosing the option to use MacroAssembler::stop() 
instead which requires less code changes:
http://cr.openjdk.java.net/~chagedorn/8225653/webrev.01/

Best regards,
Christian

On 05.09.19 18:04, Christian Hagedorn wrote:
> Hi Vladimir
> 
> Thank you for your review!
> 
> On 05.09.19 16:11, Vladimir Ivanov wrote:
>> What do you think about reusing MacroAssembler::stop()? Otherwise, you 
>> have to add MacroAssembler::halt for x86-32 as well.
>>
> 
> As we have discussed, this is another option to use 
> MacroAssembler::stop() by changing the assert in 
> MacroAssembler::debug32/64 into a guarantee() or fatal() to also crash 
> with a message in a product build. This change should be possible as 
> debug32/64 is currently only used in debug mode. I think choosing 
> fatal() would be better because the halt reason will then show up in the 
> hs_err file.
> 
> Which option is preferable?
> 
> Best regards,
> Christian
> 
>> On 04/09/2019 10:43, Christian Hagedorn wrote:
>>> Hi
>>>
>>> Please review the following patch:
>>> https://bugs.openjdk.java.net/browse/JDK-8225653
>>> http://cr.openjdk.java.net/~chagedorn/8225653/webrev.00/
>>>
>>> This enhancement provides more information about executing a HaltNode 
>>> on x86. Up to now, an illegal instruction was emitted for a HaltNode 
>>> which resulted in a crash with SIGILL at runtime if executed. This 
>>> made it hard to determine if the crash was actually caused by a 
>>> HaltNode or not without doing a closer analysis.
>>>
>>> This patch adds a string to each HaltNode stating the purpose of it. 
>>> Code generation for a HaltNode adds a runtime call which executes 
>>> fatal() with the newly provided HaltNode string. The VM then crashes 
>>> with this additional information which should make a further analysis 
>>> easier. I filed an RFE [1] to also provide this information for 
>>> architectures other than x86.
>>>
>>> The assert at [2] is not necessary anymore since there is a new 
>>> DEFINE_CLASS_ID entry for HaltNode with ID 15. ClassMask_Halt now 
>>> equals USHORT_MAX, making the assert always true. It might be a good 
>>> idea to file an RFE to change the current encoding as the next entry 
>>> with ID 16 will exceed the 16 bit limit.
>>>
>>> Thank you!
>>>
>>> Best regards,
>>> Christian
>>>
>>>
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8230552
>>> [2] 
>>> http://hg.openjdk.java.net/jdk/jdk/file/d8f22418ca99/src/hotspot/share/opto/node.hpp#l750 
>>>