BPF List
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Alan Maguire <alan.maguire@oracle.com>
Cc: Jiri Olsa <jolsa@kernel.org>,
	Clark Williams <williams@redhat.com>,
	dwarves@vger.kernel.org, bpf@vger.kernel.org,
	Andrii Nakryiko <andrii@kernel.org>,
	Yonghong Song <yonghong.song@linux.dev>,
	Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: [PATCH 22/31] dwarf_loader, btf_loader: Replace stale FIXME/XXX comments with explanations
Date: Wed, 29 Jul 2026 16:07:22 -0300	[thread overview]
Message-ID: <20260729190733.72876-23-acme@kernel.org> (raw)
In-Reply-To: <20260729190733.72876-1-acme@kernel.org>

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Several FIXME and XXX comments referenced issues that were already
resolved or described intentional behavior that was never going to
change.  Replace them with proper explanations of why the code
behaves the way it does:

- Call site tags (DW_TAG_call_site): useful for debuggers, not for
  pahole's type reconstruction.

- Formal parameters in inline expansions: duplicates of the abstract
  origin's parameters, not needed for type reconstruction.

- Template value parameters on ftypes: already attached to the
  ftype's template value param list and used by class__fprintf.

- DW_TAG_dwarf_procedure: compiler-internal DWARF expressions with
  no type information to extract.

- BTF_KIND_DATASEC: describes runtime variable placement in ELF
  sections, intentionally ignored for type reconstruction.

Assisted-by: Claude:claude-sonnet-4-5
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 btf_loader.c   |  5 +++--
 dwarf_loader.c | 52 ++++++++++++++++++++++----------------------------
 2 files changed, 26 insertions(+), 31 deletions(-)

diff --git a/btf_loader.c b/btf_loader.c
index caead39775da54da..5857c3b6d2fc4b2b 100644
--- a/btf_loader.c
+++ b/btf_loader.c
@@ -428,8 +428,9 @@ static int create_new_datasec(struct cu *cu __maybe_unused, const struct btf_typ
 	//cu__add_tag_with_id(cu, &datasec->tag, id);
 
 	/*
-	 * FIXME: this will not be used to reconstruct some original C code,
-	 * its about runtime placement of variables so just ignore this for now
+	 * BTF_KIND_DATASEC describes runtime variable placement in ELF
+	 * sections, not C type information.  Not needed for pahole's
+	 * type reconstruction, so intentionally ignored.
 	 */
 	return 0;
 }
diff --git a/dwarf_loader.c b/dwarf_loader.c
index 04dffae3504efa06..61d55bd70d643731 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -2529,13 +2529,10 @@ static int die__process_inline_expansion(Dwarf_Die *die, struct lexblock *lexblo
 		case DW_TAG_GNU_call_site:
 		case DW_TAG_GNU_call_site_parameter:
 			/*
- 			 * FIXME: read http://www.dwarfstd.org/ShowIssue.php?issue=100909.2&type=open
- 			 * and write proper support.
-			 *
-			 * From a quick read there is not much we can use in
-			 * the existing dwarves tools, so just stop warning the user,
-			 * developers will find these notes if wanting to use in a
-			 * new tool.
+			 * Call site tags describe interprocedural call
+			 * metadata (callee, parameters, return values).
+			 * Useful for debuggers but not for pahole's type
+			 * reconstruction.  Silently skip them.
 			 */
 			continue;
 		case DW_TAG_lexical_block:
@@ -2544,15 +2541,10 @@ static int die__process_inline_expansion(Dwarf_Die *die, struct lexblock *lexblo
 			continue;
 		case DW_TAG_formal_parameter:
 			/*
-			 * FIXME:
-			 * So far DW_TAG_inline_routine had just an
-			 * abstract origin, but starting with
-			 * /usr/lib/openoffice.org/basis3.0/program/libdbalx.so
-			 * I realized it really has to be handled as a
-			 * DW_TAG_function... Lets just get the types
-			 * for 1.8, then fix this properly.
-			 *
-			 * cu__tag_not_handled(cu, die);
+			 * Inline expansions can have their own formal
+			 * parameter children duplicating the abstract
+			 * origin's parameters.  These are not needed
+			 * for type reconstruction — skip them.
 			 */
 			continue;
 		case DW_TAG_inlined_subroutine:
@@ -2635,13 +2627,10 @@ static int die__process_function(Dwarf_Die *die, struct ftype *ftype,
 		case DW_TAG_GNU_call_site:
 		case DW_TAG_GNU_call_site_parameter:
 			/*
-			 * XXX: read http://www.dwarfstd.org/ShowIssue.php?issue=100909.2&type=open
-			 * and write proper support.
-			 *
-			 * From a quick read there is not much we can use in
-			 * the existing dwarves tools, so just stop warning the user,
-			 * developers will find these notes if wanting to use in a
-			 * new tool.
+			 * Call site tags describe interprocedural call
+			 * metadata (callee, parameters, return values).
+			 * Useful for debuggers but not for pahole's type
+			 * reconstruction.  Silently skip them.
 			 */
 			continue;
 		case DW_TAG_dwarf_procedure:
@@ -2678,9 +2667,10 @@ static int die__process_function(Dwarf_Die *die, struct ftype *ftype,
 			continue;
 		}
 		case DW_TAG_template_value_parameter: {
-			/* FIXME: probably we'll have to attach this as a list of
-			 * template parameters to use at class__fprintf time... 
-			 * See die__process_class */
+			/*
+			 * Attached to the ftype's template value param list,
+			 * used by class__fprintf for C++ template display.
+			 */
 			struct template_value_param *tvparm = template_value_param__new(die, cu, conf);
 
 			if (tvparm == NULL)
@@ -2868,10 +2858,14 @@ static int die__process_unit(Dwarf_Die *die, struct cu *cu, struct conf_load *co
 			return -ENOMEM;
 
 		if (tag == &unsupported_tag) {
-			// XXX special case DW_TAG_dwarf_procedure, appears when looking at a recent ~/bin/perf
-			// Investigate later how to properly support this...
+			/*
+			 * DW_TAG_dwarf_procedure: compiler-internal
+			 * DWARF expressions, no type info to extract.
+			 * DW_TAG_label: skipped via conf->ignore_labels.
+			 * DW_TAG_GNU_annotation: handled elsewhere.
+			 */
 			if (dwarf_tag(die) != DW_TAG_dwarf_procedure &&
-			    dwarf_tag(die) != DW_TAG_label && // conf->ignore_labels == true, see die__process_tag()
+			    dwarf_tag(die) != DW_TAG_label &&
 			    dwarf_tag(die) != DW_TAG_GNU_annotation)
 				tag__print_not_supported(die);
 			continue;
-- 
2.55.0


  parent reply	other threads:[~2026-07-29 19:08 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-29 19:07 [PATCHES 00/31] pahole: Bug fixes and small improvements Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 01/31] cmake: Update minimum required version from 3.5 to 3.10 Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 02/31] Fix -Wsign-compare warnings across the codebase Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 03/31] btf_encoder: Fix interior pointer free and missing NULL check Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 04/31] dwarves: Fix missing list head initialization in type__clone_members Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 05/31] pahole: Fix instance memory leak on early returns in prototype__stdio_fprintf_value Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 06/31] ctf_loader, libctf: Fix error path resource leaks Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 07/31] dwarves: Don't search for holes before member byte sizes are cached Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 08/31] dwarf_loader: Allocate type_dcu via dwarf_cu__new to fix dangling stack pointer Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 09/31] dwarf_loader: Fix annotation failure leaks in variable and typedef creation Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 10/31] btf_encoder: Use btf_encoder__tag_type() for all type ID computations Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 11/31] pahole: Fix --errno typo that decrements instead of negating Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 12/31] dwarves: Fix heap buffer overflow in languages__parse realloc Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 13/31] btf_encoder: Fix early cleanup crashes in btf_encoder__new/delete Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 14/31] btf_encoder, libctf: Add elf_strptr NULL checks and fix kfunc bounds Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 15/31] dwarf_loader: Fix --fixup_silly_bitfields condition check Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 16/31] dwarf_loader: Skip libdw__lock when elfutils is built thread-safe Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 17/31] dwarf_loader: Fix data race in tag__init() decl_file string cache Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 18/31] pahole: Fix parse_btf_features("all") being a silent no-op Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 19/31] dwarves: Fix variable shadowing in __cus__find_struct_by_name() Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 20/31] dutil: Add exec_objcopy() shell-injection-safe helper Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 21/31] btf_encoder: Fall back to objcopy when llvm-objcopy is not available Arnaldo Carvalho de Melo
2026-07-29 19:07 ` Arnaldo Carvalho de Melo [this message]
2026-07-29 19:07 ` [PATCH 23/31] pahole: Skip inline expansions during BTF encoding Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 24/31] pahole: Use fseek for seekable files in --prettify and --seek_bytes Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 25/31] pahole: Guard pipe_seek() against negative offsets Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 26/31] gobuffer: Remove 5 dead functions found via coverage analysis Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 27/31] dwarves: Remove 6 " Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 28/31] pfunct, dwarves_fprintf: Mark file-local functions as static Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 29/31] btf_encoder: Fix multi-dimensional array encoding Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 30/31] btf_loader: Fix multi-dimensional array loading Arnaldo Carvalho de Melo
2026-07-29 19:07 ` [PATCH 31/31] btfdiff: Remove --flat_arrays now that pahole encodes multi dim arrays in BTF Arnaldo Carvalho de Melo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260729190733.72876-23-acme@kernel.org \
    --to=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=alan.maguire@oracle.com \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=dwarves@vger.kernel.org \
    --cc=jolsa@kernel.org \
    --cc=williams@redhat.com \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox