This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On Thu, Jan 12, 2012 at 1:29 PM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote: > On Thu, 12 Jan 2012 21:35:21 +0100, Paul Pluzhnikov wrote: >> +/* Modify PATH to contain only "directory/" part of PATH. >> + ? If there were no, directory separators in PATH, PATH will be empty >> + ? string on return. ?*/ >> + >> +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; >> + ? ?} > > Empty line. Done. >> + ?/* If I is -1 then no directory is present there and DIR will be "". ?*/ >> + ?path[i+1] = '\0'; > > It was there already but there should be `i + 1'. Done. >> +} >> + >> +/* Find separate debuginfo for OBJFILE (using .gnu_debuglink section). >> + ? Returns pathname, or NULL. ?*/ >> + >> +char * >> +find_separate_debug_file_by_debuglink (struct objfile *objfile) >> +{ >> + ?char *debuglink; >> + ?char *dir1, *dir2, *canon_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; >> + >> + ?dir1 = xstrdup (objfile->name); >> + ?terminate_after_last_dir_separator (dir1); >> + ?canon_dir = lrealpath (dir1); > > lrealpath can return NULL. ?GDB did not crash before. ?It will now. Where? canon_dir is passed into the utility function, which checks for NULL (as it did before). >> + >> + ?debugfile = find_separate_debug_file (dir1, canon_dir, debuglink, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? crc32, objfile); >> + ?xfree (canon_dir); >> + >> + ?if (debugfile != NULL) >> + ? ?goto cleanup1; >> + >> + ?/* For PR gdb/9538, try again with realpath (if different from the >> + ? ? original). ?*/ >> + ?dir2 = lrealpath (objfile->name); > > Maybe some optimization would be helpful. ?realpath is expensive and the > directory path is already canonicalized. ?Something like lstat (objfile->name) > and do this step only if it is a symlink. Wouldn't lstat need a configury #ifdef to make it build? But there is no need to do realpath on the directory again, so I just pass dir2 as canon_dir. > Not a requirement from me (modern systems use .build-id anyway). Not all of them. I've hit this bug while debugging libc on pre-release Ubuntu. >> + ?if (dir2 == NULL) >> + ? ?goto cleanup1; >> + >> + ?terminate_after_last_dir_separator (dir2); >> + ?if (strcmp (dir1, dir2) == 0) >> + ? ?/* Same directory, no point retrying. ?*/ >> + ? ?goto cleanup2; > > There was some discussion with conclusion it should be written as, nitpick: > ? ? ?if (strcmp (dir1, dir2) == 0) > ? ? ? ?{ > ? ? ? ? ?/* Same directory, no point retrying. ?*/ > ? ? ? ? ?goto cleanup2; > ? ? ? ?} Done. > >> + >> + ?canon_dir = lrealpath (dir2); >> + ?debugfile = find_separate_debug_file (dir2, canon_dir, debuglink, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? crc32, objfile); >> + ?xfree (canon_dir); >> + >> + cleanup2: >> + ?xfree (dir2); >> + cleanup1: > > Maybe it should be finally rewritten to cleanups but it may be out of the > scope of this patch. Done. > >> + ?xfree (dir1); >> + ?xfree (debuglink); >> >> -cleanup_return_debugfile: >> - ?xfree (canon_name); >> - ?xfree (basename); >> - ?xfree (dir); >> ? ?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 20:29:53 -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,10 +55,26 @@ if [gdb_gnu_strip_debug $binfile] { >> ? ? ?return -1 >> ?} >> >> +# >> +# PR gdb/9538. ?Verify that symlinked executable still finds the separate >> +# debuginfo. >> +# >> +set old_subdir ${subdir} >> +set subdir ${subdir}/pr9538 >> +remote_exec build "mkdir ${subdir}" >> +remote_exec build "ln -s ${binfile} ${subdir}" > > You should clean up first any stale files there. Done. > > >> +clean_restart ${testfile}${EXEEXT} >> +if { $gdb_file_cmd_debug_info != "debug" } then { >> + ? ?fail "No debug information found." >> +} >> + >> +# make sure we are not holding subdir/binary open. >> ?gdb_exit >> -gdb_start >> -gdb_reinitialize_dir $srcdir/$subdir >> -gdb_load ${binfile} >> + >> +remote_exec build "rm -rf ${subdir}" > > It is not great the FAIL is not easily reproducible after it happens. Ok. We now leave state as is, and cleanup just before re-creating it. > >> +set subdir ${old_subdir} >> + >> +clean_restart ${testfile}${EXEEXT} >> ?if { $gdb_file_cmd_debug_info != "debug" } then { >> ? ? ?fail "No debug information found." >> ?} > > Re-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.
Attachment:
gdb-symlink-pr9538-20120112a.txt
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |