Clang warns:
util/block-info.c:298:18: error: result of comparison against a string
literal is unspecified (use an explicit string comparison function
instead) [-Werror,-Wstring-compare]
if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
^ ~~~~~~~~~~~~~~~
util/block-info.c:298:51: error: result of comparison against a string
literal is unspecified (use an explicit string comparison function
instead) [-Werror,-Wstring-compare]
if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
^ ~~~~~~~~~~~~~~~
util/block-info.c:298:18: error: result of comparison against a string
literal is unspecified (use an explicit string
comparison function instead) [-Werror,-Wstring-compare]
if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
^ ~~~~~~~~~~~~~~~
util/block-info.c:298:51: error: result of comparison against a string
literal is unspecified (use an explicit string comparison function
instead) [-Werror,-Wstring-compare]
if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
^ ~~~~~~~~~~~~~~~
util/map.c:434:15: error: result of comparison against a string literal
is unspecified (use an explicit string comparison function instead)
[-Werror,-Wstring-compare]
if (srcline != SRCLINE_UNKNOWN)
^ ~~~~~~~~~~~~~~~
Link: https://github.com/ClangBuiltLinux/linux/issues/900
Signed-off-by: Nick Desaulniers <[email protected]>
---
Note: was generated off of mainline; can rebase on -next if it doesn't
apply cleanly.
tools/perf/builtin-diff.c | 3 ++-
tools/perf/util/block-info.c | 3 ++-
tools/perf/util/map.c | 2 +-
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index f8b6ae557d8b..c03c36fde7e2 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -1312,7 +1312,8 @@ static int cycles_printf(struct hist_entry *he, struct hist_entry *pair,
end_line = map__srcline(he->ms.map, bi->sym->start + bi->end,
he->ms.sym);
- if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
+ if ((strncmp(start_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0) &&
+ (strncmp(end_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0)) {
scnprintf(buf, sizeof(buf), "[%s -> %s] %4ld",
start_line, end_line, block_he->diff.cycles);
} else {
diff --git a/tools/perf/util/block-info.c b/tools/perf/util/block-info.c
index c4b030bf6ec2..fbbb6d640dad 100644
--- a/tools/perf/util/block-info.c
+++ b/tools/perf/util/block-info.c
@@ -295,7 +295,8 @@ static int block_range_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
end_line = map__srcline(he->ms.map, bi->sym->start + bi->end,
he->ms.sym);
- if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
+ if ((strncmp(start_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0) &&
+ (strncmp(end_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0)) {
scnprintf(buf, sizeof(buf), "[%s -> %s]",
start_line, end_line);
} else {
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index a08ca276098e..95428511300d 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -431,7 +431,7 @@ int map__fprintf_srcline(struct map *map, u64 addr, const char *prefix,
if (map && map->dso) {
char *srcline = map__srcline(map, addr, NULL);
- if (srcline != SRCLINE_UNKNOWN)
+ if (strncmp(srcline, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0)
ret = fprintf(fp, "%s%s", prefix, srcline);
free_srcline(srcline);
}
--
2.17.1
On Sun, Feb 23, 2020 at 11:35 AM Nick Desaulniers
<[email protected]> wrote:
>
> Clang warns:
>
> util/block-info.c:298:18: error: result of comparison against a string
> literal is unspecified (use an explicit string comparison function
> instead) [-Werror,-Wstring-compare]
> if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> ^ ~~~~~~~~~~~~~~~
> util/block-info.c:298:51: error: result of comparison against a string
> literal is unspecified (use an explicit string comparison function
> instead) [-Werror,-Wstring-compare]
> if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> ^ ~~~~~~~~~~~~~~~
> util/block-info.c:298:18: error: result of comparison against a string
> literal is unspecified (use an explicit string
> comparison function instead) [-Werror,-Wstring-compare]
> if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> ^ ~~~~~~~~~~~~~~~
> util/block-info.c:298:51: error: result of comparison against a string
> literal is unspecified (use an explicit string comparison function
> instead) [-Werror,-Wstring-compare]
> if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> ^ ~~~~~~~~~~~~~~~
> util/map.c:434:15: error: result of comparison against a string literal
> is unspecified (use an explicit string comparison function instead)
> [-Werror,-Wstring-compare]
> if (srcline != SRCLINE_UNKNOWN)
> ^ ~~~~~~~~~~~~~~~
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/900
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> Note: was generated off of mainline; can rebase on -next if it doesn't
> apply cleanly.
Looks good to me. Some more context:
https://clang.llvm.org/docs/DiagnosticsReference.html#wstring-compare
The spec says:
J.1 Unspecified behavior
The following are unspecified:
.. Whether two string literals result in distinct arrays (6.4.5).
> tools/perf/builtin-diff.c | 3 ++-
> tools/perf/util/block-info.c | 3 ++-
> tools/perf/util/map.c | 2 +-
> 3 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
> index f8b6ae557d8b..c03c36fde7e2 100644
> --- a/tools/perf/builtin-diff.c
> +++ b/tools/perf/builtin-diff.c
> @@ -1312,7 +1312,8 @@ static int cycles_printf(struct hist_entry *he, struct hist_entry *pair,
> end_line = map__srcline(he->ms.map, bi->sym->start + bi->end,
> he->ms.sym);
>
> - if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> + if ((strncmp(start_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0) &&
> + (strncmp(end_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0)) {
> scnprintf(buf, sizeof(buf), "[%s -> %s] %4ld",
> start_line, end_line, block_he->diff.cycles);
> } else {
> diff --git a/tools/perf/util/block-info.c b/tools/perf/util/block-info.c
> index c4b030bf6ec2..fbbb6d640dad 100644
> --- a/tools/perf/util/block-info.c
> +++ b/tools/perf/util/block-info.c
> @@ -295,7 +295,8 @@ static int block_range_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
> end_line = map__srcline(he->ms.map, bi->sym->start + bi->end,
> he->ms.sym);
>
> - if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> + if ((strncmp(start_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0) &&
> + (strncmp(end_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0)) {
> scnprintf(buf, sizeof(buf), "[%s -> %s]",
> start_line, end_line);
> } else {
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index a08ca276098e..95428511300d 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -431,7 +431,7 @@ int map__fprintf_srcline(struct map *map, u64 addr, const char *prefix,
>
> if (map && map->dso) {
> char *srcline = map__srcline(map, addr, NULL);
> - if (srcline != SRCLINE_UNKNOWN)
> + if (strncmp(srcline, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0)
> ret = fprintf(fp, "%s%s", prefix, srcline);
> free_srcline(srcline);
> }
> --
> 2.17.1
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200223193456.25291-1-nick.desaulniers%40gmail.com.
From: Ian Rogers
> Sent: 24 February 2020 05:56
> On Sun, Feb 23, 2020 at 11:35 AM Nick Desaulniers
> <[email protected]> wrote:
> >
> > Clang warns:
> >
> > util/block-info.c:298:18: error: result of comparison against a string
> > literal is unspecified (use an explicit string comparison function
> > instead) [-Werror,-Wstring-compare]
> > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > ^ ~~~~~~~~~~~~~~~
> > util/block-info.c:298:51: error: result of comparison against a string
> > literal is unspecified (use an explicit string comparison function
> > instead) [-Werror,-Wstring-compare]
> > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > ^ ~~~~~~~~~~~~~~~
> > util/block-info.c:298:18: error: result of comparison against a string
> > literal is unspecified (use an explicit string
> > comparison function instead) [-Werror,-Wstring-compare]
> > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > ^ ~~~~~~~~~~~~~~~
> > util/block-info.c:298:51: error: result of comparison against a string
> > literal is unspecified (use an explicit string comparison function
> > instead) [-Werror,-Wstring-compare]
> > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > ^ ~~~~~~~~~~~~~~~
> > util/map.c:434:15: error: result of comparison against a string literal
> > is unspecified (use an explicit string comparison function instead)
> > [-Werror,-Wstring-compare]
> > if (srcline != SRCLINE_UNKNOWN)
> > ^ ~~~~~~~~~~~~~~~
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/900
> > Signed-off-by: Nick Desaulniers <[email protected]>
> > ---
> > Note: was generated off of mainline; can rebase on -next if it doesn't
> > apply cleanly.
>
> Looks good to me. Some more context:
> https://clang.llvm.org/docs/DiagnosticsReference.html#wstring-compare
> The spec says:
> J.1 Unspecified behavior
> The following are unspecified:
> .. Whether two string literals result in distinct arrays (6.4.5).
Just change the (probable):
#define SRCLINE_UNKNOWN "unknown"
with
static const char SRC_LINE_UNKNOWN[] = "unk";
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
On Mon, Feb 24, 2020 at 8:03 AM David Laight <[email protected]> wrote:
>
> From: Ian Rogers
> > Sent: 24 February 2020 05:56
> > On Sun, Feb 23, 2020 at 11:35 AM Nick Desaulniers
> > <[email protected]> wrote:
> > >
> > > Clang warns:
> > >
> > > util/block-info.c:298:18: error: result of comparison against a string
> > > literal is unspecified (use an explicit string comparison function
> > > instead) [-Werror,-Wstring-compare]
> > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > ^ ~~~~~~~~~~~~~~~
> > > util/block-info.c:298:51: error: result of comparison against a string
> > > literal is unspecified (use an explicit string comparison function
> > > instead) [-Werror,-Wstring-compare]
> > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > ^ ~~~~~~~~~~~~~~~
> > > util/block-info.c:298:18: error: result of comparison against a string
> > > literal is unspecified (use an explicit string
> > > comparison function instead) [-Werror,-Wstring-compare]
> > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > ^ ~~~~~~~~~~~~~~~
> > > util/block-info.c:298:51: error: result of comparison against a string
> > > literal is unspecified (use an explicit string comparison function
> > > instead) [-Werror,-Wstring-compare]
> > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > ^ ~~~~~~~~~~~~~~~
> > > util/map.c:434:15: error: result of comparison against a string literal
> > > is unspecified (use an explicit string comparison function instead)
> > > [-Werror,-Wstring-compare]
> > > if (srcline != SRCLINE_UNKNOWN)
> > > ^ ~~~~~~~~~~~~~~~
> > >
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/900
> > > Signed-off-by: Nick Desaulniers <[email protected]>
> > > ---
> > > Note: was generated off of mainline; can rebase on -next if it doesn't
> > > apply cleanly.
Reviewed-by: Ian Rogers <[email protected]>
> > Looks good to me. Some more context:
> > https://clang.llvm.org/docs/DiagnosticsReference.html#wstring-compare
> > The spec says:
> > J.1 Unspecified behavior
> > The following are unspecified:
> > .. Whether two string literals result in distinct arrays (6.4.5).
>
> Just change the (probable):
> #define SRCLINE_UNKNOWN "unknown"
> with
> static const char SRC_LINE_UNKNOWN[] = "unk";
>
> David
The SRCLINE_UNKNOWN is used to convey information. Having multiple
distinct pointers (static) would mean the compiler could likely remove
all comparisons as the compiler could prove that pointer is never
returned by a function - ie comparisons are either known to be true
(!=) or false (==).
Thanks,
Ian
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
On Mon, Feb 24, 2020 at 10:20 AM 'Ian Rogers' via Clang Built Linux
<[email protected]> wrote:
>
> On Mon, Feb 24, 2020 at 8:03 AM David Laight <[email protected]> wrote:
> >
> > From: Ian Rogers
> > > Sent: 24 February 2020 05:56
> > > On Sun, Feb 23, 2020 at 11:35 AM Nick Desaulniers
> > > <[email protected]> wrote:
> > > >
> > > > Clang warns:
> > > >
> > > > util/block-info.c:298:18: error: result of comparison against a string
> > > > literal is unspecified (use an explicit string comparison function
> > > > instead) [-Werror,-Wstring-compare]
> > > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > > ^ ~~~~~~~~~~~~~~~
> > > > util/block-info.c:298:51: error: result of comparison against a string
> > > > literal is unspecified (use an explicit string comparison function
> > > > instead) [-Werror,-Wstring-compare]
> > > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > > ^ ~~~~~~~~~~~~~~~
> > > > util/block-info.c:298:18: error: result of comparison against a string
> > > > literal is unspecified (use an explicit string
> > > > comparison function instead) [-Werror,-Wstring-compare]
> > > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > > ^ ~~~~~~~~~~~~~~~
> > > > util/block-info.c:298:51: error: result of comparison against a string
> > > > literal is unspecified (use an explicit string comparison function
> > > > instead) [-Werror,-Wstring-compare]
> > > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > > ^ ~~~~~~~~~~~~~~~
> > > > util/map.c:434:15: error: result of comparison against a string literal
> > > > is unspecified (use an explicit string comparison function instead)
> > > > [-Werror,-Wstring-compare]
> > > > if (srcline != SRCLINE_UNKNOWN)
> > > > ^ ~~~~~~~~~~~~~~~
> > > >
> > > > Link: https://github.com/ClangBuiltLinux/linux/issues/900
> > > > Signed-off-by: Nick Desaulniers <[email protected]>
> > > > ---
> > > > Note: was generated off of mainline; can rebase on -next if it doesn't
> > > > apply cleanly.
>
> Reviewed-by: Ian Rogers <[email protected]>
>
> > > Looks good to me. Some more context:
> > > https://clang.llvm.org/docs/DiagnosticsReference.html#wstring-compare
> > > The spec says:
> > > J.1 Unspecified behavior
> > > The following are unspecified:
> > > .. Whether two string literals result in distinct arrays (6.4.5).
> >
> > Just change the (probable):
> > #define SRCLINE_UNKNOWN "unknown"
> > with
> > static const char SRC_LINE_UNKNOWN[] = "unk";
> >
> > David
>
>
> The SRCLINE_UNKNOWN is used to convey information. Having multiple
> distinct pointers (static) would mean the compiler could likely remove
> all comparisons as the compiler could prove that pointer is never
> returned by a function - ie comparisons are either known to be true
> (!=) or false (==).
I wouldn't define a static string in a header. Though I could:
1. forward declare it in the header with extern linkage.
2. define it in *one* .c source file.
Thoughts on that vs the current approach?
--
Thanks,
~Nick Desaulniers
From: Nick Desaulniers
> Sent: 24 February 2020 22:06
> On Mon, Feb 24, 2020 at 10:20 AM 'Ian Rogers' via Clang Built Linux
> <[email protected]> wrote:
> >
> > On Mon, Feb 24, 2020 at 8:03 AM David Laight <[email protected]> wrote:
> > >
> > > From: Ian Rogers
> > > > Sent: 24 February 2020 05:56
> > > > On Sun, Feb 23, 2020 at 11:35 AM Nick Desaulniers
> > > > <[email protected]> wrote:
> > > > >
> > > > > Clang warns:
> > > > >
> > > > > util/block-info.c:298:18: error: result of comparison against a string
> > > > > literal is unspecified (use an explicit string comparison function
> > > > > instead) [-Werror,-Wstring-compare]
> > > > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > > > ^ ~~~~~~~~~~~~~~~
> > > > > util/block-info.c:298:51: error: result of comparison against a string
> > > > > literal is unspecified (use an explicit string comparison function
> > > > > instead) [-Werror,-Wstring-compare]
> > > > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > > > ^ ~~~~~~~~~~~~~~~
> > > > > util/block-info.c:298:18: error: result of comparison against a string
> > > > > literal is unspecified (use an explicit string
> > > > > comparison function instead) [-Werror,-Wstring-compare]
> > > > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > > > ^ ~~~~~~~~~~~~~~~
> > > > > util/block-info.c:298:51: error: result of comparison against a string
> > > > > literal is unspecified (use an explicit string comparison function
> > > > > instead) [-Werror,-Wstring-compare]
> > > > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > > > ^ ~~~~~~~~~~~~~~~
> > > > > util/map.c:434:15: error: result of comparison against a string literal
> > > > > is unspecified (use an explicit string comparison function instead)
> > > > > [-Werror,-Wstring-compare]
> > > > > if (srcline != SRCLINE_UNKNOWN)
> > > > > ^ ~~~~~~~~~~~~~~~
> > > > >
> > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/900
> > > > > Signed-off-by: Nick Desaulniers <[email protected]>
> > > > > ---
> > > > > Note: was generated off of mainline; can rebase on -next if it doesn't
> > > > > apply cleanly.
> >
> > Reviewed-by: Ian Rogers <[email protected]>
> >
> > > > Looks good to me. Some more context:
> > > > https://clang.llvm.org/docs/DiagnosticsReference.html#wstring-compare
> > > > The spec says:
> > > > J.1 Unspecified behavior
> > > > The following are unspecified:
> > > > .. Whether two string literals result in distinct arrays (6.4.5).
> > >
> > > Just change the (probable):
> > > #define SRCLINE_UNKNOWN "unknown"
> > > with
> > > static const char SRC_LINE_UNKNOWN[] = "unk";
> > >
> > > David
> >
> >
> > The SRCLINE_UNKNOWN is used to convey information. Having multiple
> > distinct pointers (static) would mean the compiler could likely remove
> > all comparisons as the compiler could prove that pointer is never
> > returned by a function - ie comparisons are either known to be true
> > (!=) or false (==).
>
> I wouldn't define a static string in a header. Though I could:
> 1. forward declare it in the header with extern linkage.
> 2. define it in *one* .c source file.
>
> Thoughts on that vs the current approach?
The string compares are just stupid.
If the 'fake' strings are not printed you could use:
#define SRCLINE_UNKNOWN ((const char *)1)
Otherwise defining the strings in one of the C files is better.
Relying on the linker to merge the strings from different compilation
units is so broken...
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
On Tue, Feb 25, 2020 at 1:35 AM David Laight <[email protected]> wrote:
>
> From: Nick Desaulniers
> > Sent: 24 February 2020 22:06
> > On Mon, Feb 24, 2020 at 10:20 AM 'Ian Rogers' via Clang Built Linux
> > <[email protected]> wrote:
> > >
> > > On Mon, Feb 24, 2020 at 8:03 AM David Laight <[email protected]> wrote:
> > > >
> > > > From: Ian Rogers
> > > > > Sent: 24 February 2020 05:56
> > > > > On Sun, Feb 23, 2020 at 11:35 AM Nick Desaulniers
> > > > > <[email protected]> wrote:
> > > > > >
> > > > > > Clang warns:
> > > > > >
> > > > > > util/block-info.c:298:18: error: result of comparison against a string
> > > > > > literal is unspecified (use an explicit string comparison function
> > > > > > instead) [-Werror,-Wstring-compare]
> > > > > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > > > > ^ ~~~~~~~~~~~~~~~
> > > > > > util/block-info.c:298:51: error: result of comparison against a string
> > > > > > literal is unspecified (use an explicit string comparison function
> > > > > > instead) [-Werror,-Wstring-compare]
> > > > > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > > > > ^ ~~~~~~~~~~~~~~~
> > > > > > util/block-info.c:298:18: error: result of comparison against a string
> > > > > > literal is unspecified (use an explicit string
> > > > > > comparison function instead) [-Werror,-Wstring-compare]
> > > > > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > > > > ^ ~~~~~~~~~~~~~~~
> > > > > > util/block-info.c:298:51: error: result of comparison against a string
> > > > > > literal is unspecified (use an explicit string comparison function
> > > > > > instead) [-Werror,-Wstring-compare]
> > > > > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > > > > ^ ~~~~~~~~~~~~~~~
> > > > > > util/map.c:434:15: error: result of comparison against a string literal
> > > > > > is unspecified (use an explicit string comparison function instead)
> > > > > > [-Werror,-Wstring-compare]
> > > > > > if (srcline != SRCLINE_UNKNOWN)
> > > > > > ^ ~~~~~~~~~~~~~~~
> > > > > >
> > > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/900
> > > > > > Signed-off-by: Nick Desaulniers <[email protected]>
> > > > > > ---
> > > > > > Note: was generated off of mainline; can rebase on -next if it doesn't
> > > > > > apply cleanly.
> > >
> > > Reviewed-by: Ian Rogers <[email protected]>
> > >
> > > > > Looks good to me. Some more context:
> > > > > https://clang.llvm.org/docs/DiagnosticsReference.html#wstring-compare
> > > > > The spec says:
> > > > > J.1 Unspecified behavior
> > > > > The following are unspecified:
> > > > > .. Whether two string literals result in distinct arrays (6.4.5).
> > > >
> > > > Just change the (probable):
> > > > #define SRCLINE_UNKNOWN "unknown"
> > > > with
> > > > static const char SRC_LINE_UNKNOWN[] = "unk";
> > > >
> > > > David
> > >
> > >
> > > The SRCLINE_UNKNOWN is used to convey information. Having multiple
> > > distinct pointers (static) would mean the compiler could likely remove
> > > all comparisons as the compiler could prove that pointer is never
> > > returned by a function - ie comparisons are either known to be true
> > > (!=) or false (==).
> >
> > I wouldn't define a static string in a header. Though I could:
> > 1. forward declare it in the header with extern linkage.
> > 2. define it in *one* .c source file.
> >
> > Thoughts on that vs the current approach?
>
> The string compares are just stupid.
> If the 'fake' strings are not printed you could use:
> #define SRCLINE_UNKNOWN ((const char *)1)
>
> Otherwise defining the strings in one of the C files is better.
> Relying on the linker to merge the strings from different compilation
> units is so broken...
Note: it looks like free_srcline() already does strcmp, so my patch
basically does a more consistent job for string comparisons. Forward
declaring then defining in tools/perf/util/srcline.c involves changing
the function signatures and struct members to `const char*` rather
than `char*`, which is of questionable value. I wouldn't mind
changing my patch to just use strcmp instead of strncmp, or convert
free_srcline() to use strncmp instead, if folks felt strongly about
being consistent. Otherwise I think my patch with Ian's Reviewed-by is
the best approach.
--
Thanks,
~Nick Desaulniers
Em Tue, Feb 25, 2020 at 01:28:50PM -0800, Nick Desaulniers escreveu:
> On Tue, Feb 25, 2020 at 1:35 AM David Laight <[email protected]> wrote:
> >
> > From: Nick Desaulniers
> > > Sent: 24 February 2020 22:06
> > > On Mon, Feb 24, 2020 at 10:20 AM 'Ian Rogers' via Clang Built Linux
> > > <[email protected]> wrote:
> > > >
> > > > On Mon, Feb 24, 2020 at 8:03 AM David Laight <[email protected]> wrote:
> > > > >
> > > > > From: Ian Rogers
> > > > > > Sent: 24 February 2020 05:56
> > > > > > On Sun, Feb 23, 2020 at 11:35 AM Nick Desaulniers
> > > > > > <[email protected]> wrote:
> > > > > > >
> > > > > > > Clang warns:
> > > > > > >
> > > > > > > util/block-info.c:298:18: error: result of comparison against a string
> > > > > > > literal is unspecified (use an explicit string comparison function
> > > > > > > instead) [-Werror,-Wstring-compare]
> > > > > > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > > > > > ^ ~~~~~~~~~~~~~~~
> > > > > > > util/block-info.c:298:51: error: result of comparison against a string
> > > > > > > literal is unspecified (use an explicit string comparison function
> > > > > > > instead) [-Werror,-Wstring-compare]
> > > > > > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > > > > > ^ ~~~~~~~~~~~~~~~
> > > > > > > util/block-info.c:298:18: error: result of comparison against a string
> > > > > > > literal is unspecified (use an explicit string
> > > > > > > comparison function instead) [-Werror,-Wstring-compare]
> > > > > > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > > > > > ^ ~~~~~~~~~~~~~~~
> > > > > > > util/block-info.c:298:51: error: result of comparison against a string
> > > > > > > literal is unspecified (use an explicit string comparison function
> > > > > > > instead) [-Werror,-Wstring-compare]
> > > > > > > if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
> > > > > > > ^ ~~~~~~~~~~~~~~~
> > > > > > > util/map.c:434:15: error: result of comparison against a string literal
> > > > > > > is unspecified (use an explicit string comparison function instead)
> > > > > > > [-Werror,-Wstring-compare]
> > > > > > > if (srcline != SRCLINE_UNKNOWN)
> > > > > > > ^ ~~~~~~~~~~~~~~~
> > > > > > >
> > > > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/900
> > > > > > > Signed-off-by: Nick Desaulniers <[email protected]>
> > > > > > > ---
> > > > > > > Note: was generated off of mainline; can rebase on -next if it doesn't
> > > > > > > apply cleanly.
> > > >
> > > > Reviewed-by: Ian Rogers <[email protected]>
> > > >
> > > > > > Looks good to me. Some more context:
> > > > > > https://clang.llvm.org/docs/DiagnosticsReference.html#wstring-compare
> > > > > > The spec says:
> > > > > > J.1 Unspecified behavior
> > > > > > The following are unspecified:
> > > > > > .. Whether two string literals result in distinct arrays (6.4.5).
> > > > >
> > > > > Just change the (probable):
> > > > > #define SRCLINE_UNKNOWN "unknown"
> > > > > with
> > > > > static const char SRC_LINE_UNKNOWN[] = "unk";
> > > > >
> > > > > David
> > > >
> > > >
> > > > The SRCLINE_UNKNOWN is used to convey information. Having multiple
> > > > distinct pointers (static) would mean the compiler could likely remove
> > > > all comparisons as the compiler could prove that pointer is never
> > > > returned by a function - ie comparisons are either known to be true
> > > > (!=) or false (==).
> > >
> > > I wouldn't define a static string in a header. Though I could:
> > > 1. forward declare it in the header with extern linkage.
> > > 2. define it in *one* .c source file.
> > >
> > > Thoughts on that vs the current approach?
> >
> > The string compares are just stupid.
> > If the 'fake' strings are not printed you could use:
> > #define SRCLINE_UNKNOWN ((const char *)1)
> >
> > Otherwise defining the strings in one of the C files is better.
> > Relying on the linker to merge the strings from different compilation
> > units is so broken...
>
> Note: it looks like free_srcline() already does strcmp, so my patch
> basically does a more consistent job for string comparisons. Forward
> declaring then defining in tools/perf/util/srcline.c involves changing
> the function signatures and struct members to `const char*` rather
> than `char*`, which is of questionable value. I wouldn't mind
> changing my patch to just use strcmp instead of strncmp, or convert
> free_srcline() to use strncmp instead, if folks felt strongly about
> being consistent. Otherwise I think my patch with Ian's Reviewed-by is
> the best approach.
Fair enough, applying it with Ian's reviewed-by,
- Arnaldo
The following commit has been merged into the perf/urgent branch of tip:
Commit-ID: cfd3bc752a3f5529506d279deb42e3bc8055695b
Gitweb: https://git.kernel.org/tip/cfd3bc752a3f5529506d279deb42e3bc8055695b
Author: Nick Desaulniers <[email protected]>
AuthorDate: Sun, 23 Feb 2020 11:34:49 -08:00
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitterDate: Fri, 06 Mar 2020 08:30:29 -03:00
perf diff: Fix undefined string comparision spotted by clang's -Wstring-compare
clang warns:
util/block-info.c:298:18: error: result of comparison against a string
literal is unspecified (use an explicit string comparison function
instead) [-Werror,-Wstring-compare]
if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
^ ~~~~~~~~~~~~~~~
util/block-info.c:298:51: error: result of comparison against a string
literal is unspecified (use an explicit string comparison function
instead) [-Werror,-Wstring-compare]
if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
^ ~~~~~~~~~~~~~~~
util/block-info.c:298:18: error: result of comparison against a string
literal is unspecified (use an explicit string
comparison function instead) [-Werror,-Wstring-compare]
if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
^ ~~~~~~~~~~~~~~~
util/block-info.c:298:51: error: result of comparison against a string
literal is unspecified (use an explicit string comparison function
instead) [-Werror,-Wstring-compare]
if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
^ ~~~~~~~~~~~~~~~
util/map.c:434:15: error: result of comparison against a string literal
is unspecified (use an explicit string comparison function instead)
[-Werror,-Wstring-compare]
if (srcline != SRCLINE_UNKNOWN)
^ ~~~~~~~~~~~~~~~
Reviewer Notes:
Looks good to me. Some more context:
https://clang.llvm.org/docs/DiagnosticsReference.html#wstring-compare
The spec says:
J.1 Unspecified behavior
The following are unspecified:
.. Whether two string literals result in distinct arrays (6.4.5).
Signed-off-by: Nick Desaulniers <[email protected]>
Reviewed-by: Ian Rogers <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Changbin Du <[email protected]>
Cc: Jin Yao <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: John Keeping <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Song Liu <[email protected]>
Cc: [email protected]
Link: https://github.com/ClangBuiltLinux/linux/issues/900
Link: http://lore.kernel.org/lkml/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-diff.c | 3 ++-
tools/perf/util/block-info.c | 3 ++-
tools/perf/util/map.c | 2 +-
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index f8b6ae5..c03c36f 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -1312,7 +1312,8 @@ static int cycles_printf(struct hist_entry *he, struct hist_entry *pair,
end_line = map__srcline(he->ms.map, bi->sym->start + bi->end,
he->ms.sym);
- if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
+ if ((strncmp(start_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0) &&
+ (strncmp(end_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0)) {
scnprintf(buf, sizeof(buf), "[%s -> %s] %4ld",
start_line, end_line, block_he->diff.cycles);
} else {
diff --git a/tools/perf/util/block-info.c b/tools/perf/util/block-info.c
index c4b030b..fbbb6d6 100644
--- a/tools/perf/util/block-info.c
+++ b/tools/perf/util/block-info.c
@@ -295,7 +295,8 @@ static int block_range_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
end_line = map__srcline(he->ms.map, bi->sym->start + bi->end,
he->ms.sym);
- if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
+ if ((strncmp(start_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0) &&
+ (strncmp(end_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0)) {
scnprintf(buf, sizeof(buf), "[%s -> %s]",
start_line, end_line);
} else {
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index a08ca27..9542851 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -431,7 +431,7 @@ int map__fprintf_srcline(struct map *map, u64 addr, const char *prefix,
if (map && map->dso) {
char *srcline = map__srcline(map, addr, NULL);
- if (srcline != SRCLINE_UNKNOWN)
+ if (strncmp(srcline, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0)
ret = fprintf(fp, "%s%s", prefix, srcline);
free_srcline(srcline);
}
The following commit has been merged into the perf/core branch of tip:
Commit-ID: c395c3553d6870541f1c283479aea2a6f26364d5
Gitweb: https://git.kernel.org/tip/c395c3553d6870541f1c283479aea2a6f26364d5
Author: Nick Desaulniers <[email protected]>
AuthorDate: Sun, 23 Feb 2020 11:34:49 -08:00
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitterDate: Wed, 04 Mar 2020 10:28:08 -03:00
perf diff: Fix undefined string comparison spotted by clang's -Wstring-compare
clang warns:
util/block-info.c:298:18: error: result of comparison against a string
literal is unspecified (use an explicit string comparison function
instead) [-Werror,-Wstring-compare]
if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
^ ~~~~~~~~~~~~~~~
util/block-info.c:298:51: error: result of comparison against a string
literal is unspecified (use an explicit string comparison function
instead) [-Werror,-Wstring-compare]
if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
^ ~~~~~~~~~~~~~~~
util/block-info.c:298:18: error: result of comparison against a string
literal is unspecified (use an explicit string
comparison function instead) [-Werror,-Wstring-compare]
if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
^ ~~~~~~~~~~~~~~~
util/block-info.c:298:51: error: result of comparison against a string
literal is unspecified (use an explicit string comparison function
instead) [-Werror,-Wstring-compare]
if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
^ ~~~~~~~~~~~~~~~
util/map.c:434:15: error: result of comparison against a string literal
is unspecified (use an explicit string comparison function instead)
[-Werror,-Wstring-compare]
if (srcline != SRCLINE_UNKNOWN)
^ ~~~~~~~~~~~~~~~
Reviewer Notes:
Looks good to me. Some more context:
https://clang.llvm.org/docs/DiagnosticsReference.html#wstring-compare
The spec says:
J.1 Unspecified behavior
The following are unspecified:
.. Whether two string literals result in distinct arrays (6.4.5).
Signed-off-by: Nick Desaulniers <[email protected]>
Reviewed-by: Ian Rogers <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Changbin Du <[email protected]>
Cc: Jin Yao <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: John Keeping <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Song Liu <[email protected]>
Cc: [email protected]
Link: https://github.com/ClangBuiltLinux/linux/issues/900
Link: http://lore.kernel.org/lkml/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-diff.c | 3 ++-
tools/perf/util/block-info.c | 3 ++-
tools/perf/util/map.c | 2 +-
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index f8b6ae5..c03c36f 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -1312,7 +1312,8 @@ static int cycles_printf(struct hist_entry *he, struct hist_entry *pair,
end_line = map__srcline(he->ms.map, bi->sym->start + bi->end,
he->ms.sym);
- if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
+ if ((strncmp(start_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0) &&
+ (strncmp(end_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0)) {
scnprintf(buf, sizeof(buf), "[%s -> %s] %4ld",
start_line, end_line, block_he->diff.cycles);
} else {
diff --git a/tools/perf/util/block-info.c b/tools/perf/util/block-info.c
index c4b030b..fbbb6d6 100644
--- a/tools/perf/util/block-info.c
+++ b/tools/perf/util/block-info.c
@@ -295,7 +295,8 @@ static int block_range_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
end_line = map__srcline(he->ms.map, bi->sym->start + bi->end,
he->ms.sym);
- if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
+ if ((strncmp(start_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0) &&
+ (strncmp(end_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0)) {
scnprintf(buf, sizeof(buf), "[%s -> %s]",
start_line, end_line);
} else {
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index a08ca27..9542851 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -431,7 +431,7 @@ int map__fprintf_srcline(struct map *map, u64 addr, const char *prefix,
if (map && map->dso) {
char *srcline = map__srcline(map, addr, NULL);
- if (srcline != SRCLINE_UNKNOWN)
+ if (strncmp(srcline, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0)
ret = fprintf(fp, "%s%s", prefix, srcline);
free_srcline(srcline);
}