Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220826012614.552860-1-james.hilliard1@gmail.com> <CAEf4BzahSjt1huoJXAxSj-ycVjGKJm_dFinsZFBHtRGj7apiUQ@mail.gmail.com>
In-Reply-To: <CAEf4BzahSjt1huoJXAxSj-ycVjGKJm_dFinsZFBHtRGj7apiUQ@mail.gmail.com>
From:   James Hilliard <james.hilliard1@gmail.com>
Date:   Sat, 27 Aug 2022 04:47:14 -0600
Message-ID: <CADvTj4p8pykr4Vy-YSLcaXsfmuWAmZ9ay-np6mxTdwOT1LBnXw@mail.gmail.com>
Subject: Re: [PATCH] selftests/bpf: Fix conflicts with built-in functions in bpf_iter_ksym
To:     Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc:     bpf@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <martin.lau@linux.dev>,
        Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>,
        Stanislav Fomichev <sdf@google.com>,
        Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>,
        Mykola Lysenko <mykolal@fb.com>, Shuah Khan <shuah@kernel.org>,
        Alan Maguire <alan.maguire@oracle.com>,
        linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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
> >