diff mbox series

[v2] bpf: ensure correct string offsets in BTF.ext

Message ID 20220118211427.7602-1-david.faust@oracle.com
State Committed
Commit 7db42268ce4bc77bc13f13ba0899221747255bb5
Headers
Series [v2] bpf: ensure correct string offsets in BTF.ext |

Commit Message

David Faust Jan. 18, 2022, 9:14 p.m. UTC 
  [Changed from v1: Adjust to account for file renaming so patch applies.]

BPF CO-RE relocations contain offsets to strings buffered in the BTF
string table. These BTF-specific strings are stored in memory in the
CTF auxilliary strtab, which at output time is concatenated onto the end
of the standard strtab.

Previously, these string offsets were computed at the time the
relocations were created. But strings could be added to the standard
strtab after this point, causing the offsets to no longer be correct.

Compute the offsets just before output instead, when they are sure to no
longer change.

Tested for bpf-unknown-none. OK to install?
Thanks.

gcc/ChangeLog:

	* config/bpf/coreout.cc (bpf_core_reloc_add): Do not account
	for base strtab offset yet as it may change.
	(output_asm_btfext_core_reloc): Do so here instead.
	(output_btfext_core_sections): Likewise.
---
 gcc/config/bpf/coreout.cc | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

Comments

Jose E. Marchesi Jan. 18, 2022, 9:18 p.m. UTC | #1 
Hi David.

> [Changed from v1: Adjust to account for file renaming so patch applies.]
>
> BPF CO-RE relocations contain offsets to strings buffered in the BTF
> string table. These BTF-specific strings are stored in memory in the
> CTF auxilliary strtab, which at output time is concatenated onto the end
> of the standard strtab.
>
> Previously, these string offsets were computed at the time the
> relocations were created. But strings could be added to the standard
> strtab after this point, causing the offsets to no longer be correct.
>
> Compute the offsets just before output instead, when they are sure to no
> longer change.
>
> Tested for bpf-unknown-none. OK to install?
> Thanks.

OK.
Thanks.

>
> gcc/ChangeLog:
>
> 	* config/bpf/coreout.cc (bpf_core_reloc_add): Do not account
> 	for base strtab offset yet as it may change.
> 	(output_asm_btfext_core_reloc): Do so here instead.
> 	(output_btfext_core_sections): Likewise.
> ---
>  gcc/config/bpf/coreout.cc | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/config/bpf/coreout.cc b/gcc/config/bpf/coreout.cc
> index 4ec12ecd305..cceaaa969cc 100644
> --- a/gcc/config/bpf/coreout.cc
> +++ b/gcc/config/bpf/coreout.cc
> @@ -168,11 +168,8 @@ bpf_core_reloc_add (const tree type, const char * section_name,
>    bpf_core_reloc_ref bpfcr = ggc_cleared_alloc<bpf_core_reloc_t> ();
>    ctf_container_ref ctfc = ctf_get_tu_ctfc ();
>  
> -  /* Buffer the access string in the auxiliary strtab. Since the two string
> -     tables are concatenated, add the length of the first to the offset.  */
> -  size_t strtab_len = ctfc_get_strtab_len (ctfc, CTF_STRTAB);
> +  /* Buffer the access string in the auxiliary strtab.  */
>    ctf_add_string (ctfc, buf, &(bpfcr->bpfcr_astr_off), CTF_AUX_STRTAB);
> -  bpfcr->bpfcr_astr_off += strtab_len;
>  
>    bpfcr->bpfcr_type = get_btf_id (ctf_lookup_tree_type (ctfc, type));
>    bpfcr->bpfcr_insn_label = label;
> @@ -191,7 +188,6 @@ bpf_core_reloc_add (const tree type, const char * section_name,
>    sec = ggc_cleared_alloc<bpf_core_section_t> ();
>  
>    ctf_add_string (ctfc, section_name, &sec->name_offset, CTF_AUX_STRTAB);
> -  sec->name_offset += strtab_len;
>    if (strcmp (section_name, ""))
>      ctfc->ctfc_aux_strlen += strlen (section_name) + 1;
>  
> @@ -287,6 +283,9 @@ output_btfext_header (void)
>  static void
>  output_asm_btfext_core_reloc (bpf_core_reloc_ref bpfcr)
>  {
> +  bpfcr->bpfcr_astr_off += ctfc_get_strtab_len (ctf_get_tu_ctfc (),
> +						CTF_STRTAB);
> +
>    dw2_assemble_integer (4, gen_rtx_LABEL_REF (Pmode, bpfcr->bpfcr_insn_label));
>    fprintf (asm_out_file, "\t%s bpfcr_insn\n", ASM_COMMENT_START);
>  
> @@ -323,6 +322,11 @@ output_btfext_core_sections (void)
>        /* Section name offset, refers to the offset of a string with the name of
>  	 the section to which these CORE relocations refer, e.g. '.text'.
>  	 The string is buffered in the BTF strings table.  */
> +
> +      /* BTF specific strings are in CTF_AUX_STRTAB, which is concatenated
> +	 after CTF_STRTAB. Add the length of STRTAB to the final offset.  */
> +      sec->name_offset += ctfc_get_strtab_len (ctf_get_tu_ctfc (), CTF_STRTAB);
> +
>        dw2_asm_output_data (4, sec->name_offset,  "btfext_secinfo_sec_name_off");
>        dw2_asm_output_data (4, vec_safe_length (sec->relocs),
>  			   "btfext_secinfo_num_recs");
David Faust Jan. 18, 2022, 9:45 p.m. UTC | #2 
On 1/18/22 13:18, Jose E. Marchesi wrote:
> 
> Hi David.
> 
>> [Changed from v1: Adjust to account for file renaming so patch applies.]
>>
>> BPF CO-RE relocations contain offsets to strings buffered in the BTF
>> string table. These BTF-specific strings are stored in memory in the
>> CTF auxilliary strtab, which at output time is concatenated onto the end
>> of the standard strtab.
>>
>> Previously, these string offsets were computed at the time the
>> relocations were created. But strings could be added to the standard
>> strtab after this point, causing the offsets to no longer be correct.
>>
>> Compute the offsets just before output instead, when they are sure to no
>> longer change.
>>
>> Tested for bpf-unknown-none. OK to install?
>> Thanks.
> 
> OK.
> Thanks.

Pushed, thanks

> 
>>
>> gcc/ChangeLog:
>>
>> 	* config/bpf/coreout.cc (bpf_core_reloc_add): Do not account
>> 	for base strtab offset yet as it may change.
>> 	(output_asm_btfext_core_reloc): Do so here instead.
>> 	(output_btfext_core_sections): Likewise.
>> ---
>>   gcc/config/bpf/coreout.cc | 14 +++++++++-----
>>   1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/gcc/config/bpf/coreout.cc b/gcc/config/bpf/coreout.cc
>> index 4ec12ecd305..cceaaa969cc 100644
>> --- a/gcc/config/bpf/coreout.cc
>> +++ b/gcc/config/bpf/coreout.cc
>> @@ -168,11 +168,8 @@ bpf_core_reloc_add (const tree type, const char * section_name,
>>     bpf_core_reloc_ref bpfcr = ggc_cleared_alloc<bpf_core_reloc_t> ();
>>     ctf_container_ref ctfc = ctf_get_tu_ctfc ();
>>   
>> -  /* Buffer the access string in the auxiliary strtab. Since the two string
>> -     tables are concatenated, add the length of the first to the offset.  */
>> -  size_t strtab_len = ctfc_get_strtab_len (ctfc, CTF_STRTAB);
>> +  /* Buffer the access string in the auxiliary strtab.  */
>>     ctf_add_string (ctfc, buf, &(bpfcr->bpfcr_astr_off), CTF_AUX_STRTAB);
>> -  bpfcr->bpfcr_astr_off += strtab_len;
>>   
>>     bpfcr->bpfcr_type = get_btf_id (ctf_lookup_tree_type (ctfc, type));
>>     bpfcr->bpfcr_insn_label = label;
>> @@ -191,7 +188,6 @@ bpf_core_reloc_add (const tree type, const char * section_name,
>>     sec = ggc_cleared_alloc<bpf_core_section_t> ();
>>   
>>     ctf_add_string (ctfc, section_name, &sec->name_offset, CTF_AUX_STRTAB);
>> -  sec->name_offset += strtab_len;
>>     if (strcmp (section_name, ""))
>>       ctfc->ctfc_aux_strlen += strlen (section_name) + 1;
>>   
>> @@ -287,6 +283,9 @@ output_btfext_header (void)
>>   static void
>>   output_asm_btfext_core_reloc (bpf_core_reloc_ref bpfcr)
>>   {
>> +  bpfcr->bpfcr_astr_off += ctfc_get_strtab_len (ctf_get_tu_ctfc (),
>> +						CTF_STRTAB);
>> +
>>     dw2_assemble_integer (4, gen_rtx_LABEL_REF (Pmode, bpfcr->bpfcr_insn_label));
>>     fprintf (asm_out_file, "\t%s bpfcr_insn\n", ASM_COMMENT_START);
>>   
>> @@ -323,6 +322,11 @@ output_btfext_core_sections (void)
>>         /* Section name offset, refers to the offset of a string with the name of
>>   	 the section to which these CORE relocations refer, e.g. '.text'.
>>   	 The string is buffered in the BTF strings table.  */
>> +
>> +      /* BTF specific strings are in CTF_AUX_STRTAB, which is concatenated
>> +	 after CTF_STRTAB. Add the length of STRTAB to the final offset.  */
>> +      sec->name_offset += ctfc_get_strtab_len (ctf_get_tu_ctfc (), CTF_STRTAB);
>> +
>>         dw2_asm_output_data (4, sec->name_offset,  "btfext_secinfo_sec_name_off");
>>         dw2_asm_output_data (4, vec_safe_length (sec->relocs),
>>   			   "btfext_secinfo_num_recs");
diff mbox series

Patch

diff --git a/gcc/config/bpf/coreout.cc b/gcc/config/bpf/coreout.cc
index 4ec12ecd305..cceaaa969cc 100644
--- a/gcc/config/bpf/coreout.cc
+++ b/gcc/config/bpf/coreout.cc
@@ -168,11 +168,8 @@  bpf_core_reloc_add (const tree type, const char * section_name,
   bpf_core_reloc_ref bpfcr = ggc_cleared_alloc<bpf_core_reloc_t> ();
   ctf_container_ref ctfc = ctf_get_tu_ctfc ();
 
-  /* Buffer the access string in the auxiliary strtab. Since the two string
-     tables are concatenated, add the length of the first to the offset.  */
-  size_t strtab_len = ctfc_get_strtab_len (ctfc, CTF_STRTAB);
+  /* Buffer the access string in the auxiliary strtab.  */
   ctf_add_string (ctfc, buf, &(bpfcr->bpfcr_astr_off), CTF_AUX_STRTAB);
-  bpfcr->bpfcr_astr_off += strtab_len;
 
   bpfcr->bpfcr_type = get_btf_id (ctf_lookup_tree_type (ctfc, type));
   bpfcr->bpfcr_insn_label = label;
@@ -191,7 +188,6 @@  bpf_core_reloc_add (const tree type, const char * section_name,
   sec = ggc_cleared_alloc<bpf_core_section_t> ();
 
   ctf_add_string (ctfc, section_name, &sec->name_offset, CTF_AUX_STRTAB);
-  sec->name_offset += strtab_len;
   if (strcmp (section_name, ""))
     ctfc->ctfc_aux_strlen += strlen (section_name) + 1;
 
@@ -287,6 +283,9 @@  output_btfext_header (void)
 static void
 output_asm_btfext_core_reloc (bpf_core_reloc_ref bpfcr)
 {
+  bpfcr->bpfcr_astr_off += ctfc_get_strtab_len (ctf_get_tu_ctfc (),
+						CTF_STRTAB);
+
   dw2_assemble_integer (4, gen_rtx_LABEL_REF (Pmode, bpfcr->bpfcr_insn_label));
   fprintf (asm_out_file, "\t%s bpfcr_insn\n", ASM_COMMENT_START);
 
@@ -323,6 +322,11 @@  output_btfext_core_sections (void)
       /* Section name offset, refers to the offset of a string with the name of
 	 the section to which these CORE relocations refer, e.g. '.text'.
 	 The string is buffered in the BTF strings table.  */
+
+      /* BTF specific strings are in CTF_AUX_STRTAB, which is concatenated
+	 after CTF_STRTAB. Add the length of STRTAB to the final offset.  */
+      sec->name_offset += ctfc_get_strtab_len (ctf_get_tu_ctfc (), CTF_STRTAB);
+
       dw2_asm_output_data (4, sec->name_offset,  "btfext_secinfo_sec_name_off");
       dw2_asm_output_data (4, vec_safe_length (sec->relocs),
 			   "btfext_secinfo_num_recs");