> 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.

There is no need to truncate.
Just use xasprintf.

>
>> +      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 } } 
>> */

Reply via email to