<div dir="ltr"><div>The mix of backward and forward slashes doesn't impact my current project but it would be nice to have a consistent path syntax (both within a single path and also cross platforms).</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">Leonard, is it reasonable to assume that all paths in the minidumps will be<br></span><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">absolute (and thus resolving is pointless anyway)?</span></blockquote><div><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br></span></div><div><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">Yes. Mostly :) For non-Windows minidumps the way we capture module names is a bit fuzzy (we depend on loader data structures and things like /proc/self/maps).</span></div><div><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br></span></div><div><span style="font-size:12.8px">What exactly does "resolving the path" means here? Breaking down into path components and re-assembling it doesn't seem particularly interesting.</span></div><div><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br></span></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">We should probably fix the </span><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">"fullpath" property to do something smarter in the future.</span></blockquote><div><br></div><div>I agree, it would be nice to avoid hard coding path separators. </div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Apr 19, 2018 at 9:51 AM, Pavel Labath <span dir="ltr"><<a href="mailto:labath@google.com" target="_blank">labath@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Yes, I noticed that as well, but I did not want to change it, as it wasn't<br>
related to the problem I was trying to fix. I agree that not resolving the<br>
path sounds like a better idea.<br>
<br>
Leonard, is it reasonable to assume that all paths in the minidumps will be<br>
absolute (and thus resolving is pointless anyway)?<br>
<div class="HOEnZb"><div class="h5">On Thu, 19 Apr 2018 at 17:20, Greg Clayton <<a href="mailto:clayborg@gmail.com">clayborg@gmail.com</a>> wrote:<br>
<br>
> Any reason we are trying to resolve the path with the 2nd true argument<br>
to FileSpec? If you still want to resolve the path, I would suggest<br>
checking if GetArchitecture() is compatible with any host architectures<br>
before passing true.<br>
<br>
<br>
> > On Apr 19, 2018, at 2:38 AM, Pavel Labath via lldb-commits <<br>
<a href="mailto:lldb-commits@lists.llvm.org">lldb-commits@lists.llvm.org</a>> wrote:<br>
> ><br>
> > Author: labath<br>
> > Date: Thu Apr 19 02:38:42 2018<br>
> > New Revision: 330314<br>
> ><br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=330314&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=330314&view=rev</a><br>
> > Log:<br>
> > Attempt to fix TestMiniDump on windows<br>
> ><br>
> > It was failing because the modules names were coming out as<br>
> > C:\Windows\System32/MSVCP120D.<wbr>dll (last separator is a forward slash) on<br>
> > windows.<br>
> ><br>
> > There are two issues at play here:<br>
> > - the first problem is that the paths in minidump were being parsed as a<br>
> >  host path. This meant that on posix systems the whole path was<br>
> >  interpreted as a file name.<br>
> > - on windows the path was split into a directory-filename pair<br>
> >  correctly, but then when it was reconsituted, the last separator ended<br>
> >  up being a forward slash because SBFileSpec.fullpath was joining them<br>
> >  with '/' unconditionally.<br>
> ><br>
> > I fix the first issue by parsing the minidump paths according to the<br>
> > path syntax of the host which produced the dump, which should make the<br>
> > test behavior on posix&windows identical. The last path will still be a<br>
> > forward slash because of the second issue. We should probably fix the<br>
> > "fullpath" property to do something smarter in the future.<br>
> ><br>
> > Modified:<br>
> ><br>
<br>
lldb/trunk/packages/Python/<wbr>lldbsuite/test/<wbr>functionalities/postmortem/<wbr>minidump/TestMiniDump.py<br>
> >    lldb/trunk/source/Plugins/<wbr>Process/minidump/<wbr>ProcessMinidump.cpp<br>
> ><br>
> > Modified:<br>
lldb/trunk/packages/Python/<wbr>lldbsuite/test/<wbr>functionalities/postmortem/<wbr>minidump/TestMiniDump.py<br>
> > URL:<br>
<a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump/TestMiniDump.py?rev=330314&r1=330313&r2=330314&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/lldb/trunk/packages/<wbr>Python/lldbsuite/test/<wbr>functionalities/postmortem/<wbr>minidump/TestMiniDump.py?rev=<wbr>330314&r1=330313&r2=330314&<wbr>view=diff</a><br>
> ><br>
==============================<wbr>==============================<wbr>==================<br>
> > ---<br>
lldb/trunk/packages/Python/<wbr>lldbsuite/test/<wbr>functionalities/postmortem/<wbr>minidump/TestMiniDump.py<br>
(original)<br>
> > +++<br>
lldb/trunk/packages/Python/<wbr>lldbsuite/test/<wbr>functionalities/postmortem/<wbr>minidump/TestMiniDump.py<br>
Thu Apr 19 02:38:42 2018<br>
> > @@ -49,12 +49,12 @@ class MiniDumpTestCase(TestBase):<br>
> >         self.process = self.target.LoadCore("<wbr>fizzbuzz_no_heap.dmp")<br>
> >         self.assertTrue(self.process, PROCESS_IS_VALID)<br>
> >         expected_modules = [<br>
> > -            r"C:\Windows\System32\<wbr>MSVCP120D.dll",<br>
> > -            r"C:\Windows\SysWOW64\<wbr>kernel32.dll",<br>
> > -            r"C:\Users\amccarth\Documents\<wbr>Visual Studio<br>
2013\Projects\fizzbuzz\Debug\<wbr>fizzbuzz.exe",<br>
> > -            r"C:\Windows\System32\<wbr>MSVCR120D.dll",<br>
> > -            r"C:\Windows\SysWOW64\<wbr>KERNELBASE.dll",<br>
> > -            r"C:\Windows\SysWOW64\ntdll.<wbr>dll",<br>
> > +            r"C:\Windows\System32/<wbr>MSVCP120D.dll",<br>
> > +            r"C:\Windows\SysWOW64/<wbr>kernel32.dll",<br>
> > +            r"C:\Users\amccarth\Documents\<wbr>Visual Studio<br>
2013\Projects\fizzbuzz\Debug/<wbr>fizzbuzz.exe",<br>
> > +            r"C:\Windows\System32/<wbr>MSVCR120D.dll",<br>
> > +            r"C:\Windows\SysWOW64/<wbr>KERNELBASE.dll",<br>
> > +            r"C:\Windows\SysWOW64/ntdll.<wbr>dll",<br>
> >         ]<br>
> >         self.assertEqual(self.target.<wbr>GetNumModules(),<br>
len(expected_modules))<br>
> >         for module, expected in zip(self.target.modules,<br>
expected_modules):<br>
> ><br>
> > Modified: lldb/trunk/source/Plugins/<wbr>Process/minidump/<wbr>ProcessMinidump.cpp<br>
> > URL:<br>
<a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp?rev=330314&r1=330313&r2=330314&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/lldb/trunk/source/<wbr>Plugins/Process/minidump/<wbr>ProcessMinidump.cpp?rev=<wbr>330314&r1=330313&r2=330314&<wbr>view=diff</a><br>
> ><br>
==============================<wbr>==============================<wbr>==================<br>
> > --- lldb/trunk/source/Plugins/<wbr>Process/minidump/<wbr>ProcessMinidump.cpp<br>
(original)<br>
> > +++ lldb/trunk/source/Plugins/<wbr>Process/minidump/<wbr>ProcessMinidump.cpp Thu<br>
Apr 19 02:38:42 2018<br>
> > @@ -321,7 +321,8 @@ void ProcessMinidump::<wbr>ReadModuleList() {<br>
> >       m_is_wow64 = true;<br>
> >     }<br>
> ><br>
> > -    const auto file_spec = FileSpec(name.getValue(), true);<br>
> > +    const auto file_spec =<br>
> > +        FileSpec(name.getValue(), true, GetArchitecture().GetTriple())<wbr>;<br>
> >     ModuleSpec module_spec = file_spec;<br>
> >     Status error;<br>
> >     lldb::ModuleSP module_sp = GetTarget().GetSharedModule(<wbr>module_spec,<br>
&error);<br>
> ><br>
> ><br>
> > ______________________________<wbr>_________________<br>
> > lldb-commits mailing list<br>
> > <a href="mailto:lldb-commits@lists.llvm.org">lldb-commits@lists.llvm.org</a><br>
> > <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/lldb-commits</a><br>
</div></div></blockquote></div><br></div>