diff mbox series

[3/3] bpf: add line_info support to BTF.ext section.

Message ID 20240411111118.215612-3-cupertino.miranda@oracle.com
State New
Headers show
Series [1/3] bpf: support more instructions to match CO-RE relocations | expand

Commit Message

Cupertino Miranda April 11, 2024, 11:11 a.m. UTC
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 core referenced errors.

gcc/Changelog:
	* 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 emmidiatelly 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/config/bpf/bpf-protos.h                   |   2 +-
 gcc/config/bpf/bpf.cc                         | 103 ++++++++++++++---
 gcc/config/bpf/bpf.md                         |   4 +-
 gcc/config/bpf/btfext-out.cc                  | 108 +++++++++++++++++-
 gcc/config/bpf/btfext-out.h                   |   4 +
 .../gcc.target/bpf/btfext-funcinfo.c          |   3 +-
 6 files changed, 203 insertions(+), 21 deletions(-)

Comments

Jose E. Marchesi April 17, 2024, 3:14 p.m. UTC | #1
Hi Cuper.
Thanks for the patch.

> 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 core referenced errors.
>
> gcc/Changelog:
> 	* 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 emmidiatelly 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.

Is it actually necessary to emit a label (plus .BTF.ext entry) for every
instruction?

> 	* 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/config/bpf/bpf-protos.h                   |   2 +-
>  gcc/config/bpf/bpf.cc                         | 103 ++++++++++++++---
>  gcc/config/bpf/bpf.md                         |   4 +-
>  gcc/config/bpf/btfext-out.cc                  | 108 +++++++++++++++++-
>  gcc/config/bpf/btfext-out.h                   |   4 +
>  .../gcc.target/bpf/btfext-funcinfo.c          |   3 +-
>  6 files changed, 203 insertions(+), 21 deletions(-)
>
> diff --git a/gcc/config/bpf/bpf-protos.h b/gcc/config/bpf/bpf-protos.h
> index b4866d34209..ddaca50af69 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 d9141dd625a..f1a8eb8d62c 100644
> --- a/gcc/config/bpf/bpf.cc
> +++ b/gcc/config/bpf/bpf.cc
> @@ -754,14 +754,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:
>        {
> @@ -774,26 +772,20 @@ 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);
> -	  }
> -	else
> -	  output_asm_insn ("call\t%0", &target);
> +	    operands[target_index] = GEN_INT (TREE_INT_CST_LOW (TREE_VALUE (attr_args)));
>  
> +	  }
>  	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 *
> @@ -1144,6 +1136,87 @@ 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;
> +  const char *filename = NULL;
> +  unsigned int line = 0;
> +  unsigned int column = 0;
> +
> +  if(!btf_debuginfo_p ())
> +    return;

I think it would be better to put this guard in bpf_asm_out_unwind_emit
instead of bpf_output_line_info.

> +
> +  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);
> +      filename = LOCATION_FILE (loc);
> +      line = LOCATION_LINE (loc);
> +      column = LOCATION_COLUMN (loc);
> +      func_start_added = true;
> +    }
> +  else
> +    {
> +      if (GET_CODE (insn) == NOTE)
> +	return;
> +
> +      /* Already added a label for this location. This might not be fully
> +	 acurate but it is better then adding 2 entries on the same location,
> +	 which is imcompatible with the verifier expectations.  */
> +      if (func_start_added == true)
> +	{
> +	  func_start_added = false;
> +	  return;
> +	}
> +
> +      loc = INSN_LOCATION (insn);
> +      filename = LOCATION_FILE (loc);
> +      line = LOCATION_LINE (loc);
> +      column = LOCATION_COLUMN (loc);
> +
> +      if (filename == NULL || line == 0)
> +	return;
> +
> +      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 (filename != NULL && line != 0)
> +    btf_add_line_info_for (label, filename, line, 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 returing 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 95859328d25..3fdf81b86a6 100644
> --- a/gcc/config/bpf/bpf.md
> +++ b/gcc/config/bpf/bpf.md
> @@ -546,7 +546,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"
> @@ -569,7 +569,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 ff1fd0739f1..42ec48e394e 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,7 +125,8 @@ 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).  */
> @@ -235,6 +237,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 *
> @@ -429,6 +451,47 @@ btf_validate_funcinfo (btf_ext_info_sec *sec)
>      }
>  }
>  
> +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);
> +
> +  unsigned int line_column = ((0x000fffff & line) << 12)
> +			     | (0x00000fff & column);
> +
> +  info->insn_label = label;
> +
> +  if (!IS_DIR_SEPARATOR (filename[0]))
> +    {
> +      char full_filename[256];
> +
> +      /* Filename is a relative path.  */
> +      const char * cu_pwd = get_src_pwd ();
> +      gcc_assert (strlen (cu_pwd) + strlen (filename) + 2 < 256);
> +
> +      sprintf(full_filename, "%s%c%s", cu_pwd, DIR_SEPARATOR, filename);
> +      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 = line_column;
> +
> +  sec->line_info.num_info += 1;
> +  return info;
> +}
> +
>  /* Compute the section size in section for func_info, line_info and core_info
>     regions of .BTF.ext.  */
>  
> @@ -537,6 +600,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)",
> +				   elem->line_col >> 12,
> +				   elem->line_col & 0x00000fff);
> +	      elem = elem->next;
> +	    }
> +	}
> +
> +      gcc_assert (count == sec->line_info.num_info);
> +      sec = sec->next;
> +    }
> +}
> +
>  /* Output all CO-RE relocation sections.  */
>  
>  static void
> @@ -609,6 +714,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 b36309475c9..9c6848324e7 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 6fdd14574ec..0f1e0ad1e89 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 } } */
Cupertino Miranda April 17, 2024, 3:47 p.m. UTC | #2
Jose E. Marchesi writes:

> Hi Cuper.
> Thanks for the patch.
>
>> 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 core referenced errors.
>>
>> gcc/Changelog:
>> 	* 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 emmidiatelly 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.
>
> Is it actually necessary to emit a label (plus .BTF.ext entry) for every
> instruction?
Maybe BPF would be Ok (not complaining of missing line_info) with just a
single entry per function pointing to the entry instruction. That
is not what clang does. Don't know if it emits any labels either.

It is probably possible to add some logic to compute the offset from the
function label and emit with an offset to the instruction location.
In case of inline assembly we could add a symbol after, and restart
offset computation.
It will need to add code to compute the instruction encoding size, and
increment function label offset each time we emit an instruction.

Still, my personal preference is to create those labels to properly
compute instruction location then some rather intricate solution that
would lead to future complications.
I know BPF is not like all the other targets, but I am thinking of
assembly/linker relaxation.

WDYT ?

>> 	* 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/config/bpf/bpf-protos.h                   |   2 +-
>>  gcc/config/bpf/bpf.cc                         | 103 ++++++++++++++---
>>  gcc/config/bpf/bpf.md                         |   4 +-
>>  gcc/config/bpf/btfext-out.cc                  | 108 +++++++++++++++++-
>>  gcc/config/bpf/btfext-out.h                   |   4 +
>>  .../gcc.target/bpf/btfext-funcinfo.c          |   3 +-
>>  6 files changed, 203 insertions(+), 21 deletions(-)
>>
>> diff --git a/gcc/config/bpf/bpf-protos.h b/gcc/config/bpf/bpf-protos.h
>> index b4866d34209..ddaca50af69 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 d9141dd625a..f1a8eb8d62c 100644
>> --- a/gcc/config/bpf/bpf.cc
>> +++ b/gcc/config/bpf/bpf.cc
>> @@ -754,14 +754,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:
>>        {
>> @@ -774,26 +772,20 @@ 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);
>> -	  }
>> -	else
>> -	  output_asm_insn ("call\t%0", &target);
>> +	    operands[target_index] = GEN_INT (TREE_INT_CST_LOW (TREE_VALUE (attr_args)));
>>
>> +	  }
>>  	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 *
>> @@ -1144,6 +1136,87 @@ 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;
>> +  const char *filename = NULL;
>> +  unsigned int line = 0;
>> +  unsigned int column = 0;
>> +
>> +  if(!btf_debuginfo_p ())
>> +    return;
>
> I think it would be better to put this guard in bpf_asm_out_unwind_emit
> instead of bpf_output_line_info.
>
>> +
>> +  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);
>> +      filename = LOCATION_FILE (loc);
>> +      line = LOCATION_LINE (loc);
>> +      column = LOCATION_COLUMN (loc);
>> +      func_start_added = true;
>> +    }
>> +  else
>> +    {
>> +      if (GET_CODE (insn) == NOTE)
>> +	return;
>> +
>> +      /* Already added a label for this location. This might not be fully
>> +	 acurate but it is better then adding 2 entries on the same location,
>> +	 which is imcompatible with the verifier expectations.  */
>> +      if (func_start_added == true)
>> +	{
>> +	  func_start_added = false;
>> +	  return;
>> +	}
>> +
>> +      loc = INSN_LOCATION (insn);
>> +      filename = LOCATION_FILE (loc);
>> +      line = LOCATION_LINE (loc);
>> +      column = LOCATION_COLUMN (loc);
>> +
>> +      if (filename == NULL || line == 0)
>> +	return;
>> +
>> +      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 (filename != NULL && line != 0)
>> +    btf_add_line_info_for (label, filename, line, 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 returing 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 95859328d25..3fdf81b86a6 100644
>> --- a/gcc/config/bpf/bpf.md
>> +++ b/gcc/config/bpf/bpf.md
>> @@ -546,7 +546,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"
>> @@ -569,7 +569,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 ff1fd0739f1..42ec48e394e 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,7 +125,8 @@ 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).  */
>> @@ -235,6 +237,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 *
>> @@ -429,6 +451,47 @@ btf_validate_funcinfo (btf_ext_info_sec *sec)
>>      }
>>  }
>>
>> +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);
>> +
>> +  unsigned int line_column = ((0x000fffff & line) << 12)
>> +			     | (0x00000fff & column);
>> +
>> +  info->insn_label = label;
>> +
>> +  if (!IS_DIR_SEPARATOR (filename[0]))
>> +    {
>> +      char full_filename[256];
>> +
>> +      /* Filename is a relative path.  */
>> +      const char * cu_pwd = get_src_pwd ();
>> +      gcc_assert (strlen (cu_pwd) + strlen (filename) + 2 < 256);
>> +
>> +      sprintf(full_filename, "%s%c%s", cu_pwd, DIR_SEPARATOR, filename);
>> +      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 = line_column;
>> +
>> +  sec->line_info.num_info += 1;
>> +  return info;
>> +}
>> +
>>  /* Compute the section size in section for func_info, line_info and core_info
>>     regions of .BTF.ext.  */
>>
>> @@ -537,6 +600,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)",
>> +				   elem->line_col >> 12,
>> +				   elem->line_col & 0x00000fff);
>> +	      elem = elem->next;
>> +	    }
>> +	}
>> +
>> +      gcc_assert (count == sec->line_info.num_info);
>> +      sec = sec->next;
>> +    }
>> +}
>> +
>>  /* Output all CO-RE relocation sections.  */
>>
>>  static void
>> @@ -609,6 +714,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 b36309475c9..9c6848324e7 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 6fdd14574ec..0f1e0ad1e89 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 } } */
Jose E. Marchesi April 17, 2024, 5:48 p.m. UTC | #3
> Jose E. Marchesi writes:
>
>> Hi Cuper.
>> Thanks for the patch.
>>
>>> 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 core referenced errors.
>>>
>>> gcc/Changelog:
>>> 	* 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 emmidiatelly 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.
>>
>> Is it actually necessary to emit a label (plus .BTF.ext entry) for every
>> instruction?
> Maybe BPF would be Ok (not complaining of missing line_info) with just a
> single entry per function pointing to the entry instruction. That
> is not what clang does. Don't know if it emits any labels either.
>
> It is probably possible to add some logic to compute the offset from
> the function label and emit with an offset to the instruction
> location.  In case of inline assembly we could add a symbol after, and
> restart offset computation.  It will need to add code to compute the
> instruction encoding size, and increment function label offset each
> time we emit an instruction.
>
> Still, my personal preference is to create those labels to properly
> compute instruction location then some rather intricate solution that
> would lead to future complications.  I know BPF is not like all the
> other targets, but I am thinking of assembly/linker relaxation.
>
> WDYT ?

What I meant is: if it is not required to emit a line_info entry for
_every_ BPF instruction, but only for the instructions that "change" the
current location, then we better do so?

Then, regarding the labels, I assume their purpose is to get the
assembler to fill in the `insn_off' field of the bpf_line_info in the
.BTF.ext section:

    struct bpf_line_info {
        __u32   insn_off; /* [0, insn_cnt - 1] */
        __u32   file_name_off; /* offset to string table for the filename */
        __u32   line_off; /* offset to string table for the source line */
        __u32   line_col; /* line number and column number */
    };

Which makes sense, since "instruction offset" is really the business of
the assembler, not the compiler.  I agree with you making it the
compiler's business would be overcomplicated, given inline assembly and
variable-sized BPF instructions...

So, what about moving the task of creating these line_info entries
entirely to the assembler?  GCC already knows how to emit .file and .loc
directives to track location info in DWARF.

The BPF assembler could then process these and create entries in
.BTF.ext for line_info, all the fields above: insn_off, file_name_off,
line_off and line_col.

>>> 	* 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/config/bpf/bpf-protos.h                   |   2 +-
>>>  gcc/config/bpf/bpf.cc                         | 103 ++++++++++++++---
>>>  gcc/config/bpf/bpf.md                         |   4 +-
>>>  gcc/config/bpf/btfext-out.cc                  | 108 +++++++++++++++++-
>>>  gcc/config/bpf/btfext-out.h                   |   4 +
>>>  .../gcc.target/bpf/btfext-funcinfo.c          |   3 +-
>>>  6 files changed, 203 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/gcc/config/bpf/bpf-protos.h b/gcc/config/bpf/bpf-protos.h
>>> index b4866d34209..ddaca50af69 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 d9141dd625a..f1a8eb8d62c 100644
>>> --- a/gcc/config/bpf/bpf.cc
>>> +++ b/gcc/config/bpf/bpf.cc
>>> @@ -754,14 +754,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:
>>>        {
>>> @@ -774,26 +772,20 @@ 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);
>>> -	  }
>>> -	else
>>> -	  output_asm_insn ("call\t%0", &target);
>>> +	    operands[target_index] = GEN_INT (TREE_INT_CST_LOW (TREE_VALUE (attr_args)));
>>>
>>> +	  }
>>>  	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 *
>>> @@ -1144,6 +1136,87 @@ 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;
>>> +  const char *filename = NULL;
>>> +  unsigned int line = 0;
>>> +  unsigned int column = 0;
>>> +
>>> +  if(!btf_debuginfo_p ())
>>> +    return;
>>
>> I think it would be better to put this guard in bpf_asm_out_unwind_emit
>> instead of bpf_output_line_info.
>>
>>> +
>>> +  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);
>>> +      filename = LOCATION_FILE (loc);
>>> +      line = LOCATION_LINE (loc);
>>> +      column = LOCATION_COLUMN (loc);
>>> +      func_start_added = true;
>>> +    }
>>> +  else
>>> +    {
>>> +      if (GET_CODE (insn) == NOTE)
>>> +	return;
>>> +
>>> +      /* Already added a label for this location. This might not be fully
>>> +	 acurate but it is better then adding 2 entries on the same location,
>>> +	 which is imcompatible with the verifier expectations.  */
>>> +      if (func_start_added == true)
>>> +	{
>>> +	  func_start_added = false;
>>> +	  return;
>>> +	}
>>> +
>>> +      loc = INSN_LOCATION (insn);
>>> +      filename = LOCATION_FILE (loc);
>>> +      line = LOCATION_LINE (loc);
>>> +      column = LOCATION_COLUMN (loc);
>>> +
>>> +      if (filename == NULL || line == 0)
>>> +	return;
>>> +
>>> +      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 (filename != NULL && line != 0)
>>> +    btf_add_line_info_for (label, filename, line, 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 returing 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 95859328d25..3fdf81b86a6 100644
>>> --- a/gcc/config/bpf/bpf.md
>>> +++ b/gcc/config/bpf/bpf.md
>>> @@ -546,7 +546,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"
>>> @@ -569,7 +569,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 ff1fd0739f1..42ec48e394e 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,7 +125,8 @@ 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).  */
>>> @@ -235,6 +237,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 *
>>> @@ -429,6 +451,47 @@ btf_validate_funcinfo (btf_ext_info_sec *sec)
>>>      }
>>>  }
>>>
>>> +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);
>>> +
>>> +  unsigned int line_column = ((0x000fffff & line) << 12)
>>> +			     | (0x00000fff & column);
>>> +
>>> +  info->insn_label = label;
>>> +
>>> +  if (!IS_DIR_SEPARATOR (filename[0]))
>>> +    {
>>> +      char full_filename[256];
>>> +
>>> +      /* Filename is a relative path.  */
>>> +      const char * cu_pwd = get_src_pwd ();
>>> +      gcc_assert (strlen (cu_pwd) + strlen (filename) + 2 < 256);
>>> +
>>> +      sprintf(full_filename, "%s%c%s", cu_pwd, DIR_SEPARATOR, filename);
>>> +      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 = line_column;
>>> +
>>> +  sec->line_info.num_info += 1;
>>> +  return info;
>>> +}
>>> +
>>>  /* Compute the section size in section for func_info, line_info and core_info
>>>     regions of .BTF.ext.  */
>>>
>>> @@ -537,6 +600,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)",
>>> +				   elem->line_col >> 12,
>>> +				   elem->line_col & 0x00000fff);
>>> +	      elem = elem->next;
>>> +	    }
>>> +	}
>>> +
>>> +      gcc_assert (count == sec->line_info.num_info);
>>> +      sec = sec->next;
>>> +    }
>>> +}
>>> +
>>>  /* Output all CO-RE relocation sections.  */
>>>
>>>  static void
>>> @@ -609,6 +714,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 b36309475c9..9c6848324e7 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 6fdd14574ec..0f1e0ad1e89 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 } } */
Jose E. Marchesi April 17, 2024, 5:55 p.m. UTC | #4
>> Jose E. Marchesi writes:
>>
>>> Hi Cuper.
>>> Thanks for the patch.
>>>
>>>> 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 core referenced errors.
>>>>
>>>> gcc/Changelog:
>>>> 	* 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 emmidiatelly 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.
>>>
>>> Is it actually necessary to emit a label (plus .BTF.ext entry) for every
>>> instruction?
>> Maybe BPF would be Ok (not complaining of missing line_info) with just a
>> single entry per function pointing to the entry instruction. That
>> is not what clang does. Don't know if it emits any labels either.
>>
>> It is probably possible to add some logic to compute the offset from
>> the function label and emit with an offset to the instruction
>> location.  In case of inline assembly we could add a symbol after, and
>> restart offset computation.  It will need to add code to compute the
>> instruction encoding size, and increment function label offset each
>> time we emit an instruction.
>>
>> Still, my personal preference is to create those labels to properly
>> compute instruction location then some rather intricate solution that
>> would lead to future complications.  I know BPF is not like all the
>> other targets, but I am thinking of assembly/linker relaxation.
>>
>> WDYT ?
>
> What I meant is: if it is not required to emit a line_info entry for
> _every_ BPF instruction, but only for the instructions that "change" the
> current location, then we better do so?
>
> Then, regarding the labels, I assume their purpose is to get the
> assembler to fill in the `insn_off' field of the bpf_line_info in the
> .BTF.ext section:
>
>     struct bpf_line_info {
>         __u32   insn_off; /* [0, insn_cnt - 1] */
>         __u32   file_name_off; /* offset to string table for the filename */
>         __u32   line_off; /* offset to string table for the source line */
>         __u32   line_col; /* line number and column number */
>     };
>
> Which makes sense, since "instruction offset" is really the business of
> the assembler, not the compiler.  I agree with you making it the
> compiler's business would be overcomplicated, given inline assembly and
> variable-sized BPF instructions...
>
> So, what about moving the task of creating these line_info entries
> entirely to the assembler?  GCC already knows how to emit .file and .loc
> directives to track location info in DWARF.
>
> The BPF assembler could then process these and create entries in
> .BTF.ext for line_info, all the fields above: insn_off, file_name_off,
> line_off and line_col.

Regarding file_name_off, hopefully it will be possible to make the
assembler to simply expand the string table in .BTF (with the strings
read from .file directives) without having to understand/redo the whole
BTF section...

>>>> 	* 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/config/bpf/bpf-protos.h                   |   2 +-
>>>>  gcc/config/bpf/bpf.cc                         | 103 ++++++++++++++---
>>>>  gcc/config/bpf/bpf.md                         |   4 +-
>>>>  gcc/config/bpf/btfext-out.cc                  | 108 +++++++++++++++++-
>>>>  gcc/config/bpf/btfext-out.h                   |   4 +
>>>>  .../gcc.target/bpf/btfext-funcinfo.c          |   3 +-
>>>>  6 files changed, 203 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/gcc/config/bpf/bpf-protos.h b/gcc/config/bpf/bpf-protos.h
>>>> index b4866d34209..ddaca50af69 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 d9141dd625a..f1a8eb8d62c 100644
>>>> --- a/gcc/config/bpf/bpf.cc
>>>> +++ b/gcc/config/bpf/bpf.cc
>>>> @@ -754,14 +754,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:
>>>>        {
>>>> @@ -774,26 +772,20 @@ 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);
>>>> -	  }
>>>> -	else
>>>> -	  output_asm_insn ("call\t%0", &target);
>>>> +	    operands[target_index] = GEN_INT (TREE_INT_CST_LOW (TREE_VALUE (attr_args)));
>>>>
>>>> +	  }
>>>>  	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 *
>>>> @@ -1144,6 +1136,87 @@ 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;
>>>> +  const char *filename = NULL;
>>>> +  unsigned int line = 0;
>>>> +  unsigned int column = 0;
>>>> +
>>>> +  if(!btf_debuginfo_p ())
>>>> +    return;
>>>
>>> I think it would be better to put this guard in bpf_asm_out_unwind_emit
>>> instead of bpf_output_line_info.
>>>
>>>> +
>>>> +  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);
>>>> +      filename = LOCATION_FILE (loc);
>>>> +      line = LOCATION_LINE (loc);
>>>> +      column = LOCATION_COLUMN (loc);
>>>> +      func_start_added = true;
>>>> +    }
>>>> +  else
>>>> +    {
>>>> +      if (GET_CODE (insn) == NOTE)
>>>> +	return;
>>>> +
>>>> +      /* Already added a label for this location. This might not be fully
>>>> +	 acurate but it is better then adding 2 entries on the same location,
>>>> +	 which is imcompatible with the verifier expectations.  */
>>>> +      if (func_start_added == true)
>>>> +	{
>>>> +	  func_start_added = false;
>>>> +	  return;
>>>> +	}
>>>> +
>>>> +      loc = INSN_LOCATION (insn);
>>>> +      filename = LOCATION_FILE (loc);
>>>> +      line = LOCATION_LINE (loc);
>>>> +      column = LOCATION_COLUMN (loc);
>>>> +
>>>> +      if (filename == NULL || line == 0)
>>>> +	return;
>>>> +
>>>> +      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 (filename != NULL && line != 0)
>>>> +    btf_add_line_info_for (label, filename, line, 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 returing 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 95859328d25..3fdf81b86a6 100644
>>>> --- a/gcc/config/bpf/bpf.md
>>>> +++ b/gcc/config/bpf/bpf.md
>>>> @@ -546,7 +546,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"
>>>> @@ -569,7 +569,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 ff1fd0739f1..42ec48e394e 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,7 +125,8 @@ 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).  */
>>>> @@ -235,6 +237,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 *
>>>> @@ -429,6 +451,47 @@ btf_validate_funcinfo (btf_ext_info_sec *sec)
>>>>      }
>>>>  }
>>>>
>>>> +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);
>>>> +
>>>> +  unsigned int line_column = ((0x000fffff & line) << 12)
>>>> +			     | (0x00000fff & column);
>>>> +
>>>> +  info->insn_label = label;
>>>> +
>>>> +  if (!IS_DIR_SEPARATOR (filename[0]))
>>>> +    {
>>>> +      char full_filename[256];
>>>> +
>>>> +      /* Filename is a relative path.  */
>>>> +      const char * cu_pwd = get_src_pwd ();
>>>> +      gcc_assert (strlen (cu_pwd) + strlen (filename) + 2 < 256);
>>>> +
>>>> +      sprintf(full_filename, "%s%c%s", cu_pwd, DIR_SEPARATOR, filename);
>>>> +      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 = line_column;
>>>> +
>>>> +  sec->line_info.num_info += 1;
>>>> +  return info;
>>>> +}
>>>> +
>>>>  /* Compute the section size in section for func_info, line_info and core_info
>>>>     regions of .BTF.ext.  */
>>>>
>>>> @@ -537,6 +600,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)",
>>>> +				   elem->line_col >> 12,
>>>> +				   elem->line_col & 0x00000fff);
>>>> +	      elem = elem->next;
>>>> +	    }
>>>> +	}
>>>> +
>>>> +      gcc_assert (count == sec->line_info.num_info);
>>>> +      sec = sec->next;
>>>> +    }
>>>> +}
>>>> +
>>>>  /* Output all CO-RE relocation sections.  */
>>>>
>>>>  static void
>>>> @@ -609,6 +714,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 b36309475c9..9c6848324e7 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 6fdd14574ec..0f1e0ad1e89 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 mbox series

Patch

diff --git a/gcc/config/bpf/bpf-protos.h b/gcc/config/bpf/bpf-protos.h
index b4866d34209..ddaca50af69 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 d9141dd625a..f1a8eb8d62c 100644
--- a/gcc/config/bpf/bpf.cc
+++ b/gcc/config/bpf/bpf.cc
@@ -754,14 +754,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:
       {
@@ -774,26 +772,20 @@  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);
-	  }
-	else
-	  output_asm_insn ("call\t%0", &target);
+	    operands[target_index] = GEN_INT (TREE_INT_CST_LOW (TREE_VALUE (attr_args)));
 
+	  }
 	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 *
@@ -1144,6 +1136,87 @@  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;
+  const char *filename = NULL;
+  unsigned int line = 0;
+  unsigned int column = 0;
+
+  if(!btf_debuginfo_p ())
+    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);
+      filename = LOCATION_FILE (loc);
+      line = LOCATION_LINE (loc);
+      column = LOCATION_COLUMN (loc);
+      func_start_added = true;
+    }
+  else
+    {
+      if (GET_CODE (insn) == NOTE)
+	return;
+
+      /* Already added a label for this location. This might not be fully
+	 acurate but it is better then adding 2 entries on the same location,
+	 which is imcompatible with the verifier expectations.  */
+      if (func_start_added == true)
+	{
+	  func_start_added = false;
+	  return;
+	}
+
+      loc = INSN_LOCATION (insn);
+      filename = LOCATION_FILE (loc);
+      line = LOCATION_LINE (loc);
+      column = LOCATION_COLUMN (loc);
+
+      if (filename == NULL || line == 0)
+	return;
+
+      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 (filename != NULL && line != 0)
+    btf_add_line_info_for (label, filename, line, 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 returing 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 95859328d25..3fdf81b86a6 100644
--- a/gcc/config/bpf/bpf.md
+++ b/gcc/config/bpf/bpf.md
@@ -546,7 +546,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"
@@ -569,7 +569,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 ff1fd0739f1..42ec48e394e 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,7 +125,8 @@  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).  */
@@ -235,6 +237,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 *
@@ -429,6 +451,47 @@  btf_validate_funcinfo (btf_ext_info_sec *sec)
     }
 }
 
+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);
+
+  unsigned int line_column = ((0x000fffff & line) << 12)
+			     | (0x00000fff & column);
+
+  info->insn_label = label;
+
+  if (!IS_DIR_SEPARATOR (filename[0]))
+    {
+      char full_filename[256];
+
+      /* Filename is a relative path.  */
+      const char * cu_pwd = get_src_pwd ();
+      gcc_assert (strlen (cu_pwd) + strlen (filename) + 2 < 256);
+
+      sprintf(full_filename, "%s%c%s", cu_pwd, DIR_SEPARATOR, filename);
+      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 = line_column;
+
+  sec->line_info.num_info += 1;
+  return info;
+}
+
 /* Compute the section size in section for func_info, line_info and core_info
    regions of .BTF.ext.  */
 
@@ -537,6 +600,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)",
+				   elem->line_col >> 12,
+				   elem->line_col & 0x00000fff);
+	      elem = elem->next;
+	    }
+	}
+
+      gcc_assert (count == sec->line_info.num_info);
+      sec = sec->next;
+    }
+}
+
 /* Output all CO-RE relocation sections.  */
 
 static void
@@ -609,6 +714,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 b36309475c9..9c6848324e7 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 6fdd14574ec..0f1e0ad1e89 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 } } */