From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1339 invoked by alias); 7 Jan 2008 14:33:32 -0000 Received: (qmail 1327 invoked by uid 22791); 7 Jan 2008 14:33:29 -0000 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 07 Jan 2008 14:32:59 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id F04962A965E; Mon, 7 Jan 2008 09:32:57 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id PCvy2uEtFZCX; Mon, 7 Jan 2008 09:32:57 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 1F3B62A965A; Mon, 7 Jan 2008 09:32:56 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 325E1E7ACB; Mon, 7 Jan 2008 06:32:12 -0800 (PST) Date: Mon, 07 Jan 2008 14:33:00 -0000 From: Joel Brobecker To: Aleksandar Ristovski , gdb@sourceware.org, Ryan Mansfield Subject: Re: gdb_realpath: dealing with ./ and ../ Message-ID: <20080107143212.GA5923@adacore.com> References: <2F6320727174C448A52CEB63D85D11F40A3F@nova.ott.qnx.com> <20080103170846.GA17263@caradoc.them.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="AhhlLboLdkugWU4S" Content-Disposition: inline In-Reply-To: <20080103170846.GA17263@caradoc.them.org> User-Agent: Mutt/1.4.2.2i Mailing-List: contact gdb-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sourceware.org X-SW-Source: 2008-01/txt/msg00036.txt.bz2 --AhhlLboLdkugWU4S Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 2827 > An alternative would be to do some canonicalization - not > gdb_realpath, which accesses the filesystem, but just string > manipulation - on the subfile names iff nothing matches the main > file. You could remove the ".." there. Aleksandar liked this option best, but I think I liked your first suggestion more: > > Would this not introduce a bit of guessing? Consider a case where there are > > several files with the same basename but different paths (possibly specified > > using relative path elements, i.e. different pathnames like in my case). In > > this case none of the files with the same base name would perfectly fit and > > picking the first one will likely not give the correct answer. > > It's a guess, but a good one. 99.9% of the time, a C file does not > include another C file with the same basename. If the compiler is > going to generate debug info which refers to the same file by two > different names, I don't see what else we can do. I looked at the whole discussion and at the patches, and they seem pretty simple in the idea, but pretty complex in the implementation. If I undertand the problem correctly, the problem is matching our main.cc entry in the linetable back to the main unit subfile where the name&comp_unit don't exactly match character for character. Right? Going further with the idea that 99.9% of the time, one file does not depend on another file with the same name, then how about doing the matching using basenames? As a starting point, I propose the following idea: Modify start_subfile so that: 1. file name is reduced to a basename only If there is any path information inside name (such as ".." or "/foo/bar/"), it is appended to the dir name. As a special case, if name was an absolute path, then we ignore the dirname and use the dirname from the filename only. 2. subfile matching is done using the basename only. I am attaching a patch that illustrates roughly what I have in mind (not compiled, not tested). The great advantage (if it works :), is that no rewriting is necessary. The reason why I agree that the changes should be done inside start-subfiles is that we don't want to have to handle this sort of problem with every debug info reader. This was, the handling is centralized. It also looks a lot simpler than the patches I have seen that touch the DWARF reader. The one thing that this might break, however, is when the user provides a relative path in the break command: (gdb) break bar/main.c As a matter of fact, this is already broken if DW_AT_name is main.c and not bar/main.c. So I'm pretty sure that it'll break it more. But the good news is that it would be easy to fix it: Modify the matching to concat the dirname and basename and do the match with that. Does this sound like it would work? -- Joel --AhhlLboLdkugWU4S Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="buildsym.c.diff" Content-length: 3078 Index: buildsym.c =================================================================== --- buildsym.c (revision 44) +++ buildsym.c (working copy) @@ -580,7 +580,7 @@ make_blockvector (struct objfile *objfil the directory in which the file was compiled (or NULL if not known). */ void -start_subfile (char *name, char *dirname) +start_subfile_1 (char *name, char *dirname) { struct subfile *subfile; @@ -589,27 +589,11 @@ start_subfile (char *name, char *dirname for (subfile = subfiles; subfile; subfile = subfile->next) { - char *subfile_name; - - /* If NAME is an absolute path, and this subfile is not, then - attempt to create an absolute path to compare. */ - if (IS_ABSOLUTE_PATH (name) - && !IS_ABSOLUTE_PATH (subfile->name) - && subfile->dirname != NULL) - subfile_name = concat (subfile->dirname, SLASH_STRING, - subfile->name, NULL); - else - subfile_name = subfile->name; - - if (FILENAME_CMP (subfile_name, name) == 0) + if (FILENAME_CMP (subfile->name, name) == 0) { current_subfile = subfile; - if (subfile_name != subfile->name) - xfree (subfile_name); return; } - if (subfile_name != subfile->name) - xfree (subfile_name); } /* This subfile is not known. Add an entry for it. Make an entry @@ -681,6 +665,53 @@ start_subfile (char *name, char *dirname } } +/* This function is a wrapper around start_subfile_1 which rewrites + a bit NAME and DIRNAME to make sure that we are always consistent + in how we create subfiles: Turn NAME into a basename, and adjust + DIRNAME accordingly. */ + +void +start_subfile (char *name, char *dirname) +{ + char *new_name, new_dir_name; + + new_name = lbasename (name); + + /* The simple usual case: NAME is the name of a file without any path + (in other words, it's already a basename). In that case, we use + DIR_NAME as is. */ + if (FILENAME_CMP (new_name, name) == 0) + new_dir_name = dir_name; + + /* Another simple case: NAME is an absolute PATH. In that case, + we can ignore the compilation dir, and use the path inside NAME + as the dir name. */ + else if (IS_ABSOLUTE_PATH (name)) + new_dir_name = ldirname (name); + + /* Otherwise, NAME is a relative PATH, so we extract the relative + path from name, and append it at the end of DIRNAME. IF DIRNAME + is NULL, then we use that relative path as the DIRNAME. */ + else + { + char *name_relative_path = ldirname (name); + + if (dirname != NULL) + { + new_dir_name = concat (dirname, SLASH_STRING, name_relative_path, + NULL); + xfree (name_relative_path); + } + else + new_dir_name = name_relative_path; + } + + start_subfile_1 (new_name, new_dir_name); + + xfree (new_name); + xfree (new_dir_name); +} + /* For stabs readers, the first N_SO symbol is assumed to be the source file name, and the subfile struct is initialized using that assumption. If another N_SO symbol is later seen, immediately --AhhlLboLdkugWU4S--