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).
> Leonard, is it reasonable to assume that all paths in the minidumps will be > absolute (and thus resolving is pointless anyway)? 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). What exactly does "resolving the path" means here? Breaking down into path components and re-assembling it doesn't seem particularly interesting. We should probably fix the "fullpath" property to do something smarter in > the future. I agree, it would be nice to avoid hard coding path separators. On Thu, Apr 19, 2018 at 9:51 AM, Pavel Labath <lab...@google.com> wrote: > Yes, I noticed that as well, but I did not want to change it, as it wasn't > related to the problem I was trying to fix. I agree that not resolving the > path sounds like a better idea. > > Leonard, is it reasonable to assume that all paths in the minidumps will be > absolute (and thus resolving is pointless anyway)? > On Thu, 19 Apr 2018 at 17:20, Greg Clayton <clayb...@gmail.com> wrote: > > > Any reason we are trying to resolve the path with the 2nd true argument > to FileSpec? If you still want to resolve the path, I would suggest > checking if GetArchitecture() is compatible with any host architectures > before passing true. > > > > > On Apr 19, 2018, at 2:38 AM, Pavel Labath via lldb-commits < > lldb-commits@lists.llvm.org> wrote: > > > > > > Author: labath > > > Date: Thu Apr 19 02:38:42 2018 > > > New Revision: 330314 > > > > > > URL: http://llvm.org/viewvc/llvm-project?rev=330314&view=rev > > > Log: > > > Attempt to fix TestMiniDump on windows > > > > > > It was failing because the modules names were coming out as > > > C:\Windows\System32/MSVCP120D.dll (last separator is a forward slash) > on > > > windows. > > > > > > There are two issues at play here: > > > - the first problem is that the paths in minidump were being parsed as > a > > > host path. This meant that on posix systems the whole path was > > > interpreted as a file name. > > > - on windows the path was split into a directory-filename pair > > > correctly, but then when it was reconsituted, the last separator ended > > > up being a forward slash because SBFileSpec.fullpath was joining them > > > with '/' unconditionally. > > > > > > I fix the first issue by parsing the minidump paths according to the > > > path syntax of the host which produced the dump, which should make the > > > test behavior on posix&windows identical. The last path will still be a > > > forward slash because of the second issue. We should probably fix the > > > "fullpath" property to do something smarter in the future. > > > > > > Modified: > > > > > lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/ > minidump/TestMiniDump.py > > > lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp > > > > > > Modified: > lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/ > minidump/TestMiniDump.py > > > URL: > 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 > > > > ============================================================ > ================== > > > --- > lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/ > minidump/TestMiniDump.py > (original) > > > +++ > lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/ > minidump/TestMiniDump.py > Thu Apr 19 02:38:42 2018 > > > @@ -49,12 +49,12 @@ class MiniDumpTestCase(TestBase): > > > self.process = self.target.LoadCore("fizzbuzz_no_heap.dmp") > > > self.assertTrue(self.process, PROCESS_IS_VALID) > > > expected_modules = [ > > > - r"C:\Windows\System32\MSVCP120D.dll", > > > - r"C:\Windows\SysWOW64\kernel32.dll", > > > - r"C:\Users\amccarth\Documents\Visual Studio > 2013\Projects\fizzbuzz\Debug\fizzbuzz.exe", > > > - r"C:\Windows\System32\MSVCR120D.dll", > > > - r"C:\Windows\SysWOW64\KERNELBASE.dll", > > > - r"C:\Windows\SysWOW64\ntdll.dll", > > > + r"C:\Windows\System32/MSVCP120D.dll", > > > + r"C:\Windows\SysWOW64/kernel32.dll", > > > + r"C:\Users\amccarth\Documents\Visual Studio > 2013\Projects\fizzbuzz\Debug/fizzbuzz.exe", > > > + r"C:\Windows\System32/MSVCR120D.dll", > > > + r"C:\Windows\SysWOW64/KERNELBASE.dll", > > > + r"C:\Windows\SysWOW64/ntdll.dll", > > > ] > > > self.assertEqual(self.target.GetNumModules(), > len(expected_modules)) > > > for module, expected in zip(self.target.modules, > expected_modules): > > > > > > Modified: lldb/trunk/source/Plugins/Process/minidump/ > ProcessMinidump.cpp > > > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/ > Plugins/Process/minidump/ProcessMinidump.cpp?rev= > 330314&r1=330313&r2=330314&view=diff > > > > ============================================================ > ================== > > > --- lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp > (original) > > > +++ lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp Thu > Apr 19 02:38:42 2018 > > > @@ -321,7 +321,8 @@ void ProcessMinidump::ReadModuleList() { > > > m_is_wow64 = true; > > > } > > > > > > - const auto file_spec = FileSpec(name.getValue(), true); > > > + const auto file_spec = > > > + FileSpec(name.getValue(), true, GetArchitecture().GetTriple()) > ; > > > ModuleSpec module_spec = file_spec; > > > Status error; > > > lldb::ModuleSP module_sp = GetTarget().GetSharedModule( > module_spec, > &error); > > > > > > > > > _______________________________________________ > > > lldb-commits mailing list > > > lldb-commits@lists.llvm.org > > > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits >
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
