| Message ID | 20220826012614.552860-1-james.hilliard1@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | selftests/bpf: Fix conflicts with built-in functions in bpf_iter_ksym | expand |
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 > > >
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 > > > >
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 {
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(-)