[PATCH] [gdb/symtab] Factor out new_symbol_file_line

[PATCH] [gdb/symtab] Factor out new_symbol_file_line

[Tom de Vries]

I was looking at a patch [1] modifying the DW_AT_decl_file / DW_AT_call_file
handling of new_symbol, and noticed that:
- the code is fairly nested, and
- the patch adds another nesting layer.

Factor out function new_symbol_file_line out of new_symbol, handling both the
DW_AT_decl_file / DW_AT_call_file and DW_AT_call_line / DW_AT_decl_line
attributes.

Having factored out the code, simplify it using return, and modernize it using
bool and nullptr.

[1] https://sourceware.org/pipermail/gdb-patches/2026-March/226065.html
---
 gdb/dwarf2/read.c | 87 +++++++++++++++++++++++++++--------------------
 1 file changed, 51 insertions(+), 36 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 375e8a75db4..d6695627bb2 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -15426,6 +15426,55 @@ is_ada_import_or_export (dwarf2_cu *cu, const char *name,
 	  && !streq (linkagename, "adainit"));
 }
 
+/* Apply the DW_AT_{call,decl}_{file,line} attributes of DIE in CU to SYM.  */
+
+static void
+new_symbol_file_line (struct die_info *die, struct dwarf2_cu *cu,
+		      struct symbol *sym)
+{
+  struct attribute *attr = nullptr;
+
+  bool inlined_func = (die->tag == DW_TAG_inlined_subroutine);
+
+  /* Handle DW_AT_call_line / DW_AT_decl_line.  */
+  attr = dwarf2_attr (die,
+		      inlined_func ? DW_AT_call_line : DW_AT_decl_line,
+		      cu);
+  if (attr != nullptr)
+    sym->set_line (attr->unsigned_constant ().value_or (0));
+
+  /* Handle DW_AT_call_file / DW_AT_decl_file.  */
+  struct dwarf2_cu *file_cu = cu;
+  attr = dwarf2_attr (die,
+		      inlined_func ? DW_AT_call_file : DW_AT_decl_file,
+		      &file_cu);
+  if (attr == nullptr)
+    return;
+
+  std::optional<ULONGEST> index_cst = attr->unsigned_constant ();
+  if (!index_cst.has_value ())
+    return;
+
+  if (file_cu->line_header == nullptr)
+    {
+      file_and_directory fnd (nullptr, nullptr);
+      decode_line_header_for_cu (file_cu->dies, file_cu, fnd);
+    }
+
+  file_name_index file_index = (file_name_index) *index_cst;
+  struct file_entry *fe = nullptr;
+  if (file_cu->line_header != nullptr)
+    fe = file_cu->line_header->file_name_at (file_index);
+
+  if (fe == nullptr)
+    {
+      complaint (_("file index out of range"));
+      return;
+    }
+
+  sym->set_symtab (fe->symtab (*file_cu));
+}
+
 /* Given a pointer to a DWARF information entry, figure out if we need
    to make a symbol table entry for it, and if so, create a new entry
    and return a pointer to it.
@@ -15446,8 +15495,6 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
   struct attribute *attr2 = NULL;
   std::vector<symbol *> *list_to_add = nullptr;
 
-  int inlined_func = (die->tag == DW_TAG_inlined_subroutine);
-
   name = dwarf2_name (die, cu);
   if (name == nullptr && (die->tag == DW_TAG_subprogram
 			  || die->tag == DW_TAG_inlined_subroutine
@@ -15500,41 +15547,9 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	sym->set_type (type);
       else
 	sym->set_type (die_type (die, cu));
-      attr = dwarf2_attr (die,
-			  inlined_func ? DW_AT_call_line : DW_AT_decl_line,
-			  cu);
-      if (attr != nullptr)
-	sym->set_line (attr->unsigned_constant ().value_or (0));
 
-      struct dwarf2_cu *file_cu = cu;
-      attr = dwarf2_attr (die,
-			  inlined_func ? DW_AT_call_file : DW_AT_decl_file,
-			  &file_cu);
-      if (attr != nullptr)
-	{
-	  std::optional<ULONGEST> index_cst = attr->unsigned_constant ();
-	  if (index_cst.has_value ())
-	    {
-	      file_name_index file_index = (file_name_index) *index_cst;
-	      struct file_entry *fe;
-
-	      if (file_cu->line_header == nullptr)
-		{
-		  file_and_directory fnd (nullptr, nullptr);
-		  decode_line_header_for_cu (file_cu->dies, file_cu, fnd);
-		}
-
-	      if (file_cu->line_header != nullptr)
-		fe = file_cu->line_header->file_name_at (file_index);
-	      else
-		fe = NULL;
-
-	      if (fe == NULL)
-		complaint (_("file index out of range"));
-	      else
-		sym->set_symtab (fe->symtab (*file_cu));
-	    }
-	}
+      /* Handle DW_AT_{call,decl}_{file,line}.  */
+      new_symbol_file_line (die, cu, sym);
 
       switch (die->tag)
 	{

base-commit: c365a263f5dcf95982464cf6d53db00cc6f04c1c
-- 
2.51.0

Re: [PATCH] [gdb/symtab] Factor out new_symbol_file_line

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> I was looking at a patch [1] modifying the DW_AT_decl_file / DW_AT_call_file
Tom> handling of new_symbol, and noticed that:
Tom> - the code is fairly nested, and
Tom> - the patch adds another nesting layer.

Tom> Factor out function new_symbol_file_line out of new_symbol, handling both the
Tom> DW_AT_decl_file / DW_AT_call_file and DW_AT_call_line / DW_AT_decl_line
Tom> attributes.

Thanks for doing this.

I think new_symbol really needs to be split up, the way it is now is
pretty bad, and your patch is a nice step in that direction.

Tom> Having factored out the code, simplify it using return, and modernize it using
Tom> bool and nullptr.

Tom> +  struct attribute *attr = nullptr;
Tom> +
Tom> +  bool inlined_func = (die->tag == DW_TAG_inlined_subroutine);
Tom> +
Tom> +  /* Handle DW_AT_call_line / DW_AT_decl_line.  */
Tom> +  attr = dwarf2_attr (die,
Tom> +		      inlined_func ? DW_AT_call_line : DW_AT_decl_line,
Tom> +		      cu);

This line could be combined with the declaration and then the
initialization removed like

attribute *attr = dwarf2_attr (...)

Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH] [gdb/symtab] Factor out new_symbol_file_line

[Tom de Vries]

On 4/22/26 6:51 PM, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> I was looking at a patch [1] modifying the DW_AT_decl_file / DW_AT_call_file
> Tom> handling of new_symbol, and noticed that:
> Tom> - the code is fairly nested, and
> Tom> - the patch adds another nesting layer.
> 
> Tom> Factor out function new_symbol_file_line out of new_symbol, handling both the
> Tom> DW_AT_decl_file / DW_AT_call_file and DW_AT_call_line / DW_AT_decl_line
> Tom> attributes.
> 
> Thanks for doing this.
> 
> I think new_symbol really needs to be split up, the way it is now is
> pretty bad, and your patch is a nice step in that direction.
>

Hi Tom,

Thanks for the review, pushed with the nit pointed out below fixed.

I don't know whether this is what you had in mind, but I've attempted a 
split up here ( 
https://sourceware.org/pipermail/gdb-patches/2026-April/226731.html ).

Thanks,
- Tom


> Tom> Having factored out the code, simplify it using return, and modernize it using
> Tom> bool and nullptr.
> 
> Tom> +  struct attribute *attr = nullptr;
> Tom> +
> Tom> +  bool inlined_func = (die->tag == DW_TAG_inlined_subroutine);
> Tom> +
> Tom> +  /* Handle DW_AT_call_line / DW_AT_decl_line.  */
> Tom> +  attr = dwarf2_attr (die,
> Tom> +		      inlined_func ? DW_AT_call_line : DW_AT_decl_line,
> Tom> +		      cu);
> 
> This line could be combined with the declaration and then the
> initialization removed like
> 
> attribute *attr = dwarf2_attr (...)
> 
> Approved-By: Tom Tromey <tom@tromey.com>
> 
> Tom