[Lldb-commits] [lldb] r330314 - Attempt to fix TestMiniDump on windows

Leonard Mosescu via lldb-commits lldb-commits at lists.llvm.org
Thu Apr 19 10:11:39 PDT 2018


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 <labath at 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 <clayborg at 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 at 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 at lists.llvm.org
> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20180419/13870f7c/attachment.html>


More information about the lldb-commits mailing list