diff mbox series

[v2,3/5] bpf: Always emit .BTF.ext section if generating BTF

Message ID 20240227190435.35750-4-cupertino.miranda@oracle.com
State New
Headers show
Series [v2,1/5] btf: fixed type id in BTF_KIND_FUNC struct data. | expand

Commit Message

Cupertino Miranda Feb. 27, 2024, 7:04 p.m. UTC 
BPF applications, when generating BTF information should always create a
.BTF.ext section.
Current implementation was only creating it when -mco-re option was used.
This patch makes .BTF.ext always be generated for BPF target objects.
The patch also adds conditions around btf_finalize function call
such that BTF deallocation happens later for BPF target.
For BPF, btf_finalize is only called after .BTF.ext is generated.

gcc/ChangeLog:

	* config/bpf/bpf.cc (bpf_option_override): Make .BTF.ext
	enabled by default for BPF.
	(bpf_file_end): Call BTF deallocation.
	* dwarf2ctf.cc (ctf_debug_finalize): Conditionally execute BTF
	deallocation.
---
 gcc/config/bpf/bpf.cc | 20 +++++++++-----------
 gcc/dwarf2ctf.cc      | 12 ++++++------
 2 files changed, 15 insertions(+), 17 deletions(-)

Comments

David Faust Feb. 27, 2024, 8:16 p.m. UTC | #1 
On 2/27/24 11:04, Cupertino Miranda wrote:
> BPF applications, when generating BTF information should always create a
> .BTF.ext section.
> Current implementation was only creating it when -mco-re option was used.
> This patch makes .BTF.ext always be generated for BPF target objects.
> The patch also adds conditions around btf_finalize function call
> such that BTF deallocation happens later for BPF target.
> For BPF, btf_finalize is only called after .BTF.ext is generated.

Thank you, this version makes it much more clear what the patch does.

> 
> gcc/ChangeLog:
> 
> 	* config/bpf/bpf.cc (bpf_option_override): Make .BTF.ext
> 	enabled by default for BPF.
> 	(bpf_file_end): Call BTF deallocation.
> 	* dwarf2ctf.cc (ctf_debug_finalize): Conditionally execute BTF
> 	deallocation.

You are missing ChangeLog entries for bpf_asm_init_sections and
ctf_debug_finish.

The script contrib/gcc-changelog/git_check_commit.py may help
to catch those.

The code changes LGTM, so OK with the ChangeLog fixed.
Thanks.

> ---
>  gcc/config/bpf/bpf.cc | 20 +++++++++-----------
>  gcc/dwarf2ctf.cc      | 12 ++++++------
>  2 files changed, 15 insertions(+), 17 deletions(-)
> 
> diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
> index d6ca47eeecbe..4318b26b9cda 100644
> --- a/gcc/config/bpf/bpf.cc
> +++ b/gcc/config/bpf/bpf.cc
> @@ -195,10 +195,8 @@ bpf_option_override (void)
>    if (TARGET_BPF_CORE && !btf_debuginfo_p ())
>      error ("BPF CO-RE requires BTF debugging information, use %<-gbtf%>");
>  
> -  /* To support the portability needs of BPF CO-RE approach, BTF debug
> -     information includes the BPF CO-RE relocations.  */
> -  if (TARGET_BPF_CORE)
> -    write_symbols |= BTF_WITH_CORE_DEBUG;
> +  /* BPF applications always generate .BTF.ext.  */
> +  write_symbols |= BTF_WITH_CORE_DEBUG;
>  
>    /* Unlike much of the other BTF debug information, the information necessary
>       for CO-RE relocations is added to the CTF container by the BPF backend.
> @@ -218,10 +216,7 @@ bpf_option_override (void)
>    /* -gbtf implies -mcore when using the BPF backend, unless -mno-co-re
>       is specified.  */
>    if (btf_debuginfo_p () && !(target_flags_explicit & MASK_BPF_CORE))
> -    {
> -      target_flags |= MASK_BPF_CORE;
> -      write_symbols |= BTF_WITH_CORE_DEBUG;
> -    }
> +    target_flags |= MASK_BPF_CORE;
>  
>    /* Determine available features from ISA setting (-mcpu=).  */
>    if (bpf_has_jmpext == -1)
> @@ -267,7 +262,7 @@ bpf_option_override (void)
>  static void
>  bpf_asm_init_sections (void)
>  {
> -  if (TARGET_BPF_CORE)
> +  if (btf_debuginfo_p () && btf_with_core_debuginfo_p ())
>      btf_ext_init ();
>  }
>  
> @@ -279,8 +274,11 @@ bpf_asm_init_sections (void)
>  static void
>  bpf_file_end (void)
>  {
> -  if (TARGET_BPF_CORE)
> -    btf_ext_output ();
> +  if (btf_debuginfo_p () && btf_with_core_debuginfo_p ())
> +    {
> +      btf_ext_output ();
> +      btf_finalize ();
> +    }
>  }
>  
>  #undef TARGET_ASM_FILE_END
> diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
> index 93e5619933fa..dca86edfffa9 100644
> --- a/gcc/dwarf2ctf.cc
> +++ b/gcc/dwarf2ctf.cc
> @@ -944,7 +944,10 @@ ctf_debug_finalize (const char *filename, bool btf)
>    if (btf)
>      {
>        btf_output (filename);
> -      btf_finalize ();
> +      /* btf_finalize when compiling BPF applciations gets deallocated by the
> +	 BPF target in bpf_file_end.  */
> +      if (btf_debuginfo_p () && !btf_with_core_debuginfo_p ())
> +	btf_finalize ();
>      }
>  
>    else
> @@ -1027,11 +1030,8 @@ ctf_debug_finish (const char * filename)
>    /* Emit BTF debug info here when CO-RE relocations need to be generated.
>       BTF with CO-RE relocations needs to be generated when CO-RE is in effect
>       for the BPF target.  */
> -  if (btf_with_core_debuginfo_p ())
> -    {
> -      gcc_assert (btf_debuginfo_p ());
> -      ctf_debug_finalize (filename, btf_debuginfo_p ());
> -    }
> +  if (btf_debuginfo_p () && btf_with_core_debuginfo_p ())
> +    ctf_debug_finalize (filename, btf_debuginfo_p ());
>  }
>  
>  #include "gt-dwarf2ctf.h"
Cupertino Miranda Feb. 28, 2024, 7:25 p.m. UTC | #2 
Corrected and Pushed.

Thanks,
Cupertino

David Faust writes:

> On 2/27/24 11:04, Cupertino Miranda wrote:
>> BPF applications, when generating BTF information should always create a
>> .BTF.ext section.
>> Current implementation was only creating it when -mco-re option was used.
>> This patch makes .BTF.ext always be generated for BPF target objects.
>> The patch also adds conditions around btf_finalize function call
>> such that BTF deallocation happens later for BPF target.
>> For BPF, btf_finalize is only called after .BTF.ext is generated.
>
> Thank you, this version makes it much more clear what the patch does.
>
>>
>> gcc/ChangeLog:
>>
>> 	* config/bpf/bpf.cc (bpf_option_override): Make .BTF.ext
>> 	enabled by default for BPF.
>> 	(bpf_file_end): Call BTF deallocation.
>> 	* dwarf2ctf.cc (ctf_debug_finalize): Conditionally execute BTF
>> 	deallocation.
>
> You are missing ChangeLog entries for bpf_asm_init_sections and
> ctf_debug_finish.
>
> The script contrib/gcc-changelog/git_check_commit.py may help
> to catch those.
>
> The code changes LGTM, so OK with the ChangeLog fixed.
> Thanks.
>
>> ---
>>  gcc/config/bpf/bpf.cc | 20 +++++++++-----------
>>  gcc/dwarf2ctf.cc      | 12 ++++++------
>>  2 files changed, 15 insertions(+), 17 deletions(-)
>>
>> diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
>> index d6ca47eeecbe..4318b26b9cda 100644
>> --- a/gcc/config/bpf/bpf.cc
>> +++ b/gcc/config/bpf/bpf.cc
>> @@ -195,10 +195,8 @@ bpf_option_override (void)
>>    if (TARGET_BPF_CORE && !btf_debuginfo_p ())
>>      error ("BPF CO-RE requires BTF debugging information, use %<-gbtf%>");
>>
>> -  /* To support the portability needs of BPF CO-RE approach, BTF debug
>> -     information includes the BPF CO-RE relocations.  */
>> -  if (TARGET_BPF_CORE)
>> -    write_symbols |= BTF_WITH_CORE_DEBUG;
>> +  /* BPF applications always generate .BTF.ext.  */
>> +  write_symbols |= BTF_WITH_CORE_DEBUG;
>>
>>    /* Unlike much of the other BTF debug information, the information necessary
>>       for CO-RE relocations is added to the CTF container by the BPF backend.
>> @@ -218,10 +216,7 @@ bpf_option_override (void)
>>    /* -gbtf implies -mcore when using the BPF backend, unless -mno-co-re
>>       is specified.  */
>>    if (btf_debuginfo_p () && !(target_flags_explicit & MASK_BPF_CORE))
>> -    {
>> -      target_flags |= MASK_BPF_CORE;
>> -      write_symbols |= BTF_WITH_CORE_DEBUG;
>> -    }
>> +    target_flags |= MASK_BPF_CORE;
>>
>>    /* Determine available features from ISA setting (-mcpu=).  */
>>    if (bpf_has_jmpext == -1)
>> @@ -267,7 +262,7 @@ bpf_option_override (void)
>>  static void
>>  bpf_asm_init_sections (void)
>>  {
>> -  if (TARGET_BPF_CORE)
>> +  if (btf_debuginfo_p () && btf_with_core_debuginfo_p ())
>>      btf_ext_init ();
>>  }
>>
>> @@ -279,8 +274,11 @@ bpf_asm_init_sections (void)
>>  static void
>>  bpf_file_end (void)
>>  {
>> -  if (TARGET_BPF_CORE)
>> -    btf_ext_output ();
>> +  if (btf_debuginfo_p () && btf_with_core_debuginfo_p ())
>> +    {
>> +      btf_ext_output ();
>> +      btf_finalize ();
>> +    }
>>  }
>>
>>  #undef TARGET_ASM_FILE_END
>> diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
>> index 93e5619933fa..dca86edfffa9 100644
>> --- a/gcc/dwarf2ctf.cc
>> +++ b/gcc/dwarf2ctf.cc
>> @@ -944,7 +944,10 @@ ctf_debug_finalize (const char *filename, bool btf)
>>    if (btf)
>>      {
>>        btf_output (filename);
>> -      btf_finalize ();
>> +      /* btf_finalize when compiling BPF applciations gets deallocated by the
>> +	 BPF target in bpf_file_end.  */
>> +      if (btf_debuginfo_p () && !btf_with_core_debuginfo_p ())
>> +	btf_finalize ();
>>      }
>>
>>    else
>> @@ -1027,11 +1030,8 @@ ctf_debug_finish (const char * filename)
>>    /* Emit BTF debug info here when CO-RE relocations need to be generated.
>>       BTF with CO-RE relocations needs to be generated when CO-RE is in effect
>>       for the BPF target.  */
>> -  if (btf_with_core_debuginfo_p ())
>> -    {
>> -      gcc_assert (btf_debuginfo_p ());
>> -      ctf_debug_finalize (filename, btf_debuginfo_p ());
>> -    }
>> +  if (btf_debuginfo_p () && btf_with_core_debuginfo_p ())
>> +    ctf_debug_finalize (filename, btf_debuginfo_p ());
>>  }
>>
>>  #include "gt-dwarf2ctf.h"
diff mbox series

Patch

diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
index d6ca47eeecbe..4318b26b9cda 100644
--- a/gcc/config/bpf/bpf.cc
+++ b/gcc/config/bpf/bpf.cc
@@ -195,10 +195,8 @@  bpf_option_override (void)
   if (TARGET_BPF_CORE && !btf_debuginfo_p ())
     error ("BPF CO-RE requires BTF debugging information, use %<-gbtf%>");
 
-  /* To support the portability needs of BPF CO-RE approach, BTF debug
-     information includes the BPF CO-RE relocations.  */
-  if (TARGET_BPF_CORE)
-    write_symbols |= BTF_WITH_CORE_DEBUG;
+  /* BPF applications always generate .BTF.ext.  */
+  write_symbols |= BTF_WITH_CORE_DEBUG;
 
   /* Unlike much of the other BTF debug information, the information necessary
      for CO-RE relocations is added to the CTF container by the BPF backend.
@@ -218,10 +216,7 @@  bpf_option_override (void)
   /* -gbtf implies -mcore when using the BPF backend, unless -mno-co-re
      is specified.  */
   if (btf_debuginfo_p () && !(target_flags_explicit & MASK_BPF_CORE))
-    {
-      target_flags |= MASK_BPF_CORE;
-      write_symbols |= BTF_WITH_CORE_DEBUG;
-    }
+    target_flags |= MASK_BPF_CORE;
 
   /* Determine available features from ISA setting (-mcpu=).  */
   if (bpf_has_jmpext == -1)
@@ -267,7 +262,7 @@  bpf_option_override (void)
 static void
 bpf_asm_init_sections (void)
 {
-  if (TARGET_BPF_CORE)
+  if (btf_debuginfo_p () && btf_with_core_debuginfo_p ())
     btf_ext_init ();
 }
 
@@ -279,8 +274,11 @@  bpf_asm_init_sections (void)
 static void
 bpf_file_end (void)
 {
-  if (TARGET_BPF_CORE)
-    btf_ext_output ();
+  if (btf_debuginfo_p () && btf_with_core_debuginfo_p ())
+    {
+      btf_ext_output ();
+      btf_finalize ();
+    }
 }
 
 #undef TARGET_ASM_FILE_END
diff --git a/gcc/dwarf2ctf.cc b/gcc/dwarf2ctf.cc
index 93e5619933fa..dca86edfffa9 100644
--- a/gcc/dwarf2ctf.cc
+++ b/gcc/dwarf2ctf.cc
@@ -944,7 +944,10 @@  ctf_debug_finalize (const char *filename, bool btf)
   if (btf)
     {
       btf_output (filename);
-      btf_finalize ();
+      /* btf_finalize when compiling BPF applciations gets deallocated by the
+	 BPF target in bpf_file_end.  */
+      if (btf_debuginfo_p () && !btf_with_core_debuginfo_p ())
+	btf_finalize ();
     }
 
   else
@@ -1027,11 +1030,8 @@  ctf_debug_finish (const char * filename)
   /* Emit BTF debug info here when CO-RE relocations need to be generated.
      BTF with CO-RE relocations needs to be generated when CO-RE is in effect
      for the BPF target.  */
-  if (btf_with_core_debuginfo_p ())
-    {
-      gcc_assert (btf_debuginfo_p ());
-      ctf_debug_finalize (filename, btf_debuginfo_p ());
-    }
+  if (btf_debuginfo_p () && btf_with_core_debuginfo_p ())
+    ctf_debug_finalize (filename, btf_debuginfo_p ());
 }
 
 #include "gt-dwarf2ctf.h"