8293422: DWARF emitted by Clang cannot be parsed

Reviewed-by: tschatzl, ihse, stuefe

make/autoconf/flags-cflags.m4

@@ -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"

src/hotspot/share/utilities/decoder_elf.cpp

@@ -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;
 
@@ -83,6 +88,9 @@ 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;
   }
 
@@ -90,6 +98,7 @@ bool ElfDecoder::get_source_info(address pc, char* filename, size_t filename_len
                        p2i(pc), offset_in_library, filename, *line);
   DWARF_LOG_INFO("") // To structure the debug output better.
   return true;
+#endif // clang
 }
 
 

src/hotspot/share/utilities/elfFile.cpp

@@ -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;
@@ -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;
   }
 
@@ -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.
@@ -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;
@@ -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);
 }
@@ -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;
   }
 
@@ -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()) {
@@ -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__

src/hotspot/share/utilities/elfFile.hpp

@@ -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);
@@ -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.
@@ -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
@@ -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)

test/hotspot/gtest/runtime/test_os_linux.cpp

@@ -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"
@@ -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);
 }
 
@@ -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) {