[Lldb-commits] [PATCH] Make sure we perform full pathname matches when displaying source code.

jingham at apple.com jingham at apple.com
Tue Oct 21 17:18:05 PDT 2014


> On Oct 21, 2014, at 4:02 PM, Stephane Sezer <sas at fb.com> wrote:
> 
> What you’re saying is correct, this patch becomes useful only in combination with other patches that I’m getting ready to send to the mailing list now, sorry :/
> 
> I have one question about FileSpec::Equal, actually: why do we force full path lookups when one of the arguments contains a directory component? Basically, what the code does is it ignores the “full” boolean argument unless both paths are filename-only; in which case a full path comparison would have been equivalent to a filename-only comparison anyway.

"full == false" is useful when you want don't know whether one side of the compare is going to be a full name or not.  So for instance we know the debug information will always have full path names.  But data from the user (e.g. a -f option to "break set") might or might not have a directory. If it does have a directory, you want to respect that, if it doesn't you just want a file name compare.  That's the motivation behind that option in FileSpec::Equal, I just pass full=false and get this behavior.  

I note that FileSpec::Equal gets used this way about 1/3 of the time, and other times people do:

        if (!FileSpec::Equal (platform_file_spec, GetPlatformFileSpec (), (bool)platform_file_spec.GetDirectory()))

Which if you know GetPlatformFileSpec is going to be full is redundant...

If you want to switch to your version, you would also have to go through all the places where we use Equal, and determine which is the side whose FileSpec is user-specified (and thus should determine the level of comparison) and then put in code like that above.

Jim


> 
> To give a bit more context on this: I was working on hooking up target.exec-search-paths which requires us to do filename-only lookups (e.g.: if we can’t find a library we are loading, look inside our search path and do filename-only lookups to see if it’s there) and these functions started causing some problems because I was unable to do filename-only lookups when I needed to.
> 
> I attached a patch for FileSpec::Equal to this email. I’ll also send the target.exec-search-path stuff soon.
> 
> Does this make more sense?
> 
> 
> 
> -- 
> Stephane Sezer
> 
> On Oct 21, 2014, at 1:34 PM, <jingham at apple.com> <jingham at apple.com> wrote:
> 
> > I'm not sure about this fix.  The difference between true & false in the last argument to FileSpec::Equal only matters if one or the other paths is just a filename, but the other has a directory.  That shouldn't help when stepping from source file to source file because we'll only know whether the file is actually different if both have full paths.  So I'd want to know who is doing "is this file the same as that one" but only providing a base-name for one of the files.  That seems like the part that is wrong.
> > 
> > Jim
> > 
> > 
> >> On Oct 21, 2014, at 1:03 PM, Stephane Sezer <sas at fb.com> wrote:
> >> 
> >> Bump.
> >> 
> >> Could someone commit this for me? :)
> >> 
> >> -- 
> >> Stephane Sezer
> >> 
> >> On Oct 12, 2014, at 11:07 AM, Todd Fiala <tfiala at google.com> wrote:
> >> 
> >>> Hey Greg,
> >>> 
> >>> Stephane's patch looks reasonable here.  Can you have a quick look at it?  I can get it checked in if it looks okay to you.
> >>> 
> >>> Thanks,
> >>> Todd
> >>> 
> >>> On Wed, Oct 8, 2014 at 3:27 PM, Stephane Sezer <sas at fb.com> wrote:
> >>> When stepping into a function in a different file but with the same
> >>> basename, we do not reload the source file and end up displaying the
> >>> wrong snippet of code. This patch forces filename comparisons to be made
> >>> with a full path, which fixes the issue.
> >>> 
> >>> Ran tests on Mac OS.
> >>> 
> >>> 
> >>> -- 
> >>> Stephane Sezer
> >>> 
> >>> 
> >>> _______________________________________________
> >>> lldb-commits mailing list
> >>> lldb-commits at cs.uiuc.edu
> >>> https://urldefense.proofpoint.com/v1/url?u=http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=g1GoAnQQskSBaWLJWw6X6w%3D%3D%0A&m=c8clkd2pks3Uq2RmZ3FjBN9KqGEm1iIQ9%2FY3O1fdHTU%3D%0A&s=42f42ffb751e2c14fd9a60d42bc669b7cd6415c68d005314887bd157644ef557
> >>> 
> >>> 
> >>> 
> >>> 
> >>> -- 
> >>> Todd Fiala |         Software Engineer |      tfiala at google.com       
> >>> 
> >> 
> >> 
> >> _______________________________________________
> >> lldb-commits mailing list
> >> lldb-commits at cs.uiuc.edu
> >> https://urldefense.proofpoint.com/v1/url?u=http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=g1GoAnQQskSBaWLJWw6X6w%3D%3D%0A&m=c8clkd2pks3Uq2RmZ3FjBN9KqGEm1iIQ9%2FY3O1fdHTU%3D%0A&s=42f42ffb751e2c14fd9a60d42bc669b7cd6415c68d005314887bd157644ef557
> 
> <0001-Always-honor-the-full-argument-of-FileSpec-Equal.patch>





More information about the lldb-commits mailing list