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

Christian Hagedorn chagedorn at openjdk.org
Tue Oct 11 08:18:11 UTC 2022


On Thu, 22 Sep 2022 11:55:56 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Review comments from Thomas
>
> 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.

Sorry for the delay to come back to this. I think you are right. We should always first strip the prefix path and only then cut the filename to fit it into the `filename` output buffer. I've pushed an update that with a temporary buffer of size 1024 and reverted the gtest changes. Now it works with Clang and GCC without modifying the test.

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

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


More information about the hotspot-dev mailing list