public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* bpf: Throw error when external libcalls are generated.
@ 2023-11-24 16:26 Cupertino Miranda
  2023-11-27 14:50 ` Jose E. Marchesi
  0 siblings, 1 reply; 5+ messages in thread
From: Cupertino Miranda @ 2023-11-24 16:26 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jose E. Marchesi, david.faust

[-- Attachment #1: Type: text/plain, Size: 487 bytes --]


Hi everyone,

The attached patch is a temporary solution for the lack of proper linker
and external library linking of the eBPF platform.
Any calls created by the compiler, that would usually be defined within
libgcc, will endup being undefined in bpftool, when GCC the compiled
code is passed.

This patch anticipates that error to the compiler, by verifiying if
any of those calls are being generated, and reporting as an error.

Looking forward to your comments.

Cheers,
Cupertino


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: bpf_external_calls.patch --]
[-- Type: text/x-diff, Size: 5290 bytes --]

commit c2110ae497c7ff83c309f172bc265973652b760d
Author: Cupertino Miranda <cupertino.miranda@oracle.com>
Date:   Thu Nov 23 22:28:01 2023 +0000

    This patch enables errors when external calls are created.
    
    When architectural limitations or usage of builtins implies the compiler
    to create function calls to external libraries that implement the
    functionality, GCC will now report an error claiming that this function
    calls are not compatible with eBPF target.
    Examples of those are the usage of __builtin_memmove and a sign division
    in BPF ISA v3 or below that will require to call __divdi3.
    This is currently an eBPF limitation which does not support linking of
    object files but rather "raw" non linked ones. Those object files are
    loaded and relocated by libbpf and the kernel.
    
    gcc/ChangeLog:
            * config/bpf/bpf.cc (bpf_output_call): Report error in case the
            function call is for a builtin.
            (bpf_external_libcall): Added target hook to detect and report
            error when other external calls that are not builtins.

diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
index 0c9d5257c384..1c84113055b1 100644
--- a/gcc/config/bpf/bpf.cc
+++ b/gcc/config/bpf/bpf.cc
@@ -744,6 +744,15 @@ bpf_output_call (rtx target)
 	    xops[0] = GEN_INT (TREE_INT_CST_LOW (TREE_VALUE (attr_args)));
 	    output_asm_insn ("call\t%0", xops);
 	  }
+	else if (fndecl_built_in_p (decl))
+	  {
+	    /* For now lets report this as an error while we are not able to
+	       link eBPF object files.  In particular with libgcc.  */
+	    tree name = DECL_NAME (decl);
+	    error ("call to external builtin %s in function, which is not supported by "
+		   "eBPF", name != NULL_TREE ? IDENTIFIER_POINTER (name) : "(anon)");
+	    output_asm_insn ("call 0", NULL);
+	  }
 	else
 	  output_asm_insn ("call\t%0", &target);
 
@@ -763,6 +772,18 @@ bpf_output_call (rtx target)
   return "";
 }
 
+static void
+bpf_external_libcall (rtx fun)
+{
+  tree decl = SYMBOL_REF_DECL (fun);
+  tree name = DECL_NAME (decl);
+  error ("call to external libcall %s in function, which is not supported by "
+	 "eBPF", name != NULL_TREE ? IDENTIFIER_POINTER (name) : "(anon)");
+}
+
+#undef  TARGET_ASM_EXTERNAL_LIBCALL
+#define TARGET_ASM_EXTERNAL_LIBCALL bpf_external_libcall
+
 /* Print register name according to assembly dialect.  In normal
    syntax registers are printed like %rN where N is the register
    number.
diff --git a/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c b/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c
index 4036570ac601..fec720584e48 100644
--- a/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c
+++ b/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c
@@ -6,7 +6,7 @@ foo (int *p, int *expected, int desired)
 {
   return __atomic_compare_exchange (p, expected, &desired, 0,
 				    __ATOMIC_ACQUIRE, __ATOMIC_RELAXED);
-}
+} /* { dg-error "call to external builtin" } */
 
 int
 foo64 (long *p, long *expected, long desired)
diff --git a/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c b/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c
index 044a2f76474b..ea1b8e48928a 100644
--- a/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c
+++ b/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c
@@ -9,7 +9,7 @@ long
 test_atomic_fetch_add (long x)
 {
   return __atomic_fetch_add (&val, x, __ATOMIC_ACQUIRE);
-}
+} /* { dg-error "call to external builtin" } */
 
 long
 test_atomic_fetch_sub (long x)
diff --git a/gcc/testsuite/gcc.target/bpf/atomic-op-3.c b/gcc/testsuite/gcc.target/bpf/atomic-op-3.c
index b2ce28926347..fefafd6b748f 100644
--- a/gcc/testsuite/gcc.target/bpf/atomic-op-3.c
+++ b/gcc/testsuite/gcc.target/bpf/atomic-op-3.c
@@ -20,7 +20,7 @@ void
 test_atomic_and (int x)
 {
   __atomic_and_fetch (&val, x, __ATOMIC_ACQUIRE);
-}
+} /* { dg-error "call to external builtin" } */
 
 void
 test_atomic_nand (int x)
diff --git a/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c b/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c
index 3b6324e966b8..eab695bf388c 100644
--- a/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c
+++ b/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c
@@ -7,7 +7,7 @@ int foo (int *p, int *new)
   int old;
   __atomic_exchange (p, new, &old, __ATOMIC_RELAXED);
   return old;
-}
+} /* { dg-error "call to external builtin" } */
 
 int foo64 (long *p, long *new)
 {
diff --git a/gcc/testsuite/gcc.target/bpf/diag-sdiv.c b/gcc/testsuite/gcc.target/bpf/diag-sdiv.c
index c48bbf03df97..c3332558e7d2 100644
--- a/gcc/testsuite/gcc.target/bpf/diag-sdiv.c
+++ b/gcc/testsuite/gcc.target/bpf/diag-sdiv.c
@@ -7,6 +7,6 @@ foo ()
 {
   signed int x = 5;
   signed int y = 2;
-  signed int z = x / y;
+  signed int z = x / y; /* { dg-error "call to external libcall" } */
 }
 /* { dg-final { scan-assembler-not "sdiv(32)?\t%r" } } */
diff --git a/gcc/testsuite/gcc.target/bpf/diag-smod.c b/gcc/testsuite/gcc.target/bpf/diag-smod.c
index d3df308217f3..25bcb1e4ffdb 100644
--- a/gcc/testsuite/gcc.target/bpf/diag-smod.c
+++ b/gcc/testsuite/gcc.target/bpf/diag-smod.c
@@ -7,6 +7,6 @@ foo ()
 {
   signed int x = 5;
   signed int y = 2;
-  signed int z = x % y;
+  signed int z = x % y; /* { dg-error "call to external libcall" } */
 }
 /* { dg-final { scan-assembler-not "smod(32)?\t%r" } } */

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: bpf: Throw error when external libcalls are generated.
  2023-11-24 16:26 bpf: Throw error when external libcalls are generated Cupertino Miranda
@ 2023-11-27 14:50 ` Jose E. Marchesi
  2023-11-28 15:40   ` Jose E. Marchesi
  0 siblings, 1 reply; 5+ messages in thread
From: Jose E. Marchesi @ 2023-11-27 14:50 UTC (permalink / raw)
  To: Cupertino Miranda; +Cc: gcc-patches, david.faust


Hi Cuper.
OK. Thanks for the patch.

> Hi everyone,
>
> The attached patch is a temporary solution for the lack of proper linker
> and external library linking of the eBPF platform.
> Any calls created by the compiler, that would usually be defined within
> libgcc, will endup being undefined in bpftool, when GCC the compiled
> code is passed.
>
> This patch anticipates that error to the compiler, by verifiying if
> any of those calls are being generated, and reporting as an error.
>
> Looking forward to your comments.
>
> Cheers,
> Cupertino
>
> commit c2110ae497c7ff83c309f172bc265973652b760d
> Author: Cupertino Miranda <cupertino.miranda@oracle.com>
> Date:   Thu Nov 23 22:28:01 2023 +0000
>
>     This patch enables errors when external calls are created.
>     
>     When architectural limitations or usage of builtins implies the compiler
>     to create function calls to external libraries that implement the
>     functionality, GCC will now report an error claiming that this function
>     calls are not compatible with eBPF target.
>     Examples of those are the usage of __builtin_memmove and a sign division
>     in BPF ISA v3 or below that will require to call __divdi3.
>     This is currently an eBPF limitation which does not support linking of
>     object files but rather "raw" non linked ones. Those object files are
>     loaded and relocated by libbpf and the kernel.
>     
>     gcc/ChangeLog:
>             * config/bpf/bpf.cc (bpf_output_call): Report error in case the
>             function call is for a builtin.
>             (bpf_external_libcall): Added target hook to detect and report
>             error when other external calls that are not builtins.
>
> diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
> index 0c9d5257c384..1c84113055b1 100644
> --- a/gcc/config/bpf/bpf.cc
> +++ b/gcc/config/bpf/bpf.cc
> @@ -744,6 +744,15 @@ bpf_output_call (rtx target)
>  	    xops[0] = GEN_INT (TREE_INT_CST_LOW (TREE_VALUE (attr_args)));
>  	    output_asm_insn ("call\t%0", xops);
>  	  }
> +	else if (fndecl_built_in_p (decl))
> +	  {
> +	    /* For now lets report this as an error while we are not able to
> +	       link eBPF object files.  In particular with libgcc.  */
> +	    tree name = DECL_NAME (decl);
> +	    error ("call to external builtin %s in function, which is not supported by "
> +		   "eBPF", name != NULL_TREE ? IDENTIFIER_POINTER (name) : "(anon)");
> +	    output_asm_insn ("call 0", NULL);
> +	  }
>  	else
>  	  output_asm_insn ("call\t%0", &target);
>  
> @@ -763,6 +772,18 @@ bpf_output_call (rtx target)
>    return "";
>  }
>  
> +static void
> +bpf_external_libcall (rtx fun)
> +{
> +  tree decl = SYMBOL_REF_DECL (fun);
> +  tree name = DECL_NAME (decl);
> +  error ("call to external libcall %s in function, which is not supported by "
> +	 "eBPF", name != NULL_TREE ? IDENTIFIER_POINTER (name) : "(anon)");
> +}
> +
> +#undef  TARGET_ASM_EXTERNAL_LIBCALL
> +#define TARGET_ASM_EXTERNAL_LIBCALL bpf_external_libcall
> +
>  /* Print register name according to assembly dialect.  In normal
>     syntax registers are printed like %rN where N is the register
>     number.
> diff --git a/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c b/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c
> index 4036570ac601..fec720584e48 100644
> --- a/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c
> +++ b/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c
> @@ -6,7 +6,7 @@ foo (int *p, int *expected, int desired)
>  {
>    return __atomic_compare_exchange (p, expected, &desired, 0,
>  				    __ATOMIC_ACQUIRE, __ATOMIC_RELAXED);
> -}
> +} /* { dg-error "call to external builtin" } */
>  
>  int
>  foo64 (long *p, long *expected, long desired)
> diff --git a/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c b/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c
> index 044a2f76474b..ea1b8e48928a 100644
> --- a/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c
> +++ b/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c
> @@ -9,7 +9,7 @@ long
>  test_atomic_fetch_add (long x)
>  {
>    return __atomic_fetch_add (&val, x, __ATOMIC_ACQUIRE);
> -}
> +} /* { dg-error "call to external builtin" } */
>  
>  long
>  test_atomic_fetch_sub (long x)
> diff --git a/gcc/testsuite/gcc.target/bpf/atomic-op-3.c b/gcc/testsuite/gcc.target/bpf/atomic-op-3.c
> index b2ce28926347..fefafd6b748f 100644
> --- a/gcc/testsuite/gcc.target/bpf/atomic-op-3.c
> +++ b/gcc/testsuite/gcc.target/bpf/atomic-op-3.c
> @@ -20,7 +20,7 @@ void
>  test_atomic_and (int x)
>  {
>    __atomic_and_fetch (&val, x, __ATOMIC_ACQUIRE);
> -}
> +} /* { dg-error "call to external builtin" } */
>  
>  void
>  test_atomic_nand (int x)
> diff --git a/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c b/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c
> index 3b6324e966b8..eab695bf388c 100644
> --- a/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c
> +++ b/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c
> @@ -7,7 +7,7 @@ int foo (int *p, int *new)
>    int old;
>    __atomic_exchange (p, new, &old, __ATOMIC_RELAXED);
>    return old;
> -}
> +} /* { dg-error "call to external builtin" } */
>  
>  int foo64 (long *p, long *new)
>  {
> diff --git a/gcc/testsuite/gcc.target/bpf/diag-sdiv.c b/gcc/testsuite/gcc.target/bpf/diag-sdiv.c
> index c48bbf03df97..c3332558e7d2 100644
> --- a/gcc/testsuite/gcc.target/bpf/diag-sdiv.c
> +++ b/gcc/testsuite/gcc.target/bpf/diag-sdiv.c
> @@ -7,6 +7,6 @@ foo ()
>  {
>    signed int x = 5;
>    signed int y = 2;
> -  signed int z = x / y;
> +  signed int z = x / y; /* { dg-error "call to external libcall" } */
>  }
>  /* { dg-final { scan-assembler-not "sdiv(32)?\t%r" } } */
> diff --git a/gcc/testsuite/gcc.target/bpf/diag-smod.c b/gcc/testsuite/gcc.target/bpf/diag-smod.c
> index d3df308217f3..25bcb1e4ffdb 100644
> --- a/gcc/testsuite/gcc.target/bpf/diag-smod.c
> +++ b/gcc/testsuite/gcc.target/bpf/diag-smod.c
> @@ -7,6 +7,6 @@ foo ()
>  {
>    signed int x = 5;
>    signed int y = 2;
> -  signed int z = x % y;
> +  signed int z = x % y; /* { dg-error "call to external libcall" } */
>  }
>  /* { dg-final { scan-assembler-not "smod(32)?\t%r" } } */

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: bpf: Throw error when external libcalls are generated.
  2023-11-27 14:50 ` Jose E. Marchesi
@ 2023-11-28 15:40   ` Jose E. Marchesi
  2023-11-28 15:47     ` Cupertino Miranda
  0 siblings, 1 reply; 5+ messages in thread
From: Jose E. Marchesi @ 2023-11-28 15:40 UTC (permalink / raw)
  To: Cupertino Miranda; +Cc: gcc-patches, david.faust


> Hi Cuper.
> OK. Thanks for the patch.

This commit is breaking the BPF build, because libgcc emits libcalls to
__builtin_abort.  We need to rethink this.

Please revert:

  commit faf5b148588bd7fbb60ec669aefa704044037cdc
  Author: Cupertino Miranda <cupertino.miranda@oracle.com>
  Date:   Thu Nov 23 22:28:01 2023 +0000

Thanks!

>
>> Hi everyone,
>>
>> The attached patch is a temporary solution for the lack of proper linker
>> and external library linking of the eBPF platform.
>> Any calls created by the compiler, that would usually be defined within
>> libgcc, will endup being undefined in bpftool, when GCC the compiled
>> code is passed.
>>
>> This patch anticipates that error to the compiler, by verifiying if
>> any of those calls are being generated, and reporting as an error.
>>
>> Looking forward to your comments.
>>
>> Cheers,
>> Cupertino
>>
>> commit c2110ae497c7ff83c309f172bc265973652b760d
>> Author: Cupertino Miranda <cupertino.miranda@oracle.com>
>> Date:   Thu Nov 23 22:28:01 2023 +0000
>>
>>     This patch enables errors when external calls are created.
>>     
>>     When architectural limitations or usage of builtins implies the compiler
>>     to create function calls to external libraries that implement the
>>     functionality, GCC will now report an error claiming that this function
>>     calls are not compatible with eBPF target.
>>     Examples of those are the usage of __builtin_memmove and a sign division
>>     in BPF ISA v3 or below that will require to call __divdi3.
>>     This is currently an eBPF limitation which does not support linking of
>>     object files but rather "raw" non linked ones. Those object files are
>>     loaded and relocated by libbpf and the kernel.
>>     
>>     gcc/ChangeLog:
>>             * config/bpf/bpf.cc (bpf_output_call): Report error in case the
>>             function call is for a builtin.
>>             (bpf_external_libcall): Added target hook to detect and report
>>             error when other external calls that are not builtins.
>>
>> diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
>> index 0c9d5257c384..1c84113055b1 100644
>> --- a/gcc/config/bpf/bpf.cc
>> +++ b/gcc/config/bpf/bpf.cc
>> @@ -744,6 +744,15 @@ bpf_output_call (rtx target)
>>  	    xops[0] = GEN_INT (TREE_INT_CST_LOW (TREE_VALUE (attr_args)));
>>  	    output_asm_insn ("call\t%0", xops);
>>  	  }
>> +	else if (fndecl_built_in_p (decl))
>> +	  {
>> +	    /* For now lets report this as an error while we are not able to
>> +	       link eBPF object files.  In particular with libgcc.  */
>> +	    tree name = DECL_NAME (decl);
>> +	    error ("call to external builtin %s in function, which is not supported by "
>> +		   "eBPF", name != NULL_TREE ? IDENTIFIER_POINTER (name) : "(anon)");
>> +	    output_asm_insn ("call 0", NULL);
>> +	  }
>>  	else
>>  	  output_asm_insn ("call\t%0", &target);
>>  
>> @@ -763,6 +772,18 @@ bpf_output_call (rtx target)
>>    return "";
>>  }
>>  
>> +static void
>> +bpf_external_libcall (rtx fun)
>> +{
>> +  tree decl = SYMBOL_REF_DECL (fun);
>> +  tree name = DECL_NAME (decl);
>> +  error ("call to external libcall %s in function, which is not supported by "
>> +	 "eBPF", name != NULL_TREE ? IDENTIFIER_POINTER (name) : "(anon)");
>> +}
>> +
>> +#undef  TARGET_ASM_EXTERNAL_LIBCALL
>> +#define TARGET_ASM_EXTERNAL_LIBCALL bpf_external_libcall
>> +
>>  /* Print register name according to assembly dialect.  In normal
>>     syntax registers are printed like %rN where N is the register
>>     number.
>> diff --git a/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c b/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c
>> index 4036570ac601..fec720584e48 100644
>> --- a/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c
>> +++ b/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c
>> @@ -6,7 +6,7 @@ foo (int *p, int *expected, int desired)
>>  {
>>    return __atomic_compare_exchange (p, expected, &desired, 0,
>>  				    __ATOMIC_ACQUIRE, __ATOMIC_RELAXED);
>> -}
>> +} /* { dg-error "call to external builtin" } */
>>  
>>  int
>>  foo64 (long *p, long *expected, long desired)
>> diff --git a/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c b/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c
>> index 044a2f76474b..ea1b8e48928a 100644
>> --- a/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c
>> +++ b/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c
>> @@ -9,7 +9,7 @@ long
>>  test_atomic_fetch_add (long x)
>>  {
>>    return __atomic_fetch_add (&val, x, __ATOMIC_ACQUIRE);
>> -}
>> +} /* { dg-error "call to external builtin" } */
>>  
>>  long
>>  test_atomic_fetch_sub (long x)
>> diff --git a/gcc/testsuite/gcc.target/bpf/atomic-op-3.c b/gcc/testsuite/gcc.target/bpf/atomic-op-3.c
>> index b2ce28926347..fefafd6b748f 100644
>> --- a/gcc/testsuite/gcc.target/bpf/atomic-op-3.c
>> +++ b/gcc/testsuite/gcc.target/bpf/atomic-op-3.c
>> @@ -20,7 +20,7 @@ void
>>  test_atomic_and (int x)
>>  {
>>    __atomic_and_fetch (&val, x, __ATOMIC_ACQUIRE);
>> -}
>> +} /* { dg-error "call to external builtin" } */
>>  
>>  void
>>  test_atomic_nand (int x)
>> diff --git a/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c b/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c
>> index 3b6324e966b8..eab695bf388c 100644
>> --- a/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c
>> +++ b/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c
>> @@ -7,7 +7,7 @@ int foo (int *p, int *new)
>>    int old;
>>    __atomic_exchange (p, new, &old, __ATOMIC_RELAXED);
>>    return old;
>> -}
>> +} /* { dg-error "call to external builtin" } */
>>  
>>  int foo64 (long *p, long *new)
>>  {
>> diff --git a/gcc/testsuite/gcc.target/bpf/diag-sdiv.c b/gcc/testsuite/gcc.target/bpf/diag-sdiv.c
>> index c48bbf03df97..c3332558e7d2 100644
>> --- a/gcc/testsuite/gcc.target/bpf/diag-sdiv.c
>> +++ b/gcc/testsuite/gcc.target/bpf/diag-sdiv.c
>> @@ -7,6 +7,6 @@ foo ()
>>  {
>>    signed int x = 5;
>>    signed int y = 2;
>> -  signed int z = x / y;
>> +  signed int z = x / y; /* { dg-error "call to external libcall" } */
>>  }
>>  /* { dg-final { scan-assembler-not "sdiv(32)?\t%r" } } */
>> diff --git a/gcc/testsuite/gcc.target/bpf/diag-smod.c b/gcc/testsuite/gcc.target/bpf/diag-smod.c
>> index d3df308217f3..25bcb1e4ffdb 100644
>> --- a/gcc/testsuite/gcc.target/bpf/diag-smod.c
>> +++ b/gcc/testsuite/gcc.target/bpf/diag-smod.c
>> @@ -7,6 +7,6 @@ foo ()
>>  {
>>    signed int x = 5;
>>    signed int y = 2;
>> -  signed int z = x % y;
>> +  signed int z = x % y; /* { dg-error "call to external libcall" } */
>>  }
>>  /* { dg-final { scan-assembler-not "smod(32)?\t%r" } } */

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: bpf: Throw error when external libcalls are generated.
  2023-11-28 15:40   ` Jose E. Marchesi
@ 2023-11-28 15:47     ` Cupertino Miranda
  0 siblings, 0 replies; 5+ messages in thread
From: Cupertino Miranda @ 2023-11-28 15:47 UTC (permalink / raw)
  To: Jose E. Marchesi; +Cc: gcc-patches, david.faust


Reverted! Apologies for the mistake.

Jose E. Marchesi writes:

>> Hi Cuper.
>> OK. Thanks for the patch.
>
> This commit is breaking the BPF build, because libgcc emits libcalls to
> __builtin_abort.  We need to rethink this.
>
> Please revert:
>
>   commit faf5b148588bd7fbb60ec669aefa704044037cdc
>   Author: Cupertino Miranda <cupertino.miranda@oracle.com>
>   Date:   Thu Nov 23 22:28:01 2023 +0000
>
> Thanks!
>
>>
>>> Hi everyone,
>>>
>>> The attached patch is a temporary solution for the lack of proper linker
>>> and external library linking of the eBPF platform.
>>> Any calls created by the compiler, that would usually be defined within
>>> libgcc, will endup being undefined in bpftool, when GCC the compiled
>>> code is passed.
>>>
>>> This patch anticipates that error to the compiler, by verifiying if
>>> any of those calls are being generated, and reporting as an error.
>>>
>>> Looking forward to your comments.
>>>
>>> Cheers,
>>> Cupertino
>>>
>>> commit c2110ae497c7ff83c309f172bc265973652b760d
>>> Author: Cupertino Miranda <cupertino.miranda@oracle.com>
>>> Date:   Thu Nov 23 22:28:01 2023 +0000
>>>
>>>     This patch enables errors when external calls are created.
>>>
>>>     When architectural limitations or usage of builtins implies the compiler
>>>     to create function calls to external libraries that implement the
>>>     functionality, GCC will now report an error claiming that this function
>>>     calls are not compatible with eBPF target.
>>>     Examples of those are the usage of __builtin_memmove and a sign division
>>>     in BPF ISA v3 or below that will require to call __divdi3.
>>>     This is currently an eBPF limitation which does not support linking of
>>>     object files but rather "raw" non linked ones. Those object files are
>>>     loaded and relocated by libbpf and the kernel.
>>>
>>>     gcc/ChangeLog:
>>>             * config/bpf/bpf.cc (bpf_output_call): Report error in case the
>>>             function call is for a builtin.
>>>             (bpf_external_libcall): Added target hook to detect and report
>>>             error when other external calls that are not builtins.
>>>
>>> diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
>>> index 0c9d5257c384..1c84113055b1 100644
>>> --- a/gcc/config/bpf/bpf.cc
>>> +++ b/gcc/config/bpf/bpf.cc
>>> @@ -744,6 +744,15 @@ bpf_output_call (rtx target)
>>>  	    xops[0] = GEN_INT (TREE_INT_CST_LOW (TREE_VALUE (attr_args)));
>>>  	    output_asm_insn ("call\t%0", xops);
>>>  	  }
>>> +	else if (fndecl_built_in_p (decl))
>>> +	  {
>>> +	    /* For now lets report this as an error while we are not able to
>>> +	       link eBPF object files.  In particular with libgcc.  */
>>> +	    tree name = DECL_NAME (decl);
>>> +	    error ("call to external builtin %s in function, which is not supported by "
>>> +		   "eBPF", name != NULL_TREE ? IDENTIFIER_POINTER (name) : "(anon)");
>>> +	    output_asm_insn ("call 0", NULL);
>>> +	  }
>>>  	else
>>>  	  output_asm_insn ("call\t%0", &target);
>>>
>>> @@ -763,6 +772,18 @@ bpf_output_call (rtx target)
>>>    return "";
>>>  }
>>>
>>> +static void
>>> +bpf_external_libcall (rtx fun)
>>> +{
>>> +  tree decl = SYMBOL_REF_DECL (fun);
>>> +  tree name = DECL_NAME (decl);
>>> +  error ("call to external libcall %s in function, which is not supported by "
>>> +	 "eBPF", name != NULL_TREE ? IDENTIFIER_POINTER (name) : "(anon)");
>>> +}
>>> +
>>> +#undef  TARGET_ASM_EXTERNAL_LIBCALL
>>> +#define TARGET_ASM_EXTERNAL_LIBCALL bpf_external_libcall
>>> +
>>>  /* Print register name according to assembly dialect.  In normal
>>>     syntax registers are printed like %rN where N is the register
>>>     number.
>>> diff --git a/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c b/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c
>>> index 4036570ac601..fec720584e48 100644
>>> --- a/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c
>>> +++ b/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c
>>> @@ -6,7 +6,7 @@ foo (int *p, int *expected, int desired)
>>>  {
>>>    return __atomic_compare_exchange (p, expected, &desired, 0,
>>>  				    __ATOMIC_ACQUIRE, __ATOMIC_RELAXED);
>>> -}
>>> +} /* { dg-error "call to external builtin" } */
>>>
>>>  int
>>>  foo64 (long *p, long *expected, long desired)
>>> diff --git a/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c b/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c
>>> index 044a2f76474b..ea1b8e48928a 100644
>>> --- a/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c
>>> +++ b/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c
>>> @@ -9,7 +9,7 @@ long
>>>  test_atomic_fetch_add (long x)
>>>  {
>>>    return __atomic_fetch_add (&val, x, __ATOMIC_ACQUIRE);
>>> -}
>>> +} /* { dg-error "call to external builtin" } */
>>>
>>>  long
>>>  test_atomic_fetch_sub (long x)
>>> diff --git a/gcc/testsuite/gcc.target/bpf/atomic-op-3.c b/gcc/testsuite/gcc.target/bpf/atomic-op-3.c
>>> index b2ce28926347..fefafd6b748f 100644
>>> --- a/gcc/testsuite/gcc.target/bpf/atomic-op-3.c
>>> +++ b/gcc/testsuite/gcc.target/bpf/atomic-op-3.c
>>> @@ -20,7 +20,7 @@ void
>>>  test_atomic_and (int x)
>>>  {
>>>    __atomic_and_fetch (&val, x, __ATOMIC_ACQUIRE);
>>> -}
>>> +} /* { dg-error "call to external builtin" } */
>>>
>>>  void
>>>  test_atomic_nand (int x)
>>> diff --git a/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c b/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c
>>> index 3b6324e966b8..eab695bf388c 100644
>>> --- a/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c
>>> +++ b/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c
>>> @@ -7,7 +7,7 @@ int foo (int *p, int *new)
>>>    int old;
>>>    __atomic_exchange (p, new, &old, __ATOMIC_RELAXED);
>>>    return old;
>>> -}
>>> +} /* { dg-error "call to external builtin" } */
>>>
>>>  int foo64 (long *p, long *new)
>>>  {
>>> diff --git a/gcc/testsuite/gcc.target/bpf/diag-sdiv.c b/gcc/testsuite/gcc.target/bpf/diag-sdiv.c
>>> index c48bbf03df97..c3332558e7d2 100644
>>> --- a/gcc/testsuite/gcc.target/bpf/diag-sdiv.c
>>> +++ b/gcc/testsuite/gcc.target/bpf/diag-sdiv.c
>>> @@ -7,6 +7,6 @@ foo ()
>>>  {
>>>    signed int x = 5;
>>>    signed int y = 2;
>>> -  signed int z = x / y;
>>> +  signed int z = x / y; /* { dg-error "call to external libcall" } */
>>>  }
>>>  /* { dg-final { scan-assembler-not "sdiv(32)?\t%r" } } */
>>> diff --git a/gcc/testsuite/gcc.target/bpf/diag-smod.c b/gcc/testsuite/gcc.target/bpf/diag-smod.c
>>> index d3df308217f3..25bcb1e4ffdb 100644
>>> --- a/gcc/testsuite/gcc.target/bpf/diag-smod.c
>>> +++ b/gcc/testsuite/gcc.target/bpf/diag-smod.c
>>> @@ -7,6 +7,6 @@ foo ()
>>>  {
>>>    signed int x = 5;
>>>    signed int y = 2;
>>> -  signed int z = x % y;
>>> +  signed int z = x % y; /* { dg-error "call to external libcall" } */
>>>  }
>>>  /* { dg-final { scan-assembler-not "smod(32)?\t%r" } } */

^ permalink raw reply	[flat|nested] 5+ messages in thread

* bpf: Throw error when external libcalls are generated.
@ 2023-11-27 14:40 Cupertino Miranda
  0 siblings, 0 replies; 5+ messages in thread
From: Cupertino Miranda @ 2023-11-27 14:40 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jose E. Marchesi, david.faust


User-agent: mu4e 1.4.15; emacs 28.1
Author: Cupertino Miranda <cupertino.miranda@oracle.com>
Hi everyone,

The attached patch is a temporary solution for the lack of proper linker
and external library linking of the eBPF platform.
Any calls created by the compiler, that would usually be defined within
libgcc, will endup being undefined in bpftool, when GCC the compiled
code is passed.

This patch anticipates that error to the compiler, by verifiying if
any of those calls are being generated, and reporting as an error.

Looking forward to your comments.

Cheers,
Cupertino

commit c2110ae497c7ff83c309f172bc265973652b760d
    This patch enables errors when external calls are created.

    When architectural limitations or usage of builtins implies the compiler
    to create function calls to external libraries that implement the
    functionality, GCC will now report an error claiming that this function
    calls are not compatible with eBPF target.
    Examples of those are the usage of __builtin_memmove and a sign division
    in BPF ISA v3 or below that will require to call __divdi3.
    This is currently an eBPF limitation which does not support linking of
    object files but rather "raw" non linked ones. Those object files are
    loaded and relocated by libbpf and the kernel.

    gcc/ChangeLog:
            * config/bpf/bpf.cc (bpf_output_call): Report error in case the
            function call is for a builtin.
            (bpf_external_libcall): Added target hook to detect and report
            error when other external calls that are not builtins.

diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
index 0c9d5257c384..1c84113055b1 100644
--- a/gcc/config/bpf/bpf.cc
+++ b/gcc/config/bpf/bpf.cc
@@ -744,6 +744,15 @@ bpf_output_call (rtx target)
 	    xops[0] = GEN_INT (TREE_INT_CST_LOW (TREE_VALUE (attr_args)));
 	    output_asm_insn ("call\t%0", xops);
 	  }
+	else if (fndecl_built_in_p (decl))
+	  {
+	    /* For now lets report this as an error while we are not able to
+	       link eBPF object files.  In particular with libgcc.  */
+	    tree name = DECL_NAME (decl);
+	    error ("call to external builtin %s in function, which is not supported by "
+		   "eBPF", name != NULL_TREE ? IDENTIFIER_POINTER (name) : "(anon)");
+	    output_asm_insn ("call 0", NULL);
+	  }
 	else
 	  output_asm_insn ("call\t%0", &target);

@@ -763,6 +772,18 @@ bpf_output_call (rtx target)
   return "";
 }

+static void
+bpf_external_libcall (rtx fun)
+{
+  tree decl = SYMBOL_REF_DECL (fun);
+  tree name = DECL_NAME (decl);
+  error ("call to external libcall %s in function, which is not supported by "
+	 "eBPF", name != NULL_TREE ? IDENTIFIER_POINTER (name) : "(anon)");
+}
+
+#undef  TARGET_ASM_EXTERNAL_LIBCALL
+#define TARGET_ASM_EXTERNAL_LIBCALL bpf_external_libcall
+
 /* Print register name according to assembly dialect.  In normal
    syntax registers are printed like %rN where N is the register
    number.
diff --git a/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c b/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c
index 4036570ac601..fec720584e48 100644
--- a/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c
+++ b/gcc/testsuite/gcc.target/bpf/atomic-cmpxchg-2.c
@@ -6,7 +6,7 @@ foo (int *p, int *expected, int desired)
 {
   return __atomic_compare_exchange (p, expected, &desired, 0,
 				    __ATOMIC_ACQUIRE, __ATOMIC_RELAXED);
-}
+} /* { dg-error "call to external builtin" } */

 int
 foo64 (long *p, long *expected, long desired)
diff --git a/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c b/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c
index 044a2f76474b..ea1b8e48928a 100644
--- a/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c
+++ b/gcc/testsuite/gcc.target/bpf/atomic-fetch-op-3.c
@@ -9,7 +9,7 @@ long
 test_atomic_fetch_add (long x)
 {
   return __atomic_fetch_add (&val, x, __ATOMIC_ACQUIRE);
-}
+} /* { dg-error "call to external builtin" } */

 long
 test_atomic_fetch_sub (long x)
diff --git a/gcc/testsuite/gcc.target/bpf/atomic-op-3.c b/gcc/testsuite/gcc.target/bpf/atomic-op-3.c
index b2ce28926347..fefafd6b748f 100644
--- a/gcc/testsuite/gcc.target/bpf/atomic-op-3.c
+++ b/gcc/testsuite/gcc.target/bpf/atomic-op-3.c
@@ -20,7 +20,7 @@ void
 test_atomic_and (int x)
 {
   __atomic_and_fetch (&val, x, __ATOMIC_ACQUIRE);
-}
+} /* { dg-error "call to external builtin" } */

 void
 test_atomic_nand (int x)
diff --git a/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c b/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c
index 3b6324e966b8..eab695bf388c 100644
--- a/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c
+++ b/gcc/testsuite/gcc.target/bpf/atomic-xchg-2.c
@@ -7,7 +7,7 @@ int foo (int *p, int *new)
   int old;
   __atomic_exchange (p, new, &old, __ATOMIC_RELAXED);
   return old;
-}
+} /* { dg-error "call to external builtin" } */

 int foo64 (long *p, long *new)
 {
diff --git a/gcc/testsuite/gcc.target/bpf/diag-sdiv.c b/gcc/testsuite/gcc.target/bpf/diag-sdiv.c
index c48bbf03df97..c3332558e7d2 100644
--- a/gcc/testsuite/gcc.target/bpf/diag-sdiv.c
+++ b/gcc/testsuite/gcc.target/bpf/diag-sdiv.c
@@ -7,6 +7,6 @@ foo ()
 {
   signed int x = 5;
   signed int y = 2;
-  signed int z = x / y;
+  signed int z = x / y; /* { dg-error "call to external libcall" } */
 }
 /* { dg-final { scan-assembler-not "sdiv(32)?\t%r" } } */
diff --git a/gcc/testsuite/gcc.target/bpf/diag-smod.c b/gcc/testsuite/gcc.target/bpf/diag-smod.c
index d3df308217f3..25bcb1e4ffdb 100644
--- a/gcc/testsuite/gcc.target/bpf/diag-smod.c
+++ b/gcc/testsuite/gcc.target/bpf/diag-smod.c
@@ -7,6 +7,6 @@ foo ()
 {
   signed int x = 5;
   signed int y = 2;
-  signed int z = x % y;
+  signed int z = x % y; /* { dg-error "call to external libcall" } */
 }
 /* { dg-final { scan-assembler-not "smod(32)?\t%r" } } */

Date: Mon, 27 Nov 2023 14:40:15 +0000
Message-ID: <87o7ffb6pc.fsf@oracle.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-11-28 15:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-24 16:26 bpf: Throw error when external libcalls are generated Cupertino Miranda
2023-11-27 14:50 ` Jose E. Marchesi
2023-11-28 15:40   ` Jose E. Marchesi
2023-11-28 15:47     ` Cupertino Miranda
2023-11-27 14:40 Cupertino Miranda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).