The 6.1.18 kernel of Fedora 36 x86_64 also has this problem.

This can be replicated with rhel8 (works) versusus rhel9 (doesn't work).  But I
don't think it's related to kernel:  After copying my test binary from rhel8
over to rhel9 (and making sure it's md5-identical, things started to work even on rhel9!

The following comes from my rhel8 binary where things work fine:

> 8 x86_64 # eu-readelf --debug-dump=info a.out 
> 
> DWARF section [26] '.debug_info' at offset 0x2e70:
>  [Offset]
>  Compilation unit at offset 0:
>  Version: 4, Abbreviation section offset: 0, Address size: 8, Offset size: 4
>  [     b]  compile_unit         abbrev: 1
>            producer             (strp) "GNU C17 8.5.0 20210514 (Red Hat 8.5.0-19) -mtune=generic -march=x86-64 -g"
>            language             (data1) C99 (12)
>            name                 (strp) "test.c"
>            comp_dir             (strp) "/root/build"
>            low_pc               (addr) 0x0000000000400536 <main>
>            high_pc              (data8) 11 (0x0000000000400541)
>            stmt_list            (sec_offset) 0
>  [    2d]    subprogram           abbrev: 2
>              external             (flag_present) yes
>              name                 (strp) "main"
>              decl_file            (data1) test.c (1)
>              decl_line            (data1) 1
>              decl_column          (data1) 5
>              prototyped           (flag_present) yes
>              type                 (ref4) [    4b]
>              low_pc               (addr) 0x0000000000400536 <main>
>              high_pc              (data8) 11 (0x0000000000400541)
>              frame_base           (exprloc) 
>               [ 0] call_frame_cfa
>              GNU_all_call_sites   (flag_present) yes
>  [    4b]    base_type            abbrev: 3
>              byte_size            (data1) 4
>              encoding             (data1) signed (5)
>              name                 (string) "int"
> 8 x86_64 #

Now here's what my rhel9 binary shows:

> 9 x86_64 # eu-readelf --debug-dump=info a.out 
> 
> DWARF section [27] '.debug_info' at offset 0x47a0:
>  [Offset]
>  Compilation unit at offset 0:
>  Version: 5, Abbreviation section offset: 0, Address size: 8, Offset size: 4
>  Unit type: compile (1)
>  [     c]  compile_unit         abbrev: 1
>            producer             (strp) "GNU C17 11.3.1 20221121 (Red Hat 11.3.1-4) -mtune=generic -march=x86-64-v2 -g"
>            language             (data1) C11 (29)
>            name                 (line_strp) "test.c"
>            comp_dir             (line_strp) "/root/build"
>            low_pc               (addr) 0x0000000000401106 <main>
>            high_pc              (data8) 11 (0x0000000000401111)
>            stmt_list            (sec_offset) 0
>  [    2e]    subprogram           abbrev: 2
>              external             (flag_present) yes
>              name                 (strp) "main"
>              decl_file            (data1) test.c (1)
>              decl_line            (data1) 1
>              decl_column          (data1) 5
>              prototyped           (flag_present) yes
>              type                 (ref4) [    4c]
>              low_pc               (addr) 0x0000000000401106 <main>
>              high_pc              (data8) 11 (0x0000000000401111)
>              frame_base           (exprloc) 
>               [ 0] call_frame_cfa
>              call_all_calls       (flag_present) yes
>  [    4c]    base_type            abbrev: 3
>              byte_size            (data1) 4
>              encoding             (data1) signed (5)
>              name                 (string) "int"
> 9 x86_64 # 

I've verified that -march, doesn't cause the difference.  But enforcing DWARF4
on rhel9 with `gcc -gdwarf-4` makes things work again on rhel9!  So I think it's the DWARF version that makes the difference :)


The print_ubacktrace_fileline() call propagates down to _stp_linenumber_lookup(address, task, &filename, ...) which already returns the NULL filename.  Stepping in, this calls _stp_linenumber_lookup(), which returns struct _stp_module with a NULL debug_line member.  Looking farther...

Had some additional discussion with Martin on email on how to fix this. For this PR (and 29984) there are two parts that need to be addressed:

1) recognize the dwarf 5 format line info in translate.cxx.  It looks like https://sourceware.org/bugzilla/show_bug.cgi?id=29984 mentions an area that needs to be changed to so systemtap copies the line information.

2) have the runtime _stp_linenumber_lookup in runtime/sym.c be able to handle the dwarf 5 line number headers.

Need to refer to the dwarf debuginfo manuals for dwarf4 and dwarf5 from https://dwarfstd.org/, section 6.2.4 "The Line Number Program Header" for details on the differences.  There are differences in the dwarf5 line number header: address_size, segment_selector_size, directories_count, directories, etc. in dwarf5 .

Created attachment 14964 [details]
A WIP patch showing line numbers, but no file names yet

First time I see DWARF this closely.  The attached WIP patch allows for showing the line numbers already.  The DWARF structure change roughly is as follows:

DWARF-4                                      DWARF-5
==================================================================
unit_length                                  unit_length
version (uhalf)                              version (uhalf)
---                                          address_size (ubyte)
---                                          segment_selector_size (ubyte)
header_length                                header_length
minimum_instruction_length (ubyte)           minimum_instruction_length (ubyte)
maximum_operations_per_instruction (ubyte)   maximum_operations_per_instruction (ubyte)
default_is_stmt (ubyte)                      default_is_stmt (ubyte)
line_base (sbyte)                            line_base (sbyte)
line_range (ubyte)                           line_range (ubyte)
opcode_base (ubyte)                          opcode_base (ubyte)
standard_opcode_lengths (array of ubyte)     standard_opcode_lengths (array of ubyte)
include_directories (sequence of path names) ---
file_names (sequence of file entries)        ---
---                                          directory_entry_format_count (ubyte)
---                                          directory_entry_format (sequence of ULEB128 pairs)
---                                          directories_count (ULEB128)
---                                          directories (sequence of directory names)
---                                          file_name_entry_format_count (ubyte)
---                                          file_name_entry_format (sequence of ULEB128 pairs)
---                                          file_names_count (ULEB128)
---                                          file_names (sequence of file name entries)



So, the attached WIP patch merely jumps over address_size and segment_selector_size for now. Looking into how to display the file name now...

Created attachment 15161 [details]
WIP patch

The attached patch doesn't properly parse the .debug_line header (yet) to find correct pointers to the .debug_line_str section.  Next version will try to do that.

Demo:
-----------------------------------
# cat a.c
int main(void) {
    return 0;
}
# 
# gcc -g -gdwarf-5 a.c -o prog5
# gcc -g -gdwarf-4 a.c -o prog4
#
# eu-readelf --debug-dump=info prog4 | fgrep -i produc
           producer             (strp) "GNU C17 13.1.1 20230614 (Red Hat 13.1.1-4) -mtune=generic -march=x86-64 -g -gdwarf-4"
# eu-readelf --debug-dump=info prog5 | fgrep -i produc
           producer             (strp) "GNU C17 13.1.1 20230614 (Red Hat 13.1.1-4) -mtune=generic -march=x86-64 -g -gdwarf-5"
# 
# stap --ldd -e 'probe process.function("main") { print_ubacktrace_fileline() }' -c ./prog4 
 0x40110a : main+0x4/0xe at /root/test/a.c:2 [/root/test/prog4]
 0x7f9d3c42814a : __libc_start_call_main+0x7a/0xb0 [/usr/lib64/libc.so.6]
 0x7f9d3c42820b : __libc_start_main@GLIBC_2.2.5+0x8b/0x160 [/usr/lib64/libc.so.6]
 0x401045 : _start+0x25/0x30 [/root/test/prog4]
# 
# stap --ldd -e 'probe process.function("main") { print_ubacktrace_fileline() }' -c ./prog5
 0x40110a : main+0x4/0xe at /root/test/a.c:2 [/root/test/prog5]
 0x7fba8ae2814a : __libc_start_call_main+0x7a/0xb0 [/usr/lib64/libc.so.6]
 0x7fba8ae2820b : __libc_start_main@GLIBC_2.2.5+0x8b/0x160 [/usr/lib64/libc.so.6]
 0x401045 : _start+0x25/0x30 [/root/test/prog5]
#

Created attachment 15165 [details]
possible patch

The attached patch seems to work for me, tested with gcc and clang, 32 bit and 64 "hello world" kind of program.

--- a/runtime/sym.c	
+++ a/runtime/sym.c	
@@ -355,6 +355,121 @@ static void _stp_filename_lookup(struct _stp_module *mod, char ** filename,
     }
 }
 
+static void _stp_filename_lookup_2(struct _stp_module *mod, char ** filename,
+                                   uint8_t *dirsecp, uint8_t *enddirsecp,
+                                   uint8_t *strsecp, uint8_t *endstrsecp,
+                                   unsigned int length,
+                                   unsigned fileidx, int user, int compat_task)
+{

Would be cooler if you called this _stp_filename_lookup_5 :)

+  // Pointer to the .debug_line section
+  // pointing at at just after standard_opcode_lengths
+  // which is tha last header item common to DWARF v4 and v5.

s/tha/the/

+  uint8_t *debug_line_p = dirsecp;
+  // Pointer to the beginning of .debug_line_str section
+  uint8_t *debug_line_str_p = strsecp;
+
+  uint8_t directory_entry_format_count = 0, file_name_entry_format_count = 0,
+          directories_count = 0, file_names_count = 0;
+
+  struct encpair { uint16_t desc; uint16_t form; };
+  struct encpair dir_enc[20], file_enc[20];

Where does the 20 come from?

+  // Source files and directories
+  struct dirinfo { uint8_t offset; char *name; };
+  struct dirinfo src_dir[20];
+  struct fileinfo { uint8_t offset; uint8_t dirindex; char *name; };
+  struct fileinfo src_file[20];
+  static char fullpath [MAXSTRINGLEN];

Likewise, 20?

+
+  // Initialize the *filename
+  *filename = "unknown";
+
+  // Next comes directory_entry_format_count
+  if (debug_line_str_p + 1 > enddirsecp)
+    return;
+  directory_entry_format_count = *debug_line_p++;

Sanity check this value since you seem to be limited to 20?
But this can be 255.

+  // Next comes directory_entry_format
+  for (int i = 0; i < directory_entry_format_count; i++)
+    {
+      dir_enc[i].desc = read_pointer ((const uint8_t **) &debug_line_p, enddirsecp, DW_EH_PE_leb128, user, compat_task);
+      dir_enc[i].form = read_pointer ((const uint8_t **) &debug_line_p, enddirsecp, DW_EH_PE_leb128, user, compat_task);
+    }
+
+  // Next comes directories_count
+  directories_count = read_pointer ((const uint8_t **) &debug_line_p, enddirsecp, DW_EH_PE_leb128, user, compat_task);

And directories_count can be way larger than 20.

+  // Next come directories
+  // See elfutil's print_form_data() in readelf.c for an analogy of what happens below
+  for (int i=0; i < directories_count; i++)
+      for (int j=0; j < directory_entry_format_count; j++)
+          switch (dir_enc[j].form)
+            {
+              case DW_FORM_line_strp:
+                src_dir[i].offset = (uint8_t) read_pointer ((const uint8_t **) &debug_line_p, enddirsecp, DW_EH_PE_data4, user, compat_task);
+                if (strsecp + src_dir[i].offset > endstrsecp)
+                  return;
+                src_dir[i].name = strsecp + src_dir[i].offset;
+                break;
+              default:
+                _stp_error("BUG: Unknown form %d encountered while parsing source dir\n", dir_enc[j].form);
+                return;
+            }

I assume gcc and clang only use DW_FORM_line_strp, but you might want to check some others. I wouldn't be surprised if some use DW_FORM_strp.

+  // Next comes file_name_entry_format_count
+  if (debug_line_p + 1 > enddirsecp)
+    return;
+  file_name_entry_format_count = *debug_line_p++;
+

This can be up to 255 too. Sanity check?

+  // Next comes file_name_entry_format
+  for (int i = 0; i < file_name_entry_format_count; i++)
+    {
+      file_enc[i].desc = read_pointer ((const uint8_t **) &debug_line_p, enddirsecp, DW_EH_PE_leb128, user, compat_task);
+      file_enc[i].form = read_pointer ((const uint8_t **) &debug_line_p, enddirsecp, DW_EH_PE_leb128, user, compat_task);
+    }
+
+  // Next comes the file_names_count
+  // See elfutil's print_form_data() in readelf.c for an analogy of what happens below
+  file_names_count = read_pointer ((const uint8_t **) &debug_line_p, enddirsecp, DW_EH_PE_leb128, user, compat_task);

Again, this can be way larger than 20.

+  // Next come the files
+  for (int i=0; i < file_names_count; i++)
+      for (int j=0; j < file_name_entry_format_count; j++)
+          switch (file_enc[j].form)
+            {
+              case DW_FORM_line_strp:
+                src_file[i].offset = (uint8_t) read_pointer ((const uint8_t **) &debug_line_p, enddirsecp, DW_EH_PE_data4, user, compat_task);
+                if (strsecp + src_file[i].offset > endstrsecp)
+                  return;
+                src_file[i].name = strsecp + src_file[i].offset;
+                break;
+              case DW_FORM_data16:
+                // This is how clang encodes the md5sum, skip it
+                if (debug_line_p + 16 > enddirsecp)
+                  return;
+                debug_line_p += 16;
+                break;
+              case DW_FORM_udata:
+                src_file[i].dirindex = (uint8_t) read_pointer ((const uint8_t **) &debug_line_p, enddirsecp, DW_EH_PE_leb128, user, compat_task);
+                break;
+              default:
+                _stp_error("BUG: Unknown form %d encountered while parsing source file\n", file_enc[j].form);
+                return;
+            }

Looks good. But again surprised you aren't seeing any other forms. Although these are definitely the required ones.

+  // Put it together
+  // - requested file index is fileidx
+  //   (based on the line number program)
+  // - find directory respective to this file
+  // - and attach slash and the file name itself
+  strlcpy(fullpath, src_dir[src_file[fileidx].dirindex].name, MAXSTRINGLEN);
+  strlcat(fullpath, "/", MAXSTRINGLEN);
+  strlcat(fullpath, src_file[fileidx].name, MAXSTRINGLEN);
+  *filename = fullpath;
+
+}
+
 #endif /* STP_NEED_LINE_DATA */

OK.

 unsigned long _stp_linenumber_lookup(unsigned long addr, struct task_struct *task, char ** filename, int need_filename)
@@ -363,6 +478,7 @@ unsigned long _stp_linenumber_lookup(unsigned long addr, struct task_struct *tas
   struct _stp_section *sec;
   const char *modname = NULL;
   uint8_t *linep, *enddatap;
+  uint8_t *str_linep, *str_enddatap;
   int compat_task = _stp_is_compat_task();
   int user = (task ? 1 : 0);
 
@@ -405,10 +521,16 @@ unsigned long _stp_linenumber_lookup(unsigned long addr, struct task_struct *tas
       addr = addr - offset;
     }
 
-
+  // The .debug_line section
   linep = m->debug_line;
   enddatap = m->debug_line + m->debug_line_len;
 
+  // _stp_printf("XXX the beginning of .debug_line section is at address %p\n", linep);
+
+  // The .debug_line_str section
+  str_linep = m->debug_line_str;
+  str_enddatap = m->debug_line_str + m->debug_line_str_len;
+
   while (linep < enddatap)
     {
       // State machine "curr" values are updated directly.

OK. So str_linep and str_enddatap are derived from m.

@@ -442,6 +564,16 @@ unsigned long _stp_linenumber_lookup(unsigned long addr, struct task_struct *tas
 
       version = read_pointer ((const uint8_t **) &linep, endunitp, DW_EH_PE_data2, user, compat_task);
 
+      // Need to skip over DWARF 5's address_size and segment_selector_size right
+      // to hdr_length (analogy to what happens in pass2's dump_line_tables_check()
+      // PR29984
+      if (version >= 5)
+        {
+          if (unlikely (linep + 2 > enddatap))
+            return 0;
+          linep += 2;
+        }
+
       if (length == 4)
         hdr_length = (uint64_t) read_pointer ((const uint8_t **) &linep, endunitp, DW_EH_PE_data4, user, compat_task);
       else
@@ -613,8 +745,15 @@ unsigned long _stp_linenumber_lookup(unsigned long addr, struct task_struct *tas
             if (row_end_sequence == 0 && row_addr <= addr && addr < curr_addr)
               {
                 if (need_filename)
-                  _stp_filename_lookup(m, filename, dirsecp, endhdrp,
-                                       row_file_idx, user, compat_task);
+                  {
+                    if (version > 4)
+                      _stp_filename_lookup_2(m, filename, dirsecp, endhdrp,
+                                             str_linep, str_enddatap, length,
+                                             row_file_idx, user, compat_task);
+                    else
+                      _stp_filename_lookup(m, filename, dirsecp, endhdrp,
+                                           row_file_idx, user, compat_task);
+                  }
                 return row_linenum;
               }

Aren't you stuffing the str_linep and lenght into the module m?
You don't need to pass them here then.
To make things look more similar to _stp_filename_lookup.

--- a/runtime/sym.h	
+++ a/runtime/sym.h	
@@ -116,12 +116,14 @@ struct _stp_module {
 	   Note index for .debug_frame (hdr) is per section. */
 	void *debug_frame;
 	void *eh_frame;
-	void *unwind_hdr;	
-  void *debug_line;
+	void *unwind_hdr;
+	void *debug_line;
+	void *debug_line_str;
 	uint32_t debug_frame_len;
 	uint32_t eh_frame_len;
 	uint32_t unwind_hdr_len;
-  uint32_t debug_line_len;
+	uint32_t debug_line_len;
+	uint32_t debug_line_str_len;
 	unsigned long eh_frame_addr; /* Orig load address (offset) .eh_frame */
 	unsigned long unwind_hdr_addr; /* same for .eh_frame_hdr */

OK.

--- a/runtime/unwind/unwind.h	
+++ a/runtime/unwind/unwind.h	
@@ -466,6 +466,13 @@ static const struct {
 #define	DW_OP_xderef_size	0x95
 #define	DW_OP_nop		0x96
 
+#define DW_LNCT_path		0x1
+#define DW_LNCT_directory_index	0x2
+
+#define DW_FORM_udata		0x0f
+#define DW_FORM_data16          0x1e
+#define DW_FORM_line_strp	0x1f
+
 struct unwind_item {
 	enum item_location {
 		Same,     /* no state */

OK. Slightly surprised you aren't seeing DW_FORM_data[124]. But maybe no producer optimizes for that.

--- a/translate.cxx	
+++ a/translate.cxx	
@@ -6601,6 +6601,8 @@ struct unwindsym_dump_context
   Dwarf_Addr eh_frame_hdr_addr;
   void *debug_line;
   size_t debug_line_len;
+  void *debug_line_str;
+  size_t debug_line_str_len;
 
   set<string> undone_unwindsym_modules;
 };

OK

@@ -7056,6 +7058,16 @@ dump_line_tables_check (void *data, size_t data_len)
       version  = *((uint16_t *)ptr);
       ptr += 2;
 
+      // Need to skip over DWARF 5's address_size and segment_selector_size right
+      // to hdr_length (analogy to what happens in pass2's dump_line_tables_check()
+      // PR29984
+      if (version >= 5)
+      {
+        if (ptr + 2 > (uint8_t *)data + data_len)
+            return DWARF_CB_ABORT;
+        ptr += 2;
+      }
+
       if (unit_length <= (2 + length))
         return DWARF_CB_ABORT;
 
OK. Might want to write the check as if (ptr - (uint8_t *)data > data_len - 2) to prevent overflow (although you might have checked for that before).

@@ -7123,6 +7135,7 @@ dump_line_tables (Dwfl_Module *m, unwindsym_dump_context *c,
       const char *sh_name;
       shdr = gelf_getshdr(scn, &shdr_mem);
       sh_name = elf_strptr(elf, ehdr->e_shstrndx, shdr->sh_name);
+
       // decompression is done via dwarf_begin_elf / global_read / check_section
       // / elf_compress_gnu / __libelf_decompress in libelf/elf_compress_gnu.c
       if (strcmp(sh_name, ".debug_line") == 0

Random whitespace change. OK, but why?

@@ -7133,7 +7146,15 @@ dump_line_tables (Dwfl_Module *m, unwindsym_dump_context *c,
             return;
           c->debug_line = data->d_buf;
           c->debug_line_len = data->d_size;
-          break;
+          continue;
+        }
+       if (strcmp(sh_name, ".debug_line_str") == 0
+           || strcmp(sh_name, ".zdebug_line_str") == 0)
+        {
+          data = elf_rawdata(scn, NULL);
+          c->debug_line_str = data->d_buf;
+          c->debug_line_str_len = data->d_size;
+          continue;
         }
     }

OK

@@ -7412,8 +7433,12 @@ dump_unwindsym_cxt_table(systemtap_session& session, ostream& output,
 			 const string& secname, unsigned secindex,
 			 const string& table, void*& data, size_t& len)
 {
-  if (data == NULL || len == 0)
-    return;
+  // XXX DWARF4 doesn't have .debug_line_str section, but we need to generate
+  // empty section data to stap_symbols.c to keep it syntactically correct
+  // This is a hack and needs to be improved.
+  //
+  // if (data == NULL || len == 0)
+  //   return;
 
   if (len > MAX_UNWIND_TABLE_SIZE)
     {

Yeah, this needs to improved.
What exactly is the output like when data == NULL || len == 0?
Does it currently only work for DWARF5?

@@ -7474,6 +7499,8 @@ dump_unwindsym_cxt (Dwfl_Module *m,
   Dwarf_Addr eh_frame_hdr_addr = c->eh_frame_hdr_addr;
   void *debug_line = c->debug_line;
   size_t debug_line_len = c->debug_line_len;
+  void *debug_line_str = c->debug_line_str;
+  size_t debug_line_str_len = c->debug_line_str_len;
 
   dump_unwindsym_cxt_table(c->session, c->output, modname, stpmod_idx, "", 0,
 			   "debug_frame", debug_frame, debug_len);
@@ -7487,6 +7514,9 @@ dump_unwindsym_cxt (Dwfl_Module *m,
   dump_unwindsym_cxt_table(c->session, c->output, modname, stpmod_idx, "", 0,
 			   "debug_line", debug_line, debug_line_len);
 
+  dump_unwindsym_cxt_table(c->session, c->output, modname, stpmod_idx, "", 0,
+			   "debug_line_str", debug_line_str, debug_line_str_len);
+
   if (c->session.need_unwind && debug_frame == NULL && eh_frame == NULL)
     {
       // There would be only a small benefit to warning.  A user

OK

@@ -7505,6 +7535,13 @@ dump_unwindsym_cxt (Dwfl_Module *m,
                                   dwfl_errmsg (-1));
     }
 
+  if (c->session.need_lines && debug_line_str == NULL)
+    {
+      if (c->session.verbose > 2)
+        c->session.print_warning ("No debug line str data for " + modname + ", " +
+                                  dwfl_errmsg (-1));
+    }
+

Does this always warn for DWARF4?

   for (unsigned secidx = 0; secidx < c->seclist.size(); secidx++)
     {
       c->output << "static struct _stp_symbol "
@@ -7685,11 +7722,16 @@ dump_unwindsym_cxt (Dwfl_Module *m,
       c->output << ".debug_line = "
 		<< "_stp_module_" << stpmod_idx << "_debug_line, \n";
       c->output << ".debug_line_len = " << debug_line_len << ", \n";
+      c->output << ".debug_line_str = "
+		<< "_stp_module_" << stpmod_idx << "_debug_line_str, \n";
+      c->output << ".debug_line_str_len = " << debug_line_str_len << ", \n";
       c->output << "#else\n";
     }
 
   c->output << ".debug_line = NULL,\n";
   c->output << ".debug_line_len = 0,\n";
+  c->output << ".debug_line_str = NULL,\n";
+  c->output << ".debug_line_str_len = 0,\n";
 
   if (debug_line != NULL)
     c->output << "#endif /* STP_NEED_LINE_DATA */\n";
@@ -7873,6 +7915,8 @@ dump_unwindsyms (Dwfl_Module *m,
 
   c->debug_line = NULL;
   c->debug_line_len = 0;
+  c->debug_line_str = NULL;
+  c->debug_line_str_len = 0;
   if (res == DWARF_CB_OK && c->session.need_lines)
     // we dont set res = dump_line_tables() because unwindsym stuff should still
     // get dumped to the output even if gathering debug_line data fails
@@ -8044,6 +8088,8 @@ emit_symbol_data (systemtap_session& s)
 				 0, /* eh_frame_hdr_addr */
 				 NULL, /* debug_line */
 				 0, /* debug_line_len */
+				 NULL, /* debug_line_str */
+				 0, /* debug_line_str_len */
 				 s.unwindsym_modules };
 
   // Micro optimization, mainly to speed up tiny regression tests
@@ -8139,6 +8185,8 @@ self_unwind_declarations(unwindsym_dump_context *ctx)
   ctx->output << ".debug_frame_len = 0,\n";
   ctx->output << ".debug_line = NULL,\n";
   ctx->output << ".debug_line_len = 0,\n";
+  ctx->output << ".debug_line_str = NULL,\n";
+  ctx->output << ".debug_line_str_len = 0,\n";
   ctx->output << "};\n";
 }

All the code that adds the extra debug_line_str and debug_line_str_len looks correct.

Created attachment 15183 [details]
possible patch

(In reply to Mark Wielaard from comment #7)
> --- a/runtime/sym.c	
> +++ a/runtime/sym.c	
> @@ -355,6 +355,121 @@ static void _stp_filename_lookup(struct _stp_module
> *mod, char ** filename,
>      }
>  }
>  
> +static void _stp_filename_lookup_2(struct _stp_module *mod, char **
> filename,
> +                                   uint8_t *dirsecp, uint8_t *enddirsecp,
> +                                   uint8_t *strsecp, uint8_t *endstrsecp,
> +                                   unsigned int length,
> +                                   unsigned fileidx, int user, int
> compat_task)
> +{
> 
> Would be cooler if you called this _stp_filename_lookup_5 :)

Renamed!

> 
> +  // Pointer to the .debug_line section
> +  // pointing at at just after standard_opcode_lengths
> +  // which is tha last header item common to DWARF v4 and v5.
> 
> s/tha/the/

Fixed (also the s/at\ at/at/ one line above)

> 
> +  uint8_t *debug_line_p = dirsecp;
> +  // Pointer to the beginning of .debug_line_str section
> +  uint8_t *debug_line_str_p = strsecp;
> +
> +  uint8_t directory_entry_format_count = 0, file_name_entry_format_count =
> 0,
> +          directories_count = 0, file_names_count = 0;
> +
> +  struct encpair { uint16_t desc; uint16_t form; };
> +  struct encpair dir_enc[20], file_enc[20];
> 
> Where does the 20 come from?

I saw in elfutils's readelf.c that 255 is used there.  But when I use that here, I'm getting a weird error that I don't get:

======================== 8< =============================
f40 x86_64 # stap --ldd -e 'probe process.function("main") { print_ubacktrace_fileline() }' -c ./prog5
 0x40110a : main+0x4/0xe at /root/build/test/test.c:2 [/root/build/test/prog5]
 0x7fb94bf2b14a : __libc_start_call_main+0x7a/0xb0 [/usr/lib64/libc.so.6]
 0x7fb94bf2b20b : __libc_start_main@GLIBC_2.2.5+0x8b/0x160 [/usr/lib64/libc.so.6]
 0x401045 : _start+0x25/0x30 [/root/build/test/prog5]
f40 x86_64 # pwd
/root/build/test
f40 x86_64 # stap --ldd -e 'probe process.function("main") { print_ubacktrace_fileline() }' -c ./prog5
In file included from /usr/local/share/systemtap/runtime/linux/runtime.h:341,
                 from /usr/local/share/systemtap/runtime/runtime.h:26,
                 from /tmp/stapitMvxE/stap_c2ef1d639ec0c14a23679aff69af0d76_1622_src.c:21:
/usr/local/share/systemtap/runtime/sym.c: In function ‘_stp_filename_lookup_2.constprop’:
/usr/local/share/systemtap/runtime/sym.c:471:1: error: the frame size of 10272 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
  471 | }
      | ^
cc1: all warnings being treated as errors
make[1]: *** [scripts/Makefile.build:243: /tmp/stapitMvxE/stap_c2ef1d639ec0c14a23679aff69af0d76_1622_src.o] Error 1
make: *** [Makefile:1931: /tmp/stapitMvxE] Error 2
WARNING: kbuild exited with status: 2
Pass 4: compilation failed.  [man error::pass4]
Kernel version 6.6.0 is outside tested range 2.6.32 ... 6.5
f40 x86_64 #
======================== 8< =============================

So, for now I've at least introduced a new macro STP_MAX_DW_SOURCES and used that across the update.  Also I've added some more checks based on this macro, see below.

> 
> +  // Source files and directories
> +  struct dirinfo { uint8_t offset; char *name; };
> +  struct dirinfo src_dir[20];
> +  struct fileinfo { uint8_t offset; uint8_t dirindex; char *name; };
> +  struct fileinfo src_file[20];
> +  static char fullpath [MAXSTRINGLEN];
> 
> Likewise, 20?

See above.

> 
> +
> +  // Initialize the *filename
> +  *filename = "unknown";
> +
> +  // Next comes directory_entry_format_count
> +  if (debug_line_str_p + 1 > enddirsecp)
> +    return;
> +  directory_entry_format_count = *debug_line_p++;
> 
> Sanity check this value since you seem to be limited to 20?
> But this can be 255.

See above.

> 
> +  // Next comes directory_entry_format
> +  for (int i = 0; i < directory_entry_format_count; i++)
> +    {
> +      dir_enc[i].desc = read_pointer ((const uint8_t **) &debug_line_p,
> enddirsecp, DW_EH_PE_leb128, user, compat_task);
> +      dir_enc[i].form = read_pointer ((const uint8_t **) &debug_line_p,
> enddirsecp, DW_EH_PE_leb128, user, compat_task);
> +    }
> +
> +  // Next comes directories_count
> +  directories_count = read_pointer ((const uint8_t **) &debug_line_p,
> enddirsecp, DW_EH_PE_leb128, user, compat_task);
> 
> And directories_count can be way larger than 20.

See above.

> 
> +  // Next come directories
> +  // See elfutil's print_form_data() in readelf.c for an analogy of what
> happens below
> +  for (int i=0; i < directories_count; i++)
> +      for (int j=0; j < directory_entry_format_count; j++)
> +          switch (dir_enc[j].form)
> +            {
> +              case DW_FORM_line_strp:
> +                src_dir[i].offset = (uint8_t) read_pointer ((const uint8_t
> **) &debug_line_p, enddirsecp, DW_EH_PE_data4, user, compat_task);
> +                if (strsecp + src_dir[i].offset > endstrsecp)
> +                  return;
> +                src_dir[i].name = strsecp + src_dir[i].offset;
> +                break;
> +              default:
> +                _stp_error("BUG: Unknown form %d encountered while parsing
> source dir\n", dir_enc[j].form);
> +                return;
> +            }
> 
> I assume gcc and clang only use DW_FORM_line_strp, but you might want to
> check some others. I wouldn't be surprised if some use DW_FORM_strp.
> 
> +  // Next comes file_name_entry_format_count
> +  if (debug_line_p + 1 > enddirsecp)
> +    return;
> +  file_name_entry_format_count = *debug_line_p++;
> +
> 
> This can be up to 255 too. Sanity check?

See above (sanity check added).

> 
> +  // Next comes file_name_entry_format
> +  for (int i = 0; i < file_name_entry_format_count; i++)
> +    {
> +      file_enc[i].desc = read_pointer ((const uint8_t **) &debug_line_p,
> enddirsecp, DW_EH_PE_leb128, user, compat_task);
> +      file_enc[i].form = read_pointer ((const uint8_t **) &debug_line_p,
> enddirsecp, DW_EH_PE_leb128, user, compat_task);
> +    }
> +
> +  // Next comes the file_names_count
> +  // See elfutil's print_form_data() in readelf.c for an analogy of what
> happens below
> +  file_names_count = read_pointer ((const uint8_t **) &debug_line_p,
> enddirsecp, DW_EH_PE_leb128, user, compat_task);
> 
> Again, this can be way larger than 20.

See above.

> 
> +  // Next come the files
> +  for (int i=0; i < file_names_count; i++)
> +      for (int j=0; j < file_name_entry_format_count; j++)
> +          switch (file_enc[j].form)
> +            {
> +              case DW_FORM_line_strp:
> +                src_file[i].offset = (uint8_t) read_pointer ((const uint8_t
> **) &debug_line_p, enddirsecp, DW_EH_PE_data4, user, compat_task);
> +                if (strsecp + src_file[i].offset > endstrsecp)
> +                  return;
> +                src_file[i].name = strsecp + src_file[i].offset;
> +                break;
> +              case DW_FORM_data16:
> +                // This is how clang encodes the md5sum, skip it
> +                if (debug_line_p + 16 > enddirsecp)
> +                  return;
> +                debug_line_p += 16;
> +                break;
> +              case DW_FORM_udata:
> +                src_file[i].dirindex = (uint8_t) read_pointer ((const
> uint8_t **) &debug_line_p, enddirsecp, DW_EH_PE_leb128, user, compat_task);
> +                break;
> +              default:
> +                _stp_error("BUG: Unknown form %d encountered while parsing
> source file\n", file_enc[j].form);
> +                return;
> +            }
> 
> Looks good. But again surprised you aren't seeing any other forms. Although
> these are definitely the required ones.

I've worked with what I saw in practice with gcc and clang.

> 
> +  // Put it together
> +  // - requested file index is fileidx
> +  //   (based on the line number program)
> +  // - find directory respective to this file
> +  // - and attach slash and the file name itself
> +  strlcpy(fullpath, src_dir[src_file[fileidx].dirindex].name, MAXSTRINGLEN);
> +  strlcat(fullpath, "/", MAXSTRINGLEN);
> +  strlcat(fullpath, src_file[fileidx].name, MAXSTRINGLEN);
> +  *filename = fullpath;
> +
> +}
> +
>  #endif /* STP_NEED_LINE_DATA */
> 
> OK.
> 
>  unsigned long _stp_linenumber_lookup(unsigned long addr, struct task_struct
> *task, char ** filename, int need_filename)
> @@ -363,6 +478,7 @@ unsigned long _stp_linenumber_lookup(unsigned long addr,
> struct task_struct *tas
>    struct _stp_section *sec;
>    const char *modname = NULL;
>    uint8_t *linep, *enddatap;
> +  uint8_t *str_linep, *str_enddatap;
>    int compat_task = _stp_is_compat_task();
>    int user = (task ? 1 : 0);
>  
> @@ -405,10 +521,16 @@ unsigned long _stp_linenumber_lookup(unsigned long
> addr, struct task_struct *tas
>        addr = addr - offset;
>      }
>  
> -
> +  // The .debug_line section
>    linep = m->debug_line;
>    enddatap = m->debug_line + m->debug_line_len;
>  
> +  // _stp_printf("XXX the beginning of .debug_line section is at address
> %p\n", linep);
> +
> +  // The .debug_line_str section
> +  str_linep = m->debug_line_str;
> +  str_enddatap = m->debug_line_str + m->debug_line_str_len;
> +
>    while (linep < enddatap)
>      {
>        // State machine "curr" values are updated directly.
> 
> OK. So str_linep and str_enddatap are derived from m.
> 
> @@ -442,6 +564,16 @@ unsigned long _stp_linenumber_lookup(unsigned long
> addr, struct task_struct *tas
>  
>        version = read_pointer ((const uint8_t **) &linep, endunitp,
> DW_EH_PE_data2, user, compat_task);
>  
> +      // Need to skip over DWARF 5's address_size and segment_selector_size
> right
> +      // to hdr_length (analogy to what happens in pass2's
> dump_line_tables_check()
> +      // PR29984
> +      if (version >= 5)
> +        {
> +          if (unlikely (linep + 2 > enddatap))
> +            return 0;
> +          linep += 2;
> +        }
> +
>        if (length == 4)
>          hdr_length = (uint64_t) read_pointer ((const uint8_t **) &linep,
> endunitp, DW_EH_PE_data4, user, compat_task);
>        else
> @@ -613,8 +745,15 @@ unsigned long _stp_linenumber_lookup(unsigned long
> addr, struct task_struct *tas
>              if (row_end_sequence == 0 && row_addr <= addr && addr <
> curr_addr)
>                {
>                  if (need_filename)
> -                  _stp_filename_lookup(m, filename, dirsecp, endhdrp,
> -                                       row_file_idx, user, compat_task);
> +                  {
> +                    if (version > 4)
> +                      _stp_filename_lookup_2(m, filename, dirsecp, endhdrp,
> +                                             str_linep, str_enddatap,
> length,
> +                                             row_file_idx, user,
> compat_task);
> +                    else
> +                      _stp_filename_lookup(m, filename, dirsecp, endhdrp,
> +                                           row_file_idx, user, compat_task);
> +                  }
>                  return row_linenum;
>                }
> 
> Aren't you stuffing the str_linep and lenght into the module m?
> You don't need to pass them here then.
> To make things look more similar to _stp_filename_lookup.

Right!  Fixed this.

>
> --- a/runtime/sym.h	
> +++ a/runtime/sym.h	
> @@ -116,12 +116,14 @@ struct _stp_module {
>  	   Note index for .debug_frame (hdr) is per section. */
>  	void *debug_frame;
>  	void *eh_frame;
> -	void *unwind_hdr;	
> -  void *debug_line;
> +	void *unwind_hdr;
> +	void *debug_line;
> +	void *debug_line_str;
>  	uint32_t debug_frame_len;
>  	uint32_t eh_frame_len;
>  	uint32_t unwind_hdr_len;
> -  uint32_t debug_line_len;
> +	uint32_t debug_line_len;
> +	uint32_t debug_line_str_len;
>  	unsigned long eh_frame_addr; /* Orig load address (offset) .eh_frame */
>  	unsigned long unwind_hdr_addr; /* same for .eh_frame_hdr */
> 
> OK.
> 
> --- a/runtime/unwind/unwind.h	
> +++ a/runtime/unwind/unwind.h	
> @@ -466,6 +466,13 @@ static const struct {
>  #define	DW_OP_xderef_size	0x95
>  #define	DW_OP_nop		0x96
>  
> +#define DW_LNCT_path		0x1
> +#define DW_LNCT_directory_index	0x2
> +
> +#define DW_FORM_udata		0x0f
> +#define DW_FORM_data16          0x1e
> +#define DW_FORM_line_strp	0x1f
> +
>  struct unwind_item {
>  	enum item_location {
>  		Same,     /* no state */
> 
> OK. Slightly surprised you aren't seeing DW_FORM_data[124]. But maybe no
> producer optimizes for that.

I see; I was working with what gcc and clang were able to generate in practice.

> 
> --- a/translate.cxx	
> +++ a/translate.cxx	
> @@ -6601,6 +6601,8 @@ struct unwindsym_dump_context
>    Dwarf_Addr eh_frame_hdr_addr;
>    void *debug_line;
>    size_t debug_line_len;
> +  void *debug_line_str;
> +  size_t debug_line_str_len;
>  
>    set<string> undone_unwindsym_modules;
>  };
> 
> OK
> 
> @@ -7056,6 +7058,16 @@ dump_line_tables_check (void *data, size_t data_len)
>        version  = *((uint16_t *)ptr);
>        ptr += 2;
>  
> +      // Need to skip over DWARF 5's address_size and segment_selector_size
> right
> +      // to hdr_length (analogy to what happens in pass2's
> dump_line_tables_check()
> +      // PR29984
> +      if (version >= 5)
> +      {
> +        if (ptr + 2 > (uint8_t *)data + data_len)
> +            return DWARF_CB_ABORT;
> +        ptr += 2;
> +      }
> +
>        if (unit_length <= (2 + length))
>          return DWARF_CB_ABORT;
>  
> OK. Might want to write the check as if (ptr - (uint8_t *)data > data_len -
> 2) to prevent overflow (although you might have checked for that before).

This check already is visible here, a few lines above.  Just in a different (but equivalent) form.

> 
> @@ -7123,6 +7135,7 @@ dump_line_tables (Dwfl_Module *m,
> unwindsym_dump_context *c,
>        const char *sh_name;
>        shdr = gelf_getshdr(scn, &shdr_mem);
>        sh_name = elf_strptr(elf, ehdr->e_shstrndx, shdr->sh_name);
> +
>        // decompression is done via dwarf_begin_elf / global_read /
> check_section
>        // / elf_compress_gnu / __libelf_decompress in
> libelf/elf_compress_gnu.c
>        if (strcmp(sh_name, ".debug_line") == 0
> 
> Random whitespace change. OK, but why?

Dropped.

> 
> @@ -7133,7 +7146,15 @@ dump_line_tables (Dwfl_Module *m,
> unwindsym_dump_context *c,
>              return;
>            c->debug_line = data->d_buf;
>            c->debug_line_len = data->d_size;
> -          break;
> +          continue;
> +        }
> +       if (strcmp(sh_name, ".debug_line_str") == 0
> +           || strcmp(sh_name, ".zdebug_line_str") == 0)
> +        {
> +          data = elf_rawdata(scn, NULL);
> +          c->debug_line_str = data->d_buf;
> +          c->debug_line_str_len = data->d_size;
> +          continue;
>          }
>      }
> 
> OK
> 
> @@ -7412,8 +7433,12 @@ dump_unwindsym_cxt_table(systemtap_session& session,
> ostream& output,
>  			 const string& secname, unsigned secindex,
>  			 const string& table, void*& data, size_t& len)
>  {
> -  if (data == NULL || len == 0)
> -    return;
> +  // XXX DWARF4 doesn't have .debug_line_str section, but we need to
> generate
> +  // empty section data to stap_symbols.c to keep it syntactically correct
> +  // This is a hack and needs to be improved.
> +  //
> +  // if (data == NULL || len == 0)
> +  //   return;
>  
>    if (len > MAX_UNWIND_TABLE_SIZE)
>      {
> 
> Yeah, this needs to improved.
> What exactly is the output like when data == NULL || len == 0?
> Does it currently only work for DWARF5?

So this actually works for both DWARF4 and DWARF5.  It even works for a combination of those (!).  What gets generated in case the .debug_line_str section isn't there is something like:

...
static uint8_t _stp_module_1_debug_line_str[] =  
  {};
...

BUT given that similar constructs are in stap_symbols.c anyway, such as totally unrelated to this patch:

...
static uint8_t _stp_module_2_debug_frame[] =
  {};
...

So since this isn't an unprecedented syntactical construct within stap_symbols.c, I suspect this might actually be a good solution. If that's the case, then the only thing that needs to be removed is the comment about the need to improve this... Thoughts?

> 
> @@ -7474,6 +7499,8 @@ dump_unwindsym_cxt (Dwfl_Module *m,
>    Dwarf_Addr eh_frame_hdr_addr = c->eh_frame_hdr_addr;
>    void *debug_line = c->debug_line;
>    size_t debug_line_len = c->debug_line_len;
> +  void *debug_line_str = c->debug_line_str;
> +  size_t debug_line_str_len = c->debug_line_str_len;
>  
>    dump_unwindsym_cxt_table(c->session, c->output, modname, stpmod_idx, "",
> 0,
>  			   "debug_frame", debug_frame, debug_len);
> @@ -7487,6 +7514,9 @@ dump_unwindsym_cxt (Dwfl_Module *m,
>    dump_unwindsym_cxt_table(c->session, c->output, modname, stpmod_idx, "",
> 0,
>  			   "debug_line", debug_line, debug_line_len);
>  
> +  dump_unwindsym_cxt_table(c->session, c->output, modname, stpmod_idx, "",
> 0,
> +			   "debug_line_str", debug_line_str, debug_line_str_len);
> +
>    if (c->session.need_unwind && debug_frame == NULL && eh_frame == NULL)
>      {
>        // There would be only a small benefit to warning.  A user
> 
> OK
> 
> @@ -7505,6 +7535,13 @@ dump_unwindsym_cxt (Dwfl_Module *m,
>                                    dwfl_errmsg (-1));
>      }
>  
> +  if (c->session.need_lines && debug_line_str == NULL)
> +    {
> +      if (c->session.verbose > 2)
> +        c->session.print_warning ("No debug line str data for " + modname +
> ", " +
> +                                  dwfl_errmsg (-1));
> +    }
> +
> 
> Does this always warn for DWARF4?

Yes, it does:

f40 x86_64 # stap --ldd -e 'probe process.function("main") { print_ubacktrace_fileline() }' -c ./prog4 -vvvvv --poison-cache |& fgrep 'No debug line str data for'
WARNING: No debug line str data for /lib/modules/6.6.0-0.rc6.47.fc40.x86_64/vdso/vdso32.so, no error
WARNING: No debug line str data for /lib/modules/6.6.0-0.rc6.47.fc40.x86_64/vdso/vdso64.so, no error
WARNING: No debug line str data for /lib64/libc.so.6, no error
WARNING: No debug line str data for /root/build/test/prog4, no error
^[[A^C
f40 x86_64 # stap --ldd -e 'probe process.function("main") { print_ubacktrace_fileline() }' -c ./prog5 -vvvvv --poison-cache |& fgrep 'No debug line str data for'
WARNING: No debug line str data for /lib/modules/6.6.0-0.rc6.47.fc40.x86_64/vdso/vdso32.so, no error
WARNING: No debug line str data for /lib/modules/6.6.0-0.rc6.47.fc40.x86_64/vdso/vdso64.so, no error
WARNING: No debug line str data for /lib64/libc.so.6, no error
^C
f40 x86_64 # 

> 
>    for (unsigned secidx = 0; secidx < c->seclist.size(); secidx++)
>      {
>        c->output << "static struct _stp_symbol "
> @@ -7685,11 +7722,16 @@ dump_unwindsym_cxt (Dwfl_Module *m,
>        c->output << ".debug_line = "
>  		<< "_stp_module_" << stpmod_idx << "_debug_line, \n";
>        c->output << ".debug_line_len = " << debug_line_len << ", \n";
> +      c->output << ".debug_line_str = "
> +		<< "_stp_module_" << stpmod_idx << "_debug_line_str, \n";
> +      c->output << ".debug_line_str_len = " << debug_line_str_len << ", \n";
>        c->output << "#else\n";
>      }
>  
>    c->output << ".debug_line = NULL,\n";
>    c->output << ".debug_line_len = 0,\n";
> +  c->output << ".debug_line_str = NULL,\n";
> +  c->output << ".debug_line_str_len = 0,\n";
>  
>    if (debug_line != NULL)
>      c->output << "#endif /* STP_NEED_LINE_DATA */\n";
> @@ -7873,6 +7915,8 @@ dump_unwindsyms (Dwfl_Module *m,
>  
>    c->debug_line = NULL;
>    c->debug_line_len = 0;
> +  c->debug_line_str = NULL;
> +  c->debug_line_str_len = 0;
>    if (res == DWARF_CB_OK && c->session.need_lines)
>      // we dont set res = dump_line_tables() because unwindsym stuff should
> still
>      // get dumped to the output even if gathering debug_line data fails
> @@ -8044,6 +8088,8 @@ emit_symbol_data (systemtap_session& s)
>  				 0, /* eh_frame_hdr_addr */
>  				 NULL, /* debug_line */
>  				 0, /* debug_line_len */
> +				 NULL, /* debug_line_str */
> +				 0, /* debug_line_str_len */
>  				 s.unwindsym_modules };
>  
>    // Micro optimization, mainly to speed up tiny regression tests
> @@ -8139,6 +8185,8 @@ self_unwind_declarations(unwindsym_dump_context *ctx)
>    ctx->output << ".debug_frame_len = 0,\n";
>    ctx->output << ".debug_line = NULL,\n";
>    ctx->output << ".debug_line_len = 0,\n";
> +  ctx->output << ".debug_line_str = NULL,\n";
> +  ctx->output << ".debug_line_str_len = 0,\n";
>    ctx->output << "};\n";
>  }
> 
> All the code that adds the extra debug_line_str and debug_line_str_len looks
> correct.

Thanks a ton for the review!  I'm attaching an updated patch based on your comments.  Please review.

Created attachment 15186 [details]
possible patch

I've tested the patch on all supported arches for RHEL8 and RHEL9 and attached updated patch. Since apparently the RHEL8 kernel build system uses -std=gnu90, I've dropped the initial loop declarations.  Other than that the patch seems to work fine with both GCC and clang, on both RHEL8 and RHEL9, all supported arches.

I've pushed the current state of things to a private branch:
https://sourceware.org/git/?p=systemtap.git;a=shortlog;h=refs/heads/mcermak-﷒0﷓

Fixed in commit 367392917b68653abb16baa98cd1a4d02605f3de .

*** Bug 29984 has been marked as a duplicate of this bug. ***