RFR: 8368787: Error reporting: hs_err files should show instructions when referencing code in nmethods

[Martin Doerr]

On Mon, 6 Oct 2025 13:18:01 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Thanks for looking at this PR!
>> Hotspot currently dumps code (hex or disassembled) when the nmethod is on stack of the crashing thread. That is completely missing when it's not on stack.
>> 
>> Should we print both, hex dump and disassembly?
>> 
>> Interesting. I haven't tried with ZGC. Did you find more relocations which don't point to an instruction start?
>> We could ignore relocations with format `ZBarrierRelocationFormatStoreGoodAfterMov` on x86. (Or find the correct start in this case.) E.g. we could fix it like this:
>> 
>> diff --git a/src/hotspot/share/code/codeBlob.cpp b/src/hotspot/share/code/codeBlob.cpp
>> index 6511b4689ed..2e4d49a81c1 100644
>> --- a/src/hotspot/share/code/codeBlob.cpp
>> +++ b/src/hotspot/share/code/codeBlob.cpp
>> @@ -52,6 +52,9 @@
>>  #ifdef COMPILER1
>>  #include "c1/c1_Runtime1.hpp"
>>  #endif
>> +#if defined(AMD64) && INCLUDE_ZGC
>> +#include "gc/z/zBarrierSetAssembler.hpp"
>> +#endif
>>  
>>  #include <type_traits>
>>  
>> @@ -919,6 +922,10 @@ void CodeBlob::dump_for_addr(address addr, outputStream* st, bool verbose) const
>>          // disassemble correctly at instruction start addresses.)
>>          RelocIterator iter(nm, start);
>>          while (iter.next() && iter.addr() < addr) { // find relocation before addr
>> +#if defined(AMD64) && INCLUDE_ZGC
>> +          // There's a relocation which doesn't point to an instruction start:
>> +          if ((iter.type() != relocInfo::barrier_type) || (iter.format() != ZBarrierRelocationFormatStoreGoodAfterMov))
>> +#endif
>>            start = iter.addr();
>>          }
>>          if (iter.has_current()) {
>
>> Hotspot currently dumps code (hex or disassembled) when the nmethod is on stack of the crashing thread. That is completely missing when it's not on stack. [...] Should we print both, hex dump and disassembly?
> 
> Yes, I think if we know the location is within nmethod, it makes sense to dump around the location. 
> 
> I think hex dump is most bullet-proof, as we can always disassemble offline it at different offsets. I don't think we want to specialize for reloc types, it does not gain us much? Also, relocs solve the variable-sized encoding only if you are lucky to hit the reloc right at the location you are decoding, right? Anything in between relocs is still pretty foggy. I suspect current patch would work in 99% of the cases, as it is hard to imagine e.g. the value in the register that points into nmethod and _does not_ have some sort of reloc.
> 
> Then I also suspect that disassemblers actually able to figure the instruction boundaries pretty well? Because I don't quite see how our usual printout of `decode(pc - 64, pc + 64)` would otherwise work: `pc-64` starts at arbitrary boundary. You might want to check if this whole reloc thing is even needed. What happens if we just do `Disassembler::decode(MAX2(nm->entry_point(), addr - 64), MIN2(nm->code_end(), addr + 64))`?

@shipilev: I have made some improvements after your feedback. Please take another look! Thanks!

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

PR Comment: https://git.openjdk.org/jdk/pull/27530#issuecomment-3380913610