| Message ID | 20241013173742.71882-1-atrajeev@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | tools/perf/tests: Fix compilation error with strncpy in tests/tool_pmu | expand |
Hello Athira, On Sun, Oct 13, 2024 at 11:07:42PM +0530, Athira Rajeev wrote: > perf fails to compile on systems with GCC version11 > as below: > > In file included from /usr/include/string.h:519, > from /home/athir/perf-tools-next/tools/include/linux/bitmap.h:5, > from /home/athir/perf-tools-next/tools/perf/util/pmu.h:5, > from /home/athir/perf-tools-next/tools/perf/util/evsel.h:14, > from /home/athir/perf-tools-next/tools/perf/util/evlist.h:14, > from tests/tool_pmu.c:3: > In function ‘strncpy’, > inlined from ‘do_test’ at tests/tool_pmu.c:25:3: > /usr/include/bits/string_fortified.h:95:10: error: ‘__builtin_strncpy’ specified bound 128 equals destination size [-Werror=stringop-truncation] > 95 | return __builtin___strncpy_chk (__dest, __src, __len, > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 96 | __glibc_objsize (__dest)); > | ~~~~~~~~~~~~~~~~~~~~~~~~~ > > The compile error is from strncpy refernce in do_test: > strncpy(str, tool_pmu__event_to_str(ev), sizeof(str)); > > This behaviour is not observed with GCC version 8, but observed > with GCC version 11 . This is message from gcc for detecting > truncation while using strncpu. Use snprintf instead of strncpy > here to be safe. > > Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com> I found this issue now and thanks for the quick fix. I will push to perf-tools-next soon. Thanks, Namhyung > --- > tools/perf/tests/tool_pmu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/perf/tests/tool_pmu.c b/tools/perf/tests/tool_pmu.c > index 94d0dd8fd3cb..297cc8c55579 100644 > --- a/tools/perf/tests/tool_pmu.c > +++ b/tools/perf/tests/tool_pmu.c > @@ -22,7 +22,7 @@ static int do_test(enum tool_pmu_event ev, bool with_pmu) > if (with_pmu) > snprintf(str, sizeof(str), "tool/%s/", tool_pmu__event_to_str(ev)); > else > - strncpy(str, tool_pmu__event_to_str(ev), sizeof(str)); > + snprintf(str, sizeof(str), "%s", tool_pmu__event_to_str(ev)); > > parse_events_error__init(&err); > ret = parse_events(evlist, str, &err); > -- > 2.43.5 >
On Sun, Oct 13, 2024 at 11:07:42PM +0530, Athira Rajeev wrote: > perf fails to compile on systems with GCC version11 > as below: I have one more build failure on 32 bit systems. I'll carry this as well. Thanks, Namhyung ---8<--- From 2e2c7ca3691d223d94ea383a6b688e35579d14d5 Mon Sep 17 00:00:00 2001 From: Namhyung Kim <namhyung@kernel.org> Date: Mon, 14 Oct 2024 10:34:17 -0700 Subject: [PATCH] perf tools: Fix compiler error in util/tool_pmu.c util/tool_pmu.c: In function 'evsel__tool_pmu_read': util/tool_pmu.c:419:55: error: passing argument 2 of 'tool_pmu__read_event' from incompatible pointer type [-Werror=incompatible-pointer-types] 419 | if (!tool_pmu__read_event(ev, &val)) { | ^~~~ | | | long unsigned int * util/tool_pmu.c:335:56: note: expected 'u64 *' {aka 'long long unsigned int *'} but argument is of type 'long unsigned int *' 335 | bool tool_pmu__read_event(enum tool_pmu_event ev, u64 *result) | ~~~~~^~~~~~ Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/util/tool_pmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/util/tool_pmu.c b/tools/perf/util/tool_pmu.c index ea9d50f0252023dc..bd1cee643eb506df 100644 --- a/tools/perf/util/tool_pmu.c +++ b/tools/perf/util/tool_pmu.c @@ -394,7 +394,7 @@ bool tool_pmu__read_event(enum tool_pmu_event ev, u64 *result) int evsel__tool_pmu_read(struct evsel *evsel, int cpu_map_idx, int thread) { __u64 *start_time, cur_time, delta_start; - unsigned long val; + u64 val; int fd, err = 0; struct perf_counts_values *count, *old_count = NULL; bool adjust = false;
On Sun, 13 Oct 2024 23:07:42 +0530, Athira Rajeev wrote: > perf fails to compile on systems with GCC version11 > as below: > > In file included from /usr/include/string.h:519, > from /home/athir/perf-tools-next/tools/include/linux/bitmap.h:5, > from /home/athir/perf-tools-next/tools/perf/util/pmu.h:5, > from /home/athir/perf-tools-next/tools/perf/util/evsel.h:14, > from /home/athir/perf-tools-next/tools/perf/util/evlist.h:14, > from tests/tool_pmu.c:3: > In function ‘strncpy’, > inlined from ‘do_test’ at tests/tool_pmu.c:25:3: > /usr/include/bits/string_fortified.h:95:10: error: ‘__builtin_strncpy’ specified bound 128 equals destination size [-Werror=stringop-truncation] > 95 | return __builtin___strncpy_chk (__dest, __src, __len, > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 96 | __glibc_objsize (__dest)); > | ~~~~~~~~~~~~~~~~~~~~~~~~~ > > [...] Applied to perf-tools-next, thanks! Best regards, Namhyung
> On 14 Oct 2024, at 10:56 PM, Namhyung Kim <namhyung@kernel.org> wrote: > > Hello Athira, > > On Sun, Oct 13, 2024 at 11:07:42PM +0530, Athira Rajeev wrote: >> perf fails to compile on systems with GCC version11 >> as below: >> >> In file included from /usr/include/string.h:519, >> from /home/athir/perf-tools-next/tools/include/linux/bitmap.h:5, >> from /home/athir/perf-tools-next/tools/perf/util/pmu.h:5, >> from /home/athir/perf-tools-next/tools/perf/util/evsel.h:14, >> from /home/athir/perf-tools-next/tools/perf/util/evlist.h:14, >> from tests/tool_pmu.c:3: >> In function ‘strncpy’, >> inlined from ‘do_test’ at tests/tool_pmu.c:25:3: >> /usr/include/bits/string_fortified.h:95:10: error: ‘__builtin_strncpy’ specified bound 128 equals destination size [-Werror=stringop-truncation] >> 95 | return __builtin___strncpy_chk (__dest, __src, __len, >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> 96 | __glibc_objsize (__dest)); >> | ~~~~~~~~~~~~~~~~~~~~~~~~~ >> >> The compile error is from strncpy refernce in do_test: >> strncpy(str, tool_pmu__event_to_str(ev), sizeof(str)); >> >> This behaviour is not observed with GCC version 8, but observed >> with GCC version 11 . This is message from gcc for detecting >> truncation while using strncpu. Use snprintf instead of strncpy >> here to be safe. >> >> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com> > > I found this issue now and thanks for the quick fix. I will push to > perf-tools-next soon. > > Thanks, > Namhyung > Sure Thanks Namhyung for pulling in the change Athira >> --- >> tools/perf/tests/tool_pmu.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tools/perf/tests/tool_pmu.c b/tools/perf/tests/tool_pmu.c >> index 94d0dd8fd3cb..297cc8c55579 100644 >> --- a/tools/perf/tests/tool_pmu.c >> +++ b/tools/perf/tests/tool_pmu.c >> @@ -22,7 +22,7 @@ static int do_test(enum tool_pmu_event ev, bool with_pmu) >> if (with_pmu) >> snprintf(str, sizeof(str), "tool/%s/", tool_pmu__event_to_str(ev)); >> else >> - strncpy(str, tool_pmu__event_to_str(ev), sizeof(str)); >> + snprintf(str, sizeof(str), "%s", tool_pmu__event_to_str(ev)); >> >> parse_events_error__init(&err); >> ret = parse_events(evlist, str, &err); >> -- >> 2.43.5
On Wed, Oct 16, 2024 at 5:30 AM Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote: > > > > > On 14 Oct 2024, at 10:56 PM, Namhyung Kim <namhyung@kernel.org> wrote: > > > > Hello Athira, > > > > On Sun, Oct 13, 2024 at 11:07:42PM +0530, Athira Rajeev wrote: > >> perf fails to compile on systems with GCC version11 > >> as below: > >> > >> In file included from /usr/include/string.h:519, > >> from /home/athir/perf-tools-next/tools/include/linux/bitmap.h:5, > >> from /home/athir/perf-tools-next/tools/perf/util/pmu.h:5, > >> from /home/athir/perf-tools-next/tools/perf/util/evsel.h:14, > >> from /home/athir/perf-tools-next/tools/perf/util/evlist.h:14, > >> from tests/tool_pmu.c:3: > >> In function ‘strncpy’, > >> inlined from ‘do_test’ at tests/tool_pmu.c:25:3: > >> /usr/include/bits/string_fortified.h:95:10: error: ‘__builtin_strncpy’ specified bound 128 equals destination size [-Werror=stringop-truncation] > >> 95 | return __builtin___strncpy_chk (__dest, __src, __len, > >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > >> 96 | __glibc_objsize (__dest)); > >> | ~~~~~~~~~~~~~~~~~~~~~~~~~ > >> > >> The compile error is from strncpy refernce in do_test: > >> strncpy(str, tool_pmu__event_to_str(ev), sizeof(str)); > >> > >> This behaviour is not observed with GCC version 8, but observed > >> with GCC version 11 . This is message from gcc for detecting > >> truncation while using strncpu. Use snprintf instead of strncpy > >> here to be safe. > >> > >> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com> > > > > I found this issue now and thanks for the quick fix. I will push to > > perf-tools-next soon. > > > > Thanks, > > Namhyung > > > > Sure > > Thanks Namhyung for pulling in the change > > Athira Thanks for the fixes. As this is test code I don't think performance, style, etc. matter much. The GCC strncpy warnings are annoying imo, I'm not sure it makes sense for them to be enabled. I see in the kernel lots of "sizeof(foo)-1" as a workaround. strlcpy looks like a better alternative but it gets a checkpatch warning as in the kernel strscpy is preferred. Perhaps we should create a strscpy shim. Thanks, Ian > >> --- > >> tools/perf/tests/tool_pmu.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/tools/perf/tests/tool_pmu.c b/tools/perf/tests/tool_pmu.c > >> index 94d0dd8fd3cb..297cc8c55579 100644 > >> --- a/tools/perf/tests/tool_pmu.c > >> +++ b/tools/perf/tests/tool_pmu.c > >> @@ -22,7 +22,7 @@ static int do_test(enum tool_pmu_event ev, bool with_pmu) > >> if (with_pmu) > >> snprintf(str, sizeof(str), "tool/%s/", tool_pmu__event_to_str(ev)); > >> else > >> - strncpy(str, tool_pmu__event_to_str(ev), sizeof(str)); > >> + snprintf(str, sizeof(str), "%s", tool_pmu__event_to_str(ev)); > >> > >> parse_events_error__init(&err); > >> ret = parse_events(evlist, str, &err); > >> -- > >> 2.43.5 > >
> On 16 Oct 2024, at 8:36 PM, Ian Rogers <irogers@google.com> wrote: > > On Wed, Oct 16, 2024 at 5:30 AM Athira Rajeev > <atrajeev@linux.vnet.ibm.com> wrote: >> >> >> >>> On 14 Oct 2024, at 10:56 PM, Namhyung Kim <namhyung@kernel.org> wrote: >>> >>> Hello Athira, >>> >>> On Sun, Oct 13, 2024 at 11:07:42PM +0530, Athira Rajeev wrote: >>>> perf fails to compile on systems with GCC version11 >>>> as below: >>>> >>>> In file included from /usr/include/string.h:519, >>>> from /home/athir/perf-tools-next/tools/include/linux/bitmap.h:5, >>>> from /home/athir/perf-tools-next/tools/perf/util/pmu.h:5, >>>> from /home/athir/perf-tools-next/tools/perf/util/evsel.h:14, >>>> from /home/athir/perf-tools-next/tools/perf/util/evlist.h:14, >>>> from tests/tool_pmu.c:3: >>>> In function ‘strncpy’, >>>> inlined from ‘do_test’ at tests/tool_pmu.c:25:3: >>>> /usr/include/bits/string_fortified.h:95:10: error: ‘__builtin_strncpy’ specified bound 128 equals destination size [-Werror=stringop-truncation] >>>> 95 | return __builtin___strncpy_chk (__dest, __src, __len, >>>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>>> 96 | __glibc_objsize (__dest)); >>>> | ~~~~~~~~~~~~~~~~~~~~~~~~~ >>>> >>>> The compile error is from strncpy refernce in do_test: >>>> strncpy(str, tool_pmu__event_to_str(ev), sizeof(str)); >>>> >>>> This behaviour is not observed with GCC version 8, but observed >>>> with GCC version 11 . This is message from gcc for detecting >>>> truncation while using strncpu. Use snprintf instead of strncpy >>>> here to be safe. >>>> >>>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com> >>> >>> I found this issue now and thanks for the quick fix. I will push to >>> perf-tools-next soon. >>> >>> Thanks, >>> Namhyung >>> >> >> Sure >> >> Thanks Namhyung for pulling in the change >> >> Athira > > Thanks for the fixes. As this is test code I don't think performance, > style, etc. matter much. The GCC strncpy warnings are annoying imo, > I'm not sure it makes sense for them to be enabled. I see in the > kernel lots of "sizeof(foo)-1" as a workaround. strlcpy looks like a > better alternative but it gets a checkpatch warning as in the kernel > strscpy is preferred. Perhaps we should create a strscpy shim. > IIUC, strscpy is defined in kernel source and is currently not used in tools side. Ian, Your point here is that we can create an interface in tools side so that we can later safely use strscpy in cases like this ? Thanks Athira > Thanks, > Ian > >>>> --- >>>> tools/perf/tests/tool_pmu.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/tools/perf/tests/tool_pmu.c b/tools/perf/tests/tool_pmu.c >>>> index 94d0dd8fd3cb..297cc8c55579 100644 >>>> --- a/tools/perf/tests/tool_pmu.c >>>> +++ b/tools/perf/tests/tool_pmu.c >>>> @@ -22,7 +22,7 @@ static int do_test(enum tool_pmu_event ev, bool with_pmu) >>>> if (with_pmu) >>>> snprintf(str, sizeof(str), "tool/%s/", tool_pmu__event_to_str(ev)); >>>> else >>>> - strncpy(str, tool_pmu__event_to_str(ev), sizeof(str)); >>>> + snprintf(str, sizeof(str), "%s", tool_pmu__event_to_str(ev)); >>>> >>>> parse_events_error__init(&err); >>>> ret = parse_events(evlist, str, &err); >>>> -- >>>> 2.43.5
On Wed, Oct 16, 2024 at 11:23 AM Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote: > > > > > On 16 Oct 2024, at 8:36 PM, Ian Rogers <irogers@google.com> wrote: > > > > On Wed, Oct 16, 2024 at 5:30 AM Athira Rajeev > > <atrajeev@linux.vnet.ibm.com> wrote: > >> > >> > >> > >>> On 14 Oct 2024, at 10:56 PM, Namhyung Kim <namhyung@kernel.org> wrote: > >>> > >>> Hello Athira, > >>> > >>> On Sun, Oct 13, 2024 at 11:07:42PM +0530, Athira Rajeev wrote: > >>>> perf fails to compile on systems with GCC version11 > >>>> as below: > >>>> > >>>> In file included from /usr/include/string.h:519, > >>>> from /home/athir/perf-tools-next/tools/include/linux/bitmap.h:5, > >>>> from /home/athir/perf-tools-next/tools/perf/util/pmu.h:5, > >>>> from /home/athir/perf-tools-next/tools/perf/util/evsel.h:14, > >>>> from /home/athir/perf-tools-next/tools/perf/util/evlist.h:14, > >>>> from tests/tool_pmu.c:3: > >>>> In function ‘strncpy’, > >>>> inlined from ‘do_test’ at tests/tool_pmu.c:25:3: > >>>> /usr/include/bits/string_fortified.h:95:10: error: ‘__builtin_strncpy’ specified bound 128 equals destination size [-Werror=stringop-truncation] > >>>> 95 | return __builtin___strncpy_chk (__dest, __src, __len, > >>>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > >>>> 96 | __glibc_objsize (__dest)); > >>>> | ~~~~~~~~~~~~~~~~~~~~~~~~~ > >>>> > >>>> The compile error is from strncpy refernce in do_test: > >>>> strncpy(str, tool_pmu__event_to_str(ev), sizeof(str)); > >>>> > >>>> This behaviour is not observed with GCC version 8, but observed > >>>> with GCC version 11 . This is message from gcc for detecting > >>>> truncation while using strncpu. Use snprintf instead of strncpy > >>>> here to be safe. > >>>> > >>>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com> > >>> > >>> I found this issue now and thanks for the quick fix. I will push to > >>> perf-tools-next soon. > >>> > >>> Thanks, > >>> Namhyung > >>> > >> > >> Sure > >> > >> Thanks Namhyung for pulling in the change > >> > >> Athira > > > > Thanks for the fixes. As this is test code I don't think performance, > > style, etc. matter much. The GCC strncpy warnings are annoying imo, > > I'm not sure it makes sense for them to be enabled. I see in the > > kernel lots of "sizeof(foo)-1" as a workaround. strlcpy looks like a > > better alternative but it gets a checkpatch warning as in the kernel > > strscpy is preferred. Perhaps we should create a strscpy shim. > > > > IIUC, strscpy is defined in kernel source and is currently not used in tools side. > > Ian, > Your point here is that we can create an interface in tools side so that we can later safely use strscpy in cases like this ? Yeah. Tbh the string code is a bit chaotic in the code base. There's some stuff using char*s, some asprintf, there's strbuf then there all the kernel ways of doing things. I lose track of why strscpy is superior to strlcpy, but strlcpy will get you a checkpath warning. The standard kernel solution to the GCC strncpy warning I've seen, changing "strncpy(a, b, sizeof(a))" to "strncpy(a, b, sizeof(a) - 1)", looks dodgy and possibly worse to me. The point of the warning I think relates to strncpy not necessarily \0 terminating the string and you need to reserve at least 1 character for this, while strlcpy guarantees the \0 termination. Doing "sizeof(..) - 1" likely confuses the checker making it give up, but now you have less space to copy a potential \0 as well as not getting a \0. Thanks, Ian > Thanks > Athira > > > Thanks, > > Ian > > > >>>> --- > >>>> tools/perf/tests/tool_pmu.c | 2 +- > >>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>> > >>>> diff --git a/tools/perf/tests/tool_pmu.c b/tools/perf/tests/tool_pmu.c > >>>> index 94d0dd8fd3cb..297cc8c55579 100644 > >>>> --- a/tools/perf/tests/tool_pmu.c > >>>> +++ b/tools/perf/tests/tool_pmu.c > >>>> @@ -22,7 +22,7 @@ static int do_test(enum tool_pmu_event ev, bool with_pmu) > >>>> if (with_pmu) > >>>> snprintf(str, sizeof(str), "tool/%s/", tool_pmu__event_to_str(ev)); > >>>> else > >>>> - strncpy(str, tool_pmu__event_to_str(ev), sizeof(str)); > >>>> + snprintf(str, sizeof(str), "%s", tool_pmu__event_to_str(ev)); > >>>> > >>>> parse_events_error__init(&err); > >>>> ret = parse_events(evlist, str, &err); > >>>> -- > >>>> 2.43.5 > >
diff --git a/tools/perf/tests/tool_pmu.c b/tools/perf/tests/tool_pmu.c index 94d0dd8fd3cb..297cc8c55579 100644 --- a/tools/perf/tests/tool_pmu.c +++ b/tools/perf/tests/tool_pmu.c @@ -22,7 +22,7 @@ static int do_test(enum tool_pmu_event ev, bool with_pmu) if (with_pmu) snprintf(str, sizeof(str), "tool/%s/", tool_pmu__event_to_str(ev)); else - strncpy(str, tool_pmu__event_to_str(ev), sizeof(str)); + snprintf(str, sizeof(str), "%s", tool_pmu__event_to_str(ev)); parse_events_error__init(&err); ret = parse_events(evlist, str, &err);
perf fails to compile on systems with GCC version11 as below: In file included from /usr/include/string.h:519, from /home/athir/perf-tools-next/tools/include/linux/bitmap.h:5, from /home/athir/perf-tools-next/tools/perf/util/pmu.h:5, from /home/athir/perf-tools-next/tools/perf/util/evsel.h:14, from /home/athir/perf-tools-next/tools/perf/util/evlist.h:14, from tests/tool_pmu.c:3: In function ‘strncpy’, inlined from ‘do_test’ at tests/tool_pmu.c:25:3: /usr/include/bits/string_fortified.h:95:10: error: ‘__builtin_strncpy’ specified bound 128 equals destination size [-Werror=stringop-truncation] 95 | return __builtin___strncpy_chk (__dest, __src, __len, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 96 | __glibc_objsize (__dest)); | ~~~~~~~~~~~~~~~~~~~~~~~~~ The compile error is from strncpy refernce in do_test: strncpy(str, tool_pmu__event_to_str(ev), sizeof(str)); This behaviour is not observed with GCC version 8, but observed with GCC version 11 . This is message from gcc for detecting truncation while using strncpu. Use snprintf instead of strncpy here to be safe. Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com> --- tools/perf/tests/tool_pmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)