All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 1/3] perf report: compile tips.txt in perf binary
@ 2021-05-22  6:20 Denys Zagorui
  2021-05-22  6:20 ` [PATCH v7 2/3] perf tests: avoid storing an absolute path " Denys Zagorui
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Denys Zagorui @ 2021-05-22  6:20 UTC (permalink / raw)
  To: jolsa, linux-kernel, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung

It seems there is some need to have an ability to invoke perf from
build directory without installation
(84cfac7f05e1: perf tools: Set and pass DOCDIR to builtin-report.c)
DOCDIR definition contains an absolute path to kernel source directory.
It is build machine related info and it makes perf binary unreproducible.

This can be avoided by compiling tips.txt in perf directly.

Signed-off-by: Denys Zagorui <dzagorui@cisco.com>
---
 tools/perf/Build               |  2 +-
 tools/perf/Documentation/Build |  9 +++++++++
 tools/perf/builtin-report.c    | 34 +++++++++++++++++++++++++---------
 tools/perf/util/util.c         | 28 ----------------------------
 tools/perf/util/util.h         |  2 --
 5 files changed, 35 insertions(+), 40 deletions(-)
 create mode 100644 tools/perf/Documentation/Build

diff --git a/tools/perf/Build b/tools/perf/Build
index db61dbe2b543..3a2e768d7576 100644
--- a/tools/perf/Build
+++ b/tools/perf/Build
@@ -45,12 +45,12 @@ CFLAGS_perf.o              += -DPERF_HTML_PATH="BUILD_STR($(htmldir_SQ))"	\
 			      -DPREFIX="BUILD_STR($(prefix_SQ))"
 CFLAGS_builtin-trace.o	   += -DSTRACE_GROUPS_DIR="BUILD_STR($(STRACE_GROUPS_DIR_SQ))"
 CFLAGS_builtin-report.o	   += -DTIPDIR="BUILD_STR($(tipdir_SQ))"
-CFLAGS_builtin-report.o	   += -DDOCDIR="BUILD_STR($(srcdir_SQ)/Documentation)"
 
 perf-y += util/
 perf-y += arch/
 perf-y += ui/
 perf-y += scripts/
 perf-$(CONFIG_TRACE) += trace/beauty/
+perf-y += Documentation/
 
 gtk-y += ui/gtk/
diff --git a/tools/perf/Documentation/Build b/tools/perf/Documentation/Build
new file mode 100644
index 000000000000..83e16764caa4
--- /dev/null
+++ b/tools/perf/Documentation/Build
@@ -0,0 +1,9 @@
+perf-y += tips.o
+
+quiet_cmd_ld_tips = LD       $@
+      cmd_ld_tips = $(LD) -r -b binary -o $@ $<
+
+$(OUTPUT)Documentation/tips.o: Documentation/tips.txt FORCE
+	$(call rule_mkdir)
+	$(call if_changed,ld_tips)
+
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 36f9ccfeb38a..4f2c7ee8fea1 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -47,7 +47,6 @@
 #include "util/time-utils.h"
 #include "util/auxtrace.h"
 #include "util/units.h"
-#include "util/util.h" // perf_tip()
 #include "ui/ui.h"
 #include "ui/progress.h"
 #include "util/block-info.h"
@@ -109,6 +108,9 @@ struct report {
 	int			nr_block_reports;
 };
 
+extern char _binary_Documentation_tips_txt_start[];
+extern char _binary_Documentation_tips_txt_end[];
+
 static int report__config(const char *var, const char *value, void *cb)
 {
 	struct report *rep = cb;
@@ -614,19 +616,33 @@ static int report__gtk_browse_hists(struct report *rep, const char *help)
 	return hist_browser(rep->session->evlist, help, NULL, rep->min_percent);
 }
 
+static const char *perf_tip(void)
+{
+	char *start = _binary_Documentation_tips_txt_start;
+	char *tok, *tmp, *prev;
+	int pick, size;
+
+	size = _binary_Documentation_tips_txt_end - start;
+	pick = random() % size;
+
+	_binary_Documentation_tips_txt_start[size - 1] = 0;
+
+	for (tok = strtok_r(start, "\n", &tmp); tok;
+	     tok = strtok_r(NULL, "\n", &tmp)) {
+		if (pick < (tok - start))
+			return prev;
+		prev = tok;
+	}
+
+	return prev;
+}
+
 static int report__browse_hists(struct report *rep)
 {
 	int ret;
 	struct perf_session *session = rep->session;
 	struct evlist *evlist = session->evlist;
-	const char *help = perf_tip(system_path(TIPDIR));
-
-	if (help == NULL) {
-		/* fallback for people who don't install perf ;-) */
-		help = perf_tip(DOCDIR);
-		if (help == NULL)
-			help = "Cannot load tips.txt file, please install perf!";
-	}
+	const char *help = perf_tip();
 
 	switch (use_browser) {
 	case 1:
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 37a9492edb3e..3bba74e431ed 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -379,34 +379,6 @@ fetch_kernel_version(unsigned int *puint, char *str,
 	return 0;
 }
 
-const char *perf_tip(const char *dirpath)
-{
-	struct strlist *tips;
-	struct str_node *node;
-	char *tip = NULL;
-	struct strlist_config conf = {
-		.dirname = dirpath,
-		.file_only = true,
-	};
-
-	tips = strlist__new("tips.txt", &conf);
-	if (tips == NULL)
-		return errno == ENOENT ? NULL :
-			"Tip: check path of tips.txt or get more memory! ;-p";
-
-	if (strlist__nr_entries(tips) == 0)
-		goto out;
-
-	node = strlist__entry(tips, random() % strlist__nr_entries(tips));
-	if (asprintf(&tip, "Tip: %s", node->s) < 0)
-		tip = (char *)"Tip: get more memory! ;-)";
-
-out:
-	strlist__delete(tips);
-
-	return tip;
-}
-
 char *perf_exe(char *buf, int len)
 {
 	int n = readlink("/proc/self/exe", buf, len);
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index ad737052e597..80b194ee6c7d 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -39,8 +39,6 @@ int fetch_kernel_version(unsigned int *puint,
 #define KVER_FMT	"%d.%d.%d"
 #define KVER_PARAM(x)	KVER_VERSION(x), KVER_PATCHLEVEL(x), KVER_SUBLEVEL(x)
 
-const char *perf_tip(const char *dirpath);
-
 #ifndef HAVE_SCHED_GETCPU_SUPPORT
 int sched_getcpu(void);
 #endif
-- 
2.26.2.Cisco


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v7 2/3] perf tests: avoid storing an absolute path in perf binary
  2021-05-22  6:20 [PATCH v7 1/3] perf report: compile tips.txt in perf binary Denys Zagorui
@ 2021-05-22  6:20 ` Denys Zagorui
  2021-05-23 16:30   ` Jiri Olsa
  2021-05-22  6:20 ` [PATCH v7 3/3] perf parse-events: add bison --file-prefix-map option Denys Zagorui
  2021-05-26 14:07 ` [PATCH v7 1/3] perf report: compile tips.txt in perf binary Arnaldo Carvalho de Melo
  2 siblings, 1 reply; 7+ messages in thread
From: Denys Zagorui @ 2021-05-22  6:20 UTC (permalink / raw)
  To: jolsa, linux-kernel, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung

python binding test uses PYTHONPATH definition to find python/perf.so
library. This definition is an absolute path that makes perf binary
unreproducible. This path can be found during runtime execution.

Signed-off-by: Denys Zagorui <dzagorui@cisco.com>
---
 tools/perf/tests/Build        |  2 +-
 tools/perf/tests/python-use.c | 32 ++++++++++++++++++++++++++++++--
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 650aec19d490..a20098dcdbc4 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -98,5 +98,5 @@ perf-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
 endif
 
 CFLAGS_attr.o         += -DBINDIR="BUILD_STR($(bindir_SQ))" -DPYTHON="BUILD_STR($(PYTHON_WORD))"
-CFLAGS_python-use.o   += -DPYTHONPATH="BUILD_STR($(OUTPUT)python)" -DPYTHON="BUILD_STR($(PYTHON_WORD))"
+CFLAGS_python-use.o   += -DPYTHON="BUILD_STR($(PYTHON_WORD))"
 CFLAGS_dwarf-unwind.o += -fno-optimize-sibling-calls
diff --git a/tools/perf/tests/python-use.c b/tools/perf/tests/python-use.c
index 98c6d474aa6f..e196faf1140d 100644
--- a/tools/perf/tests/python-use.c
+++ b/tools/perf/tests/python-use.c
@@ -8,18 +8,46 @@
 #include <linux/compiler.h>
 #include "tests.h"
 #include "util/debug.h"
+#include "util/util.h"
+#include <sys/stat.h>
+#include <limits.h>
+#include <libgen.h>
 
 int test__python_use(struct test *test __maybe_unused, int subtest __maybe_unused)
 {
 	char *cmd;
 	int ret;
+	char *exec_path;
+	char *buf;
+	char *pythonpath = NULL;
+	struct stat sb;
 
-	if (asprintf(&cmd, "echo \"import sys ; sys.path.append('%s'); import perf\" | %s %s",
-		     PYTHONPATH, PYTHON, verbose > 0 ? "" : "2> /dev/null") < 0)
+	buf = malloc(PATH_MAX);
+	if (buf == NULL)
 		return -1;
 
+	perf_exe(buf, PATH_MAX);
+	exec_path = dirname(buf);
+
+	if (asprintf(&pythonpath, "%s/python", exec_path) < 0) {
+		ret = -1;
+		goto out;
+	}
+
+	if (stat(pythonpath, &sb) || !S_ISDIR(sb.st_mode))
+		pythonpath[0] = 0;
+
+	if (asprintf(&cmd, "echo \"import sys ; sys.path.append('%s'); import perf\" | %s %s",
+		     pythonpath, PYTHON, verbose > 0 ? "" : "2> /dev/null") < 0) {
+		ret = -1;
+		goto out;
+	}
+
 	pr_debug("python usage test: \"%s\"\n", cmd);
 	ret = system(cmd) ? -1 : 0;
 	free(cmd);
+out:
+	free(buf);
+	free(pythonpath);
 	return ret;
 }
-- 
2.26.2.Cisco


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v7 3/3] perf parse-events: add bison --file-prefix-map option
  2021-05-22  6:20 [PATCH v7 1/3] perf report: compile tips.txt in perf binary Denys Zagorui
  2021-05-22  6:20 ` [PATCH v7 2/3] perf tests: avoid storing an absolute path " Denys Zagorui
@ 2021-05-22  6:20 ` Denys Zagorui
  2021-05-26 14:07 ` [PATCH v7 1/3] perf report: compile tips.txt in perf binary Arnaldo Carvalho de Melo
  2 siblings, 0 replies; 7+ messages in thread
From: Denys Zagorui @ 2021-05-22  6:20 UTC (permalink / raw)
  To: jolsa, linux-kernel, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung

During perf build with O= bison stores full paths in generated files and
those paths are stored in resulting perf binary. Starting from v3.7.1.
those paths can be remapped by using --file-prefix-map option. Use this
option if it possible to make perf binary more reproducible.

Signed-off-by: Denys Zagorui <dzagorui@cisco.com>
---
 tools/perf/Makefile.config | 9 +++++++++
 tools/perf/util/Build      | 6 +++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 406a9519145e..dacd16874d3d 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -202,6 +202,12 @@ ifeq ($(call get-executable,$(BISON)),)
   dummy := $(error Error: $(BISON) is missing on this system, please install it)
 endif
 
+ifneq ($(OUTPUT),)
+  ifeq ($(shell expr $(shell $(BISON) --version | grep bison | sed -e 's/.\+ \([0-9]\+\).\([0-9]\+\).\([0-9]\+\)/\1\2\3/g') \>\= 371), 1)
+    BISON_FILE_PREFIX_MAP := --file-prefix-map=$(OUTPUT)=
+  endif
+endif
+
 # Treat warnings as errors unless directed not to
 ifneq ($(WERROR),0)
   CORE_CFLAGS += -Werror
@@ -1228,6 +1234,9 @@ $(call detected_var,LIBDIR)
 $(call detected_var,GTK_CFLAGS)
 $(call detected_var,PERL_EMBED_CCOPTS)
 $(call detected_var,PYTHON_EMBED_CCOPTS)
+ifneq ($(BISON_FILE_PREFIX_MAP),)
+$(call detected_var,BISON_FILE_PREFIX_MAP)
+endif
 
 # re-generate FEATURE-DUMP as we may have called feature_check, found out
 # extra libraries to add to LDFLAGS of some other test and then redo those
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index b64bdc1a7026..95e15d1035ab 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -216,7 +216,7 @@ $(OUTPUT)util/parse-events-flex.c $(OUTPUT)util/parse-events-flex.h: util/parse-
 
 $(OUTPUT)util/parse-events-bison.c $(OUTPUT)util/parse-events-bison.h: util/parse-events.y
 	$(call rule_mkdir)
-	$(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) \
+	$(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) $(BISON_FILE_PREFIX_MAP) \
 		-o $(OUTPUT)util/parse-events-bison.c -p parse_events_
 
 $(OUTPUT)util/expr-flex.c $(OUTPUT)util/expr-flex.h: util/expr.l $(OUTPUT)util/expr-bison.c
@@ -226,7 +226,7 @@ $(OUTPUT)util/expr-flex.c $(OUTPUT)util/expr-flex.h: util/expr.l $(OUTPUT)util/e
 
 $(OUTPUT)util/expr-bison.c $(OUTPUT)util/expr-bison.h: util/expr.y
 	$(call rule_mkdir)
-	$(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) \
+	$(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) $(BISON_FILE_PREFIX_MAP) \
 		-o $(OUTPUT)util/expr-bison.c -p expr_
 
 $(OUTPUT)util/pmu-flex.c $(OUTPUT)util/pmu-flex.h: util/pmu.l $(OUTPUT)util/pmu-bison.c
@@ -236,7 +236,7 @@ $(OUTPUT)util/pmu-flex.c $(OUTPUT)util/pmu-flex.h: util/pmu.l $(OUTPUT)util/pmu-
 
 $(OUTPUT)util/pmu-bison.c $(OUTPUT)util/pmu-bison.h: util/pmu.y
 	$(call rule_mkdir)
-	$(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) \
+	$(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) $(BISON_FILE_PREFIX_MAP) \
 		-o $(OUTPUT)util/pmu-bison.c -p perf_pmu_
 
 FLEX_GE_26 := $(shell expr $(shell $(FLEX) --version | sed -e  's/flex \([0-9]\+\).\([0-9]\+\)/\1\2/g') \>\= 26)
-- 
2.26.2.Cisco


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 2/3] perf tests: avoid storing an absolute path in perf binary
  2021-05-22  6:20 ` [PATCH v7 2/3] perf tests: avoid storing an absolute path " Denys Zagorui
@ 2021-05-23 16:30   ` Jiri Olsa
  0 siblings, 0 replies; 7+ messages in thread
From: Jiri Olsa @ 2021-05-23 16:30 UTC (permalink / raw)
  To: Denys Zagorui
  Cc: linux-kernel, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung

On Fri, May 21, 2021 at 11:20:15PM -0700, Denys Zagorui wrote:
> python binding test uses PYTHONPATH definition to find python/perf.so
> library. This definition is an absolute path that makes perf binary
> unreproducible. This path can be found during runtime execution.
> 
> Signed-off-by: Denys Zagorui <dzagorui@cisco.com>
> ---
>  tools/perf/tests/Build        |  2 +-
>  tools/perf/tests/python-use.c | 32 ++++++++++++++++++++++++++++++--
>  2 files changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> index 650aec19d490..a20098dcdbc4 100644
> --- a/tools/perf/tests/Build
> +++ b/tools/perf/tests/Build
> @@ -98,5 +98,5 @@ perf-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
>  endif
>  
>  CFLAGS_attr.o         += -DBINDIR="BUILD_STR($(bindir_SQ))" -DPYTHON="BUILD_STR($(PYTHON_WORD))"
> -CFLAGS_python-use.o   += -DPYTHONPATH="BUILD_STR($(OUTPUT)python)" -DPYTHON="BUILD_STR($(PYTHON_WORD))"
> +CFLAGS_python-use.o   += -DPYTHON="BUILD_STR($(PYTHON_WORD))"
>  CFLAGS_dwarf-unwind.o += -fno-optimize-sibling-calls
> diff --git a/tools/perf/tests/python-use.c b/tools/perf/tests/python-use.c
> index 98c6d474aa6f..e196faf1140d 100644
> --- a/tools/perf/tests/python-use.c
> +++ b/tools/perf/tests/python-use.c
> @@ -8,18 +8,46 @@
>  #include <linux/compiler.h>
>  #include "tests.h"
>  #include "util/debug.h"
> +#include "util/util.h"
> +#include <sys/stat.h>
> +#include <limits.h>
> +#include <libgen.h>

sry, did not notice there was new version.. looks good, 2 nits below

thanks,
jirka


>  
>  int test__python_use(struct test *test __maybe_unused, int subtest __maybe_unused)
>  {
>  	char *cmd;
>  	int ret;
> +	char *exec_path;
> +	char *buf;
> +	char *pythonpath = NULL;
> +	struct stat sb;
>  
> -	if (asprintf(&cmd, "echo \"import sys ; sys.path.append('%s'); import perf\" | %s %s",
> -		     PYTHONPATH, PYTHON, verbose > 0 ? "" : "2> /dev/null") < 0)
> +	buf = malloc(PATH_MAX);

why bother with dynamic allocation here, we normaly do that on stack

> +	if (buf == NULL)
>  		return -1;
>  
> +	perf_exe(buf, PATH_MAX);
> +	exec_path = dirname(buf);
> +
> +	if (asprintf(&pythonpath, "%s/python", exec_path) < 0) {
> +		ret = -1;

you could initialize ret with -1

> +		goto out;
> +	}
> +
> +	if (stat(pythonpath, &sb) || !S_ISDIR(sb.st_mode))
> +		pythonpath[0] = 0;
> +
> +	if (asprintf(&cmd, "echo \"import sys ; sys.path.append('%s'); import perf\" | %s %s",
> +		     pythonpath, PYTHON, verbose > 0 ? "" : "2> /dev/null") < 0) {
> +		ret = -1;
> +		goto out;
> +	}
> +
>  	pr_debug("python usage test: \"%s\"\n", cmd);
>  	ret = system(cmd) ? -1 : 0;
>  	free(cmd);
> +out:
> +	free(buf);
> +	free(pythonpath);
>  	return ret;
>  }
> -- 
> 2.26.2.Cisco
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 1/3] perf report: compile tips.txt in perf binary
  2021-05-22  6:20 [PATCH v7 1/3] perf report: compile tips.txt in perf binary Denys Zagorui
  2021-05-22  6:20 ` [PATCH v7 2/3] perf tests: avoid storing an absolute path " Denys Zagorui
  2021-05-22  6:20 ` [PATCH v7 3/3] perf parse-events: add bison --file-prefix-map option Denys Zagorui
@ 2021-05-26 14:07 ` Arnaldo Carvalho de Melo
  2021-05-26 14:50   ` Denys Zagorui -X (dzagorui - GLOBALLOGIC INC at Cisco)
  2 siblings, 1 reply; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-05-26 14:07 UTC (permalink / raw)
  To: Denys Zagorui
  Cc: jolsa, linux-kernel, peterz, mingo, mark.rutland,
	alexander.shishkin, namhyung

Em Fri, May 21, 2021 at 11:20:14PM -0700, Denys Zagorui escreveu:
> It seems there is some need to have an ability to invoke perf from
> build directory without installation
> (84cfac7f05e1: perf tools: Set and pass DOCDIR to builtin-report.c)
> DOCDIR definition contains an absolute path to kernel source directory.
> It is build machine related info and it makes perf binary unreproducible.
> 
> This can be avoided by compiling tips.txt in perf directly.
> 
> Signed-off-by: Denys Zagorui <dzagorui@cisco.com>
> ---
>  tools/perf/Build               |  2 +-
>  tools/perf/Documentation/Build |  9 +++++++++
>  tools/perf/builtin-report.c    | 34 +++++++++++++++++++++++++---------
>  tools/perf/util/util.c         | 28 ----------------------------
>  tools/perf/util/util.h         |  2 --
>  5 files changed, 35 insertions(+), 40 deletions(-)
>  create mode 100644 tools/perf/Documentation/Build
> 
> diff --git a/tools/perf/Build b/tools/perf/Build
> index db61dbe2b543..3a2e768d7576 100644
> --- a/tools/perf/Build
> +++ b/tools/perf/Build
> @@ -45,12 +45,12 @@ CFLAGS_perf.o              += -DPERF_HTML_PATH="BUILD_STR($(htmldir_SQ))"	\
>  			      -DPREFIX="BUILD_STR($(prefix_SQ))"
>  CFLAGS_builtin-trace.o	   += -DSTRACE_GROUPS_DIR="BUILD_STR($(STRACE_GROUPS_DIR_SQ))"
>  CFLAGS_builtin-report.o	   += -DTIPDIR="BUILD_STR($(tipdir_SQ))"
> -CFLAGS_builtin-report.o	   += -DDOCDIR="BUILD_STR($(srcdir_SQ)/Documentation)"
>  
>  perf-y += util/
>  perf-y += arch/
>  perf-y += ui/
>  perf-y += scripts/
>  perf-$(CONFIG_TRACE) += trace/beauty/
> +perf-y += Documentation/
>  
>  gtk-y += ui/gtk/
> diff --git a/tools/perf/Documentation/Build b/tools/perf/Documentation/Build
> new file mode 100644
> index 000000000000..83e16764caa4
> --- /dev/null
> +++ b/tools/perf/Documentation/Build
> @@ -0,0 +1,9 @@
> +perf-y += tips.o
> +
> +quiet_cmd_ld_tips = LD       $@
> +      cmd_ld_tips = $(LD) -r -b binary -o $@ $<
> +
> +$(OUTPUT)Documentation/tips.o: Documentation/tips.txt FORCE
> +	$(call rule_mkdir)
> +	$(call if_changed,ld_tips)
> +
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 36f9ccfeb38a..4f2c7ee8fea1 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -47,7 +47,6 @@
>  #include "util/time-utils.h"
>  #include "util/auxtrace.h"
>  #include "util/units.h"
> -#include "util/util.h" // perf_tip()
>  #include "ui/ui.h"
>  #include "ui/progress.h"
>  #include "util/block-info.h"
> @@ -109,6 +108,9 @@ struct report {
>  	int			nr_block_reports;
>  };
>  
> +extern char _binary_Documentation_tips_txt_start[];
> +extern char _binary_Documentation_tips_txt_end[];
> +
>  static int report__config(const char *var, const char *value, void *cb)
>  {
>  	struct report *rep = cb;
> @@ -614,19 +616,33 @@ static int report__gtk_browse_hists(struct report *rep, const char *help)
>  	return hist_browser(rep->session->evlist, help, NULL, rep->min_percent);
>  }
>  
> +static const char *perf_tip(void)
> +{
> +	char *start = _binary_Documentation_tips_txt_start;
> +	char *tok, *tmp, *prev;
> +	int pick, size;
> +
> +	size = _binary_Documentation_tips_txt_end - start;
> +	pick = random() % size;
> +
> +	_binary_Documentation_tips_txt_start[size - 1] = 0;
> +
> +	for (tok = strtok_r(start, "\n", &tmp); tok;
> +	     tok = strtok_r(NULL, "\n", &tmp)) {
> +		if (pick < (tok - start))
> +			return prev;
> +		prev = tok;
> +	}
> +
> +	return prev;
> +}


[perfbuilder@five ~]$ export PERF_TARBALL=http://192.168.100.2/perf/perf-5.13.0-rc3.tar.xz
[perfbuilder@five ~]$ time dm
Wed May 26 11:04:00 AM -03 2021
# export PERF_TARBALL=http://192.168.100.2/perf/perf-5.13.0-rc3.tar.xz
# dm
   1     9.39 alpine:3.4                    : FAIL gcc version 5.3.0 (Alpine 5.3.0)
    builtin-report.c: In function 'cmd_report':
    builtin-report.c:560:3: error: 'prev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
       fprintf(stdout, "#\n# (%s)\n#\n", help);
       ^
    builtin-report.c:622:20: note: 'prev' was declared here
      char *tok, *tmp, *prev;
<SNIP>

  10    13.35 alpine:3.13                   : FAIL gcc version 10.2.1 20201203 (Alpine 10.2.1_pre1)
    builtin-report.c: In function 'cmd_report':
    builtin-report.c:560:3: error: 'prev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
      560 |   fprintf(stdout, "#\n# (%s)\n#\n", help);
          |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    builtin-report.c:622:20: note: 'prev' was declared here
      622 |  char *tok, *tmp, *prev;
          |                    ^~~~
    cc1: all warnings being treated as errors
  11    13.77 alpine:edge                   : FAIL gcc version 10.3.1 20210424 (Alpine 10.3.1_git20210424)
    builtin-report.c: In function 'cmd_report':
    builtin-report.c:560:3: error: 'prev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
      560 |   fprintf(stdout, "#\n# (%s)\n#\n", help);
          |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    builtin-report.c:622:20: note: 'prev' was declared here
      622 |  char *tok, *tmp, *prev;
          |                    ^~~~
    cc1: all warnings being treated as errors


I'll check later...

>  static int report__browse_hists(struct report *rep)
>  {
>  	int ret;
>  	struct perf_session *session = rep->session;
>  	struct evlist *evlist = session->evlist;
> -	const char *help = perf_tip(system_path(TIPDIR));
> -
> -	if (help == NULL) {
> -		/* fallback for people who don't install perf ;-) */
> -		help = perf_tip(DOCDIR);
> -		if (help == NULL)
> -			help = "Cannot load tips.txt file, please install perf!";
> -	}
> +	const char *help = perf_tip();
>  
>  	switch (use_browser) {
>  	case 1:
> diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
> index 37a9492edb3e..3bba74e431ed 100644
> --- a/tools/perf/util/util.c
> +++ b/tools/perf/util/util.c
> @@ -379,34 +379,6 @@ fetch_kernel_version(unsigned int *puint, char *str,
>  	return 0;
>  }
>  
> -const char *perf_tip(const char *dirpath)
> -{
> -	struct strlist *tips;
> -	struct str_node *node;
> -	char *tip = NULL;
> -	struct strlist_config conf = {
> -		.dirname = dirpath,
> -		.file_only = true,
> -	};
> -
> -	tips = strlist__new("tips.txt", &conf);
> -	if (tips == NULL)
> -		return errno == ENOENT ? NULL :
> -			"Tip: check path of tips.txt or get more memory! ;-p";
> -
> -	if (strlist__nr_entries(tips) == 0)
> -		goto out;
> -
> -	node = strlist__entry(tips, random() % strlist__nr_entries(tips));
> -	if (asprintf(&tip, "Tip: %s", node->s) < 0)
> -		tip = (char *)"Tip: get more memory! ;-)";
> -
> -out:
> -	strlist__delete(tips);
> -
> -	return tip;
> -}
> -
>  char *perf_exe(char *buf, int len)
>  {
>  	int n = readlink("/proc/self/exe", buf, len);
> diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
> index ad737052e597..80b194ee6c7d 100644
> --- a/tools/perf/util/util.h
> +++ b/tools/perf/util/util.h
> @@ -39,8 +39,6 @@ int fetch_kernel_version(unsigned int *puint,
>  #define KVER_FMT	"%d.%d.%d"
>  #define KVER_PARAM(x)	KVER_VERSION(x), KVER_PATCHLEVEL(x), KVER_SUBLEVEL(x)
>  
> -const char *perf_tip(const char *dirpath);
> -
>  #ifndef HAVE_SCHED_GETCPU_SUPPORT
>  int sched_getcpu(void);
>  #endif
> -- 
> 2.26.2.Cisco
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 1/3] perf report: compile tips.txt in perf binary
  2021-05-26 14:07 ` [PATCH v7 1/3] perf report: compile tips.txt in perf binary Arnaldo Carvalho de Melo
@ 2021-05-26 14:50   ` Denys Zagorui -X (dzagorui - GLOBALLOGIC INC at Cisco)
  2021-05-26 15:44     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 7+ messages in thread
From: Denys Zagorui -X (dzagorui - GLOBALLOGIC INC at Cisco) @ 2021-05-26 14:50 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: jolsa, linux-kernel, peterz, mingo, mark.rutland,
	alexander.shishkin, namhyung

> [perfbuilder@five ~]$ export PERF_TARBALL=http://192.168.100.2/perf/perf-5.13.0-rc3.tar.xz
> [perfbuilder@five ~]$ time dm
> Wed May 26 11:04:00 AM -03 2021
> # export PERF_TARBALL=http://192.168.100.2/perf/perf-5.13.0-rc3.tar.xz
> # dm
>   1     9.39 alpine:3.4                    : FAIL gcc version 5.3.0 (Alpine 5.3.0)
>    builtin-report.c: In function 'cmd_report':
>    builtin-report.c:560:3: error: 'prev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>       fprintf(stdout, "#\n# (%s)\n#\n", help);
>       ^
>    builtin-report.c:622:20: note: 'prev' was declared here
>      char *tok, *tmp, *prev;
> <SNIP>
>
>  10    13.35 alpine:3.13                   : FAIL gcc version 10.2.1 20201203 (Alpine 10.2.1_pre1)
>    builtin-report.c: In function 'cmd_report':
>    builtin-report.c:560:3: error: 'prev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>      560 |   fprintf(stdout, "#\n# (%s)\n#\n", help);
>          |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    builtin-report.c:622:20: note: 'prev' was declared here
>      622 |  char *tok, *tmp, *prev;
>          |                    ^~~~
>    cc1: all warnings being treated as errors
>  11    13.77 alpine:edge                   : FAIL gcc version 10.3.1 20210424 (Alpine 10.3.1_git20210424)
>    builtin-report.c: In function 'cmd_report':
>    builtin-report.c:560:3: error: 'prev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>      560 |   fprintf(stdout, "#\n# (%s)\n#\n", help);
>          |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    builtin-report.c:622:20: note: 'prev' was declared here
>      622 |  char *tok, *tmp, *prev;
>          |                    ^~~~
>    cc1: all warnings being treated as errors
>
>
> I'll check later...

prev should be initialized to _binary_Documentation_tips_txt_start, i can resend 


Thanks,
Denys

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 1/3] perf report: compile tips.txt in perf binary
  2021-05-26 14:50   ` Denys Zagorui -X (dzagorui - GLOBALLOGIC INC at Cisco)
@ 2021-05-26 15:44     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-05-26 15:44 UTC (permalink / raw)
  To: Denys Zagorui -X (dzagorui - GLOBALLOGIC INC at Cisco)
  Cc: jolsa, linux-kernel, peterz, mingo, mark.rutland,
	alexander.shishkin, namhyung

Em Wed, May 26, 2021 at 02:50:24PM +0000, Denys Zagorui -X (dzagorui - GLOBALLOGIC INC at Cisco) escreveu:
> > [perfbuilder@five ~]$ export PERF_TARBALL=http://192.168.100.2/perf/perf-5.13.0-rc3.tar.xz
> > [perfbuilder@five ~]$ time dm
> > Wed May 26 11:04:00 AM -03 2021
> > # export PERF_TARBALL=http://192.168.100.2/perf/perf-5.13.0-rc3.tar.xz
> > # dm
> >   1     9.39 alpine:3.4                    : FAIL gcc version 5.3.0 (Alpine 5.3.0)
> >    builtin-report.c: In function 'cmd_report':
> >    builtin-report.c:560:3: error: 'prev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >       fprintf(stdout, "#\n# (%s)\n#\n", help);
> >       ^
> >    builtin-report.c:622:20: note: 'prev' was declared here
> >      char *tok, *tmp, *prev;
> > <SNIP>
> >
> >  10    13.35 alpine:3.13                   : FAIL gcc version 10.2.1 20201203 (Alpine 10.2.1_pre1)
> >    builtin-report.c: In function 'cmd_report':
> >    builtin-report.c:560:3: error: 'prev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >      560 |   fprintf(stdout, "#\n# (%s)\n#\n", help);
> >          |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    builtin-report.c:622:20: note: 'prev' was declared here
> >      622 |  char *tok, *tmp, *prev;
> >          |                    ^~~~
> >    cc1: all warnings being treated as errors
> >  11    13.77 alpine:edge                   : FAIL gcc version 10.3.1 20210424 (Alpine 10.3.1_git20210424)
> >    builtin-report.c: In function 'cmd_report':
> >    builtin-report.c:560:3: error: 'prev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >      560 |   fprintf(stdout, "#\n# (%s)\n#\n", help);
> >          |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    builtin-report.c:622:20: note: 'prev' was declared here
> >      622 |  char *tok, *tmp, *prev;
> >          |                    ^~~~
> >    cc1: all warnings being treated as errors
> >
> >
> > I'll check later...
> 
> prev should be initialized to _binary_Documentation_tips_txt_start, i can resend 


More problems with that patch, so I'm removing it for now, till we have
these addressed.

- Arnaldo

When cross-building to Risc-V 64-bit:

  LD       /tmp/build/perf/Documentation/tips.o
riscv64-linux-gnu-ld: --relax and -r may not be used together
Documentation/Build:7: recipe for target '/tmp/build/perf/Documentation/tips.o' failed
make[4]: *** [/tmp/build/perf/Documentation/tips.o] Error 1
/git/perf-5.13.0-rc3/tools/build/Makefile.build:139: recipe for target 'Documentation' failed
make[3]: *** [Documentation] Error 2
make[3]: *** Waiting for unfinished jobs....

Ditto for m68k

  LD       /tmp/build/perf/Documentation/tips.o
m68k-linux-gnu-ld: failed to merge target specific data of file Documentation/tips.txt
Documentation/Build:7: recipe for target '/tmp/build/perf/Documentation/tips.o' failed
make[4]: *** [/tmp/build/perf/Documentation/tips.o] Error 1
/git/perf-5.13.0-rc3/tools/build/Makefile.build:139: recipe for target 'Documentation' failed
make[3]: *** [Documentation] Error 2
make[3]: *** Waiting for unfinished jobs....



That same problem, for other targets:

  42     7.55 fedora:34-x-ARC-uClibc        : FAIL gcc version 8.3.1 20190225 (ARCv2 ISA Linux uClibc toolchain 2019.03-rc1)
    builtin-report.c: In function 'cmd_report':
    builtin-report.c:560:3: error: 'prev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
       fprintf(stdout, "#\n# (%s)\n#\n", help);
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    builtin-report.c:622:20: note: 'prev' was declared here
      char *tok, *tmp, *prev;
                        ^~~~

  19     9.46 debian:8                      : FAIL gcc version 4.9.2 (Debian 4.9.2-10+deb8u2)
    builtin-report.c: In function 'cmd_report':
    builtin-report.c:654:7: error: 'prev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
       ret = evlist__tui_browse_hists(evlist, help, NULL, rep->min_percent,
           ^
    builtin-report.c:622:20: note: 'prev' was declared here
      char *tok, *tmp, *prev;
                        ^
    cc1: all warnings being treated as errors

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-05-26 15:44 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-22  6:20 [PATCH v7 1/3] perf report: compile tips.txt in perf binary Denys Zagorui
2021-05-22  6:20 ` [PATCH v7 2/3] perf tests: avoid storing an absolute path " Denys Zagorui
2021-05-23 16:30   ` Jiri Olsa
2021-05-22  6:20 ` [PATCH v7 3/3] perf parse-events: add bison --file-prefix-map option Denys Zagorui
2021-05-26 14:07 ` [PATCH v7 1/3] perf report: compile tips.txt in perf binary Arnaldo Carvalho de Melo
2021-05-26 14:50   ` Denys Zagorui -X (dzagorui - GLOBALLOGIC INC at Cisco)
2021-05-26 15:44     ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.