diff mbox series

[PATCHv3,1/4] gdb/dwarf: remove line_header parameter from dwarf2_decode_lines

Message ID 4fee2c4476dffd7b8acac5ba4cae0e090a85eaf1.1766414210.git.aburgess@redhat.com
State New
Headers
Series Fixes related to misplaced symtabs originating from dwz files |

Commit Message

Andrew Burgess Dec. 22, 2025, 2:41 p.m. UTC 
  The function declaration for dwarf2_decode_lines is:

  void dwarf_decode_lines (struct line_header *lh, struct dwarf2_cu *cu,
                           unrelocated_addr lowpc, bool decode_mapping)

However, it is always the case that:

  lh == cu->line_header

I propose that we simplify the dwarf_decode_lines signature by
removing the line_header parameter.  The same is true for
dwarf_decode_lines_1, which is only called from dwarf_decode_lines.

I'm proposing this change because I was looking through the DWARF
code, trying to understand how symtabs are created, and this extra
complexity just makes things harder to understand: what is the
relationship between the line_header (LH) and dwarf2_cu (CU)
parameters?  When would we ever want to process a line_header other
than the one attached to CU? (answer: never, and we don't).  This
simplification makes things easier to understand (IMHO).
There should be no user visible changes after this commit.
---
 gdb/dwarf2/line-program.c | 15 ++++++++-------
 gdb/dwarf2/line-program.h | 10 ++--------
 gdb/dwarf2/read.c         |  2 +-
 3 files changed, 11 insertions(+), 16 deletions(-)

Comments

Simon Marchi Dec. 24, 2025, 5:17 a.m. UTC | #1 
On 2025-12-22 09:41, Andrew Burgess wrote:
> The function declaration for dwarf2_decode_lines is:
> 
>   void dwarf_decode_lines (struct line_header *lh, struct dwarf2_cu *cu,
>                            unrelocated_addr lowpc, bool decode_mapping)
> 
> However, it is always the case that:
> 
>   lh == cu->line_header
> 
> I propose that we simplify the dwarf_decode_lines signature by
> removing the line_header parameter.  The same is true for
> dwarf_decode_lines_1, which is only called from dwarf_decode_lines.
> 
> I'm proposing this change because I was looking through the DWARF
> code, trying to understand how symtabs are created, and this extra
> complexity just makes things harder to understand: what is the
> relationship between the line_header (LH) and dwarf2_cu (CU)
> parameters?  When would we ever want to process a line_header other
> than the one attached to CU? (answer: never, and we don't).  This
> simplification makes things easier to understand (IMHO).
> There should be no user visible changes after this commit.

Could you do the same for dwarf2_start_subfile and lnp_state_machine?

Simon
diff mbox series

Patch

diff --git a/gdb/dwarf2/line-program.c b/gdb/dwarf2/line-program.c
index c30f70dd11a..75c4a060a09 100644
--- a/gdb/dwarf2/line-program.c
+++ b/gdb/dwarf2/line-program.c
@@ -481,12 +481,12 @@  lnp_state_machine::check_line_address (struct dwarf2_cu *cu,
 }
 
 /* Subroutine of dwarf_decode_lines to simplify it.
-   Process the line number information in LH.  */
+   Process the line number information in CU::line_header.  */
 
 static void
-dwarf_decode_lines_1 (struct line_header *lh, struct dwarf2_cu *cu,
-		      unrelocated_addr lowpc)
+dwarf_decode_lines_1 (struct dwarf2_cu *cu, unrelocated_addr lowpc)
 {
+  struct line_header *lh = cu->line_header;
   const gdb_byte *line_ptr, *extended_end;
   const gdb_byte *line_end;
   unsigned int bytes_read, extended_len;
@@ -694,11 +694,11 @@  dwarf_decode_lines_1 (struct line_header *lh, struct dwarf2_cu *cu,
 /* See dwarf2/line-program.h.  */
 
 void
-dwarf_decode_lines (struct line_header *lh, struct dwarf2_cu *cu,
-		    unrelocated_addr lowpc, bool decode_mapping)
+dwarf_decode_lines (struct dwarf2_cu *cu, unrelocated_addr lowpc,
+		    bool decode_mapping)
 {
   if (decode_mapping)
-    dwarf_decode_lines_1 (lh, cu, lowpc);
+    dwarf_decode_lines_1 (cu, lowpc);
 
   /* Make sure a symtab is created for every file, even files
      which contain only variables (i.e. no code with associated
@@ -706,7 +706,8 @@  dwarf_decode_lines (struct line_header *lh, struct dwarf2_cu *cu,
   buildsym_compunit *builder = cu->get_builder ();
   struct compunit_symtab *cust = builder->get_compunit_symtab ();
 
-  for (auto &fe : lh->file_names ())
+  struct line_header *lh = cu->line_header;
+  for (file_entry &fe : lh->file_names ())
     {
       dwarf2_start_subfile (cu, fe, *lh);
       subfile *sf = builder->get_current_subfile ();
diff --git a/gdb/dwarf2/line-program.h b/gdb/dwarf2/line-program.h
index 824f18f9613..522dd994fc8 100644
--- a/gdb/dwarf2/line-program.h
+++ b/gdb/dwarf2/line-program.h
@@ -20,12 +20,7 @@ 
 #ifndef GDB_DWARF2_LINE_PROGRAM_H
 #define GDB_DWARF2_LINE_PROGRAM_H
 
-/* Decode the Line Number Program (LNP) for the given line_header
-   structure and CU.  The actual information extracted and the type
-   of structures created from the LNP depends on the value of PST.
-
-   FND holds the CU file name and directory, if known.
-   It is used for relative paths in the line table.
+/* Decode the Line Number Program (LNP) for CU::line_header.
 
    NOTE: It is important that psymtabs have the same file name (via
    strcmp) as the corresponding symtab.  Since the directory is not
@@ -40,8 +35,7 @@ 
    for its PC<->lines mapping information.  Otherwise only the filename
    table is read in.  */
 
-extern void dwarf_decode_lines (struct line_header *lh,
-				struct dwarf2_cu *cu,
+extern void dwarf_decode_lines (struct dwarf2_cu *cu,
 				unrelocated_addr lowpc, bool decode_mapping);
 
 #endif /* GDB_DWARF2_LINE_PROGRAM_H */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 12f3e5f0e3d..6fb76ddad9d 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -6099,7 +6099,7 @@  handle_DW_AT_stmt_list (struct die_info *die, struct dwarf2_cu *cu,
      then there won't be any interesting code in the CU, but a check later on
      (in lnp_state_machine::check_line_address) will fail to properly exclude
      an entry that was removed via --gc-sections.  */
-  dwarf_decode_lines (cu->line_header, cu, lowpc, decode_mapping && have_code);
+  dwarf_decode_lines (cu, lowpc, decode_mapping && have_code);
 }
 
 /* Process DW_TAG_compile_unit or DW_TAG_partial_unit.  */