Re: [PATCH RFC RESEND] Perf: lookup dwarf unwind stack info in debug file pointed by .gnu_debuglink

[Matija Glavinic Pecotic <matija.glavinic-pecotic.ext@nokia.com>]

On 24/08/16 09:30, Jiri Olsa wrote:
> On Tue, Aug 23, 2016 at 06:18:10PM +0200, Matija Glavinic Pecotic wrote:
>> -		ret = filename__read_debuglink(filename, debuglink,
>> -					       size - (debuglink - filename));
>> +		ret = filename__read_debuglink(filename, symfile, PATH_MAX);
>> +		if (ret)
>> +			break;
>> +
>> +		/* Check predefined locations where debug file might reside:
>> +		 *  - if debuglink is absolute path, check only that one
>> +		 * If debuglink provides name w/o path, look for debug file:
>> +		 *  - in the same directory as dso
>> +		 *  - in the .debug subdirectory of dso directory
>> +		 *  - in the /usr/lib/debug/[dso directory]
>> +		 *  */
>> +		ret = 0;
>> +		if (symfile[0] == '/') {
>> +			if (!is_regular_file(symfile))
>> +				ret = -1;
>> +			else
>> +				strncpy(filename, symfile, size);
>> +			break;
>>  		}
>> +
>> +		snprintf(filename, size, "%s/%s", dso_dir, symfile);
>> +		if(is_regular_file(filename))
>> +			break;
>> +
>> +		snprintf(filename, size, "%s/.debug/%s", dso_dir, symfile);
>> +		if(is_regular_file(filename))
>> +			break;
>> +
>> +		snprintf(filename, size, "/usr/lib/debug/%s/%s",
>> +				dso_dir, symfile);
>> +		if(is_regular_file(filename))
>> +			break;
> 
> it might be more clear to follow the same way we do for vmlinux search,
> like array of possible paths and generic code to check.. search for
> vmlinux_path in symbol.c

It indeed looks better:

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 774f6ec..3ea205cc 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -35,6 +35,13 @@ char dso__symtab_origin(const struct dso *dso)
 	return origin[dso->symtab_type];
 }
 
+static const char * const debuglink_paths[] = {
+	"%.0s%s",
+	"%s/%s",
+	"%s/.debug/%s",
+	"/usr/lib/debug%s/%s"
+};
+
 int dso__read_binary_type_filename(const struct dso *dso,
 				   enum dso_binary_type type,
 				   char *root_dir, char *filename, size_t size)
@@ -44,24 +51,44 @@ int dso__read_binary_type_filename(const struct dso *dso,
 	size_t len;
 
 	switch (type) {
-	case DSO_BINARY_TYPE__DEBUGLINK: {
-		char *debuglink;
+	case DSO_BINARY_TYPE__DEBUGLINK:
+	{
+		const char *last_slash;
+		char dso_dir[PATH_MAX];
+		char symfile[PATH_MAX];
+		unsigned int i;
 
 		len = __symbol__join_symfs(filename, size, dso->long_name);
-		debuglink = filename + len;
-		while (debuglink != filename && *debuglink != '/')
-			debuglink--;
-		if (*debuglink == '/')
-			debuglink++;
+		last_slash = filename + len;
+		while (last_slash != filename && *last_slash != '/')
+			last_slash--;
 
-		ret = -1;
-		if (!is_regular_file(filename))
+		strncpy(dso_dir, filename, last_slash - filename);
+		dso_dir[last_slash-filename] = '\0';
+
+		if (!is_regular_file(filename)) {
+			ret = -1;
+			break;
+		}
+
+		ret = filename__read_debuglink(filename, symfile, PATH_MAX);
+		if (ret)
 			break;
 
-		ret = filename__read_debuglink(filename, debuglink,
-					       size - (debuglink - filename));
+		/* Check predefined locations where debug file might reside */
+		ret = -1;
+		for (i = 0; i < ARRAY_SIZE(debuglink_paths); i++) {
+			snprintf(filename, size,
+					debuglink_paths[i], dso_dir, symfile);
+			if(is_regular_file(filename)) {
+				ret = 0;
+				break;
+			}
 		}
+
 		break;
+	}

Thanks,

Matija