[Lldb-commits] [lldb] r330385 - [DWARFASTParserClang] Remove dead code. NFCI.

2018-04-19 Thread Davide Italiano via lldb-commits
Author: davide
Date: Thu Apr 19 17:44:33 2018
New Revision: 330385

URL: http://llvm.org/viewvc/llvm-project?rev=330385=rev
Log:
[DWARFASTParserClang] Remove dead code. NFCI.

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=330385=330384=330385=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Thu Apr 
19 17:44:33 2018
@@ -180,9 +180,6 @@ TypeSP DWARFASTParserClang::ParseTypeFro
   lldb_private::CompilerType type =
   GetClangASTImporter().CopyType(m_ast, dwo_type);
 
-  // printf ("copied_qual_type: ast = %p, clang_type = %p, name =
-  // '%s'\n", m_ast, copied_qual_type.getAsOpaquePtr(),
-  // external_type->GetName().GetCString());
   if (!type)
 return TypeSP();
 
@@ -255,17 +252,6 @@ TypeSP DWARFASTParserClang::ParseTypeFro
   die.GetOffset(), static_cast(context),
   context_die.GetOffset(), die.GetTagAsCString(), die.GetName());
 }
-//
-//Log *log (LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO));
-//if (log && dwarf_cu)
-//{
-//StreamString s;
-//die->DumpLocation (this, dwarf_cu, s);
-//dwarf->GetObjectFile()->GetModule()->LogMessage (log,
-//"SymbolFileDwarf::%s %s", __FUNCTION__, s.GetData());
-//
-//}
-
 Type *type_ptr = dwarf->GetDIEToType().lookup(die.GetDIE());
 TypeList *type_list = dwarf->GetTypeList();
 if (type_ptr == NULL) {
@@ -553,17 +539,6 @@ TypeSP DWARFASTParserClang::ParseTypeFro
  , clang_type, resolve_state));
 
 dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
-
-//  Type* encoding_type =
-//  GetUniquedTypeForDIEOffset(encoding_uid, type_sp,
-//  NULL, 0, 0, false);
-//  if (encoding_type != NULL)
-//  {
-//  if (encoding_type != DIE_IS_BEING_PARSED)
-//  type_sp->SetEncodingType(encoding_type);
-//  else
-//  m_indirect_fixups.push_back(type_sp.get());
-//  }
   } break;
 
   case DW_TAG_structure_type:
@@ -578,7 +553,6 @@ TypeSP DWARFASTParserClang::ParseTypeFro
 size_t calling_convention 
 = llvm::dwarf::CallingConvention::DW_CC_normal;
 
-// bool struct_is_class = false;
 const size_t num_attributes = die.GetAttributes(attributes);
 if (num_attributes > 0) {
   uint32_t i;
@@ -3550,22 +3524,6 @@ size_t DWARFASTParserClang::ParseChildPa
   is_artificial = form_value.Boolean();
   break;
 case DW_AT_location:
-//  if (form_value.BlockData())
-//  {
-//  const DWARFDataExtractor&
-//  debug_info_data = debug_info();
-//  uint32_t block_length =
-//  form_value.Unsigned();
-//  DWARFDataExtractor
-//  location(debug_info_data,
-//  form_value.BlockData() -
-//  debug_info_data.GetDataStart(),
-//  block_length);
-//  }
-//  else
-//  {
-//  }
-//  break;
 case DW_AT_const_value:
 case DW_AT_default_value:
 case DW_AT_description:


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


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

2018-04-19 Thread Leonard Mosescu via lldb-commits
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  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  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=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 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=330313=330314=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:
> 

[Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps

2018-04-19 Thread Leonard Mosescu via Phabricator via lldb-commits
lemo added subscribers: bgianfo, penryu.
lemo added a comment.

> It looks like nobody except me is worried about the
>  module-without-an-object-file situation, so I guess we can try this out and
>  see how it goes.

Sorry Pavel, I meant to respond to this: most of the code seems to
explicitly handle this case (module-without-object-file), I just had to fix
a couple of cases. It's possible that more fixes will be required, but the
intention seems to be to accommodate for missing object files so at least
in this area I didn't have to break new ground.

I also considered creating placeholder object files, but it proved a bit
more intrusive since there are numerous places where it's assumed that
object files map to a real file which can be read and written to. Maybe at
some point we'll need to reconsider this (placeholder object files) but for
the initial iteration the placeholder modules seems to be sufficient. The
only downside I noticed is mostly cosmetic, for example things like "target
modules dump objfile" may print empty lines for the missing object files.

The test you've added here has been failing on windows though. I've tried

> to fix this in r330314, but it meant modifying the module.file.fullpath
>  output expectations. I'm not sure where you're going with the minidump
>  support, but if you are bothered by module.file.fullpath containing a
>  forward slash, you may want to look into fixing the SBFileSpec behavior.

Thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D45700



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps

2018-04-19 Thread Leonard Mosescu via lldb-commits
>
> It looks like nobody except me is worried about the
> module-without-an-object-file situation, so I guess we can try this out and
> see how it goes.
>

Sorry Pavel, I meant to respond to this: most of the code seems to
explicitly handle this case (module-without-object-file), I just had to fix
a couple of cases. It's possible that more fixes will be required, but the
intention seems to be to accommodate for missing object files so at least
in this area I didn't have to break new ground.

I also considered creating placeholder object files, but it proved a bit
more intrusive since there are numerous places where it's assumed that
object files map to a real file which can be read and written to. Maybe at
some point we'll need to reconsider this (placeholder object files) but for
the initial iteration the placeholder modules seems to be sufficient. The
only downside I noticed is mostly cosmetic, for example things like "target
modules dump objfile" may print empty lines for the missing object files.

The test you've added here has been failing on windows though. I've tried
> to fix this in r330314, but it meant modifying the module.file.fullpath
> output expectations. I'm not sure where you're going with the minidump
> support, but if you are bothered by module.file.fullpath containing a
> forward slash, you may want to look into fixing the SBFileSpec behavior.
>

Thanks!


On Thu, Apr 19, 2018 at 2:47 AM, Pavel Labath via Phabricator <
revi...@reviews.llvm.org> wrote:

> labath added a comment.
>
> It looks like nobody except me is worried about the
> module-without-an-object-file situation, so I guess we can try this out and
> see how it goes.
>
> The test you've added here has been failing on windows though. I've tried
> to fix this in r330314, but it meant modifying the module.file.fullpath
> output expectations. I'm not sure where you're going with the minidump
> support, but if you are bothered by module.file.fullpath containing a
> forward slash, you may want to look into fixing the SBFileSpec behavior.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D45700
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


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

2018-04-19 Thread Pavel Labath via lldb-commits
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  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=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 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=330313=330314=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=330313=330314=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,
);
> >
> >
> > ___
> > lldb-commits mailing list
> > lldb-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
___
lldb-commits 

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

2018-04-19 Thread Greg Clayton via lldb-commits
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 
>  wrote:
> 
> Author: labath
> Date: Thu Apr 19 02:38:42 2018
> New Revision: 330314
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=330314=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 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=330313=330314=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=330313=330314=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, 
> );
> 
> 
> ___
> 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


[Lldb-commits] [PATCH] D45700: Improve LLDB's handling of non-local minidumps

2018-04-19 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

It looks like nobody except me is worried about the 
module-without-an-object-file situation, so I guess we can try this out and see 
how it goes.

The test you've added here has been failing on windows though. I've tried to 
fix this in r330314, but it meant modifying the module.file.fullpath output 
expectations. I'm not sure where you're going with the minidump support, but if 
you are bothered by module.file.fullpath containing a forward slash, you may 
want to look into fixing the SBFileSpec behavior.


Repository:
  rL LLVM

https://reviews.llvm.org/D45700



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


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

2018-04-19 Thread Pavel Labath via lldb-commits
Author: labath
Date: Thu Apr 19 02:38:42 2018
New Revision: 330314

URL: http://llvm.org/viewvc/llvm-project?rev=330314=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 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=330313=330314=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=330313=330314=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, 
);


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits