Hi Cupertino,
On 2/27/26 08:08, Cupertino Miranda wrote:
> Hi everyone, David
>
> This is v2 for line_info support.
> Looking forward to your review.
Very nice.
I tried it with a couple of tests locally and it works well, now
we get accurate source line info from verifier errors :)
I have one small comment below, but otherwise LGTM.
OK with that addressed.
Thanks!
>
> Cheers,
> Cupertino
>
> Changes from v1:
> - Corrected typos.
> - Fixed line_col encoding and created macros.
> - Added a test case for line_info.
> - Improved line and column info repetition detection.
> Previous patch was generating multiple entries for the same line and
> column data for consecutive labels.
>
>
> This patch adds line_info debug information support to .BTF.ext
> sections.
>
> Line info information is used by the BPF verifier to improve error
> reporting and give more precise source code referenced errors.
>
> gcc/ChangeLog:
> PR target/113453
> * config/bpf/bpf-protos.h (bpf_output_call): Change prototype.
> * config/bpf/bpf.cc (bpf_output_call): Change to adapt operands
> and return
> the instruction template instead of immediately emit asm and
> not allow proper final expected execution flow.
> (bpf_output_line_info): Add function to introduce line info
> entries in respective structures
> (bpf_asm_out_unwind_emit): Add function as hook to
> TARGET_ASM_UNWIND_EMIT. This hook is called before any
> instruction is emitted.
> * config/bpf/bpf.md: Change calls to bpf_output_call.
> * config/bpf/btfext-out.cc (struct btf_ext_lineinfo): Add fields
> to struct.
> (bpf_create_lineinfo, btf_add_line_info_for): Add support
> function to insert line_info data in respective structures.
> (output_btfext_line_info): Function to emit line_info data in
> .BTF.ext section.
> (btf_ext_output): Call output_btfext_line_info.
> * config/bpf/btfext-out.h: Add prototype for
> btf_add_line_info_for.
>
> gcc/testsuite/ChangeLog:
> PR target/113453
> * gcc.target/bpf/btfext-funcinfo.c: Adapt test.
> * gcc.target/bpf/btfext-lineinfo.c: New test.
> ---
> gcc/config/bpf/bpf-protos.h | 2 +-
> gcc/config/bpf/bpf.cc | 105 +++++++++++++---
> gcc/config/bpf/bpf.md | 4 +-
> gcc/config/bpf/btfext-out.cc | 114 +++++++++++++++++-
> gcc/config/bpf/btfext-out.h | 4 +
> .../gcc.target/bpf/btfext-funcinfo.c | 3 +-
> .../gcc.target/bpf/btfext-lineinfo.c | 51 ++++++++
> 7 files changed, 262 insertions(+), 21 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/bpf/btfext-lineinfo.c
>
> diff --git a/gcc/config/bpf/bpf-protos.h b/gcc/config/bpf/bpf-protos.h
> index 4a4799e9bf22..1113cc3e6d17 100644
> --- a/gcc/config/bpf/bpf-protos.h
> +++ b/gcc/config/bpf/bpf-protos.h
> @@ -23,7 +23,7 @@ along with GCC; see the file COPYING3. If not see
> /* Routines implemented in bpf.cc. */
>
> extern HOST_WIDE_INT bpf_initial_elimination_offset (int, int);
> -extern const char *bpf_output_call (rtx);
> +extern const char *bpf_output_call (const char *templ, rtx *, int
> target_index);
> extern void bpf_target_macros (cpp_reader *);
> extern void bpf_print_operand (FILE *, rtx, int);
> extern void bpf_print_operand_address (FILE *, rtx);
> diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
> index d91013ad140b..4773d789d8e3 100644
> --- a/gcc/config/bpf/bpf.cc
> +++ b/gcc/config/bpf/bpf.cc
> @@ -745,14 +745,12 @@ bpf_output_destructor (rtx symbol, int priority
> ATTRIBUTE_UNUSED)
> bpf.md. */
>
> const char *
> -bpf_output_call (rtx target)
> +bpf_output_call (const char *templ, rtx *operands, int target_index)
> {
> - rtx xops[1];
> -
> + rtx target = operands[target_index];
> switch (GET_CODE (target))
> {
> case CONST_INT:
> - output_asm_insn ("call\t%0", &target);
> break;
> case SYMBOL_REF:
> {
> @@ -765,26 +763,21 @@ bpf_output_call (rtx target)
> {
> tree attr_args = TREE_VALUE (attr);
>
> - xops[0] = GEN_INT (TREE_INT_CST_LOW (TREE_VALUE (attr_args)));
> - output_asm_insn ("call\t%0", xops);
> + operands[target_index] =
> + GEN_INT (TREE_INT_CST_LOW (TREE_VALUE (attr_args)));
> }
> - else
> - output_asm_insn ("call\t%0", &target);
> -
> break;
> }
> default:
> - if (TARGET_XBPF)
> - output_asm_insn ("call\t%0", &target);
> - else
> + if (!TARGET_XBPF)
> {
> error ("indirect call in function, which are not supported by eBPF");
> - output_asm_insn ("call 0", NULL);
> + operands[target_index] = GEN_INT (0);
> }
> break;
> }
>
> - return "";
> + return templ;
> }
>
> const char *
> @@ -1145,6 +1138,90 @@ bpf_debug_unwind_info ()
> #undef TARGET_DEBUG_UNWIND_INFO
> #define TARGET_DEBUG_UNWIND_INFO bpf_debug_unwind_info
>
> +/* Create a BTF.ext line_info entry. */
> +
> +static void
> +bpf_output_line_info (FILE *asm_out_file, rtx_insn *insn)
> +{
> + static unsigned int line_info_label = 1;
> + static tree cfun_decl = NULL_TREE;
> + static bool func_start_added = false;
> + const char *label = NULL;
> + unsigned int loc = 0;
> + expanded_location exploc = {NULL, 0, 0, NULL, false};
> + static expanded_location prev_exploc = {NULL, 0, 0, NULL, false};
> +
> + if(!btf_debuginfo_p () || flag_building_libgcc)
> + return;
> +
> + gcc_assert (insn != NULL_RTX);
> +
> + if (current_function_decl != cfun_decl
> + && GET_CODE (insn) == NOTE)
> + {
> + label = current_function_func_begin_label;
> + loc = DECL_SOURCE_LOCATION (current_function_decl);
> + exploc = expand_location (loc);
> + func_start_added = true;
> + prev_exploc = exploc;
> + }
> + else
> + {
> + if (GET_CODE (insn) == NOTE)
> + return;
> +
> + /* Already added a label for this location. This might not be fully
> + accurate but it is better then adding 2 entries on the same location,
> + which is incompatible with the verifier expectations. */
> + if (func_start_added == true)
> + {
> + func_start_added = false;
> + return;
> + }
> +
> +
> + if (!INSN_HAS_LOCATION (insn))
> + return;
> +
> + exploc = insn_location (insn);
> +
> + if (exploc.file == NULL || exploc.line == 0
> + || (exploc.line == prev_exploc.line
> + && exploc.column == prev_exploc.column))
> + return;
> +
> + prev_exploc = exploc;
> +
> + char tmp_label[25];
> + sprintf (tmp_label, "LI%u", line_info_label);
> + ASM_OUTPUT_LABEL (asm_out_file, tmp_label);
> + line_info_label += 1;
> + label = const_cast<char *> (ggc_strdup (tmp_label));
> + }
> +
> + cfun_decl = current_function_decl;
> +
> + if (exploc.file != NULL && exploc.line != 0)
> + btf_add_line_info_for (label, exploc.file, exploc.line, exploc.column);
> +}
> +
> +
> +/* This hook is defined as a way for BPF target to create a label before each
> + emitted instruction and emit line_info information. This data is later
> + output in .BTF.ext section.
> + This approach expects TARGET_EMIT_BEFORE_INSN to be returning TRUE as
> + this function needs to be called before the instruction is emitted.
> Current
> + default behaviour returns TRUE and the hook is left undefined. */
> +
> +static void
> +bpf_asm_out_unwind_emit (FILE *asm_out_file, rtx_insn *insn)
> +{
> + bpf_output_line_info (asm_out_file, insn);
> +}
> +
> +#undef TARGET_ASM_UNWIND_EMIT
> +#define TARGET_ASM_UNWIND_EMIT bpf_asm_out_unwind_emit
> +
> /* Output assembly directives to assemble data of various sized and
> alignments. */
>
> diff --git a/gcc/config/bpf/bpf.md b/gcc/config/bpf/bpf.md
> index 0eec006d3cfe..bc739f2746c0 100644
> --- a/gcc/config/bpf/bpf.md
> +++ b/gcc/config/bpf/bpf.md
> @@ -545,7 +545,7 @@
> ;; operands[2] is next_arg_register
> ;; operands[3] is struct_value_size_rtx.
> ""
> - { return bpf_output_call (operands[0]); }
> + { return bpf_output_call ("call\t%0", operands, 0); }
> [(set_attr "type" "jmp")])
>
> (define_expand "call_value"
> @@ -568,7 +568,7 @@
> ;; operands[3] is next_arg_register
> ;; operands[4] is struct_value_size_rtx.
> ""
> - { return bpf_output_call (operands[1]); }
> + { return bpf_output_call ("call\t%1", operands, 1); }
> [(set_attr "type" "jmp")])
>
> (define_insn "sibcall"
> diff --git a/gcc/config/bpf/btfext-out.cc b/gcc/config/bpf/btfext-out.cc
> index 04bc60d69b0e..9a7e34aeb634 100644
> --- a/gcc/config/bpf/btfext-out.cc
> +++ b/gcc/config/bpf/btfext-out.cc
> @@ -32,6 +32,7 @@
> #include "rtl.h"
> #include "tree-pretty-print.h"
> #include "cgraph.h"
> +#include "toplev.h" /* get_src_pwd */
>
> #include "btfext-out.h"
>
> @@ -124,14 +125,19 @@ struct GTY ((chain_next ("%h.next"))) btf_ext_funcinfo
> /* A lineinfo record, in the .BTF.ext lineinfo section. */
> struct GTY ((chain_next ("%h.next"))) btf_ext_lineinfo
> {
> - uint32_t insn_off; /* Offset of the instruction. */
> + const char *insn_label; /* Instruction label. */
> + const char *file_name; /* Source-code file name. */
> uint32_t file_name_off; /* Offset of file name in BTF string table. */
> uint32_t line_off; /* Offset of source line in BTF string table. */
> - uint32_t line_col; /* Line number (bits 31-11) and column (11-0). */
> + uint32_t line_col; /* Line number (bits 31-10) and column (9-0). */
>
> struct btf_ext_lineinfo *next; /* Linked list to collect line_info elems.
> */
> };
>
> +#define BTF_LINE_INFO_LINE(line_col) ((line_col) >> 10)
> +#define BTF_LINE_INFO_COLUMN(line_col) ((line_col) & 0x3ff)
> +#define BTF_LINE_INFO_LINE_COL(line, column) (line << 10 | (column & 0x3ff))
> +
> /* Internal representation of a BPF CO-RE relocation record. */
> struct GTY ((chain_next ("%h.next"))) btf_ext_core_reloc {
> ctf_dtdef_ref bpfcr_type; /* BTF type involved in relocation. */
> @@ -235,6 +241,26 @@ bpf_create_or_find_funcinfo (const char *fnname, const
> char *sec_name,
> return *head;
> }
>
> +/* Function to create a lineinfo node in info. */
> +
> +static struct btf_ext_lineinfo *
> +bpf_create_lineinfo (const char *sec_name, btf_ext_info_sec **in_sec = NULL)
> +{
> + struct btf_ext_info_sec *sec_elem =
> + btfext_info_sec_find_or_add (sec_name, true);
> +
> + if (in_sec != NULL)
> + *in_sec = sec_elem;
> +
> + struct btf_ext_lineinfo **head =
> + SEARCH_NODE_AND_RETURN(struct btf_ext_lineinfo,
> + sec_elem->line_info.head,
> + false);
> + *head = ggc_cleared_alloc<struct btf_ext_lineinfo> ();
> +
> + return *head;
> +}
> +
> /* Function to create a core_reloc node in info. */
>
> static struct btf_ext_core_reloc *
> @@ -438,6 +464,47 @@ btf_validate_funcinfo (btf_ext_info_sec *sec)
> }
> }
>
> +#define STR_BUFF_SIZE 256
> +
> +struct btf_ext_lineinfo *
> +btf_add_line_info_for (const char *label, const char *filename,
> + unsigned int line, unsigned int column)
> +{
> + const char *sec_name = decl_section_name (current_function_decl);
> +
> + if (sec_name == NULL)
> + sec_name = ".text";
> +
> + struct btf_ext_info_sec *sec = NULL;
> + struct btf_ext_lineinfo *info =
> + bpf_create_lineinfo (sec_name, &sec);
> +
> + info->insn_label = label;
> +
> + if (!IS_DIR_SEPARATOR (filename[0]))
> + {
> + char full_filename[STR_BUFF_SIZE];
> +
> + /* Filename is a relative path. */
> + const char * cu_pwd = get_src_pwd ();
> + gcc_assert (strlen (cu_pwd) + strlen (filename) + 2 < STR_BUFF_SIZE);
> +
> + sprintf (full_filename, "%s%c%s", cu_pwd, DIR_SEPARATOR, filename);
On second look I don't like using gcc_assert here; this will result in an
ICE simply if the input filename is very long...
While I doubt that will occur much in practice, we should still deal with
it gracefully. IMO just truncating via e.g. snprintf would be fine and
much better than causing an ICE.
> + info->file_name = ggc_strdup (full_filename);
> + }
> + else
> + /* Filename is an absolute path. */
> + info->file_name = ggc_strdup (filename);
> +
> + info->file_name_off = btf_ext_add_string (info->file_name);
> + info->line_off = 0;
> + info->line_col = BTF_LINE_INFO_LINE_COL (line, column);
> +
> + sec->line_info.num_info += 1;
> + return info;
> +}
> +#undef STR_BUFF_SIZE
> +
> /* Compute the section size in section for func_info, line_info and core_info
> regions of .BTF.ext. */
>
> @@ -545,6 +612,48 @@ output_btfext_func_info (struct btf_ext_info_sec *sec)
> }
> }
>
> +/* Outputs line_info region on .BTF.ext. */
> +
> +static void
> +output_btfext_line_info (struct btf_ext_info_sec *sec)
> +{
> + unsigned int str_aux_off = ctfc_get_strtab_len (ctf_get_tu_ctfc (),
> + CTF_STRTAB);
> + bool executed = false;
> + while (sec != NULL)
> + {
> + uint32_t count = 0;
> + if (sec->line_info.num_info > 0)
> + {
> + if (executed == false && (executed = true))
> + dw2_asm_output_data (4, 16, "LineInfo entry size");
> + dw2_asm_output_data (4, sec->sec_name_off + str_aux_off,
> + "LineInfo section string for %s",
> + sec->sec_name);
> + dw2_asm_output_data (4, sec->line_info.num_info, "Number of entries");
> +
> + struct btf_ext_lineinfo *elem = sec->line_info.head;
> + while (elem != NULL)
> + {
> + count += 1;
> + dw2_asm_output_offset (4, elem->insn_label, NULL, "insn_label");
> +
> + unsigned int file_name_off = btf_ext_add_string (elem->file_name);
> + dw2_asm_output_data (4, file_name_off + str_aux_off,
> + "file_name_off");
> + dw2_asm_output_data (4, elem->line_off, "line_off");
> + dw2_asm_output_data (4, elem->line_col, "(line, col) (%u, %u)",
> + BTF_LINE_INFO_LINE (elem->line_col),
> + BTF_LINE_INFO_COLUMN (elem->line_col));
> + elem = elem->next;
> + }
> + }
> +
> + gcc_assert (count == sec->line_info.num_info);
> + sec = sec->next;
> + }
> +}
> +
> /* Output all CO-RE relocation sections. */
>
> static void
> @@ -622,6 +731,7 @@ btf_ext_output (void)
>
> output_btfext_header ();
> output_btfext_func_info (btf_ext);
> + output_btfext_line_info (btf_ext);
> if (TARGET_BPF_CORE)
> output_btfext_core_sections ();
>
> diff --git a/gcc/config/bpf/btfext-out.h b/gcc/config/bpf/btfext-out.h
> index a4c87ffd1f02..fe9b3768643e 100644
> --- a/gcc/config/bpf/btfext-out.h
> +++ b/gcc/config/bpf/btfext-out.h
> @@ -99,6 +99,10 @@ extern int bpf_core_get_sou_member_index
> (ctf_container_ref, const tree);
>
> struct btf_ext_funcinfo *btf_add_func_info_for (tree decl,
> const char *label);
> +struct btf_ext_lineinfo *
> +btf_add_line_info_for (const char *label, const char *filename,
> + unsigned int line, unsigned int column);
> +
> unsigned int btf_ext_add_string (const char *str);
>
> #ifdef __cplusplus
> diff --git a/gcc/testsuite/gcc.target/bpf/btfext-funcinfo.c
> b/gcc/testsuite/gcc.target/bpf/btfext-funcinfo.c
> index fbbefeae68f0..f7f5b44b1ff4 100644
> --- a/gcc/testsuite/gcc.target/bpf/btfext-funcinfo.c
> +++ b/gcc/testsuite/gcc.target/bpf/btfext-funcinfo.c
> @@ -39,6 +39,5 @@ int bar_func (struct T *t)
> /* { dg-final { scan-assembler-times "ascii \"bar_sec.0\"\[\t
> \]+\[^\n\]*btf_aux_string" 1 } } */
> /* { dg-final { scan-assembler-times "FuncInfo entry size" 1 } } */
>
> -/* { dg-final { scan-assembler-times ".4byte\t0x1\t# Number of entries" 3 }
> } */
> -/* { dg-final { scan-assembler-times ".4byte\t0x2\t# Number of entries" 1 }
> } */
> +/* { dg-final { scan-assembler-times "FuncInfo
> section\[^\n\]*\n\[^0\]*0x1\t# Number of entries" 2 } } */
> /* { dg-final { scan-assembler-times "Required padding" 1 } } */
> diff --git a/gcc/testsuite/gcc.target/bpf/btfext-lineinfo.c
> b/gcc/testsuite/gcc.target/bpf/btfext-lineinfo.c
> new file mode 100644
> index 000000000000..dc1c297d663e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/bpf/btfext-lineinfo.c
> @@ -0,0 +1,51 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -dA -gbtf -masm=normal" } */
> +
> +struct T {
> + int a;
> + int b;
> + struct U {
> + int c;
> + struct V {
> + int d;
> + int e[4];
> + int f;
> + } v;
> + } u;
> +} __attribute__((preserve_access_index));
> +
> +__attribute__((section("foo_sec"), used))
> +int foo_func (struct T *t)
> +{
> + t->u.c = 5;
> + return t->u.v.e[3];
> +}
> +
> +__attribute__((section("bar_sec"), used))
> +int bar_func (struct T *t)
> +{
> + int *x = &(t->u.v.f);
> + int old = *x;
> + *x = 4;
> + return old;
> +}
> +
> +/* { dg-final { scan-assembler-times "LineInfo section string for foo_sec" 1
> } } */
> +/* { dg-final { scan-assembler-times "LineInfo section string for bar_sec" 1
> } } */
> +/* { dg-final { scan-assembler-times "label for function foo_func" 1 } } */
> +/* { dg-final { scan-assembler-times "label for function bar_func" 1 } } */
> +
> +
> +/* { dg-final { scan-assembler-times "btfext-lineinfo\.c.0\"\[\t
> \]+\[^\n\]*btf_aux_string" 1 } } */
> +/* { dg-final { scan-assembler-times "4byte\[\t \]+\.LFB.\[\t \]+#
> insn_label" 2 } } */
> +/* { dg-final { scan-assembler-times "4byte\[\t \]+LI.\[\t \]+# insn_label"
> 6 } } */
> +
> +/* { dg-final { scan-assembler-times "# \\(line, col\\)" 8 } } */
> +/* { dg-final { scan-assembler-times "# \\(line, col\\) \\(18, 5\\)" 1 } } */
> +/* { dg-final { scan-assembler-times "# \\(line, col\\) \\(20, 10\\)" 1 } }
> */
> +/* { dg-final { scan-assembler-times "# \\(line, col\\) \\(21, 18\\)" 1 } }
> */
> +/* { dg-final { scan-assembler-times "# \\(line, col\\) \\(22, 1\\)" 1 } } */
> +/* { dg-final { scan-assembler-times "# \\(line, col\\) \\(25, 5\\)" 1 } } */
> +/* { dg-final { scan-assembler-times "# \\(line, col\\) \\(28, 7\\)" 1 } } */
> +/* { dg-final { scan-assembler-times "# \\(line, col\\) \\(29, 6\\)" 1 } } */
> +/* { dg-final { scan-assembler-times "# \\(line, col\\) \\(31, 1\\)" 1 } } */
