From 922db21d7e094c363313f9787acdd47d774651af Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Mon, 12 Jun 2023 11:10:46 -0300 Subject: [PATCH] perf srcline: Optimize comparision against SRCLINE_UNKNOWN This is a string constant that gets returned and then strcmp() around, we can instead just do a pointer comparision. That requires a new global variable to comply with these warnings from some versions of clang and gcc: 41 68.95 fedora:rawhide : FAIL clang version 16.0.4 (Fedora 16.0.4-1.fc39) result of comparison against a string literal is unspecified (use an explicit string comparison function instead) [-Werror,-Wstring-compare] if (start_line != SRCLINE_UNKNOWN && ^ ~~~~~~~~~~~~~~~ 41 Ack comments: Agreed, the strcmps make me nervous as they won't distinguish heap from a global meaning we could end up with things like pointers to freed memory. The comparison with the global is always going to be same imo. Acked-by: Ian Rogers Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Ali Saidi Cc: Andi Kleen Cc: Athira Rajeev Cc: Brian Robbins Cc: Changbin Du Cc: Dmitrii Dolgov <9erthalion6@gmail.com> Cc: Fangrui Song Cc: German Gomez Cc: Ingo Molnar Cc: Ivan Babrou Cc: James Clark Cc: Jing Zhang Cc: Jiri Olsa Cc: John Garry Cc: K Prateek Nayak Cc: Kan Liang Cc: Leo Yan Cc: Liam Howlett Cc: Mark Rutland Cc: Miguel Ojeda Cc: Mike Leach Cc: Namhyung Kim Cc: Naveen N. Rao Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Sean Christopherson Cc: Steinar H. Gunderson Cc: Suzuki Poulouse Cc: Wenyu Liu Cc: Will Deacon Cc: Yang Jihong Cc: Ye Xingchen Cc: Yuan Can Link: https://lore.kernel.org/lkml/ZIcoJytUEz4UgQYR@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-diff.c | 4 ++-- tools/perf/util/block-info.c | 4 ++-- tools/perf/util/hist.c | 2 +- tools/perf/util/map.c | 2 +- tools/perf/util/sort.c | 2 +- tools/perf/util/srcline.c | 4 +++- tools/perf/util/srcline.h | 3 ++- 7 files changed, 12 insertions(+), 9 deletions(-) diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c index eec8956..e8a1b16 100644 --- a/tools/perf/builtin-diff.c +++ b/tools/perf/builtin-diff.c @@ -1378,8 +1378,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 ((strncmp(start_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0) && - (strncmp(end_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0)) { + if (start_line != SRCLINE_UNKNOWN && + end_line != SRCLINE_UNKNOWN) { 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 08279b1..591fc1e 100644 --- a/tools/perf/util/block-info.c +++ b/tools/perf/util/block-info.c @@ -296,8 +296,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 ((strncmp(start_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0) && - (strncmp(end_line, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0)) { + if (start_line != SRCLINE_UNKNOWN && + end_line != SRCLINE_UNKNOWN) { scnprintf(buf, sizeof(buf), "[%s -> %s]", start_line, end_line); } else { diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index 0a10bcc..3dc8a49 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -484,7 +484,7 @@ static int hist_entry__init(struct hist_entry *he, goto err_infos; } - if (he->srcline && strcmp(he->srcline, SRCLINE_UNKNOWN)) { + if (he->srcline && he->srcline != SRCLINE_UNKNOWN) { he->srcline = strdup(he->srcline); if (he->srcline == NULL) goto err_rawdata; diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c index c77e2fc..f30d349 100644 --- a/tools/perf/util/map.c +++ b/tools/perf/util/map.c @@ -496,7 +496,7 @@ int map__fprintf_srcline(struct map *map, u64 addr, const char *prefix, if (dso) { char *srcline = map__srcline(map, addr, NULL); - if (strncmp(srcline, SRCLINE_UNKNOWN, strlen(SRCLINE_UNKNOWN)) != 0) + if (srcline != SRCLINE_UNKNOWN) ret = fprintf(fp, "%s%s", prefix, srcline); zfree_srcline(&srcline); } diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c index 047c360..6aa1c7f 100644 --- a/tools/perf/util/sort.c +++ b/tools/perf/util/sort.c @@ -643,7 +643,7 @@ static char *hist_entry__get_srcfile(struct hist_entry *e) sf = __get_srcline(map__dso(map), map__rip_2objdump(map, e->ip), e->ms.sym, false, true, true, e->ip); - if (!strcmp(sf, SRCLINE_UNKNOWN)) + if (sf == SRCLINE_UNKNOWN) return no_srcfile; p = strchr(sf, ':'); if (p && *sf) { diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c index b8e5965..aec596a 100644 --- a/tools/perf/util/srcline.c +++ b/tools/perf/util/srcline.c @@ -23,6 +23,8 @@ bool srcline_full_filename; +char *srcline__unknown = (char *)"??:0"; + static const char *dso__name(struct dso *dso) { const char *dso_name; @@ -809,7 +811,7 @@ void zfree_srcline(char **srcline) if (*srcline == NULL) return; - if (strcmp(*srcline, SRCLINE_UNKNOWN)) + if (*srcline != SRCLINE_UNKNOWN) free(*srcline); *srcline = NULL; diff --git a/tools/perf/util/srcline.h b/tools/perf/util/srcline.h index a15c7db..167645b 100644 --- a/tools/perf/util/srcline.h +++ b/tools/perf/util/srcline.h @@ -25,7 +25,8 @@ char *srcline__tree_find(struct rb_root_cached *tree, u64 addr); /* delete all srclines within the tree */ void srcline__tree_delete(struct rb_root_cached *tree); -#define SRCLINE_UNKNOWN ((char *) "??:0") +extern char *srcline__unknown; +#define SRCLINE_UNKNOWN srcline__unknown struct inline_list { struct symbol *symbol; -- 2.7.4