Re: [RFC PATCH V2] s390/perf: fix 'start' address of module's map

From: Songshan Gong
Date: Mon Jul 18 2016 - 23:35:36 EST




å 7/19/2016 9:50 AM, Songshan Gong åé:


å 7/19/2016 9:37 AM, Songshan Gong åé:


å 7/18/2016 10:07 PM, Jiri Olsa åé:
On Fri, Jul 15, 2016 at 06:15:11PM +0800, Song Shan Gong wrote:
At preset, when creating module's map, perf gets 'start' address by
parsing
'/proc/modules', but it's module base address, isn't the start
address of
'.text' section. In most archs, it's OK. But for s390, it places
'GOT' and
'PLT' relocations before '.text' section. So there exists an offset
between
module base address and '.text' section, which will incur wrong symbol
resolution for modules.

Fix this bug by getting 'start' address of module's map from parsing
'/sys/module/[module name]/sections/.text', not from '/proc/modules'.

Signed-off-by: Song Shan Gong <gongss@xxxxxxxxxxxxxxxxxx>
---
tools/perf/arch/s390/util/Build | 2 ++
tools/perf/arch/s390/util/sym-handling.c | 27
+++++++++++++++++++++++++++
tools/perf/util/machine.c | 8 ++++++++
tools/perf/util/machine.h | 1 +
4 files changed, 38 insertions(+)
create mode 100644 tools/perf/arch/s390/util/sym-handling.c

diff --git a/tools/perf/arch/s390/util/Build
b/tools/perf/arch/s390/util/Build
index 8a61372..5e322ed 100644
--- a/tools/perf/arch/s390/util/Build
+++ b/tools/perf/arch/s390/util/Build
@@ -2,3 +2,5 @@ libperf-y += header.o
libperf-y += kvm-stat.o

libperf-$(CONFIG_DWARF) += dwarf-regs.o
+
+libperf-y += sym-handling.o
diff --git a/tools/perf/arch/s390/util/sym-handling.c
b/tools/perf/arch/s390/util/sym-handling.c
new file mode 100644
index 0000000..ff51336
--- /dev/null
+++ b/tools/perf/arch/s390/util/sym-handling.c
@@ -0,0 +1,27 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include "util.h"
+#include "machine.h"
+#include "api/fs/fs.h"
+
+int arch__fix_module_text_start(u64 *start, const char *name)
+{
+ char path[PATH_MAX];
+ char *module_name = NULL;
+ int len;
+
+ if (!(module_name = strdup(name)))
+ return -1;
+
+ len = strlen(module_name);
+ module_name[len - 1] = '\0';
+ snprintf(path, PATH_MAX, "module/%s/sections/.text",
+ module_name + 1);

why can't you use 'name' in here? I can't see the reason
you allocated module_name..

Oh, yes.

Sorry, just ignore the upper reply.

I use the 'module_name' here instead of 'name', because 'name' is
something like '[module_name]', I need strip the '[' and ']', so I have
to set the tail ']' to '\0' to indicate the string end, which will need
change the constant variable 'name'.

eeee, I found a new way to implement it.

I could use '%.*s' to format the 'path', 'name' is enough indeed.

It's so concise now. Appreciate.


+
+ if (sysfs__read_ull(path, (unsigned long long *)start) < 0) {
+ free(module_name);
+ return -1;
+ }

leaking module_name in here

yes, thanks.

+ return 0;
+}

SNIP

thanks,
jirka




--
SongShan Gong