perf/urgent fixes

hi Ingo,
please consider pulling

thanks,
jirka

The following changes since commit 9f7ff8931e3c5ddc8535476971ec9501e9555c05:

perf/x86: Fix RAPL rdmsrl_safe() usage (2014-04-24 08:12:41 +0200)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git tags/perf-urgent-for-mingo

for you to fetch changes up to 9a126728165eef9b1e9349fe57b9747ac6c6f7e3:

perf tests x86: Fix stack map lookup in dwarf unwind test (2014-04-30 17:02:31 +0200)

----------------------------------------------------------------
perf/urgent fixes:

. Fix perf to use non-executable stack, again (Mathias Krause)

. Remove extra '/' character in events file path (Xia Kaixu)

. Search for modules in %s/lib/modules/%s (Richard Yao)

. Build related fixies plus static build test (Jiri Olsa)

. Fix stack map lookup in dwarf unwind test (Jiri Olsa)

----------------------------------------------------------------
Jiri Olsa (4):
perf tools: Use LDFLAGS instead of ALL_LDFLAGS perf tools: Fix bfd dependency libraries detection perf tests: Add static build make test
perf tests x86: Fix stack map lookup in dwarf unwind test

Mathias Krause (1):
perf x86: Fix perf to use non-executable stack, again

Richard Yao (1):
perf machine: Search for modules in %s/lib/modules/%s

Xia Kaixu (1):
perf tools: Remove extra '/' character in events file path

 tools/lib/api/fs/debugfs.c               |  4 ++--
 tools/perf/Makefile.perf                 |  2 +-
 tools/perf/arch/x86/tests/dwarf-unwind.c |  2 +-
 tools/perf/arch/x86/tests/regs_load.S    |  8 ++++++-
 tools/perf/config/Makefile               | 38 +++++++++++++++++++++++---------
 tools/perf/tests/make                    |  2 ++
 tools/perf/util/machine.c                | 16 ++++++++++----
 7 files changed, 52 insertions(+), 20 deletions(-)

We no longer use ALL_LDFLAGS, Replacing with LDFLAGS.

---
 tools/perf/Makefile.perf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index e969233..895edd3 100644
@@ -589,7 +589,7 @@ $(GTK_OBJS): $(OUTPUT)%.o: %.c $(LIB_H)
 	$(QUIET_CC)$(CC) -o $@ -c -fPIC $(CFLAGS) $(GTK_CFLAGS) $<
 
 $(OUTPUT)libperf-gtk.so: $(GTK_OBJS) $(PERFLIBS)
-	$(QUIET_LINK)$(CC) -o $@ -shared $(ALL_LDFLAGS) $(filter %.o,$^) $(GTK_LIBS)
+	$(QUIET_LINK)$(CC) -o $@ -shared $(LDFLAGS) $(filter %.o,$^) $(GTK_LIBS)
 
 $(OUTPUT)builtin-help.o: builtin-help.c $(OUTPUT)common-cmds.h $(OUTPUT)PERF-CFLAGS
 	$(QUIET_CC)$(CC) -o $@ -c $(CFLAGS) \

1.8.3.1

There's false assumption in the library detection code assuming -liberty and -lz are always present once bfd is detected. The fails on Ubuntu (14.04) as reported by Ingo.

Forcing the bdf dependency libraries detection any time bfd library is detected.

---
 tools/perf/config/Makefile | 34 +++++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 11 deletions(-)

index a71fb39..a57d59e 100644
@@ -194,7 +194,10 @@ VF_FEATURE_TESTS =			\
 	stackprotector-all		\
 	timerfd				\
 	libunwind-debug-frame		\
-	bionic
+	bionic				\
+	liberty				\
+	liberty-z			\
+	cplus-demangle
 
 # Set FEATURE_CHECK_(C|LD)FLAGS-all for all CORE_FEATURE_TESTS features.
 # If in the future we need per-feature checks/flags for features not

@@ -512,7 +515,21 @@ else
 endif
 
 ifeq ($(feature-libbfd), 1)
-  EXTLIBS += -lbfd -lz -liberty
+  EXTLIBS += -lbfd
+
+  # call all detections now so we get correct
+  # status in VF output
+  $(call feature_check,liberty)
+  $(call feature_check,liberty-z)
+  $(call feature_check,cplus-demangle)
+
+  ifeq ($(feature-liberty), 1)
+    EXTLIBS += -liberty
+  else
+    ifeq ($(feature-liberty-z), 1)
+      EXTLIBS += -liberty -lz
+    endif
+  endif
 endif
 
 ifdef NO_DEMANGLE

@@ -523,15 +540,10 @@ else
     CFLAGS += -DHAVE_CPLUS_DEMANGLE_SUPPORT
   else
     ifneq ($(feature-libbfd), 1)
-      $(call feature_check,liberty)
-      ifeq ($(feature-liberty), 1)
-        EXTLIBS += -lbfd -liberty
-      else
-        $(call feature_check,liberty-z)
-        ifeq ($(feature-liberty-z), 1)
-          EXTLIBS += -lbfd -liberty -lz
-        else
-          $(call feature_check,cplus-demangle)
+      ifneq ($(feature-liberty), 1)
+        ifneq ($(feature-liberty-z), 1)
+          # we dont have neither HAVE_CPLUS_DEMANGLE_SUPPORT
+          # or any of 'bfd iberty z' trinity
           ifeq ($(feature-cplus-demangle), 1)
             EXTLIBS += -liberty
             CFLAGS += -DHAVE_CPLUS_DEMANGLE_SUPPORT

1.8.3.1

Adding test for building static perf build into the automated suite. Also available via following commands:

  $ make -f tests/make make_static
  - make_static: cd . && make -f Makefile DESTDIR=/tmp/tmp.7u5MlB4njo LDFLAGS=-static
  $ make -f tests/make make_static_O
  - make_static_O: cd . && make -f Makefile O=/tmp/tmp.Ay6r3wEmtX DESTDIR=/tmp/tmp.vK0KQwO0Vi LDFLAGS=-static

---
 tools/perf/tests/make | 2 ++
 1 file changed, 2 insertions(+)

index 5daeae1..2f92d6e 100644
@@ -46,6 +46,7 @@ make_install_man    := install-man
 make_install_html   := install-html
 make_install_info   := install-info
 make_install_pdf    := install-pdf
+make_static         := LDFLAGS=-static
 
 # all the NO_* variable combined
 make_minimal        := NO_LIBPERL=1 NO_LIBPYTHON=1 NO_NEWT=1 NO_GTK2=1

@@ -87,6 +88,7 @@ run += make_install_bin
 # run += make_install_info
 # run += make_install_pdf
 run += make_minimal
+run += make_static
 
 ifneq ($(call has,ctags),)
 run += make_tags

1.8.3.1

From: Richard Yao <[email protected]>

Modules installed outside of the kernel's build system should go into "%s/lib/modules/%s/extra", but at present, perf will only look at them when they are in "%s/lib/modules/%s/kernel". Lets encourage good citizenship by relaxing this requirement to "%s/lib/modules/%s". This way open source modules that are out-of-tree have no incentive to start populating a directory reserved for in-kernel modules and I can stop hex-editing my system's perf binary when profiling OSS out-of-tree modules.

Feedback from Namhyung Kim correctly revealed that the hex-edits that I had been doing meant that perf was also traversing the build and source symlinks in %s/lib/modules/%s. That is undesireable, so we explicitly exclude them from traversal with a minor tweak to the traversal routine.

---
 tools/perf/util/machine.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

index a53cd0b..27c2a5e 100644
@@ -717,7 +717,7 @@ static char *get_kernel_version(const char *root_dir)
 }
 
 static int map_groups__set_modules_path_dir(struct map_groups *mg,
-				const char *dir_name)
+				const char *dir_name, int depth)
 {
 	struct dirent *dent;
 	DIR *dir = opendir(dir_name);

@@ -742,7 +742,15 @@ static int map_groups__set_modules_path_dir(struct map_groups *mg,
 			    !strcmp(dent->d_name, ".."))
 				continue;
 
-			ret = map_groups__set_modules_path_dir(mg, path);
+			/* Do not follow top-level source and build symlinks */
+			if (depth == 0) {
+				if (!strcmp(dent->d_name, "source") ||
+				    !strcmp(dent->d_name, "build"))
+					continue;
+			}
+
+			ret = map_groups__set_modules_path_dir(mg, path,
+							       depth + 1);
 			if (ret < 0)
 				goto out;
 		} else {

@@ -786,11 +794,11 @@ static int machine__set_modules_path(struct machine *machine)
 	if (!version)
 		return -1;
 
-	snprintf(modules_path, sizeof(modules_path), "%s/lib/modules/%s/kernel",
+	snprintf(modules_path, sizeof(modules_path), "%s/lib/modules/%s",
 		 machine->root_dir, version);
 	free(version);
 
-	return map_groups__set_modules_path_dir(&machine->kmaps, modules_path);
+	return map_groups__set_modules_path_dir(&machine->kmaps, modules_path, 0);
 }
 
 static int machine__create_module(void *arg, const char *name, u64 start)

1.8.3.1

From: Xia Kaixu <[email protected]>

The array debugfs_known_mountpoints[] will cause extra '/' character output.
Remove it.

pre:

$ perf probe -l
/sys/kernel/debug//tracing/uprobe_events file does not exist -
please rebuild kernel with CONFIG_UPROBE_EVENTS.

post:

$ perf probe -l
/sys/kernel/debug/tracing/uprobe_events file does not exist -
please rebuild kernel with CONFIG_UPROBE_EVENTS.

---
 tools/lib/api/fs/debugfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 7c43479..a74fba6 100644
@@ -12,8 +12,8 @@
 char debugfs_mountpoint[PATH_MAX + 1] = "/sys/kernel/debug";
 
 static const char * const debugfs_known_mountpoints[] = {
-	"/sys/kernel/debug/",
-	"/debug/",
+	"/sys/kernel/debug",
+	"/debug",
 	0,
 };
 

1.8.3.1

From: Mathias Krause <[email protected]>

arch/x86/tests/regs_load.S is missing the linker note about the stack requirements, therefore making the linker fall back to an executable stack. As this object gets linked against the final perf binary, it'll needlessly end up with an executable stack. Fix this by adding the appropriate linker note.

Also add a global linker flag to prevent future regressions, as suggested by Jiri. This way perf won't get an executable stack even if we fail to add the .GNU-stack linker note to future assembler files. Though, doing so might create regressions the other way around, when (statically) linking against libraries needing an executable stack. But, apparently, regressing in that direction is wanted as it is an indicator of poor code quality -- or just missing linker notes.

---
 tools/perf/arch/x86/tests/regs_load.S | 8 +++++++-
 tools/perf/config/Makefile            | 4 ++++
 2 files changed, 11 insertions(+), 1 deletion(-)

index 99167bf..60875d5 100644
@@ -1,4 +1,3 @@
-
 #include <linux/linkage.h>
 
 #define AX	 0

@@ -90,3 +89,10 @@ ENTRY(perf_regs_load)
 	ret
 ENDPROC(perf_regs_load)
 #endif
+
+/*
+ * We need to provide note.GNU-stack section, saying that we want
+ * NOT executable stack. Otherwise the final linking will assume that
+ * the ELF stack should not be restricted at all and set it RWX.
+ */
+.section .note.GNU-stack,"",@progbits

index a57d59e..802cf54 100644
@@ -117,6 +117,10 @@ CFLAGS += -Wall
 CFLAGS += -Wextra
 CFLAGS += -std=gnu99
 
+# Enforce a non-executable stack, as we may regress (again) in the future by
+# adding assembler files missing the .GNU-stack linker note.
+LDFLAGS += -Wl,-z,noexecstack
+
 EXTLIBS = -lelf -lpthread -lrt -lm -ldl
 
 ifneq ($(OUTPUT),)

1.8.3.1

Previous commit 'perf x86: Fix perf to use non-executable stack, again' moved stack map into MAP__VARIABLE map type again. Fixing the dwarf unwind test stack map lookup appropriately.

---
 tools/perf/arch/x86/tests/dwarf-unwind.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index b8c0102..83bc238 100644
@@ -23,7 +23,7 @@ static int sample_ustack(struct perf_sample *sample,
 
 	sp = (unsigned long) regs[PERF_REG_X86_SP];
 
-	map = map_groups__find(&thread->mg, MAP__FUNCTION, (u64) sp);
+	map = map_groups__find(&thread->mg, MAP__VARIABLE, (u64) sp);
 	if (!map) {
 		pr_debug("failed to get stack map\n");
 		free(buf);

1.8.3.1

Pulled, thanks a lot Jiri!

Ingo