[lldb-dev] FileSpec and normalization questions

[Greg Clayton via lldb-dev]

> On Apr 19, 2018, at 11:33 AM, Jim Ingham <jingham at apple.com> wrote:
> 
> I can't see any reason why we benefit from having these differently spelled equivalent paths floating around.  You want to be careful to preserve the path syntax, since it would be weird to be cross debugging to a Windows machine and have to type Posix paths.  But other than that, removing all the extraneous cruft seems goodness to me.

We translate all windows slashes all the time in any FileSpec, so it doesn't really matter what the user types.

> Were you proposing just that you get the DWARF parser to do this, or were you proposing that that become a requirement for SymbolFile parsers, so that we can drop all the code that normalizes paths from debug information before comparisons?  We still have to normalize paths from users, but it would be nice not to have to do it if we know the source is debug info.
> 

I was proposing to fix FileSpec::SetFile(), when false is passed for the "resolve" parameter, to always normalize all paths. Or I could just fix this inside of SymbolFileDWARF. I would rather do this at the "FileSpec" level since it will normalize our comparisons and just remove a ton of issue we can run into. So I vote for fixing FileSpec to "do the right thing".

Greg

> Jim
> 
> 
>> On Apr 19, 2018, at 11:14 AM, Greg Clayton via lldb-dev <lldb-dev at lists.llvm.org> wrote:
>> 
>> We currently have DWARF that has a DW_AT_comp_dir that is set to "./" followed by any number of extra '/' characters. I would like to have this path normalized as we parse the DWARF so we don't end up with line tables with a variety of ".//+" prefixes on each source file.
>> 
>> While looking to solve this issue, I took a look at the functionality that is in FileSpec right now. In:
>> 
>> void FileSpec::SetFile(llvm::StringRef pathname, bool resolve, PathSyntax syntax);
>> 
>> 
>> This function always calls a cheaper normalize function:
>> 
>> namespace {
>>  void Normalize(llvm::SmallVectorImpl<char> &path, FileSpec::PathSyntax syntax);
>> }
>> 
>> This function does nothing for posix paths, but switches backward slashes to forward slashes. 
>> 
>> We have a FileSpec function named FileSpec::GetNormalizedPath() which will do much more path normalization on a path by removing redundant "." and ".." and "//". 
>> 
>> I can fix my DWARF issue in a few ways:
>> 1 - fix FileSpec::SetFile() to still call ::Normalize() but have it do more work and have it normalize out the and redundant or relative path info
>> 2 - call FileSpec::GetNormalizedPath() on each comp dir before using it to actually normalize it
>> 
>> The main question is: do we want paths floating around LLDB that aren't normalized? It seems like having a mixture of theses path will lead to issues in LLDB so I would vote for solution #1.
>> 
>> Also, looking at the tests for normalizing paths I found the following pairs of pre-normalized and post-normalization paths for posix:
>> 
>>      {"//", "//"},
>>      {"//net", "//net"},
>> 
>> Why wouldn't we reduce "//" to just "/" for posix? And why wouldn't we reduce "//net" to "/net"?
>> 
>> Also I found:
>> 
>>      {"./foo", "foo"},
>> 
>> Do we prefer to not have "./foo" to stay as "./foo"? Seems like if users know that their debug info has line table entries that are "./foo/foo.c" that they might type this in:
>> 
>> (lldb) b ./foo/foo.c:12
>> 
>> But this will fail since it might not match the "foo/foo.c:12" that might result from path normalization. We don't normalize right now so it doesn't matter and things would match, but part of my fix is normalizing a path in the DWARF that is currently ".////////foo/foo.c" down to either "./foo/foo.c" or "foo/foo.c" so it will matter depending on what we decide here. 
>> 
>> Any input would be appreciated.
>> 
>> Greg Clayton
>> 
>> _______________________________________________
>> lldb-dev mailing list
>> lldb-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>