From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17562 invoked by alias); 12 Jan 2012 17:24:12 -0000 Received: (qmail 17542 invoked by uid 22791); 12 Jan 2012 17:24:09 -0000 X-SWARE-Spam-Status: No, hits=-3.1 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,RCVD_IN_DNSWL_LOW,TW_BJ,TW_CP,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail-vw0-f41.google.com (HELO mail-vw0-f41.google.com) (209.85.212.41) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 12 Jan 2012 17:23:44 +0000 Received: by vbnl22 with SMTP id l22so612488vbn.0 for ; Thu, 12 Jan 2012 09:23:43 -0800 (PST) Received: by 10.52.90.71 with SMTP id bu7mr2217425vdb.63.1326389023794; Thu, 12 Jan 2012 09:23:43 -0800 (PST) MIME-Version: 1.0 Received: by 10.52.90.71 with SMTP id bu7mr2217418vdb.63.1326389023719; Thu, 12 Jan 2012 09:23:43 -0800 (PST) Received: by 10.220.229.1 with HTTP; Thu, 12 Jan 2012 09:23:43 -0800 (PST) In-Reply-To: <20120112030648.14DBE190AFD@elbrus2.mtv.corp.google.com> References: <20120112030648.14DBE190AFD@elbrus2.mtv.corp.google.com> Date: Thu, 12 Jan 2012 17:28:00 -0000 Message-ID: Subject: Re: [patch] Fix for PR gdb/9538 (loading of separate debuginfo and symlinks). From: Doug Evans To: Paul Pluzhnikov Cc: gdb-patches@sourceware.org X-System-Of-Record: true Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-01/txt/msg00413.txt.bz2 On Wed, Jan 11, 2012 at 7:06 PM, Paul Pluzhnikov w= rote: > 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 =A0Paul Pluzhnikov =A0 > > =A0 =A0 =A0 =A0PR gdb/9538 > =A0 =A0 =A0 =A0* symfile.c (find_separate_debug_file): New function. > =A0 =A0 =A0 =A0(terminate_after_last_dir_separator): Likewise. > =A0 =A0 =A0 =A0(find_separate_debug_file_by_debuglink): Also try realpath. > > > testsuite/ChangeLog: > > =A0 =A0 =A0 =A0PR gdb/9538 > =A0 =A0 =A0 =A0* 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/... seque= nce? Fine with those changes. > > > > Index: symfile.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > RCS file: /cvs/src/src/gdb/symfile.c,v > retrieving revision 1.325 > diff -u -p -r1.325 symfile.c > --- symfile.c =A0 12 Jan 2012 00:00:01 -0000 =A0 =A0 =A01.325 > +++ symfile.c =A0 12 Jan 2012 01:34:01 -0000 > @@ -1441,35 +1441,15 @@ show_debug_file_directory (struct ui_fil > =A0#define DEBUG_SUBDIRECTORY ".debug" > =A0#endif > > -char * > -find_separate_debug_file_by_debuglink (struct objfile *objfile) > -{ > - =A0char *basename, *debugdir; > - =A0char *dir =3D NULL; > - =A0char *debugfile =3D NULL; > - =A0char *canon_name =3D NULL; > - =A0unsigned long crc32; > +static char * > +find_separate_debug_file (const char *dir, const char *debuglink, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 unsigned long crc32, st= ruct objfile *objfile) > +{ > + =A0char *debugdir; > + =A0char *debugfile; > + =A0char *canon_name; > =A0 int i; > > - =A0basename =3D get_debug_link_info (objfile, &crc32); > - > - =A0if (basename =3D=3D NULL) > - =A0 =A0/* There's no separate debug info, hence there's no way we could > - =A0 =A0 =A0 load it =3D> no warning. =A0*/ > - =A0 =A0goto cleanup_return_debugfile; > - > - =A0dir =3D xstrdup (objfile->name); > - > - =A0/* Strip off the final filename part, leaving the directory name, > - =A0 =A0 followed by a slash. =A0The directory can be relative or absolu= te. =A0*/ > - =A0for (i =3D strlen(dir) - 1; i >=3D 0; i--) > - =A0 =A0{ > - =A0 =A0 =A0if (IS_DIR_SEPARATOR (dir[i])) > - =A0 =A0 =A0 break; > - =A0 =A0} > - =A0/* If I is -1 then no directory is present there and DIR will be "".= =A0*/ > - =A0dir[i+1] =3D '\0'; > - > =A0 /* Set I to max (strlen (canon_name), strlen (dir)). =A0*/ > =A0 canon_name =3D lrealpath (dir); > =A0 i =3D strlen (dir); > @@ -1480,12 +1460,12 @@ find_separate_debug_file_by_debuglink (s > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 + i > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 + strlen (DEBUG_SUBDIRECTORY) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 + strlen ("/") > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0+ strlen (basename) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0+ strlen (debuglink) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 + 1); > > =A0 /* First try in the same directory as the original file. =A0*/ > =A0 strcpy (debugfile, dir); > - =A0strcat (debugfile, basename); > + =A0strcat (debugfile, debuglink); > > =A0 if (separate_debug_file_exists (debugfile, crc32, objfile)) > =A0 =A0 goto cleanup_return_debugfile; > @@ -1494,7 +1474,7 @@ find_separate_debug_file_by_debuglink (s > =A0 strcpy (debugfile, dir); > =A0 strcat (debugfile, DEBUG_SUBDIRECTORY); > =A0 strcat (debugfile, "/"); > - =A0strcat (debugfile, basename); > + =A0strcat (debugfile, debuglink); > > =A0 if (separate_debug_file_exists (debugfile, crc32, objfile)) > =A0 =A0 goto cleanup_return_debugfile; > @@ -1520,7 +1500,7 @@ find_separate_debug_file_by_debuglink (s > =A0 =A0 =A0 debugfile[debugdir_end - debugdir] =3D 0; > =A0 =A0 =A0 strcat (debugfile, "/"); > =A0 =A0 =A0 strcat (debugfile, dir); > - =A0 =A0 =A0strcat (debugfile, basename); > + =A0 =A0 =A0strcat (debugfile, debuglink); > > =A0 =A0 =A0 if (separate_debug_file_exists (debugfile, crc32, objfile)) > =A0 =A0 =A0 =A0goto cleanup_return_debugfile; > @@ -1536,7 +1516,7 @@ find_separate_debug_file_by_debuglink (s > =A0 =A0 =A0 =A0 =A0debugfile[debugdir_end - debugdir] =3D 0; > =A0 =A0 =A0 =A0 =A0strcat (debugfile, canon_name + strlen (gdb_sysroot)); > =A0 =A0 =A0 =A0 =A0strcat (debugfile, "/"); > - =A0 =A0 =A0 =A0 strcat (debugfile, basename); > + =A0 =A0 =A0 =A0 strcat (debugfile, debuglink); > > =A0 =A0 =A0 =A0 =A0if (separate_debug_file_exists (debugfile, crc32, objf= ile)) > =A0 =A0 =A0 =A0 =A0 =A0goto cleanup_return_debugfile; > @@ -1551,8 +1531,61 @@ find_separate_debug_file_by_debuglink (s > > =A0cleanup_return_debugfile: > =A0 xfree (canon_name); > - =A0xfree (basename); > + =A0return debugfile; > +} > + > +static void > +terminate_after_last_dir_separator (char *path) > +{ > + =A0int i; > + > + =A0/* Strip off the final filename part, leaving the directory name, > + =A0 =A0 followed by a slash. =A0The directory can be relative or absolu= te. =A0*/ > + =A0for (i =3D strlen(path) - 1; i >=3D 0; i--) > + =A0 =A0{ > + =A0 =A0 =A0if (IS_DIR_SEPARATOR (path[i])) > + =A0 =A0 =A0 break; > + =A0 =A0} > + =A0/* If I is -1 then no directory is present there and DIR will be "".= =A0*/ > + =A0path[i+1] =3D '\0'; > +} > + > +char * > +find_separate_debug_file_by_debuglink (struct objfile *objfile) > +{ > + =A0char *debuglink; > + =A0char *dir; > + =A0char *debugfile; > + =A0unsigned long crc32; > + > + =A0debuglink =3D get_debug_link_info (objfile, &crc32); > + > + =A0if (debuglink =3D=3D NULL) > + =A0 =A0/* There's no separate debug info, hence there's no way we could > + =A0 =A0 =A0 load it =3D> no warning. =A0*/ > + =A0 =A0return NULL; > + > + =A0dir =3D xstrdup (objfile->name); > + =A0terminate_after_last_dir_separator (dir); > + > + =A0debugfile =3D find_separate_debug_file (dir, debuglink, crc32, objfi= le); > =A0 xfree (dir); > + > + =A0if (debugfile !=3D NULL) > + =A0 =A0goto cleanup_return_debugfile; > + > + =A0/* For PR gdb/9538, try again with realpath. =A0*/ > + =A0dir =3D lrealpath (objfile->name); > + =A0if (dir =3D=3D NULL) > + =A0 =A0goto cleanup_return_debugfile; > + > + =A0terminate_after_last_dir_separator (dir); > + =A0debugfile =3D find_separate_debug_file (dir, debuglink, crc32, objfi= le); > + =A0xfree (dir); > + > + cleanup_return_debugfile: > + =A0xfree (debuglink); > + > =A0 return debugfile; > =A0} > > Index: testsuite/gdb.base/sepdebug.exp > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > 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 =A0 =A0 4 Jan 2012 08:17:46 -0000 =A0= =A0 =A0 1.33 > +++ testsuite/gdb.base/sepdebug.exp =A0 =A0 12 Jan 2012 01:34:01 -0000 > @@ -45,7 +45,7 @@ if =A0{ [gdb_compile "${srcdir}/${subdir}/ > > =A0# Note: the procedure gdb_gnu_strip_debug will produce an executable c= alled > =A0# ${binfile}, which is just like the executable ($binfile) but without > -# the debuginfo. Instead $binfile has a .gnudebuglink section which cont= ains > +# the debuginfo. Instead $binfile has a .gnu_debuglink section which con= tains > =A0# the name of a debuginfo only file. This file will be stored in the > =A0# gdb.base/ subdirectory. > > @@ -55,9 +55,25 @@ if [gdb_gnu_strip_debug $binfile] { > =A0 =A0 return -1 > =A0} > > +# > +# PR gdb/9538. =A0Verify that symlinked executable still finds the separ= ate > +# debuginfo. > +# > =A0gdb_exit > =A0gdb_start > =A0gdb_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 !=3D "debug" } then { > + =A0 =A0fail "No debug information found." > +} > +gdb_exit > +exec rm -rf ${subsubdir} > + > +gdb_start > +gdb_reinitialize_dir $srcdir/$subdir > =A0gdb_load ${binfile} > =A0if { $gdb_file_cmd_debug_info !=3D "debug" } then { > =A0 =A0 fail "No debug information found."