Skip to content

Commit

Permalink
8293422: DWARF emitted by Clang cannot be parsed
Browse files Browse the repository at this point in the history
Reviewed-by: tschatzl, ihse, stuefe
  • Loading branch information
chhagedorn committed Nov 21, 2022
1 parent 59d8f67 commit 8b8d848
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 49 deletions.
7 changes: 6 additions & 1 deletion make/autoconf/flags-cflags.m4
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,12 @@ AC_DEFUN([FLAGS_SETUP_DEBUG_SYMBOLS],
)
fi
CFLAGS_DEBUG_SYMBOLS="-g"
# -gdwarf-4 and -gdwarf-aranges were introduced in clang 5.0
GDWARF_FLAGS="-gdwarf-4 -gdwarf-aranges"
FLAGS_COMPILER_CHECK_ARGUMENTS(ARGUMENT: [${GDWARF_FLAGS}],
IF_FALSE: [GDWARF_FLAGS=""])
CFLAGS_DEBUG_SYMBOLS="-g ${GDWARF_FLAGS}"
ASFLAGS_DEBUG_SYMBOLS="-g"
elif test "x$TOOLCHAIN_TYPE" = xxlc; then
CFLAGS_DEBUG_SYMBOLS="-g1"
Expand Down
11 changes: 10 additions & 1 deletion src/hotspot/share/utilities/decoder_elf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,12 @@ bool ElfDecoder::decode(address addr, char *buf, int buflen, int* offset, const
}

bool ElfDecoder::get_source_info(address pc, char* filename, size_t filename_len, int* line, bool is_pc_after_call) {
assert(filename != nullptr && filename_len > 0 && line != nullptr, "Argument error");
#if defined(__clang_major__) && (__clang_major__ < 5)
DWARF_LOG_ERROR("The DWARF parser only supports Clang 5.0+.");
return false;
#else
assert(filename != nullptr && line != nullptr, "arguments should not be null");
assert(filename_len > 1, "buffer must be able to at least hold 1 character with a null terminator");
filename[0] = '\0';
*line = -1;

Expand Down Expand Up @@ -83,13 +88,17 @@ bool ElfDecoder::get_source_info(address pc, char* filename, size_t filename_len
unsigned_offset_in_library, filepath);

if (!file->get_source_info(unsigned_offset_in_library, filename, filename_len, line, is_pc_after_call)) {
// Return sane values.
filename[0] = '\0';
*line = -1;
return false;
}

DWARF_LOG_SUMMARY("pc: " PTR_FORMAT ", offset: " INT32_FORMAT_X_0 ", filename: %s, line: %u",
p2i(pc), offset_in_library, filename, *line);
DWARF_LOG_INFO("") // To structure the debug output better.
return true;
#endif // clang
}


Expand Down
99 changes: 77 additions & 22 deletions src/hotspot/share/utilities/elfFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ bool ElfFile::DwarfFilePath::set(const char* src) {
}

bool ElfFile::DwarfFilePath::set_after_last_slash(const char* src) {
char* last_slash = strrchr(_path, '/');
char* last_slash = strrchr(_path, *os::file_separator());
if (last_slash == nullptr) {
// Should always find a slash.
return false;
Expand Down Expand Up @@ -1201,7 +1201,7 @@ bool DwarfFile::LineNumberProgram::read_header() {
return false;
}

if (!_reader.read_sbyte(&_header._line_base)) {
if (!_reader.read_byte(&_header._line_base)) {
return false;
}

Expand Down Expand Up @@ -1608,14 +1608,13 @@ bool DwarfFile::LineNumberProgram::get_filename_from_header(const uint32_t file_
_reader.set_position(_header._file_names_offset);
uint32_t current_index = 1; // file_names start at index 1
while (_reader.has_bytes_left()) {
if (!_reader.read_string(filename, filename_len)) {
// Either an error while reading or we have reached the end of the file_names. Both should not happen.
return false;
}

if (current_index == file_index) {
// Found correct file.
return true;
return read_filename(filename, filename_len);
} else if (!_reader.read_string()) { // We don't care about this filename string. Read and ignore it.
// Either an error while reading or we have reached the end of the file_names section before reaching the file_index.
// Both should not happen.
return false;
}

// We don't care about these values.
Expand All @@ -1630,6 +1629,64 @@ bool DwarfFile::LineNumberProgram::get_filename_from_header(const uint32_t file_
return false;
}

// Read the filename into the provided 'filename' buffer. If it does not fit, an alternative smaller tag will be emitted
// in order to let the DWARF parser succeed. The line number with a function name will almost always be sufficient to get
// to the actual source code location.
bool DwarfFile::LineNumberProgram::read_filename(char* filename, const size_t filename_len) {
char next_char;
if (!_reader.read_non_null_char(&next_char)) {
// Either error while reading or read an empty string which indicates the end of the file_names section.
// Both should not happen.
return false;
}

filename[0] = next_char;
size_t index = 1;
bool overflow_filename = false; // Is the currently read filename overflowing the provided 'filename' buffer?
while (next_char != '\0' && _reader.has_bytes_left()) {
if (!_reader.read_byte(&next_char)) {
return false;
}
if (next_char == *os::file_separator()) {
// Skip file separator to get to the actual filename and reset the buffer and overflow flag. GCC does not emit
// file separators while Clang does.
index = 0;
overflow_filename = false;
} else if (index == filename_len) {
// Just keep reading as we could read another file separator and reset the buffer again. But don't bother to store
// the additionally read characters as it would not fit into the buffer anyway.
overflow_filename = true;
} else {
assert(!overflow_filename, "sanity check");
filename[index] = next_char;
index++;
}
}

if (overflow_filename) {
// 'filename' buffer overflow. Store either a generic overflow message or a minimal filename.
write_filename_for_overflow(filename, filename_len);
}
return true;
}

// Try to write a generic overflow message to the provided buffer. If it does not fit, store the minimal filename "L"
// which always fits to get the source information in the form "L:line_number".
void DwarfFile::LineNumberProgram::write_filename_for_overflow(char* filename, const size_t filename_len) {
DWARF_LOG_ERROR("DWARF filename string is too large to fit into the provided buffer of size %zu.", filename_len);
const size_t filename_overflow_message_length = strlen(overflow_filename) + 1;
if (filename_overflow_message_length <= filename_len) {
jio_snprintf(filename, filename_overflow_message_length, "%s", overflow_filename);
DWARF_LOG_ERROR("Use overflow filename: %s", overflow_filename);
} else {
// Buffer too small of generic overflow message.
DWARF_LOG_ERROR("Too small for overflow filename, use minimal filename: %c", minimal_overflow_filename);
assert(filename_len > 1, "sanity check");
filename[0] = minimal_overflow_filename;
filename[1] = '\0';
}
}

void DwarfFile::LineNumberProgram::LineNumberProgramState::reset_fields() {
_address = 0;
_op_index = 0;
Expand Down Expand Up @@ -1700,12 +1757,7 @@ bool DwarfFile::MarkedDwarfFileReader::move_position(const long offset) {
return set_position(_current_pos + offset);
}

bool DwarfFile::MarkedDwarfFileReader::read_sbyte(int8_t* result) {
_current_pos++;
return read(result, 1);
}

bool DwarfFile::MarkedDwarfFileReader::read_byte(uint8_t* result) {
bool DwarfFile::MarkedDwarfFileReader::read_byte(void* result) {
_current_pos++;
return read(result, 1);
}
Expand Down Expand Up @@ -1774,13 +1826,8 @@ bool DwarfFile::MarkedDwarfFileReader::read_sleb128(int64_t* result, const int8_

// If result is a nullptr, we do not care about the content of the string being read.
bool DwarfFile::MarkedDwarfFileReader::read_string(char* result, const size_t result_len) {
uint8_t next_byte;
if (!read_byte(&next_byte)) {
return false;
}

if (next_byte == 0) {
// Strings must contain at least one non-null byte.
char first_char;
if (!read_non_null_char(&first_char)) {
return false;
}

Expand All @@ -1789,9 +1836,10 @@ bool DwarfFile::MarkedDwarfFileReader::read_string(char* result, const size_t re
// Strings must contain at least one non-null byte and a null byte terminator.
return false;
}
result[0] = (char)next_byte;
result[0] = first_char;
}

uint8_t next_byte;
size_t char_index = 1;
bool exceeded_buffer = false;
while (has_bytes_left()) {
Expand Down Expand Up @@ -1821,4 +1869,11 @@ bool DwarfFile::MarkedDwarfFileReader::read_string(char* result, const size_t re
return false;
}

bool DwarfFile::MarkedDwarfFileReader::read_non_null_char(char* result) {
if (!read_byte(result)) {
return false;
}
return *result != '\0';
}

#endif // !_WINDOWS && !__APPLE__
11 changes: 8 additions & 3 deletions src/hotspot/share/utilities/elfFile.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -421,8 +421,7 @@ class DwarfFile : public ElfFile {
// Must be called to restore the old position before this file reader changed it with update_to_stored_position().
bool reset_to_previous_position();
bool move_position(long offset);
bool read_sbyte(int8_t* result);
bool read_byte(uint8_t* result);
bool read_byte(void* result);
bool read_word(uint16_t* result);
bool read_dword(uint32_t* result);
bool read_qword(uint64_t* result);
Expand All @@ -432,6 +431,7 @@ class DwarfFile : public ElfFile {
// Reads 4 bytes for 32-bit and 8 bytes for 64-bit builds.
bool read_address_sized(uintptr_t* result);
bool read_string(char* result = nullptr, size_t result_len = 0);
bool read_non_null_char(char* result);
};

// (2) Processing the .debug_aranges section to find the compilation unit which covers offset_in_library.
Expand Down Expand Up @@ -718,6 +718,9 @@ class DwarfFile : public ElfFile {
static constexpr uint8_t DW_LNE_define_file = 3;
static constexpr uint8_t DW_LNE_set_discriminator = 4; // Introduced with DWARF 4

static constexpr const char* overflow_filename = "<OVERFLOW>";
static constexpr const char minimal_overflow_filename = 'L';

// The header is defined in section 6.2.4 of the DWARF 4 spec.
struct LineNumberProgramHeader {
// The size in bytes of the line number information for this compilation unit, not including the unit_length
Expand Down Expand Up @@ -862,10 +865,12 @@ class DwarfFile : public ElfFile {
bool apply_opcode();
bool apply_extended_opcode();
bool apply_standard_opcode(uint8_t opcode);
void apply_special_opcode(const uint8_t opcode);
void apply_special_opcode(uint8_t opcode);
bool does_offset_match_entry(uintptr_t previous_address, uint32_t previous_file, uint32_t previous_line);
void print_and_store_prev_entry(uint32_t previous_file, uint32_t previous_line);
bool get_filename_from_header(uint32_t file_index, char* filename, size_t filename_len);
bool read_filename(char* filename, size_t filename_len);
static void write_filename_for_overflow(char* filename, size_t filename_len) ;

public:
LineNumberProgram(DwarfFile* dwarf_file, uint32_t offset_in_library, uint64_t debug_line_offset, bool is_pc_after_call)
Expand Down
52 changes: 30 additions & 22 deletions test/hotspot/gtest/runtime/test_os_linux.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "runtime/os.hpp"
#include "concurrentTestRunner.inline.hpp"
#include "os_linux.hpp"
#include "prims/jniCheck.hpp"
#include "unittest.hpp"
#include "utilities/align.hpp"
#include "utilities/decoder.hpp"
Expand Down Expand Up @@ -437,28 +438,14 @@ TEST(os_linux, addr_to_function_valid) {
ASSERT_TRUE(offset >= 0);
}

#ifndef PRODUCT
// Test valid address of method JNI_CreateJavaVM in jni.cpp. We should get "jni.cpp" in the buffer and a valid line number.
#if !defined(__clang_major__) || (__clang_major__ >= 5) // DWARF does not support Clang versions older than 5.0.
// Test valid address of method ReportJNIFatalError in jniCheck.hpp. We should get "jniCheck.hpp" in the buffer and a valid line number.
TEST_VM(os_linux, decoder_get_source_info_valid) {
char buf[128] = "";
int line = -1;
address valid_function_pointer = (address)JNI_CreateJavaVM;
address valid_function_pointer = (address)ReportJNIFatalError;
ASSERT_TRUE(Decoder::get_source_info(valid_function_pointer, buf, sizeof(buf), &line));
ASSERT_TRUE(strcmp(buf, "jni.cpp") == 0);
ASSERT_TRUE(line > 0);
}

// Same test as "decoder_get_source_info_valid" but with a too-small output buffer. Everything should work the same except
// that the output buffer truncates "jni.cpp" such that we find "jni.cp" instead. The line number must be found as before.
TEST_VM(os_linux, decoder_get_source_info_valid_truncated) {
char buf[128] = "";
int line = -1;
memset(buf, 'X', sizeof(buf));
address valid_function_pointer = (address)JNI_CreateJavaVM;
ASSERT_TRUE(Decoder::get_source_info(valid_function_pointer, buf, 7, &line));
ASSERT_TRUE(buf[7 - 1] == '\0');
ASSERT_TRUE(buf[7] == 'X');
ASSERT_TRUE(strcmp(buf, "jni.cp") == 0);
ASSERT_TRUE(strcmp(buf, "jniCheck.hpp") == 0);
ASSERT_TRUE(line > 0);
}

Expand All @@ -473,12 +460,33 @@ TEST_VM(os_linux, decoder_get_source_info_invalid) {
line = 12;
// We should return false but do not crash or fail in any way.
ASSERT_FALSE(Decoder::get_source_info(addr, buf, sizeof(buf), &line));
// buffer should contain "", offset should contain -1
ASSERT_TRUE(buf[0] == '\0');
ASSERT_TRUE(line == -1);
ASSERT_TRUE(buf[0] == '\0'); // Should contain "" on error
ASSERT_TRUE(line == -1); // Should contain -1 on error
}
}
#endif // NOT PRODUCT

// Test with valid address but a too small buffer to store the entire filename. Should find generic <OVERFLOW> message
// and a valid line number.
TEST_VM(os_linux, decoder_get_source_info_valid_overflow) {
char buf[11] = "";
int line = -1;
address valid_function_pointer = (address)ReportJNIFatalError;
ASSERT_TRUE(Decoder::get_source_info(valid_function_pointer, buf, 11, &line));
ASSERT_TRUE(strcmp(buf, "<OVERFLOW>") == 0);
ASSERT_TRUE(line > 0);
}

// Test with valid address but a too small buffer that can neither store the entire filename nor the generic <OVERFLOW>
// message. We should find "L" as filename and a valid line number.
TEST_VM(os_linux, decoder_get_source_info_valid_overflow_minimal) {
char buf[2] = "";
int line = -1;
address valid_function_pointer = (address)ReportJNIFatalError;
ASSERT_TRUE(Decoder::get_source_info(valid_function_pointer, buf, 2, &line));
ASSERT_TRUE(strcmp(buf, "L") == 0); // Overflow message does not fit, so we fall back to "L:line_number"
ASSERT_TRUE(line > 0); // Line should correctly be found and returned
}
#endif // clang

#ifdef __GLIBC__
TEST_VM(os_linux, glibc_mallinfo_wrapper) {
Expand Down

1 comment on commit 8b8d848

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.