[patch] Fix for PR gdb/9538 (loading of separate debuginfo and symlinks).
Doug Evans
dje@google.com
Thu Jan 12 17:28:00 GMT 2012
On Wed, Jan 11, 2012 at 7:06 PM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> Greetings,
>
> Attached is a proposed fix for PR gdb/9538.
>
> Mostly it just moves code around, and adds a "try realpath(objfile->name)
> if searching with objfile->name fails."
>
> The added test fails before the patch, and succeeds after it.
>
> Tested on Linux/x86_64.
>
> Thanks,
>
> --
> Paul Pluzhnikov
>
>
> 2012-01-11 Paul Pluzhnikov <ppluzhnikov@google.com>
>
> PR gdb/9538
> * symfile.c (find_separate_debug_file): New function.
> (terminate_after_last_dir_separator): Likewise.
> (find_separate_debug_file_by_debuglink): Also try realpath.
>
>
> testsuite/ChangeLog:
>
> PR gdb/9538
> * gdb.base/sepdebug.exp: New test.
Howdy.
The symfile.c change is ok with me, modulo can you add comments for
each of the functions?
[caveat: I think it doesn't properly handle paths with dos drives, but
the code already has that problem, so no requirement to fix that in
this patch]
I see sepdebug.exp uses remote_exec so best use that instead of exec.
Also, I'm not sure what the portability requirements are w.r.t. symlinks.
Probably best to watch for errors in the "ln -s" and handle appropriately.
[Even better, is there a utility routine that will create a symlink?
All the portability concerns could be tucked away in there.]
Also, can you use clean_restart instead of the gdb_exit/gdb_start/... sequence?
Fine with those changes.
>
>
>
> Index: symfile.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/symfile.c,v
> retrieving revision 1.325
> diff -u -p -r1.325 symfile.c
> --- symfile.c 12 Jan 2012 00:00:01 -0000 1.325
> +++ symfile.c 12 Jan 2012 01:34:01 -0000
> @@ -1441,35 +1441,15 @@ show_debug_file_directory (struct ui_fil
> #define DEBUG_SUBDIRECTORY ".debug"
> #endif
>
> -char *
> -find_separate_debug_file_by_debuglink (struct objfile *objfile)
> -{
> - char *basename, *debugdir;
> - char *dir = NULL;
> - char *debugfile = NULL;
> - char *canon_name = NULL;
> - unsigned long crc32;
> +static char *
> +find_separate_debug_file (const char *dir, const char *debuglink,
> + unsigned long crc32, struct objfile *objfile)
> +{
> + char *debugdir;
> + char *debugfile;
> + char *canon_name;
> int i;
>
> - basename = get_debug_link_info (objfile, &crc32);
> -
> - if (basename == NULL)
> - /* There's no separate debug info, hence there's no way we could
> - load it => no warning. */
> - goto cleanup_return_debugfile;
> -
> - dir = xstrdup (objfile->name);
> -
> - /* Strip off the final filename part, leaving the directory name,
> - followed by a slash. The directory can be relative or absolute. */
> - for (i = strlen(dir) - 1; i >= 0; i--)
> - {
> - if (IS_DIR_SEPARATOR (dir[i]))
> - break;
> - }
> - /* If I is -1 then no directory is present there and DIR will be "". */
> - dir[i+1] = '\0';
> -
> /* Set I to max (strlen (canon_name), strlen (dir)). */
> canon_name = lrealpath (dir);
> i = strlen (dir);
> @@ -1480,12 +1460,12 @@ find_separate_debug_file_by_debuglink (s
> + i
> + strlen (DEBUG_SUBDIRECTORY)
> + strlen ("/")
> - + strlen (basename)
> + + strlen (debuglink)
> + 1);
>
> /* First try in the same directory as the original file. */
> strcpy (debugfile, dir);
> - strcat (debugfile, basename);
> + strcat (debugfile, debuglink);
>
> if (separate_debug_file_exists (debugfile, crc32, objfile))
> goto cleanup_return_debugfile;
> @@ -1494,7 +1474,7 @@ find_separate_debug_file_by_debuglink (s
> strcpy (debugfile, dir);
> strcat (debugfile, DEBUG_SUBDIRECTORY);
> strcat (debugfile, "/");
> - strcat (debugfile, basename);
> + strcat (debugfile, debuglink);
>
> if (separate_debug_file_exists (debugfile, crc32, objfile))
> goto cleanup_return_debugfile;
> @@ -1520,7 +1500,7 @@ find_separate_debug_file_by_debuglink (s
> debugfile[debugdir_end - debugdir] = 0;
> strcat (debugfile, "/");
> strcat (debugfile, dir);
> - strcat (debugfile, basename);
> + strcat (debugfile, debuglink);
>
> if (separate_debug_file_exists (debugfile, crc32, objfile))
> goto cleanup_return_debugfile;
> @@ -1536,7 +1516,7 @@ find_separate_debug_file_by_debuglink (s
> debugfile[debugdir_end - debugdir] = 0;
> strcat (debugfile, canon_name + strlen (gdb_sysroot));
> strcat (debugfile, "/");
> - strcat (debugfile, basename);
> + strcat (debugfile, debuglink);
>
> if (separate_debug_file_exists (debugfile, crc32, objfile))
> goto cleanup_return_debugfile;
> @@ -1551,8 +1531,61 @@ find_separate_debug_file_by_debuglink (s
>
> cleanup_return_debugfile:
> xfree (canon_name);
> - xfree (basename);
> + return debugfile;
> +}
> +
> +static void
> +terminate_after_last_dir_separator (char *path)
> +{
> + int i;
> +
> + /* Strip off the final filename part, leaving the directory name,
> + followed by a slash. The directory can be relative or absolute. */
> + for (i = strlen(path) - 1; i >= 0; i--)
> + {
> + if (IS_DIR_SEPARATOR (path[i]))
> + break;
> + }
> + /* If I is -1 then no directory is present there and DIR will be "". */
> + path[i+1] = '\0';
> +}
> +
> +char *
> +find_separate_debug_file_by_debuglink (struct objfile *objfile)
> +{
> + char *debuglink;
> + char *dir;
> + char *debugfile;
> + unsigned long crc32;
> +
> + debuglink = get_debug_link_info (objfile, &crc32);
> +
> + if (debuglink == NULL)
> + /* There's no separate debug info, hence there's no way we could
> + load it => no warning. */
> + return NULL;
> +
> + dir = xstrdup (objfile->name);
> + terminate_after_last_dir_separator (dir);
> +
> + debugfile = find_separate_debug_file (dir, debuglink, crc32, objfile);
> xfree (dir);
> +
> + if (debugfile != NULL)
> + goto cleanup_return_debugfile;
> +
> + /* For PR gdb/9538, try again with realpath. */
> + dir = lrealpath (objfile->name);
> + if (dir == NULL)
> + goto cleanup_return_debugfile;
> +
> + terminate_after_last_dir_separator (dir);
> + debugfile = find_separate_debug_file (dir, debuglink, crc32, objfile);
> + xfree (dir);
> +
> + cleanup_return_debugfile:
> + xfree (debuglink);
> +
> return debugfile;
> }
>
> Index: testsuite/gdb.base/sepdebug.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/gdb.base/sepdebug.exp,v
> retrieving revision 1.33
> diff -u -p -r1.33 sepdebug.exp
> --- testsuite/gdb.base/sepdebug.exp 4 Jan 2012 08:17:46 -0000 1.33
> +++ testsuite/gdb.base/sepdebug.exp 12 Jan 2012 01:34:01 -0000
> @@ -45,7 +45,7 @@ if { [gdb_compile "${srcdir}/${subdir}/
>
> # Note: the procedure gdb_gnu_strip_debug will produce an executable called
> # ${binfile}, which is just like the executable ($binfile) but without
> -# the debuginfo. Instead $binfile has a .gnudebuglink section which contains
> +# the debuginfo. Instead $binfile has a .gnu_debuglink section which contains
> # the name of a debuginfo only file. This file will be stored in the
> # gdb.base/ subdirectory.
>
> @@ -55,9 +55,25 @@ if [gdb_gnu_strip_debug $binfile] {
> return -1
> }
>
> +#
> +# PR gdb/9538. Verify that symlinked executable still finds the separate
> +# debuginfo.
> +#
> gdb_exit
> gdb_start
> gdb_reinitialize_dir $srcdir/$subdir
> +set subsubdir ${objdir}/${subdir}/pr9538
> +exec mkdir ${subsubdir}
> +exec ln -s ${binfile} ${subsubdir}
> +gdb_load ${subsubdir}/${testfile}${EXEEXT}
> +if { $gdb_file_cmd_debug_info != "debug" } then {
> + fail "No debug information found."
> +}
> +gdb_exit
> +exec rm -rf ${subsubdir}
> +
> +gdb_start
> +gdb_reinitialize_dir $srcdir/$subdir
> gdb_load ${binfile}
> if { $gdb_file_cmd_debug_info != "debug" } then {
> fail "No debug information found."
More information about the Gdb-patches
mailing list