RFR: 8293422: DWARF emitted by Clang cannot be parsed [v3]

[Thomas Schatzl]

On Wed, 21 Sep 2022 13:20:53 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> The DWARF debugging symbols emitted by Clang is different from what GCC is emitting. While GCC produces a complete `.debug_aranges` section (which is required in the DWARF parser), Clang does not. As a result, the DWARF parser cannot find the necessary information to proceed and create the line number information:
>> 
>> The `.debug_aranges` section contains address range to compilation unit offset mappings. The parsing algorithm can just walk through all these entries to find the correct address range that contains the library offset of the current pc. This gives us the compilation unit offset into the `.debug_info` section from where we can proceed to parse the line number information.
>> 
>> Without a complete `.debug_aranges` section, we fail with an assertion that we could not find the correct entry. Since [JDK-8293402](https://bugs.openjdk.org/browse/JDK-8293402), we will still get the complete stack trace at least. Nevertheless, we should still fix this assertion failure of course. But that would require a different parsing approach. We need to parse the entire `.debug_info` section instead to get to the correct compilation unit. This, however, would require a lot more work. 
>> 
>> I therefore suggest to disable DWARF parsing for Clang for now and file an RFE to support Clang in the future with a different parsing approach. I'm using the `__clang__` `ifdef` to bail out in `get_source_info()` and disable the `gtests`. I've noticed that we are currently running the `gtests` with `NOT PRODUCT` which I think is not necessary - the gtests should also work fine with product builds. I've corrected this as well but that could also be done separately.
>> 
>> Thanks,
>> Christian
>
> Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review comments from Thomas

I'll leave it up to you what to do about the path stripping problem mentioned in the comment for `DwarfFile::LineNumberProgram::get_filename_from_header`.

src/hotspot/share/utilities/elfFile.cpp line 1613:

> 1611:     if (current_index == file_index) {
> 1612:       // Found correct file.
> 1613:       strip_path_prefix(filename, filename_len);

After some digging I believe this is the wrong place to strip the path prefix, and causes the strange workaround in the `decoder_get_source_info_valid_truncated` gtest.

The call to `_reader.read_string()` above should do the stripping as it is read; if like in the gtest, the given `filename_len` is too small to contain the original string, the `strip_path_prefix` tries to strip the too small buffer, but what has actually been intended was probably stripping the entire path and then limiting the return value.

I.e. a more useful implementation of this would be reading the string into a temporary buffer, stripping the path prefix and then copying the result to the output buffer.

I can see the reasoning for why the current implementation is as is, it is nowhere specified what the contents of the filename string in `debug_aranges` should be, and what should be actually printed.

Looking at callers of this method, it might actually a problem when using clang: `VMError::print_native_stack` uses a 128 byte buffer, `NativeCallStack::print_on` uses a 1024 byte buffer.

I can see that at least 128 bytes would be just a bit small, but then we (Oracle) do not use clang. It's up to you to fix this imo.

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

Marked as reviewed by tschatzl (Reviewer).

PR: https://git.openjdk.org/jdk/pull/10287