* [PATCH] selftests/bpf: Fix conflicts with built-in functions in bpf_iter_ksym
@ 2022-08-26 1:26 James Hilliard
2022-08-27 4:55 ` Andrii Nakryiko
0 siblings, 1 reply; 6+ messages in thread
From: James Hilliard @ 2022-08-26 1:26 UTC (permalink / raw)
To: bpf
Cc: James Hilliard, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Mykola Lysenko, Shuah Khan, Alan Maguire, linux-kselftest,
linux-kernel
Both tolower and toupper are built in c functions, we should not
redefine them as this can result in a build error.
Fixes the following errors:
progs/bpf_iter_ksym.c:10:20: error: conflicting types for built-in function 'tolower'; expected 'int(int)' [-Werror=builtin-declaration-mismatch]
10 | static inline char tolower(char c)
| ^~~~~~~
progs/bpf_iter_ksym.c:5:1: note: 'tolower' is declared in header '<ctype.h>'
4 | #include <bpf/bpf_helpers.h>
+++ |+#include <ctype.h>
5 |
progs/bpf_iter_ksym.c:17:20: error: conflicting types for built-in function 'toupper'; expected 'int(int)' [-Werror=builtin-declaration-mismatch]
17 | static inline char toupper(char c)
| ^~~~~~~
progs/bpf_iter_ksym.c:17:20: note: 'toupper' is declared in header '<ctype.h>'
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
tools/testing/selftests/bpf/progs/bpf_iter_ksym.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c b/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c
index 285c008cbf9c..9ba14c37bbcc 100644
--- a/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c
+++ b/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c
@@ -7,14 +7,14 @@ char _license[] SEC("license") = "GPL";
unsigned long last_sym_value = 0;
-static inline char tolower(char c)
+static inline char to_lower(char c)
{
if (c >= 'A' && c <= 'Z')
c += ('a' - 'A');
return c;
}
-static inline char toupper(char c)
+static inline char to_upper(char c)
{
if (c >= 'a' && c <= 'z')
c -= ('a' - 'A');
@@ -54,7 +54,7 @@ int dump_ksym(struct bpf_iter__ksym *ctx)
type = iter->type;
if (iter->module_name[0]) {
- type = iter->exported ? toupper(type) : tolower(type);
+ type = iter->exported ? to_upper(type) : to_lower(type);
BPF_SEQ_PRINTF(seq, "0x%llx %c %s [ %s ] ",
value, type, iter->name, iter->module_name);
} else {
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests/bpf: Fix conflicts with built-in functions in bpf_iter_ksym
2022-08-26 1:26 [PATCH] selftests/bpf: Fix conflicts with built-in functions in bpf_iter_ksym James Hilliard
@ 2022-08-27 4:55 ` Andrii Nakryiko
2022-08-27 10:47 ` James Hilliard
0 siblings, 1 reply; 6+ messages in thread
From: Andrii Nakryiko @ 2022-08-27 4:55 UTC (permalink / raw)
To: James Hilliard
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Mykola Lysenko,
Shuah Khan, Alan Maguire, linux-kselftest, linux-kernel
On Thu, Aug 25, 2022 at 6:26 PM James Hilliard
<james.hilliard1@gmail.com> wrote:
>
> Both tolower and toupper are built in c functions, we should not
Really? How come? Can you point out where this is specified in C
standard? From what I can tell you have to include <ctype.h> to get
toupper()/tolower().
This seems like yet another GCC-BPF quirk?
> redefine them as this can result in a build error.
>
> Fixes the following errors:
> progs/bpf_iter_ksym.c:10:20: error: conflicting types for built-in function 'tolower'; expected 'int(int)' [-Werror=builtin-declaration-mismatch]
> 10 | static inline char tolower(char c)
> | ^~~~~~~
> progs/bpf_iter_ksym.c:5:1: note: 'tolower' is declared in header '<ctype.h>'
> 4 | #include <bpf/bpf_helpers.h>
> +++ |+#include <ctype.h>
> 5 |
> progs/bpf_iter_ksym.c:17:20: error: conflicting types for built-in function 'toupper'; expected 'int(int)' [-Werror=builtin-declaration-mismatch]
> 17 | static inline char toupper(char c)
> | ^~~~~~~
> progs/bpf_iter_ksym.c:17:20: note: 'toupper' is declared in header '<ctype.h>'
>
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
> tools/testing/selftests/bpf/progs/bpf_iter_ksym.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c b/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c
> index 285c008cbf9c..9ba14c37bbcc 100644
> --- a/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c
> +++ b/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c
> @@ -7,14 +7,14 @@ char _license[] SEC("license") = "GPL";
>
> unsigned long last_sym_value = 0;
>
> -static inline char tolower(char c)
> +static inline char to_lower(char c)
> {
> if (c >= 'A' && c <= 'Z')
> c += ('a' - 'A');
> return c;
> }
>
> -static inline char toupper(char c)
> +static inline char to_upper(char c)
> {
> if (c >= 'a' && c <= 'z')
> c -= ('a' - 'A');
> @@ -54,7 +54,7 @@ int dump_ksym(struct bpf_iter__ksym *ctx)
> type = iter->type;
>
> if (iter->module_name[0]) {
> - type = iter->exported ? toupper(type) : tolower(type);
> + type = iter->exported ? to_upper(type) : to_lower(type);
> BPF_SEQ_PRINTF(seq, "0x%llx %c %s [ %s ] ",
> value, type, iter->name, iter->module_name);
> } else {
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests/bpf: Fix conflicts with built-in functions in bpf_iter_ksym
2022-08-27 4:55 ` Andrii Nakryiko
@ 2022-08-27 10:47 ` James Hilliard
2022-08-27 17:08 ` Andrii Nakryiko
0 siblings, 1 reply; 6+ messages in thread
From: James Hilliard @ 2022-08-27 10:47 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Mykola Lysenko,
Shuah Khan, Alan Maguire, linux-kselftest, linux-kernel
On Fri, Aug 26, 2022 at 10:55 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Aug 25, 2022 at 6:26 PM James Hilliard
> <james.hilliard1@gmail.com> wrote:
> >
> > Both tolower and toupper are built in c functions, we should not
>
> Really? How come? Can you point out where this is specified in C
> standard? From what I can tell you have to include <ctype.h> to get
> toupper()/tolower().
See background on this sort of issue:
https://stackoverflow.com/a/20582607
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=12213
(C99, 7.1.3p1) "All identifiers with external linkage in any of the following
subclauses (including the future library directions) are always reserved
for use as identifiers with external linkage."
>
> This seems like yet another GCC-BPF quirk?
Seems GCC takes a stricter interpretation of the standard here than
llvm.
It's also documented behavior in GCC:
https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-std-2
See:
The ISO C90 functions abort, abs, acos, asin, atan2, atan, calloc, ceil,
cosh, cos, exit, exp, fabs, floor, fmod, fprintf, fputs, free, frexp, fscanf,
isalnum, isalpha, iscntrl, isdigit, isgraph, islower, isprint, ispunct, isspace,
isupper, isxdigit, tolower, toupper, labs, ldexp, log10, log, malloc, memchr,
memcmp, memcpy, memset, modf, pow, printf, putchar, puts, realloc,
scanf, sinh, sin, snprintf, sprintf, sqrt, sscanf, strcat, strchr,
strcmp, strcpy,
strcspn, strlen, strncat, strncmp, strncpy, strpbrk, strrchr, strspn, strstr,
tanh, tan, vfprintf, vprintf and vsprintf are all recognized as
built-in functions
unless -fno-builtin is specified (or -fno-builtin-function is specified for an
individual function).
We could disable builtin functions but it seems more correct to just
rename it so that it doesn't conflict.
>
> > redefine them as this can result in a build error.
> >
> > Fixes the following errors:
> > progs/bpf_iter_ksym.c:10:20: error: conflicting types for built-in function 'tolower'; expected 'int(int)' [-Werror=builtin-declaration-mismatch]
> > 10 | static inline char tolower(char c)
> > | ^~~~~~~
> > progs/bpf_iter_ksym.c:5:1: note: 'tolower' is declared in header '<ctype.h>'
> > 4 | #include <bpf/bpf_helpers.h>
> > +++ |+#include <ctype.h>
> > 5 |
> > progs/bpf_iter_ksym.c:17:20: error: conflicting types for built-in function 'toupper'; expected 'int(int)' [-Werror=builtin-declaration-mismatch]
> > 17 | static inline char toupper(char c)
> > | ^~~~~~~
> > progs/bpf_iter_ksym.c:17:20: note: 'toupper' is declared in header '<ctype.h>'
> >
> > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > ---
> > tools/testing/selftests/bpf/progs/bpf_iter_ksym.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c b/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c
> > index 285c008cbf9c..9ba14c37bbcc 100644
> > --- a/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c
> > +++ b/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c
> > @@ -7,14 +7,14 @@ char _license[] SEC("license") = "GPL";
> >
> > unsigned long last_sym_value = 0;
> >
> > -static inline char tolower(char c)
> > +static inline char to_lower(char c)
> > {
> > if (c >= 'A' && c <= 'Z')
> > c += ('a' - 'A');
> > return c;
> > }
> >
> > -static inline char toupper(char c)
> > +static inline char to_upper(char c)
> > {
> > if (c >= 'a' && c <= 'z')
> > c -= ('a' - 'A');
> > @@ -54,7 +54,7 @@ int dump_ksym(struct bpf_iter__ksym *ctx)
> > type = iter->type;
> >
> > if (iter->module_name[0]) {
> > - type = iter->exported ? toupper(type) : tolower(type);
> > + type = iter->exported ? to_upper(type) : to_lower(type);
> > BPF_SEQ_PRINTF(seq, "0x%llx %c %s [ %s ] ",
> > value, type, iter->name, iter->module_name);
> > } else {
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests/bpf: Fix conflicts with built-in functions in bpf_iter_ksym
2022-08-27 10:47 ` James Hilliard
@ 2022-08-27 17:08 ` Andrii Nakryiko
2022-12-03 0:40 ` James Hilliard
0 siblings, 1 reply; 6+ messages in thread
From: Andrii Nakryiko @ 2022-08-27 17:08 UTC (permalink / raw)
To: James Hilliard
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Mykola Lysenko,
Shuah Khan, Alan Maguire, linux-kselftest, linux-kernel
On Sat, Aug 27, 2022 at 3:47 AM James Hilliard
<james.hilliard1@gmail.com> wrote:
>
> On Fri, Aug 26, 2022 at 10:55 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Thu, Aug 25, 2022 at 6:26 PM James Hilliard
> > <james.hilliard1@gmail.com> wrote:
> > >
> > > Both tolower and toupper are built in c functions, we should not
> >
> > Really? How come? Can you point out where this is specified in C
> > standard? From what I can tell you have to include <ctype.h> to get
> > toupper()/tolower().
>
> See background on this sort of issue:
> https://stackoverflow.com/a/20582607
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=12213
>
> (C99, 7.1.3p1) "All identifiers with external linkage in any of the following
> subclauses (including the future library directions) are always reserved
> for use as identifiers with external linkage."
>
Sigh, ok. Ghost functions: you can't use them without ctype.h, but you
can't define your own either. Very nice.
Can you please put all these details into a commit for the future? With that:
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> >
> > This seems like yet another GCC-BPF quirk?
>
> Seems GCC takes a stricter interpretation of the standard here than
> llvm.
>
> It's also documented behavior in GCC:
> https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-std-2
>
> See:
> The ISO C90 functions abort, abs, acos, asin, atan2, atan, calloc, ceil,
> cosh, cos, exit, exp, fabs, floor, fmod, fprintf, fputs, free, frexp, fscanf,
> isalnum, isalpha, iscntrl, isdigit, isgraph, islower, isprint, ispunct, isspace,
> isupper, isxdigit, tolower, toupper, labs, ldexp, log10, log, malloc, memchr,
> memcmp, memcpy, memset, modf, pow, printf, putchar, puts, realloc,
> scanf, sinh, sin, snprintf, sprintf, sqrt, sscanf, strcat, strchr,
> strcmp, strcpy,
> strcspn, strlen, strncat, strncmp, strncpy, strpbrk, strrchr, strspn, strstr,
> tanh, tan, vfprintf, vprintf and vsprintf are all recognized as
> built-in functions
> unless -fno-builtin is specified (or -fno-builtin-function is specified for an
> individual function).
>
> We could disable builtin functions but it seems more correct to just
> rename it so that it doesn't conflict.
>
> >
> > > redefine them as this can result in a build error.
> > >
> > > Fixes the following errors:
> > > progs/bpf_iter_ksym.c:10:20: error: conflicting types for built-in function 'tolower'; expected 'int(int)' [-Werror=builtin-declaration-mismatch]
> > > 10 | static inline char tolower(char c)
> > > | ^~~~~~~
> > > progs/bpf_iter_ksym.c:5:1: note: 'tolower' is declared in header '<ctype.h>'
> > > 4 | #include <bpf/bpf_helpers.h>
> > > +++ |+#include <ctype.h>
> > > 5 |
> > > progs/bpf_iter_ksym.c:17:20: error: conflicting types for built-in function 'toupper'; expected 'int(int)' [-Werror=builtin-declaration-mismatch]
> > > 17 | static inline char toupper(char c)
> > > | ^~~~~~~
> > > progs/bpf_iter_ksym.c:17:20: note: 'toupper' is declared in header '<ctype.h>'
> > >
> > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > > ---
> > > tools/testing/selftests/bpf/progs/bpf_iter_ksym.c | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c b/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c
> > > index 285c008cbf9c..9ba14c37bbcc 100644
> > > --- a/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c
> > > +++ b/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c
> > > @@ -7,14 +7,14 @@ char _license[] SEC("license") = "GPL";
> > >
> > > unsigned long last_sym_value = 0;
> > >
> > > -static inline char tolower(char c)
> > > +static inline char to_lower(char c)
> > > {
> > > if (c >= 'A' && c <= 'Z')
> > > c += ('a' - 'A');
> > > return c;
> > > }
> > >
> > > -static inline char toupper(char c)
> > > +static inline char to_upper(char c)
> > > {
> > > if (c >= 'a' && c <= 'z')
> > > c -= ('a' - 'A');
> > > @@ -54,7 +54,7 @@ int dump_ksym(struct bpf_iter__ksym *ctx)
> > > type = iter->type;
> > >
> > > if (iter->module_name[0]) {
> > > - type = iter->exported ? toupper(type) : tolower(type);
> > > + type = iter->exported ? to_upper(type) : to_lower(type);
> > > BPF_SEQ_PRINTF(seq, "0x%llx %c %s [ %s ] ",
> > > value, type, iter->name, iter->module_name);
> > > } else {
> > > --
> > > 2.34.1
> > >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests/bpf: Fix conflicts with built-in functions in bpf_iter_ksym
2022-08-27 17:08 ` Andrii Nakryiko
@ 2022-12-03 0:40 ` James Hilliard
2022-12-03 0:53 ` Andrii Nakryiko
0 siblings, 1 reply; 6+ messages in thread
From: James Hilliard @ 2022-12-03 0:40 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Mykola Lysenko,
Shuah Khan, Alan Maguire, linux-kselftest, linux-kernel
On Sat, Aug 27, 2022 at 1:08 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Sat, Aug 27, 2022 at 3:47 AM James Hilliard
> <james.hilliard1@gmail.com> wrote:
> >
> > On Fri, Aug 26, 2022 at 10:55 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Thu, Aug 25, 2022 at 6:26 PM James Hilliard
> > > <james.hilliard1@gmail.com> wrote:
> > > >
> > > > Both tolower and toupper are built in c functions, we should not
> > >
> > > Really? How come? Can you point out where this is specified in C
> > > standard? From what I can tell you have to include <ctype.h> to get
> > > toupper()/tolower().
> >
> > See background on this sort of issue:
> > https://stackoverflow.com/a/20582607
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=12213
> >
> > (C99, 7.1.3p1) "All identifiers with external linkage in any of the following
> > subclauses (including the future library directions) are always reserved
> > for use as identifiers with external linkage."
> >
>
> Sigh, ok. Ghost functions: you can't use them without ctype.h, but you
> can't define your own either. Very nice.
>
> Can you please put all these details into a commit for the future? With that:
Sure
>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
Is this good to apply or should I send a v2?
>
> > >
> > > This seems like yet another GCC-BPF quirk?
> >
> > Seems GCC takes a stricter interpretation of the standard here than
> > llvm.
> >
> > It's also documented behavior in GCC:
> > https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-std-2
> >
> > See:
> > The ISO C90 functions abort, abs, acos, asin, atan2, atan, calloc, ceil,
> > cosh, cos, exit, exp, fabs, floor, fmod, fprintf, fputs, free, frexp, fscanf,
> > isalnum, isalpha, iscntrl, isdigit, isgraph, islower, isprint, ispunct, isspace,
> > isupper, isxdigit, tolower, toupper, labs, ldexp, log10, log, malloc, memchr,
> > memcmp, memcpy, memset, modf, pow, printf, putchar, puts, realloc,
> > scanf, sinh, sin, snprintf, sprintf, sqrt, sscanf, strcat, strchr,
> > strcmp, strcpy,
> > strcspn, strlen, strncat, strncmp, strncpy, strpbrk, strrchr, strspn, strstr,
> > tanh, tan, vfprintf, vprintf and vsprintf are all recognized as
> > built-in functions
> > unless -fno-builtin is specified (or -fno-builtin-function is specified for an
> > individual function).
> >
> > We could disable builtin functions but it seems more correct to just
> > rename it so that it doesn't conflict.
> >
> > >
> > > > redefine them as this can result in a build error.
> > > >
> > > > Fixes the following errors:
> > > > progs/bpf_iter_ksym.c:10:20: error: conflicting types for built-in function 'tolower'; expected 'int(int)' [-Werror=builtin-declaration-mismatch]
> > > > 10 | static inline char tolower(char c)
> > > > | ^~~~~~~
> > > > progs/bpf_iter_ksym.c:5:1: note: 'tolower' is declared in header '<ctype.h>'
> > > > 4 | #include <bpf/bpf_helpers.h>
> > > > +++ |+#include <ctype.h>
> > > > 5 |
> > > > progs/bpf_iter_ksym.c:17:20: error: conflicting types for built-in function 'toupper'; expected 'int(int)' [-Werror=builtin-declaration-mismatch]
> > > > 17 | static inline char toupper(char c)
> > > > | ^~~~~~~
> > > > progs/bpf_iter_ksym.c:17:20: note: 'toupper' is declared in header '<ctype.h>'
> > > >
> > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > > > ---
> > > > tools/testing/selftests/bpf/progs/bpf_iter_ksym.c | 6 +++---
> > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c b/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c
> > > > index 285c008cbf9c..9ba14c37bbcc 100644
> > > > --- a/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c
> > > > +++ b/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c
> > > > @@ -7,14 +7,14 @@ char _license[] SEC("license") = "GPL";
> > > >
> > > > unsigned long last_sym_value = 0;
> > > >
> > > > -static inline char tolower(char c)
> > > > +static inline char to_lower(char c)
> > > > {
> > > > if (c >= 'A' && c <= 'Z')
> > > > c += ('a' - 'A');
> > > > return c;
> > > > }
> > > >
> > > > -static inline char toupper(char c)
> > > > +static inline char to_upper(char c)
> > > > {
> > > > if (c >= 'a' && c <= 'z')
> > > > c -= ('a' - 'A');
> > > > @@ -54,7 +54,7 @@ int dump_ksym(struct bpf_iter__ksym *ctx)
> > > > type = iter->type;
> > > >
> > > > if (iter->module_name[0]) {
> > > > - type = iter->exported ? toupper(type) : tolower(type);
> > > > + type = iter->exported ? to_upper(type) : to_lower(type);
> > > > BPF_SEQ_PRINTF(seq, "0x%llx %c %s [ %s ] ",
> > > > value, type, iter->name, iter->module_name);
> > > > } else {
> > > > --
> > > > 2.34.1
> > > >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests/bpf: Fix conflicts with built-in functions in bpf_iter_ksym
2022-12-03 0:40 ` James Hilliard
@ 2022-12-03 0:53 ` Andrii Nakryiko
0 siblings, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2022-12-03 0:53 UTC (permalink / raw)
To: James Hilliard
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Mykola Lysenko,
Shuah Khan, Alan Maguire, linux-kselftest, linux-kernel
On Fri, Dec 2, 2022 at 4:41 PM James Hilliard <james.hilliard1@gmail.com> wrote:
>
> On Sat, Aug 27, 2022 at 1:08 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Sat, Aug 27, 2022 at 3:47 AM James Hilliard
> > <james.hilliard1@gmail.com> wrote:
> > >
> > > On Fri, Aug 26, 2022 at 10:55 PM Andrii Nakryiko
> > > <andrii.nakryiko@gmail.com> wrote:
> > > >
> > > > On Thu, Aug 25, 2022 at 6:26 PM James Hilliard
> > > > <james.hilliard1@gmail.com> wrote:
> > > > >
> > > > > Both tolower and toupper are built in c functions, we should not
> > > >
> > > > Really? How come? Can you point out where this is specified in C
> > > > standard? From what I can tell you have to include <ctype.h> to get
> > > > toupper()/tolower().
> > >
> > > See background on this sort of issue:
> > > https://stackoverflow.com/a/20582607
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=12213
> > >
> > > (C99, 7.1.3p1) "All identifiers with external linkage in any of the following
> > > subclauses (including the future library directions) are always reserved
> > > for use as identifiers with external linkage."
> > >
> >
> > Sigh, ok. Ghost functions: you can't use them without ctype.h, but you
> > can't define your own either. Very nice.
> >
> > Can you please put all these details into a commit for the future? With that:
>
> Sure
>
> >
> > Acked-by: Andrii Nakryiko <andrii@kernel.org>
>
> Is this good to apply or should I send a v2?
please add those details and resend v2?
>
> >
> > > >
> > > > This seems like yet another GCC-BPF quirk?
> > >
> > > Seems GCC takes a stricter interpretation of the standard here than
> > > llvm.
> > >
> > > It's also documented behavior in GCC:
> > > https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-std-2
> > >
> > > See:
> > > The ISO C90 functions abort, abs, acos, asin, atan2, atan, calloc, ceil,
> > > cosh, cos, exit, exp, fabs, floor, fmod, fprintf, fputs, free, frexp, fscanf,
> > > isalnum, isalpha, iscntrl, isdigit, isgraph, islower, isprint, ispunct, isspace,
> > > isupper, isxdigit, tolower, toupper, labs, ldexp, log10, log, malloc, memchr,
> > > memcmp, memcpy, memset, modf, pow, printf, putchar, puts, realloc,
> > > scanf, sinh, sin, snprintf, sprintf, sqrt, sscanf, strcat, strchr,
> > > strcmp, strcpy,
> > > strcspn, strlen, strncat, strncmp, strncpy, strpbrk, strrchr, strspn, strstr,
> > > tanh, tan, vfprintf, vprintf and vsprintf are all recognized as
> > > built-in functions
> > > unless -fno-builtin is specified (or -fno-builtin-function is specified for an
> > > individual function).
> > >
> > > We could disable builtin functions but it seems more correct to just
> > > rename it so that it doesn't conflict.
> > >
> > > >
> > > > > redefine them as this can result in a build error.
> > > > >
> > > > > Fixes the following errors:
> > > > > progs/bpf_iter_ksym.c:10:20: error: conflicting types for built-in function 'tolower'; expected 'int(int)' [-Werror=builtin-declaration-mismatch]
> > > > > 10 | static inline char tolower(char c)
> > > > > | ^~~~~~~
> > > > > progs/bpf_iter_ksym.c:5:1: note: 'tolower' is declared in header '<ctype.h>'
> > > > > 4 | #include <bpf/bpf_helpers.h>
> > > > > +++ |+#include <ctype.h>
> > > > > 5 |
> > > > > progs/bpf_iter_ksym.c:17:20: error: conflicting types for built-in function 'toupper'; expected 'int(int)' [-Werror=builtin-declaration-mismatch]
> > > > > 17 | static inline char toupper(char c)
> > > > > | ^~~~~~~
> > > > > progs/bpf_iter_ksym.c:17:20: note: 'toupper' is declared in header '<ctype.h>'
> > > > >
> > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > > > > ---
> > > > > tools/testing/selftests/bpf/progs/bpf_iter_ksym.c | 6 +++---
> > > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c b/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c
> > > > > index 285c008cbf9c..9ba14c37bbcc 100644
> > > > > --- a/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c
> > > > > +++ b/tools/testing/selftests/bpf/progs/bpf_iter_ksym.c
> > > > > @@ -7,14 +7,14 @@ char _license[] SEC("license") = "GPL";
> > > > >
> > > > > unsigned long last_sym_value = 0;
> > > > >
> > > > > -static inline char tolower(char c)
> > > > > +static inline char to_lower(char c)
> > > > > {
> > > > > if (c >= 'A' && c <= 'Z')
> > > > > c += ('a' - 'A');
> > > > > return c;
> > > > > }
> > > > >
> > > > > -static inline char toupper(char c)
> > > > > +static inline char to_upper(char c)
> > > > > {
> > > > > if (c >= 'a' && c <= 'z')
> > > > > c -= ('a' - 'A');
> > > > > @@ -54,7 +54,7 @@ int dump_ksym(struct bpf_iter__ksym *ctx)
> > > > > type = iter->type;
> > > > >
> > > > > if (iter->module_name[0]) {
> > > > > - type = iter->exported ? toupper(type) : tolower(type);
> > > > > + type = iter->exported ? to_upper(type) : to_lower(type);
> > > > > BPF_SEQ_PRINTF(seq, "0x%llx %c %s [ %s ] ",
> > > > > value, type, iter->name, iter->module_name);
> > > > > } else {
> > > > > --
> > > > > 2.34.1
> > > > >
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-12-03 0:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-26 1:26 [PATCH] selftests/bpf: Fix conflicts with built-in functions in bpf_iter_ksym James Hilliard
2022-08-27 4:55 ` Andrii Nakryiko
2022-08-27 10:47 ` James Hilliard
2022-08-27 17:08 ` Andrii Nakryiko
2022-12-03 0:40 ` James Hilliard
2022-12-03 0:53 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox