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

[Matija Glavinic Pecotic]

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