[Lldb-commits] [lldb] r326081 - [Support] Replace HashString with djbHash.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Mon Feb 26 03:30:13 2018
New Revision: 326081

URL: http://llvm.org/viewvc/llvm-project?rev=326081&view=rev
Log:
[Support] Replace HashString with djbHash.

This removes the HashString function from StringExtraces and replaces
its uses with calls to djbHash from DJB.h

This is *almost* NFC. While the algorithm is identical, the djbHash
implementation in StringExtras used 0 as its seed while the
implementation in DJB uses 5381. The latter has been shown to result in
less collisions and improved avalanching.

https://reviews.llvm.org/D43615
(cherry picked from commit 77f7f965bc9499a9ae768a296ca5a1f7347d1d2c)

Modified:
lldb/trunk/source/Utility/ConstString.cpp

Modified: lldb/trunk/source/Utility/ConstString.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/ConstString.cpp?rev=326081&r1=326080&r2=326081&view=diff
==
--- lldb/trunk/source/Utility/ConstString.cpp (original)
+++ lldb/trunk/source/Utility/ConstString.cpp Mon Feb 26 03:30:13 2018
@@ -11,10 +11,10 @@
 
 #include "lldb/Utility/Stream.h"
 
-#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/iterator.h"// for iterator_facade_base
 #include "llvm/Support/Allocator.h"   // for BumpPtrAllocator
+#include "llvm/Support/DJB.h" // for djbHash
 #include "llvm/Support/FormatProviders.h" // for format_provider
 #include "llvm/Support/RWMutex.h"
 #include "llvm/Support/Threading.h"
@@ -171,7 +171,7 @@ public:
 
 protected:
   uint8_t hash(const llvm::StringRef &s) const {
-uint32_t h = llvm::HashString(s);
+uint32_t h = llvm::djbHash(s);
 return ((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff;
   }
 


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

[Lldb-commits] [lldb] r326082 - Revert "[Support] Replace HashString with djbHash."

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Mon Feb 26 04:05:18 2018
New Revision: 326082

URL: http://llvm.org/viewvc/llvm-project?rev=326082&view=rev
Log:
Revert "[Support] Replace HashString with djbHash."

It looks like some of our tests depend on the ordering of hashed values.
I'm reverting my changes while I try to reproduce and fix this locally.

Failing builds:

  lab.llvm.org:8011/builders/lld-x86_64-darwin13/builds/18388
  lab.llvm.org:8011/builders/clang-cmake-x86_64-sde-avx512-linux/builds/6743
  
lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/15607

Modified:
lldb/trunk/source/Utility/ConstString.cpp

Modified: lldb/trunk/source/Utility/ConstString.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/ConstString.cpp?rev=326082&r1=326081&r2=326082&view=diff
==
--- lldb/trunk/source/Utility/ConstString.cpp (original)
+++ lldb/trunk/source/Utility/ConstString.cpp Mon Feb 26 04:05:18 2018
@@ -11,10 +11,10 @@
 
 #include "lldb/Utility/Stream.h"
 
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/iterator.h"// for iterator_facade_base
 #include "llvm/Support/Allocator.h"   // for BumpPtrAllocator
-#include "llvm/Support/DJB.h" // for djbHash
 #include "llvm/Support/FormatProviders.h" // for format_provider
 #include "llvm/Support/RWMutex.h"
 #include "llvm/Support/Threading.h"
@@ -171,7 +171,7 @@ public:
 
 protected:
   uint8_t hash(const llvm::StringRef &s) const {
-uint32_t h = llvm::djbHash(s);
+uint32_t h = llvm::HashString(s);
 return ((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff;
   }
 


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

[Lldb-commits] [lldb] r326091 - Re-land: "[Support] Replace HashString with djbHash."

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Mon Feb 26 07:16:42 2018
New Revision: 326091

URL: http://llvm.org/viewvc/llvm-project?rev=326091&view=rev
Log:
Re-land: "[Support] Replace HashString with djbHash."

This patch removes the HashString function from StringExtraces and
replaces its uses with calls to djbHash from DJB.h.

This change is *almost* NFC. While the algorithm is identical, the
djbHash implementation in StringExtras used 0 as its default seed while
the implementation in DJB uses 5381. The latter has been shown to result
in less collisions and improved avalanching and is used by the DWARF
accelerator tables.

Because some test were implicitly relying on the hash order, I've
reverted to using zero as a seed for the following two files:

  lld/include/lld/Core/SymbolTable.h
  llvm/lib/Support/StringMap.cpp

Differential revision: https://reviews.llvm.org/D43615

Modified:
lldb/trunk/source/Utility/ConstString.cpp

Modified: lldb/trunk/source/Utility/ConstString.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/ConstString.cpp?rev=326091&r1=326090&r2=326091&view=diff
==
--- lldb/trunk/source/Utility/ConstString.cpp (original)
+++ lldb/trunk/source/Utility/ConstString.cpp Mon Feb 26 07:16:42 2018
@@ -11,10 +11,10 @@
 
 #include "lldb/Utility/Stream.h"
 
-#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/iterator.h"// for iterator_facade_base
 #include "llvm/Support/Allocator.h"   // for BumpPtrAllocator
+#include "llvm/Support/DJB.h" // for djbHash
 #include "llvm/Support/FormatProviders.h" // for format_provider
 #include "llvm/Support/RWMutex.h"
 #include "llvm/Support/Threading.h"
@@ -171,7 +171,7 @@ public:
 
 protected:
   uint8_t hash(const llvm::StringRef &s) const {
-uint32_t h = llvm::HashString(s);
+uint32_t h = llvm::djbHash(s);
 return ((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff;
   }
 


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

[Lldb-commits] [lldb] r326095 - Fix tabs/spaces indentation problem in TestUnicodeSymbols.py

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Mon Feb 26 07:53:31 2018
New Revision: 326095

URL: http://llvm.org/viewvc/llvm-project?rev=326095&view=rev
Log:
Fix tabs/spaces indentation problem in TestUnicodeSymbols.py

Differential Revision: https://reviews.llvm.org/D43705

Modified:

lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py?rev=326095&r1=326094&r2=326095&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py 
Mon Feb 26 07:53:31 2018
@@ -10,10 +10,10 @@ class TestUnicodeSymbols(TestBase):
 
 def test_union_members(self):
 self.build()
-   spec = lldb.SBModuleSpec()
-   spec.SetFileSpec(lldb.SBFileSpec(self.getBuildArtifact("a.out")))
-   module = lldb.SBModule(spec)
-   self.assertTrue(module.IsValid())
-   mytype = module.FindFirstType("foobár")
-   self.assertTrue(mytype.IsValid())
-   self.assertTrue(mytype.IsPointerType())
+spec = lldb.SBModuleSpec()
+spec.SetFileSpec(lldb.SBFileSpec(self.getBuildArtifact("a.out")))
+module = lldb.SBModule(spec)
+self.assertTrue(module.IsValid())
+mytype = module.FindFirstType("foobár")
+self.assertTrue(mytype.IsValid())
+self.assertTrue(mytype.IsPointerType())


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

[Lldb-commits] [PATCH] D43705: Fix tabs/spaces in TestUnicodeSymbols.py

[Adrian McCarthy via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL326095: Fix tabs/spaces indentation problem in 
TestUnicodeSymbols.py (authored by amccarth, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D43705?vs=135746&id=135905#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43705

Files:
  lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py


Index: 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py
@@ -10,10 +10,10 @@
 
 def test_union_members(self):
 self.build()
-   spec = lldb.SBModuleSpec()
-   spec.SetFileSpec(lldb.SBFileSpec(self.getBuildArtifact("a.out")))
-   module = lldb.SBModule(spec)
-   self.assertTrue(module.IsValid())
-   mytype = module.FindFirstType("foobár")
-   self.assertTrue(mytype.IsValid())
-   self.assertTrue(mytype.IsPointerType())
+spec = lldb.SBModuleSpec()
+spec.SetFileSpec(lldb.SBFileSpec(self.getBuildArtifact("a.out")))
+module = lldb.SBModule(spec)
+self.assertTrue(module.IsValid())
+mytype = module.FindFirstType("foobár")
+self.assertTrue(mytype.IsValid())
+self.assertTrue(mytype.IsPointerType())


Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py
@@ -10,10 +10,10 @@
 
 def test_union_members(self):
 self.build()
-	spec = lldb.SBModuleSpec()
-	spec.SetFileSpec(lldb.SBFileSpec(self.getBuildArtifact("a.out")))
-	module = lldb.SBModule(spec)
-	self.assertTrue(module.IsValid())
-	mytype = module.FindFirstType("foobár")
-	self.assertTrue(mytype.IsValid())
-	self.assertTrue(mytype.IsPointerType())
+spec = lldb.SBModuleSpec()
+spec.SetFileSpec(lldb.SBFileSpec(self.getBuildArtifact("a.out")))
+module = lldb.SBModule(spec)
+self.assertTrue(module.IsValid())
+mytype = module.FindFirstType("foobár")
+self.assertTrue(mytype.IsValid())
+self.assertTrue(mytype.IsPointerType())
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Greg Clayton via Phabricator via lldb-commits]

clayborg updated this revision to Diff 135913.
clayborg added a comment.
Herald added subscribers: JDevlieghere, eraman, arichardson, emaste.

Updated to top of tree sources and verified it passes all tests on linux.


https://reviews.llvm.org/D32167

Files:
  include/lldb/lldb-enumerations.h
  include/lldb/lldb-forward.h
  packages/Python/lldbsuite/test/lldbinline.py
  packages/Python/lldbsuite/test/lldbtest.py
  packages/Python/lldbsuite/test/make/Makefile.rules
  packages/Python/lldbsuite/test/plugins/builder_base.py
  packages/Python/lldbsuite/test/test_categories.py
  source/Core/Section.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  source/Plugins/SymbolFile/DWARF/DIERef.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserGo.cpp
  source/Plugins/SymbolFile/DWARF/DWARFAttribute.cpp
  source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
  source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.h
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Symbol/ObjectFile.cpp

Index: source/Symbol/ObjectFile.cpp
===
--- source/Symbol/ObjectFile.cpp
+++ source/Symbol/ObjectFile.cpp
@@ -360,6 +360,7 @@
   case eSectionTypeDWARFDebugRanges:
   case eSectionTypeDWARFDebugStr:
   case eSectionTypeDWARFDebugStrOffsets:
+  case eSectionTypeDWARFDebugTypes:
   case eSectionTypeDWARFAppleNames:
   case eSectionTypeDWARFAppleTypes:
   case eSectionTypeDWARFAppleNamespaces:
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -246,6 +246,7 @@
   const lldb_private::DWARFDataExtractor &get_debug_ranges_data();
   const lldb_private::DWARFDataExtractor &get_debug_str_data();
   const lldb_private::DWARFDataExtractor &get_debug_str_offsets_data();
+  const lldb_private::DWARFDataExtractor &get_debug_types_data();
   const lldb_private::DWARFDataExtractor &get_apple_names_data();
   const lldb_private::DWARFDataExtractor &get_apple_types_data();
   const lldb_private::DWARFDataExtractor &get_apple_namespaces_data();
@@ -491,6 +492,7 @@
   DWARFDataSegment m_data_debug_ranges;
   DWARFDataSegment m_data_debug_str;
   DWARFDataSegment m_data_debug_str_offsets;
+  DWARFDataSegment m_data_debug_types;
   DWARFDataSegment m_data_apple_names;
   DWARFDataSegment m_data_apple_types;
   DWARFDataSegment m_data_apple_namespaces;
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -501,21 +501,6 @@
 if (section_list == NULL)
   return 0;
 
-// On non Apple platforms we might have .debug_types debug info that
-// is created by using "-fdebug-types-section". LLDB currently will try
-// to load this debug info, but it causes crashes during debugging when
-// types are missing since it doesn't know how to parse the info in
-// the .debug_types type units. This causes all complex debug info
-// types to be unresolved. Because this causes LLDB to crash and since
-// it really doesn't provide a solid debuggiung experience, we should
-// disable trying to debug this kind of DWARF until support gets
-// added or deprecated.
-if (section_list->FindSectionByName(ConstString(".debug_types"))) {
-  m_obj_file->GetModule()->ReportWarning(
-"lldb doesn’t support .debug_types debug info");
-  return 0;
-}
-
 uint64_t debug_abbrev_file_size = 0;
 uint64_t debug_info_file_size = 0;
 uint64_t debug_line_file_size = 0;
@@ -591,8 +576,24 @@
 const DWARFDataExtractor &
 SymbolFileDWARF::GetCachedSectionData(lldb::SectionType sect_type,
   DWARFDataSegment &data_segment) {
-  llvm::call_once(data_segment.m_flag, [this, sect_type, &data_segment] {
+  llvm::call_once(data_segment.m_flag, [&] {
 this->LoadSectionData(sect_type, std::ref(data_segment.m_data));
+if (sect_type == eSectionTypeDWARFDebugTypes) {
+  // To add .debug_types support in DWARF 4 and earli

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Davide Italiano via Phabricator via lldb-commits]

davide requested changes to this revision.
davide added a comment.
This revision now requires changes to proceed.

This commit has no tests. It should have many. It's very big, so it could be 
split in several pieces.
I'd really appreciate if you can take the time to do so. For now, some comments.




Comment at: include/lldb/lldb-enumerations.h:659
// address
+  eSectionTypeDWARFDebugTypes,
   eSectionTypeOther

this needs a comment.



Comment at: packages/Python/lldbsuite/test/lldbtest.py:718
 self.getBuildDirBasename())
-
- 
+
+

spurious whitespaces, please revert.



Comment at: packages/Python/lldbsuite/test/lldbtest.py:1782
 x, target_platform, configuration.compiler)]
-
+
 if "dsym" in supported_categories:

ditto.



Comment at: source/Plugins/SymbolFile/DWARF/DIERef.cpp:50
 if (dwarf_cu) {
-  if (dwarf_cu->GetBaseObjOffset() != DW_INVALID_OFFSET)
-cu_offset = dwarf_cu->GetBaseObjOffset();
-  else
-cu_offset = dwarf_cu->GetOffset();
+  // Replace the compile unit with the type signature compile unit for
+  // type signature attributes.

Why? This needs to be explained in a comment.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h:64
   typedef std::vector CompileUnitColl;
-
+  typedef std::unordered_map TypeSignatureMap;
   //--

any reason why you can't use LLVM adt?


https://reviews.llvm.org/D32167



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

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In https://reviews.llvm.org/D32167#1019399, @davide wrote:

> This commit has no tests. It should have many.


I has full testing. Please read the commit and notice there is a new debug info 
flavor that is added. So of course it is tested.

> It's very big, so it could be split in several pieces.

This is one thing that goes together. It isn't that big. I adds support for 
.debug_types into the ObjectFile and parses the information. How would you 
propose breaking this up?

> I'd really appreciate if you can take the time to do so. For now, some 
> comments.

I believe I did take the time to test this properly. Let me know if you still 
think otherwise.




Comment at: packages/Python/lldbsuite/test/lldbtest.py:718
 self.getBuildDirBasename())
-
- 
+
+

davide wrote:
> spurious whitespaces, please revert.
It is removing spurious whitespaces and there are other changes in this file. 
Most editors these days will remove spurious spaces and we should strive to 
remove them where possibly no?



Comment at: packages/Python/lldbsuite/test/lldbtest.py:1782
 x, target_platform, configuration.compiler)]
-
+
 if "dsym" in supported_categories:

davide wrote:
> ditto.
Again, removing spurious whitespaces?



Comment at: packages/Python/lldbsuite/test/make/Makefile.rules:197
+ifeq "$(DWARF_TYPE_UNITS)" "YES"
+   DEBUG_INFO_FLAG ?= -gdwarf-4
+else

aprantl wrote:
> This shouldn't be necessary on any platform LLDB cares about. DWARF 4 should 
> be the default everywhere.
Ok, I can remove this.



Comment at: packages/Python/lldbsuite/test/plugins/builder_base.py:204
+commands.append([getMake(), "clean", getCmdLine(dictionary)])
+commands.append([getMake(), "MAKE_DSYM=NO", "DWARF_TYPE_UNITS=YES",
+getArchSpec( architecture), getCCSpec(compiler),

aprantl wrote:
> Why does the type unit configuration care about whether there are dsyms or 
> not? Shouldn't this be orthogonal?
All other "def buildXXX" functions do this, so I am following the convention.



Comment at: packages/Python/lldbsuite/test/test_categories.py:27
 'gmodules': 'Tests that can be run with -gmodules debug information',
+'dwarf_type_units' : 'Tests using the DWARF type units 
(-fdebug-types-section)',
 'expression': 'Tests related to the expression parser',

aprantl wrote:
> This is conflating two things (-fdebug-types-section and type units) DWARF5 
> doesn't have a debug_types section but it still offers type units. Clang 
> doesn't implement this yet, but GCC might, I haven't checked.
So what is the suggestion here?



Comment at: source/Plugins/SymbolFile/DWARF/DIERef.cpp:50
 if (dwarf_cu) {
-  if (dwarf_cu->GetBaseObjOffset() != DW_INVALID_OFFSET)
-cu_offset = dwarf_cu->GetBaseObjOffset();
-  else
-cu_offset = dwarf_cu->GetOffset();
+  // Replace the compile unit with the type signature compile unit for
+  // type signature attributes.

davide wrote:
> Why? This needs to be explained in a comment.
Will do



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.h:51
+  /// section as compile units and type units are in the .debug_info.
+  //--
+  void OffsetData(lldb::offset_t offset)

aprantl wrote:
> I think the //--- line might confuse Doxygen, but I'm not sure.
It doesn't. I checked the docs that show up in the doxygen in 
http://lldb.llvm.org/cpp_reference/html/index.html and these trailing comments 
don't hose it up.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp:121
+if (!cu_sp)
+  break;
+

aprantl wrote:
> Is that check common LLDB style?
I am copying the first loop.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h:64
   typedef std::vector CompileUnitColl;
-
+  typedef std::unordered_map TypeSignatureMap;
   //--

davide wrote:
> any reason why you can't use LLVM adt?
No reason. IMHO STL is fine unless there is a real reason llvm adt is better?


https://reviews.llvm.org/D32167



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

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

So, I tried this out on my machine, and got just one failure:

  ==
  FAIL: test_with_run_command_dwarf_type_units 
(TestInlinedBreakpoints.InlinedBreakpointsTestCase)
 Test 'b basic_types.cpp:176' does break (where int.cpp includes 
basic_type.cpp).
  --
  Traceback (most recent call last):
File 
"/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/lldbtest.py",
 line 1827, in dwarf_type_units_test_method
  return attrvalue(self)
File 
"/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/inlined_breakpoints/TestInlinedBreakpoints.py",
 line 24, in test_with_run_command
  self.inlined_breakpoints()
File 
"/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/inlined_breakpoints/TestInlinedBreakpoints.py",
 line 46, in inlined_breakpoints
  self, "basic_type.cpp", self.line, num_expected_locations=0)
File 
"/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/lldbutil.py",
 line 376, in run_break_set_by_file_and_line
  num_locations=num_expected_locations)
File 
"/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/lldbutil.py",
 line 572, in check_breakpoint_result
  out_num_locations))
  AssertionError: False is not True : Expecting 0 locations, got 1.

However, I am not so sure about the proliferation of debug info variants that 
we seem to be having. Right now we have two outstanding patches wanting to add 
a debug variant, which would bring the total amount of variants per test to 6. 
I'm not sure this is a tractable way forward.
IIUC, clang only puts "non-trivial" types in type units. I'm not sure how many 
of our tests even define classes/structs (i.e., for how many tests this debug 
info variant would actually be testing something new).




Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:194
+  //--
+  bool IsTypeUnit() const { return m_type_offset != DW_INVALID_OFFSET; }
+  //--

It seems weird to have an IsTypeUnit method on a compile unit, when a dwarf 
compile unit is clearly not a type unit. Would some of the refactors that Jan 
is doing (I haven't looked at those too closely as it's not really my area, but 
I know he is adding a DwarfUnit class) help with this?


https://reviews.llvm.org/D32167



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

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Ed Maste via Phabricator via lldb-commits]

emaste added inline comments.



Comment at: packages/Python/lldbsuite/test/make/Makefile.rules:197
+ifeq "$(DWARF_TYPE_UNITS)" "YES"
+   DEBUG_INFO_FLAG ?= -gdwarf-4
+else

clayborg wrote:
> aprantl wrote:
> > This shouldn't be necessary on any platform LLDB cares about. DWARF 4 
> > should be the default everywhere.
> Ok, I can remove this.
FreeBSD still defaults to DWARF2 at present.


https://reviews.llvm.org/D32167



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

[Lldb-commits] [lldb] r326112 - Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Feb 26 10:50:16 2018
New Revision: 326112

URL: http://llvm.org/viewvc/llvm-project?rev=326112&view=rev
Log:
Add "lldb-test breakpoint" command and convert the case-sensitivity test to use 
it

Summary:
The command takes two input arguments: a module to use as a debug target
and a file containing a list of commands. The command will execute each
of the breakpoint commands in the file and dump the breakpoint state
after each one.

The commands are expected to be breakpoint set/remove/etc. commands, but
I explicitly allow any lldb command here, so you can do things like
change setting which impact breakpoint resolution, etc. There is also a
"-persistent" flag, which causes lldb-test to *not* automatically clear
the breakpoint list after each command. Right now I don't use it, but
the idea behind it was that it could be used to test more complex
combinations of breakpoint commands (set+modify, set+disable, etc.).

Right now the command prints out only the basic breakpoint state, but
more information can be easily added there.  To enable easy matching of
the "at least one breakpoint location found" state, the command
explicitly prints out the string "At least one breakpoint location.".

To enable testing of breakpoints set with an absolute paths, I add the
ability to perform rudimentary substitutions on the commands: right now
the string %p is replaced by the directory which contains the command
file (so, under normal circumstances, this will perform the same
substitution as lit would do for %p).

I use this command to rewrite the TestBreakpointCaseSensitivity test --
the test was checking about a dozen breakpoint commands, but it was
launching a new process for each one, so it took about 90 seconds to
run. The new test takes about 0.3 seconds for me, which is approximately
a 300x speedup.

Reviewers: davide, zturner, jingham

Subscribers: luporl, lldb-commits

Differential Revision: https://reviews.llvm.org/D43686

Added:
lldb/trunk/lit/Breakpoint/
lldb/trunk/lit/Breakpoint/Inputs/
lldb/trunk/lit/Breakpoint/Inputs/case-sensitive.c
lldb/trunk/lit/Breakpoint/case-insensitive.test
lldb/trunk/lit/Breakpoint/case-sensitive.test
lldb/trunk/lit/Breakpoint/lit.local.cfg
Removed:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_case_sensitivity/Makefile

lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_case_sensitivity/TestBreakpointCaseSensitivity.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_case_sensitivity/main.c
Modified:
lldb/trunk/lit/lit.cfg
lldb/trunk/tools/lldb-test/lldb-test.cpp

Added: lldb/trunk/lit/Breakpoint/Inputs/case-sensitive.c
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Breakpoint/Inputs/case-sensitive.c?rev=326112&view=auto
==
--- lldb/trunk/lit/Breakpoint/Inputs/case-sensitive.c (added)
+++ lldb/trunk/lit/Breakpoint/Inputs/case-sensitive.c Mon Feb 26 10:50:16 2018
@@ -0,0 +1,4 @@
+int main() {
+  int x = 47; // REGEX-THIS
+  return x;
+}

Added: lldb/trunk/lit/Breakpoint/case-insensitive.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Breakpoint/case-insensitive.test?rev=326112&view=auto
==
--- lldb/trunk/lit/Breakpoint/case-insensitive.test (added)
+++ lldb/trunk/lit/Breakpoint/case-insensitive.test Mon Feb 26 10:50:16 2018
@@ -0,0 +1,44 @@
+# REQUIRES: windows
+#
+# RUN: %cc %p/Inputs/case-sensitive.c -g -o %t
+# RUN: lldb-test breakpoints %t %s | FileCheck %s
+
+breakpoint set -f case-sensitive.c -l 3
+# CHECK-LABEL: breakpoint set -f case-sensitive.c -l 3
+# CHECK: At least one location.
+
+breakpoint set -f %p/Inputs/case-sensitive.c -l 3
+# CHECK-LABEL: breakpoint set -f {{.*}}/Inputs/case-sensitive.c -l 3
+# CHECK: At least one location.
+
+breakpoint set -f %p/INPUTS/case-sensitive.c -l 3
+# CHECK-LABEL: breakpoint set -f {{.*}}/INPUTS/case-sensitive.c -l 3
+# CHECK: At least one location.
+
+breakpoint set -f Case-Sensitive.c -l 3
+# CHECK-LABEL: breakpoint set -f Case-Sensitive.c -l 3
+# CHECK: At least one location.
+
+breakpoint set -f %p/INPUTS/Case-Sensitive.c -l 3
+# CHECK-LABEL: breakpoint set -f {{.*}}/INPUTS/Case-Sensitive.c -l 3
+# CHECK: At least one location.
+
+breakpoint set -f case-sensitive.c -p REGEX-THIS
+# CHECK-LABEL: breakpoint set -f case-sensitive.c -p REGEX-THIS
+# CHECK: At least one location.
+
+breakpoint set -f %p/Inputs/case-sensitive.c -p REGEX-THIS
+# CHECK-LABEL: breakpoint set -f {{.*}}/Inputs/case-sensitive.c -p REGEX-THIS
+# CHECK: At least one location.
+
+breakpoint set -f %p/INPUTS/case-sensitive.c -p REGEX-THIS
+# CHECK-LABEL: breakpoint set -f {{.*}}/INPUTS/case-sensitive.c -p REGEX-THIS
+# CHECK: At least one location.
+
+breakpoint set -f Case-Sensitive.c -p REGEX-THIS
+# CHECK-LABEL: breakpoint set -f C

[Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL326112: Add "lldb-test breakpoint" command and 
convert the case-sensitivity test to use… (authored by labath, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D43686

Files:
  lldb/trunk/lit/Breakpoint/Inputs/case-sensitive.c
  lldb/trunk/lit/Breakpoint/case-insensitive.test
  lldb/trunk/lit/Breakpoint/case-sensitive.test
  lldb/trunk/lit/Breakpoint/lit.local.cfg
  lldb/trunk/lit/lit.cfg
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_case_sensitivity/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_case_sensitivity/TestBreakpointCaseSensitivity.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_case_sensitivity/main.c
  lldb/trunk/tools/lldb-test/lldb-test.cpp

Index: lldb/trunk/tools/lldb-test/lldb-test.cpp
===
--- lldb/trunk/tools/lldb-test/lldb-test.cpp
+++ lldb/trunk/tools/lldb-test/lldb-test.cpp
@@ -11,18 +11,22 @@
 #include "SystemInitializerTest.h"
 
 #include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
+#include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Initialization/SystemLifetimeManager.h"
+#include "lldb/Interpreter/CommandInterpreter.h"
+#include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Symbol/ClangASTContext.h"
 #include "lldb/Symbol/ClangASTImporter.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/StreamString.h"
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ManagedStatic.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include 
@@ -32,10 +36,30 @@
 using namespace llvm;
 
 namespace opts {
+static cl::SubCommand BreakpointSubcommand("breakpoints",
+   "Test breakpoint resolution");
 cl::SubCommand ModuleSubcommand("module-sections",
 "Display LLDB Module Information");
 cl::SubCommand SymbolsSubcommand("symbols", "Dump symbols for an object file");
 
+namespace breakpoint {
+static cl::opt Target(cl::Positional, cl::desc(""),
+   cl::Required, cl::sub(BreakpointSubcommand));
+static cl::opt CommandFile(cl::Positional,
+cl::desc(""),
+cl::init("-"),
+cl::sub(BreakpointSubcommand));
+static cl::opt Persistent(
+"persistent",
+cl::desc("Don't automatically remove all breakpoints before each command"),
+cl::sub(BreakpointSubcommand));
+
+static llvm::StringRef plural(uintmax_t value) { return value == 1 ? "" : "s"; }
+static void dumpState(const BreakpointList &List, LinePrinter &P);
+static std::string substitute(StringRef Cmd);
+static void evaluateBreakpoints(Debugger &Dbg);
+} // namespace breakpoint
+
 namespace module {
 cl::opt SectionContents("contents",
   cl::desc("Dump each section's contents"),
@@ -52,6 +76,101 @@
 
 static llvm::ManagedStatic DebuggerLifetime;
 
+void opts::breakpoint::dumpState(const BreakpointList &List, LinePrinter &P) {
+  P.formatLine("{0} breakpoint{1}", List.GetSize(), plural(List.GetSize()));
+  if (List.GetSize() > 0)
+P.formatLine("At least one breakpoint.");
+  for (size_t i = 0, e = List.GetSize(); i < e; ++i) {
+BreakpointSP BP = List.GetBreakpointAtIndex(i);
+P.formatLine("Breakpoint ID {0}:", BP->GetID());
+AutoIndent Indent(P, 2);
+P.formatLine("{0} location{1}.", BP->GetNumLocations(),
+ plural(BP->GetNumLocations()));
+if (BP->GetNumLocations() > 0)
+  P.formatLine("At least one location.");
+P.formatLine("{0} resolved location{1}.", BP->GetNumResolvedLocations(),
+ plural(BP->GetNumResolvedLocations()));
+if (BP->GetNumResolvedLocations() > 0)
+  P.formatLine("At least one resolved location.");
+for (size_t l = 0, le = BP->GetNumLocations(); l < le; ++l) {
+  BreakpointLocationSP Loc = BP->GetLocationAtIndex(l);
+  P.formatLine("Location ID {0}:", Loc->GetID());
+  AutoIndent Indent(P, 2);
+  P.formatLine("Enabled: {0}", Loc->IsEnabled());
+  P.formatLine("Resolved: {0}", Loc->IsResolved());
+  P.formatLine("Address: {0}+{1:x}",
+   Loc->GetAddress().GetSection()->GetName(),
+   Loc->GetAddress().GetOffset());
+}
+  }
+  P.NewLine();
+}
+
+std::string opts::breakpoint::substitute(StringRef Cmd) {
+  std::string Result;
+  raw_string_ostream OS(Result);
+  while (!Cmd.empty()) {
+switch (Cmd[0]) {
+case '%':
+  if (Cmd.consume_front("%p") && (Cmd.empty() || !isa

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In https://reviews.llvm.org/D32167#1019467, @labath wrote:

> So, I tried this out on my machine, and got just one failure:
>
>   ==
>   FAIL: test_with_run_command_dwarf_type_units 
> (TestInlinedBreakpoints.InlinedBreakpointsTestCase)
>  Test 'b basic_types.cpp:176' does break (where int.cpp includes 
> basic_type.cpp).
>   --
>   Traceback (most recent call last):
> File 
> "/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/lldbtest.py",
>  line 1827, in dwarf_type_units_test_method
>   return attrvalue(self)
> File 
> "/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/inlined_breakpoints/TestInlinedBreakpoints.py",
>  line 24, in test_with_run_command
>   self.inlined_breakpoints()
> File 
> "/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/inlined_breakpoints/TestInlinedBreakpoints.py",
>  line 46, in inlined_breakpoints
>   self, "basic_type.cpp", self.line, num_expected_locations=0)
> File 
> "/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/lldbutil.py",
>  line 376, in run_break_set_by_file_and_line
>   num_locations=num_expected_locations)
> File 
> "/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/lldbutil.py",
>  line 572, in check_breakpoint_result
>   out_num_locations))
>   AssertionError: False is not True : Expecting 0 locations, got 1.
>


I saw this too. When I ran _any_ of the variants manually, they all failed to 
set any breakpoints. Not sure this test is testing what it claims to be testing.

> However, I am not so sure about the proliferation of debug info variants that 
> we seem to be having. Right now we have two outstanding patches wanting to 
> add a debug variant, which would bring the total amount of variants per test 
> to 6. I'm not sure this is a tractable way forward.
>  IIUC, clang only puts "non-trivial" types in type units. I'm not sure how 
> many of our tests even define classes/structs (i.e., for how many tests this 
> debug info variant would actually be testing something new).

We could check after the build if the binary for the test even has a 
.debug_types sections and if not abort? That was we don't have to mark up the 
test in any way (like "this tests has structures or enums"), but we could 
ensure we test with .debug_types when needed.


https://reviews.llvm.org/D32167



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

Re: [Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

[Pavel Labath via lldb-commits]

On 23 February 2018 at 15:42, Jim Ingham  wrote:
> BTW, one thing I like about writing dotest.py tests is that it is easy to 
> craft fairly rich failure messages so if you get errors on systems you don't 
> have access to or are dealing with something that fails intermittently on a 
> bot somewhere, you have a hope of figuring out what went wrong.  Is this 
> possible with FileCheck tests?

I'm not sure this is what you had in mind, but for tests like this I
would try to do two things:
- dump out as much information as possible in the output: FileCheck
will display the snippet around the code it was not able to match, so
seeing that should give you some idea about what is broken
- make the test as hermetic as possible (i.e. avoid the situation
where it fails only on the buildbot in the first place). For example
for these tests we could disable dependent module loading/avoid
including any system headers, etc. to make sure that the test run
locally is as close to the run on the buildbots as possible.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In https://reviews.llvm.org/D32167#1019504, @clayborg wrote:

> In https://reviews.llvm.org/D32167#1019467, @labath wrote:
>
> > So, I tried this out on my machine, and got just one failure:
> >
> >   ==
> >   FAIL: test_with_run_command_dwarf_type_units 
> > (TestInlinedBreakpoints.InlinedBreakpointsTestCase)
> >  Test 'b basic_types.cpp:176' does break (where int.cpp includes 
> > basic_type.cpp).
> >   --
> >   Traceback (most recent call last):
> > File 
> > "/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/lldbtest.py",
> >  line 1827, in dwarf_type_units_test_method
> >   return attrvalue(self)
> > File 
> > "/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/inlined_breakpoints/TestInlinedBreakpoints.py",
> >  line 24, in test_with_run_command
> >   self.inlined_breakpoints()
> > File 
> > "/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/inlined_breakpoints/TestInlinedBreakpoints.py",
> >  line 46, in inlined_breakpoints
> >   self, "basic_type.cpp", self.line, num_expected_locations=0)
> > File 
> > "/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/lldbutil.py",
> >  line 376, in run_break_set_by_file_and_line
> >   num_locations=num_expected_locations)
> > File 
> > "/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/lldbutil.py",
> >  line 572, in check_breakpoint_result
> >   out_num_locations))
> >   AssertionError: False is not True : Expecting 0 locations, got 1.
> >
>
>
> I saw this too. When I ran _any_ of the variants manually, they all failed to 
> set any breakpoints. Not sure this test is testing what it claims to be 
> testing.


Actually I can repro this and will look into it


https://reviews.llvm.org/D32167



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

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D32167#1019504, @clayborg wrote:

> In https://reviews.llvm.org/D32167#1019467, @labath wrote:
>
> > However, I am not so sure about the proliferation of debug info variants 
> > that we seem to be having. Right now we have two outstanding patches 
> > wanting to add a debug variant, which would bring the total amount of 
> > variants per test to 6. I'm not sure this is a tractable way forward.
> >  IIUC, clang only puts "non-trivial" types in type units. I'm not sure how 
> > many of our tests even define classes/structs (i.e., for how many tests 
> > this debug info variant would actually be testing something new).
>
>
> We could check after the build if the binary for the test even has a 
> .debug_types sections and if not abort? That was we don't have to mark up the 
> test in any way (like "this tests has structures or enums"), but we could 
> ensure we test with .debug_types when needed.


That would help somewhat, but that's still a very "shotgun" approach to testing 
this. What I would try to do (and I think this is what Davide had in mind as 
well) is to identify the cases that are interesting from the type unit 
perspective (a single class in type unit, two classes in type units referencing 
each other, class referencing an enum, etc. (I'm not really sure about what all 
that goes into a type unit)) and then write specific tests for that (or reuse 
existing ones). For example, plenty of tests do breakpoint setting or unwinding 
and those probably don't have to be tested separately with type units (or maybe 
one test that checks that setting breakpoint on a method in type unit is 
sufficient).


https://reviews.llvm.org/D32167



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

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Right now, which tests are "run all variants" is represents more history than 
policy.  Opt out of all variants is opt in, and while I at least kick out 
obvious tests that don't really need to run all variants when I touch them, but 
I don't think we have done a careful audit.  I think the general idea of 
running variants is good, but we really should reduce the number of tests we 
run this way to the ones that actually test things that might break that way.

Note, I have (though very very occasionally) seen a dsymutil bug that broke 
line tables.  So it would be good to keep a few "set a breakpoint and run to 
it" tests just to sanity check this.  But most tests that just run somewhere 
and print a backtrace or view an integer variable or such-like do not need to 
run all variants.


https://reviews.llvm.org/D32167



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

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In https://reviews.llvm.org/D32167#1019621, @labath wrote:

> In https://reviews.llvm.org/D32167#1019504, @clayborg wrote:
>
> > In https://reviews.llvm.org/D32167#1019467, @labath wrote:
> >
> > > However, I am not so sure about the proliferation of debug info variants 
> > > that we seem to be having. Right now we have two outstanding patches 
> > > wanting to add a debug variant, which would bring the total amount of 
> > > variants per test to 6. I'm not sure this is a tractable way forward.
> > >  IIUC, clang only puts "non-trivial" types in type units. I'm not sure 
> > > how many of our tests even define classes/structs (i.e., for how many 
> > > tests this debug info variant would actually be testing something new).
> >
> >
> > We could check after the build if the binary for the test even has a 
> > .debug_types sections and if not abort? That was we don't have to mark up 
> > the test in any way (like "this tests has structures or enums"), but we 
> > could ensure we test with .debug_types when needed.
>
>
> That would help somewhat, but that's still a very "shotgun" approach to 
> testing this. What I would try to do (and I think this is what Davide had in 
> mind as well) is to identify the cases that are interesting from the type 
> unit perspective (a single class in type unit, two classes in type units 
> referencing each other, class referencing an enum, etc. (I'm not really sure 
> about what all that goes into a type unit)) and then write specific tests for 
> that (or reuse existing ones). For example, plenty of tests do breakpoint 
> setting or unwinding and those probably don't have to be tested separately 
> with type units (or maybe one test that checks that setting breakpoint on a 
> method in type unit is sufficient).


Precisely. I'm afraid what's proposed here is too coarse to be useful.


https://reviews.llvm.org/D32167



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

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D32167#1019635, @jingham wrote:

> Note, I have (though very very occasionally) seen a dsymutil bug that broke 
> line tables.  So it would be good to keep a few "set a breakpoint and run to 
> it" tests just to sanity check this.  But most tests that just run somewhere 
> and print a backtrace or view an integer variable or such-like do not need to 
> run all variants.


The thing here is that I'm not sure everything can be captured by a single 
"debug info variant" dimension. The current dimensions kinda make sense, as in 
they alter the way pretty much all of debug info is accessed (we can think of 
"dwarf" as the basic one, "dsym" and "dwo" are similar in that they place 
everything in a separate file, I'm not sure about gmodules, but that's because 
I have no idea on how it works). However, I'm not sure the same goes for type 
units. For one they are not completely mutually exclusive with the existing 
variants, so you can have type-units+regular dwarf and type units+dwo (maybe 
even type units+dsym?). But obviously we can't add another "type unit" 
dimension and do a cartesian product


https://reviews.llvm.org/D32167



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

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I am afraid of the opposite: we test what we think we need to test and our 
simple tests cases don't adequately test the feature we are adding. I can 
certainly limit the testing to very simple test cases with a one test for a 
class and one for a enum, but that wouldn't have caught the issues I ran into 
with static variables in classes that I fixed before submitting this. My point 
is it is hard to figure out what a compiler, or debugger might hose up without 
testing all possible cases. Please chime in with opinions as I will go with the 
flow on this.


https://reviews.llvm.org/D32167



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

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In https://reviews.llvm.org/D32167#1019701, @labath wrote:

> In https://reviews.llvm.org/D32167#1019635, @jingham wrote:
>
> > Note, I have (though very very occasionally) seen a dsymutil bug that broke 
> > line tables.  So it would be good to keep a few "set a breakpoint and run 
> > to it" tests just to sanity check this.  But most tests that just run 
> > somewhere and print a backtrace or view an integer variable or such-like do 
> > not need to run all variants.
>
>
> The thing here is that I'm not sure everything can be captured by a single 
> "debug info variant" dimension. The current dimensions kinda make sense, as 
> in they alter the way pretty much all of debug info is accessed (we can think 
> of "dwarf" as the basic one, "dsym" and "dwo" are similar in that they place 
> everything in a separate file, I'm not sure about gmodules, but that's 
> because I have no idea on how it works). However, I'm not sure the same goes 
> for type units. For one they are not completely mutually exclusive with the 
> existing variants, so you can have type-units+regular dwarf and type 
> units+dwo (maybe even type units+dsym?). But obviously we can't add another 
> "type unit" dimension and do a cartesian product


if you see how horrible type units are in their implementation, you might 
change your mind. If one has a positive view of type units, you might think "I 
get one type and one type only and that can only mean good things". I reality 
this feature's implementation is really messy and there are so many areas that 
can screw up. For instance if you have a class that has static members, you 
will get a nice full type in the type unit that defines everything in the 
class. But with type units you can't refer to anything within the type unit 
itself other than the top level type itself (the struct/union/class or enum). 
So you now need to declare a DW_TAG_variable that points to the static member 
of the class so you can give it an address... What happens? The type is 
duplicated in each compile unit that uses it and a skeleton incomplete version 
of the class is represented in the compile unit DWARF (not in a type unit). The 
top level DW_TAG_structure or DW_TAG_class has a DW_AT_signature that points to 
the type in the type unit, but the incomplete struct/class has _just_ the 
static members inside of it (again, in the compile unit, not in the type unit). 
Then you have a DW_TAG_variable that points to the DW_TAG_member within the 
incomplete struct or class because there is no way to point to anything within 
the type if the type is in a type unit. So I believe that we do need to fully 
test types and run the .debug_types as a separate variant. Each compiler might 
also do things a bit differently as there is nothing in the .debug_types 
specification that says that this is how things need to be. And that is just 
one thing I found while debugging .debug_types... I am sure there are more 
skeletons in the closet. If we want to support this feature we need to test it. 
I come to this conclusion after years of dealing with many different "this 
DWARF does this cool new way to do things" where it has bit us in the past and 
things break.


https://reviews.llvm.org/D32167



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

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Vedant Kumar via Phabricator via lldb-commits]

vsk added a comment.

I'm concerned about the time it takes to run the test suite. If we can get 
decent test coverage of this patch without adding a new debug info flavor, 
that'd be ideal.




Comment at: packages/Python/lldbsuite/test/lldbtest.py:718
 self.getBuildDirBasename())
-
- 
+
+

clayborg wrote:
> davide wrote:
> > spurious whitespaces, please revert.
> It is removing spurious whitespaces and there are other changes in this file. 
> Most editors these days will remove spurious spaces and we should strive to 
> remove them where possibly no?
No, we generally avoid making changes that are unrelated to the patch, as they 
make patch review and subsequent merges harder.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.h:58
+  m_start -= offset;
+  }
+  

Please clang-format your diffs.


https://reviews.llvm.org/D32167



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

[Lldb-commits] [lldb] r326130 - Partial fix for TestConflictingSymbol.py on Windows

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Mon Feb 26 13:22:39 2018
New Revision: 326130

URL: http://llvm.org/viewvc/llvm-project?rev=326130&view=rev
Log:
Partial fix for TestConflictingSymbol.py on Windows

Without this fix, the test ERRORs because the link of the inferior fails. This
patch adds the LLDB_TEST_API macro where needed and uses the new -2 magic
value for num_expected_locations to account for lazy-loading of module symbols
on Windows.

With this fix, the test itself still fails:  conflicting_symbol isn't in the
debug info nor the export table, and Windows binaries don't have an equivalent
of the ELF .symtab.  We need to understand why the test works to keep the
symbol out of the debug info.  In the mean time, having the test fail at this
point is a better indication of the remaining problem than a build error.

Differential Revision: https://reviews.llvm.org/D43688

Modified:

lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h

lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py

lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h?rev=326130&r1=326129&r2=326130&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h 
Mon Feb 26 13:22:39 2018
@@ -1,4 +1,4 @@
 #ifndef ONE_H
 #define ONE_H
-void one();
+LLDB_TEST_API void one();
 #endif

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py?rev=326130&r1=326129&r2=326130&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
 Mon Feb 26 13:22:39 2018
@@ -33,9 +33,9 @@ class TestConflictingSymbols(TestBase):
 target, ['One', 'Two'])
 
 lldbutil.run_break_set_by_source_regexp(self, '// break here',
-extra_options='-f One.c')
+extra_options='-f One.c', num_expected_locations=-2)
 lldbutil.run_break_set_by_source_regexp(self, '// break here',
-extra_options='-f Two.c')
+extra_options='-f Two.c', num_expected_locations=-2)
 lldbutil.run_break_set_by_source_regexp(self, '// break here',
 extra_options='-f main.c', num_expected_locations=1)
 

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h?rev=326130&r1=326129&r2=326130&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h 
Mon Feb 26 13:22:39 2018
@@ -1,4 +1,4 @@
 #ifndef TWO_H
 #define TWO_H
-void two();
+LLDB_TEST_API void two();
 #endif


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

[Lldb-commits] [PATCH] D43688: Partial fix for TestConflictingSymbol.py on Windows

[Adrian McCarthy via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL326130: Partial fix for TestConflictingSymbol.py on Windows 
(authored by amccarth, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D43688?vs=135668&id=135964#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43688

Files:
  lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
  
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
  lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h


Index: 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
@@ -33,9 +33,9 @@
 target, ['One', 'Two'])
 
 lldbutil.run_break_set_by_source_regexp(self, '// break here',
-extra_options='-f One.c')
+extra_options='-f One.c', num_expected_locations=-2)
 lldbutil.run_break_set_by_source_regexp(self, '// break here',
-extra_options='-f Two.c')
+extra_options='-f Two.c', num_expected_locations=-2)
 lldbutil.run_break_set_by_source_regexp(self, '// break here',
 extra_options='-f main.c', num_expected_locations=1)
 
Index: 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
@@ -1,4 +1,4 @@
 #ifndef TWO_H
 #define TWO_H
-void two();
+LLDB_TEST_API void two();
 #endif
Index: 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
@@ -1,4 +1,4 @@
 #ifndef ONE_H
 #define ONE_H
-void one();
+LLDB_TEST_API void one();
 #endif


Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
@@ -33,9 +33,9 @@
 target, ['One', 'Two'])
 
 lldbutil.run_break_set_by_source_regexp(self, '// break here',
-extra_options='-f One.c')
+extra_options='-f One.c', num_expected_locations=-2)
 lldbutil.run_break_set_by_source_regexp(self, '// break here',
-extra_options='-f Two.c')
+extra_options='-f Two.c', num_expected_locations=-2)
 lldbutil.run_break_set_by_source_regexp(self, '// break here',
 extra_options='-f main.c', num_expected_locations=1)
 
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
@@ -1,4 +1,4 @@
 #ifndef TWO_H
 #define TWO_H
-void two();
+LLDB_TEST_API void two();
 #endif
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
@@ -1,4 +1,4 @@
 #ifndef ONE_H
 #define ONE_H
-void one();
+LLDB_TEST_API void one();
 #endif
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43784: Un-XFAIL TestCallStdStringFunction.test for Windows

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth created this revision.
amccarth added a reviewer: zturner.
Herald added a subscriber: sanjoy.

This test was passing unexpectedly, which causes ninja to short-circuit out of 
running the dotest.py tests.

The bug report (https://bugs.llvm.org/show_bug.cgi?id=21765) appears to be 
obsolete (since it "blocks" another bug that's been fixed).


https://reviews.llvm.org/D43784

Files:
  lldb/lit/Expr/TestCallStdStringFunction.test


Index: lldb/lit/Expr/TestCallStdStringFunction.test
===
--- lldb/lit/Expr/TestCallStdStringFunction.test
+++ lldb/lit/Expr/TestCallStdStringFunction.test
@@ -1,6 +1,3 @@
-# XFAIL: windows
-# -> llvm.org/pr21765
-
 # RUN: %cxx %p/Inputs/call-function.cpp -g -o %t && %lldb -b -s %s -- %t | 
FileCheck %s
 
 breakpoint set --file call-function.cpp --line 52


Index: lldb/lit/Expr/TestCallStdStringFunction.test
===
--- lldb/lit/Expr/TestCallStdStringFunction.test
+++ lldb/lit/Expr/TestCallStdStringFunction.test
@@ -1,6 +1,3 @@
-# XFAIL: windows
-# -> llvm.org/pr21765
-
 # RUN: %cxx %p/Inputs/call-function.cpp -g -o %t && %lldb -b -s %s -- %t | FileCheck %s
 
 breakpoint set --file call-function.cpp --line 52
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43784: Un-XFAIL TestCallStdStringFunction.test for Windows

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Btw, today I found out that this test is passing just accidentally because the 
CHECK lines are matching the output printed by the inferior and not the results 
of the evaluated expressions (the situation may be different on windows, if you 
don't do stdin/out redirection there).


https://reviews.llvm.org/D43784



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

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 135971.
labath added a comment.
Herald added subscribers: JDevlieghere, mgorny.

This version moves the logic for the building of unified section list into the
symbol vendor plugin.

The idea was to keep the construction of section lists of individual object
files inside the ObjectFile object, and then have the SymbolVendor do the
merging. This was easy to achieve for the SymbolVendorELF, and I'm quite happy
about how things look like there.

However, I had a lot of trouble with making this work for the MachO
objectfile+symbolvendor combo, because there the section building is
intertwined with the load command parsing and the building of the unified
sections list. In particular, the parse function seems to be doing some dodgy
things like taking a section from the "unified" list and inserting it into
per-object-file list. So, here I had to leave the building of the unified list
in the ObjectFile class, and the SymbolVendor just orchestrates the functions
to be called in the right order (for this I needed to cast the ObjectFile into
ObjectFileMachO, which seemingly adds a new dependency to the symbol vendor,
but this is not really true, as the vendor was already checking the type of the
object file via the GetPluginName() function). It would be great if someone
with more MachO knowledge could look at the CreateSections function and split
it up into more manageable chunks.

The other sub-optimal design decision I had to make was due to the mutual
recursion of the Module, SymbolVendor and SymbolFile classes: SymbolVendor
creates a SymbolFile class, which expect that the Module's section list is
already constructed, but SymbolVendor is the one constructing the list. This
means that I couldn't just construct the SymbolVendor and have the Module ask
it for the section list, but I had to make the vendor directly update the
section list within the module (before constructing the symbolfile class).

On the plus side, I really like that I was able to remove the
"update_module_section_list" argument from the ObjectFile::GetSectionList
function, which was very dodgy as well, because whether the module's section
list was updated, depended on the value of the argument of the *first* call of
that function.

All in all, I think patch cleans up the unified section list handling, but only
by a small bit. Let me know what you think.


https://reviews.llvm.org/D42955

Files:
  include/lldb/Core/Module.h
  include/lldb/Symbol/ObjectFile.h
  include/lldb/Symbol/SymbolVendor.h
  include/lldb/lldb-private-interfaces.h
  lit/Modules/Inputs/stripped.yaml
  lit/Modules/Inputs/unstripped.yaml
  lit/Modules/lit.local.cfg
  lit/Modules/unified-section-list.test
  lit/lit.cfg
  source/Core/Module.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.h
  source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
  source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
  source/Plugins/SymbolVendor/CMakeLists.txt
  source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
  source/Plugins/SymbolVendor/ELF/SymbolVendorELF.h
  source/Plugins/SymbolVendor/MacOSX/CMakeLists.txt
  source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
  source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.h
  source/Symbol/ObjectFile.cpp
  source/Symbol/SymbolFile.cpp
  source/Symbol/SymbolVendor.cpp
  tools/lldb-test/lldb-test.cpp

Index: tools/lldb-test/lldb-test.cpp
===
--- tools/lldb-test/lldb-test.cpp
+++ tools/lldb-test/lldb-test.cpp
@@ -72,7 +72,6 @@
 
   for (const auto &File : opts::module::InputFilenames) {
 ModuleSpec Spec{FileSpec(File, false)};
-Spec.GetSymbolFileSpec().SetFile(File, false);
 
 auto ModulePtr = std::make_shared(Spec);
 SectionList *Sections = ModulePtr->GetSectionList();
Index: source/Symbol/SymbolVendor.cpp
===
--- source/Symbol/SymbolVendor.cpp
+++ source/Symbol/SymbolVendor.cpp
@@ -15,6 +15,7 @@
 // Project includes
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Section.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolFile.h"
@@ -31,15 +32,16 @@
 // also allow for finding separate debug information files.
 //--
 SymbolVendor *SymbolVendor::FindPlugin(const lldb::ModuleSP &module_sp,
+   SectionList &unified_list,
lldb_private::Stream *feedback_strm) {
   std::un

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 135975.
labath added a comment.

Remove the changes to source/Plugins/SymbolVendor/CMakeLists.txt that snuck in
(I was experimenting with enabling the plugin on non-mac systems, but realized
that's not possible right now).


https://reviews.llvm.org/D42955

Files:
  include/lldb/Core/Module.h
  include/lldb/Symbol/ObjectFile.h
  include/lldb/Symbol/SymbolVendor.h
  include/lldb/lldb-private-interfaces.h
  lit/Modules/Inputs/stripped.yaml
  lit/Modules/Inputs/unstripped.yaml
  lit/Modules/lit.local.cfg
  lit/Modules/unified-section-list.test
  lit/lit.cfg
  source/Core/Module.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.h
  source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp
  source/Plugins/ObjectFile/JIT/ObjectFileJIT.h
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
  source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
  source/Plugins/SymbolVendor/ELF/SymbolVendorELF.h
  source/Plugins/SymbolVendor/MacOSX/CMakeLists.txt
  source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
  source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.h
  source/Symbol/ObjectFile.cpp
  source/Symbol/SymbolFile.cpp
  source/Symbol/SymbolVendor.cpp
  tools/lldb-test/lldb-test.cpp

Index: tools/lldb-test/lldb-test.cpp
===
--- tools/lldb-test/lldb-test.cpp
+++ tools/lldb-test/lldb-test.cpp
@@ -72,7 +72,6 @@
 
   for (const auto &File : opts::module::InputFilenames) {
 ModuleSpec Spec{FileSpec(File, false)};
-Spec.GetSymbolFileSpec().SetFile(File, false);
 
 auto ModulePtr = std::make_shared(Spec);
 SectionList *Sections = ModulePtr->GetSectionList();
Index: source/Symbol/SymbolVendor.cpp
===
--- source/Symbol/SymbolVendor.cpp
+++ source/Symbol/SymbolVendor.cpp
@@ -15,6 +15,7 @@
 // Project includes
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Section.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolFile.h"
@@ -31,15 +32,16 @@
 // also allow for finding separate debug information files.
 //--
 SymbolVendor *SymbolVendor::FindPlugin(const lldb::ModuleSP &module_sp,
+   SectionList &unified_list,
lldb_private::Stream *feedback_strm) {
   std::unique_ptr instance_ap;
   SymbolVendorCreateInstance create_callback;
 
   for (size_t idx = 0;
(create_callback = PluginManager::GetSymbolVendorCreateCallbackAtIndex(
 idx)) != nullptr;
++idx) {
-instance_ap.reset(create_callback(module_sp, feedback_strm));
+instance_ap.reset(create_callback(module_sp, unified_list, feedback_strm));
 
 if (instance_ap.get()) {
   return instance_ap.release();
@@ -51,7 +53,9 @@
   if (instance_ap.get()) {
 ObjectFile *objfile = module_sp->GetObjectFile();
 if (objfile)
-  instance_ap->AddSymbolFileRepresentation(objfile->shared_from_this());
+  unified_list = *objfile->GetSectionList();
+
+instance_ap->AddSymbolFileRepresentation(objfile->shared_from_this());
   }
   return instance_ap.release();
 }
Index: source/Symbol/SymbolFile.cpp
===
--- source/Symbol/SymbolFile.cpp
+++ source/Symbol/SymbolFile.cpp
@@ -28,21 +28,6 @@
 SymbolFile *SymbolFile::FindPlugin(ObjectFile *obj_file) {
   std::unique_ptr best_symfile_ap;
   if (obj_file != nullptr) {
-
-// We need to test the abilities of this section list. So create what it
-// would
-// be with this new obj_file.
-lldb::ModuleSP module_sp(obj_file->GetModule());
-if (module_sp) {
-  // Default to the main module section list.
-  ObjectFile *module_obj_file = module_sp->GetObjectFile();
-  if (module_obj_file != obj_file) {
-// Make sure the main object file's sections are created
-module_obj_file->GetSectionList();
-obj_file->CreateSections(*module_sp->GetUnifiedSectionList());
-  }
-}
-
 // TODO: Load any plug-ins in the appropriate plug-in search paths and
 // iterate over all of them to find the best one for the job.
 
Index: source/Symbol/ObjectFile.cpp
===
--- source/Symbol/ObjectFile.cpp
+++ source/Symbol/ObjectFile.cpp
@@ -604,22 +604,6 @@
   }
 }
 
-SectionList *ObjectFile::GetSectionList(bool update_module_section_list) {
-  if (m_sections_ap.get() == nullptr) {
-if (update_module_section

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Greg Clayton via Phabricator via lldb-commits]

clayborg updated this revision to Diff 135977.
clayborg added a comment.

clang format
fixed comments
removed white space
fixed functionalities/breakpoint/inlined_breakpoints/TestInlinedBreakpoints.py 
failures


https://reviews.llvm.org/D32167

Files:
  include/lldb/lldb-enumerations.h
  include/lldb/lldb-forward.h
  packages/Python/lldbsuite/test/lldbinline.py
  packages/Python/lldbsuite/test/lldbtest.py
  packages/Python/lldbsuite/test/make/Makefile.rules
  packages/Python/lldbsuite/test/plugins/builder_base.py
  packages/Python/lldbsuite/test/test_categories.py
  source/Core/Section.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  source/Plugins/SymbolFile/DWARF/DIERef.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserGo.cpp
  source/Plugins/SymbolFile/DWARF/DWARFAttribute.cpp
  source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
  source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.h
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Symbol/ObjectFile.cpp

Index: source/Symbol/ObjectFile.cpp
===
--- source/Symbol/ObjectFile.cpp
+++ source/Symbol/ObjectFile.cpp
@@ -360,6 +360,7 @@
   case eSectionTypeDWARFDebugRanges:
   case eSectionTypeDWARFDebugStr:
   case eSectionTypeDWARFDebugStrOffsets:
+  case eSectionTypeDWARFDebugTypes:
   case eSectionTypeDWARFAppleNames:
   case eSectionTypeDWARFAppleTypes:
   case eSectionTypeDWARFAppleNamespaces:
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -246,6 +246,7 @@
   const lldb_private::DWARFDataExtractor &get_debug_ranges_data();
   const lldb_private::DWARFDataExtractor &get_debug_str_data();
   const lldb_private::DWARFDataExtractor &get_debug_str_offsets_data();
+  const lldb_private::DWARFDataExtractor &get_debug_types_data();
   const lldb_private::DWARFDataExtractor &get_apple_names_data();
   const lldb_private::DWARFDataExtractor &get_apple_types_data();
   const lldb_private::DWARFDataExtractor &get_apple_namespaces_data();
@@ -491,6 +492,7 @@
   DWARFDataSegment m_data_debug_ranges;
   DWARFDataSegment m_data_debug_str;
   DWARFDataSegment m_data_debug_str_offsets;
+  DWARFDataSegment m_data_debug_types;
   DWARFDataSegment m_data_apple_names;
   DWARFDataSegment m_data_apple_types;
   DWARFDataSegment m_data_apple_namespaces;
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -501,21 +501,6 @@
 if (section_list == NULL)
   return 0;
 
-// On non Apple platforms we might have .debug_types debug info that
-// is created by using "-fdebug-types-section". LLDB currently will try
-// to load this debug info, but it causes crashes during debugging when
-// types are missing since it doesn't know how to parse the info in
-// the .debug_types type units. This causes all complex debug info
-// types to be unresolved. Because this causes LLDB to crash and since
-// it really doesn't provide a solid debuggiung experience, we should
-// disable trying to debug this kind of DWARF until support gets
-// added or deprecated.
-if (section_list->FindSectionByName(ConstString(".debug_types"))) {
-  m_obj_file->GetModule()->ReportWarning(
-"lldb doesn’t support .debug_types debug info");
-  return 0;
-}
-
 uint64_t debug_abbrev_file_size = 0;
 uint64_t debug_info_file_size = 0;
 uint64_t debug_line_file_size = 0;
@@ -591,8 +576,24 @@
 const DWARFDataExtractor &
 SymbolFileDWARF::GetCachedSectionData(lldb::SectionType sect_type,
   DWARFDataSegment &data_segment) {
-  llvm::call_once(data_segment.m_flag, [this, sect_type, &data_segment] {
+  llvm::call_once(data_segment.m_flag, [&] {
 this->LoadSectionData(sect_type, std::ref(data_segment.m_data));
+if (sect_type == eSectionTypeDWARFDebugTypes) {
+  // To add .debug_types support in DWARF 4 and earlier wit

[Lldb-commits] [lldb] r326134 - Mark test_*int*_t_dwarf as failing on FreeBSD

[Ed Maste via lldb-commits]

Author: emaste
Date: Mon Feb 26 14:12:24 2018
New Revision: 326134

URL: http://llvm.org/viewvc/llvm-project?rev=326134&view=rev
Log:
Mark test_*int*_t_dwarf as failing on FreeBSD

Further investigation required; tests will be enabled on the buildbot
worker soon. Marking failing tests for now in order to start with a
green buildbot while investigation takes place.

llvm.org/pr36527

Modified:

lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/enum_types/TestCPP11EnumTypes.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/enum_types/TestCPP11EnumTypes.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/enum_types/TestCPP11EnumTypes.py?rev=326134&r1=326133&r2=326134&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/enum_types/TestCPP11EnumTypes.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/enum_types/TestCPP11EnumTypes.py
 Mon Feb 26 14:12:24 2018
@@ -14,6 +14,7 @@ class CPP11EnumTypesTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
+@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_int8_t(self):
 """Test C++11 enumeration class types as int8_t types."""
 self.build(
@@ -21,6 +22,7 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=int8_t"'})
 self.image_lookup_for_enum_type()
 
+@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_int16_t(self):
 """Test C++11 enumeration class types as int16_t types."""
 self.build(
@@ -28,6 +30,7 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=int16_t"'})
 self.image_lookup_for_enum_type()
 
+@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_int32_t(self):
 """Test C++11 enumeration class types as int32_t types."""
 self.build(
@@ -35,6 +38,7 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=int32_t"'})
 self.image_lookup_for_enum_type()
 
+@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_int64_t(self):
 """Test C++11 enumeration class types as int64_t types."""
 self.build(
@@ -42,6 +46,7 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=int64_t"'})
 self.image_lookup_for_enum_type()
 
+@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_uint8_t(self):
 """Test C++11 enumeration class types as uint8_t types."""
 self.build(
@@ -49,6 +54,7 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=uint8_t"'})
 self.image_lookup_for_enum_type()
 
+@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_uint16_t(self):
 """Test C++11 enumeration class types as uint16_t types."""
 self.build(
@@ -56,6 +62,7 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=uint16_t"'})
 self.image_lookup_for_enum_type()
 
+@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_uint32_t(self):
 """Test C++11 enumeration class types as uint32_t types."""
 self.build(
@@ -63,6 +70,7 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=uint32_t"'})
 self.image_lookup_for_enum_type()
 
+@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_uint64_t(self):
 """Test C++11 enumeration class types as uint64_t types."""
 self.build(


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

[Lldb-commits] [PATCH] D43784: Un-XFAIL TestCallStdStringFunction.test for Windows

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth added a comment.

Nice catch.  I'll take a closer look to see what exactly is happening on 
Windows.


https://reviews.llvm.org/D43784



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

[Lldb-commits] [lldb] r326139 - Revert r326134 due to broken buildbot

[Ed Maste via lldb-commits]

Author: emaste
Date: Mon Feb 26 14:36:41 2018
New Revision: 326139

URL: http://llvm.org/viewvc/llvm-project?rev=326139&view=rev
Log:
Revert r326134 due to broken buildbot

Modified:

lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/enum_types/TestCPP11EnumTypes.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/enum_types/TestCPP11EnumTypes.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/enum_types/TestCPP11EnumTypes.py?rev=326139&r1=326138&r2=326139&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/enum_types/TestCPP11EnumTypes.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/enum_types/TestCPP11EnumTypes.py
 Mon Feb 26 14:36:41 2018
@@ -14,7 +14,6 @@ class CPP11EnumTypesTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
-@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_int8_t(self):
 """Test C++11 enumeration class types as int8_t types."""
 self.build(
@@ -22,7 +21,6 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=int8_t"'})
 self.image_lookup_for_enum_type()
 
-@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_int16_t(self):
 """Test C++11 enumeration class types as int16_t types."""
 self.build(
@@ -30,7 +28,6 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=int16_t"'})
 self.image_lookup_for_enum_type()
 
-@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_int32_t(self):
 """Test C++11 enumeration class types as int32_t types."""
 self.build(
@@ -38,7 +35,6 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=int32_t"'})
 self.image_lookup_for_enum_type()
 
-@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_int64_t(self):
 """Test C++11 enumeration class types as int64_t types."""
 self.build(
@@ -46,7 +42,6 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=int64_t"'})
 self.image_lookup_for_enum_type()
 
-@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_uint8_t(self):
 """Test C++11 enumeration class types as uint8_t types."""
 self.build(
@@ -54,7 +49,6 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=uint8_t"'})
 self.image_lookup_for_enum_type()
 
-@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_uint16_t(self):
 """Test C++11 enumeration class types as uint16_t types."""
 self.build(
@@ -62,7 +56,6 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=uint16_t"'})
 self.image_lookup_for_enum_type()
 
-@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_uint32_t(self):
 """Test C++11 enumeration class types as uint32_t types."""
 self.build(
@@ -70,7 +63,6 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=uint32_t"'})
 self.image_lookup_for_enum_type()
 
-@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_uint64_t(self):
 """Test C++11 enumeration class types as uint64_t types."""
 self.build(


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

[Lldb-commits] [lldb] r326140 - Add a sanity check for inline testcases.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Mon Feb 26 14:40:20 2018
New Revision: 326140

URL: http://llvm.org/viewvc/llvm-project?rev=326140&view=rev
Log:
Add a sanity check for inline testcases.

When writing an inline test, there is no way to make sure that any of
the inline commands are actually executed, so this patch adds a sanity
check that at least one breakpoint was hit. This avoids a test with no
breakpoints being hit passing.

Differential Revision: https://reviews.llvm.org/D43694

Modified:
lldb/trunk/packages/Python/lldbsuite/test/lang/c/inlines/main.c
lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/c/inlines/main.c
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/inlines/main.c?rev=326140&r1=326139&r2=326140&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/inlines/main.c (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/inlines/main.c Mon Feb 26 
14:40:20 2018
@@ -21,4 +21,5 @@ void test1(int a) {
 int main() {
 test2(42);
 test1(23);
+return 0;
 }

Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py?rev=326140&r1=326139&r2=326140&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py Mon Feb 26 14:40:20 
2018
@@ -182,14 +182,23 @@ class InlineTest(TestBase):
 parser.set_breakpoints(target)
 
 process = target.LaunchSimple(None, None, self.getBuildDir())
+hit_breakpoints = 0
 
 while lldbutil.get_stopped_thread(process, lldb.eStopReasonBreakpoint):
+hit_breakpoints += 1
 thread = lldbutil.get_stopped_thread(
 process, lldb.eStopReasonBreakpoint)
 breakpoint_id = thread.GetStopReasonDataAtIndex(0)
 parser.handle_breakpoint(self, breakpoint_id)
 process.Continue()
 
+self.assertTrue(hit_breakpoints > 0,
+"inline test did not hit a single breakpoint")
+# Either the process exited or the stepping plan is complete.
+self.assertTrue(process.GetState() in [lldb.eStateStopped,
+   lldb.eStateExited],
+PROCESS_EXITED)
+
 # Utilities for testcases
 
 def check_expression(self, expression, expected_result, use_summary=True):


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

[Lldb-commits] [PATCH] D43694: Add a sanity check for inline tests

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL326140: Add a sanity check for inline testcases. (authored 
by adrian, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D43694?vs=135693&id=135990#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43694

Files:
  lldb/trunk/packages/Python/lldbsuite/test/lang/c/inlines/main.c
  lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py


Index: lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
@@ -182,14 +182,23 @@
 parser.set_breakpoints(target)
 
 process = target.LaunchSimple(None, None, self.getBuildDir())
+hit_breakpoints = 0
 
 while lldbutil.get_stopped_thread(process, lldb.eStopReasonBreakpoint):
+hit_breakpoints += 1
 thread = lldbutil.get_stopped_thread(
 process, lldb.eStopReasonBreakpoint)
 breakpoint_id = thread.GetStopReasonDataAtIndex(0)
 parser.handle_breakpoint(self, breakpoint_id)
 process.Continue()
 
+self.assertTrue(hit_breakpoints > 0,
+"inline test did not hit a single breakpoint")
+# Either the process exited or the stepping plan is complete.
+self.assertTrue(process.GetState() in [lldb.eStateStopped,
+   lldb.eStateExited],
+PROCESS_EXITED)
+
 # Utilities for testcases
 
 def check_expression(self, expression, expected_result, use_summary=True):
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/inlines/main.c
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/inlines/main.c
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/inlines/main.c
@@ -21,4 +21,5 @@
 int main() {
 test2(42);
 test1(23);
+return 0;
 }


Index: lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
@@ -182,14 +182,23 @@
 parser.set_breakpoints(target)
 
 process = target.LaunchSimple(None, None, self.getBuildDir())
+hit_breakpoints = 0
 
 while lldbutil.get_stopped_thread(process, lldb.eStopReasonBreakpoint):
+hit_breakpoints += 1
 thread = lldbutil.get_stopped_thread(
 process, lldb.eStopReasonBreakpoint)
 breakpoint_id = thread.GetStopReasonDataAtIndex(0)
 parser.handle_breakpoint(self, breakpoint_id)
 process.Continue()
 
+self.assertTrue(hit_breakpoints > 0,
+"inline test did not hit a single breakpoint")
+# Either the process exited or the stepping plan is complete.
+self.assertTrue(process.GetState() in [lldb.eStateStopped,
+   lldb.eStateExited],
+PROCESS_EXITED)
+
 # Utilities for testcases
 
 def check_expression(self, expression, expected_result, use_summary=True):
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/inlines/main.c
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/inlines/main.c
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/inlines/main.c
@@ -21,4 +21,5 @@
 int main() {
 test2(42);
 test1(23);
+return 0;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

[Owen Shaw via Phabricator via lldb-commits]

owenpshaw added a comment.

@labath, any other changes you'd like to see on this one?  Skipping the test if 
there's no xml support seemed to be the final todo.


https://reviews.llvm.org/D42145



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

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: packages/Python/lldbsuite/test/test_categories.py:27
 'gmodules': 'Tests that can be run with -gmodules debug information',
+'dwarf_type_units' : 'Tests using the DWARF type units 
(-fdebug-types-section)',
 'expression': 'Tests related to the expression parser',

clayborg wrote:
> aprantl wrote:
> > This is conflating two things (-fdebug-types-section and type units) DWARF5 
> > doesn't have a debug_types section but it still offers type units. Clang 
> > doesn't implement this yet, but GCC might, I haven't checked.
> So what is the suggestion here?
I would mention type units in the comment rather than -fdebug-types-section.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp:97
 void DWARFDebugInfo::ParseCompileUnitHeadersIfNeeded() {
   if (m_compile_units.empty()) {
 if (m_dwarf2Data != NULL) {

We probably should convert this to an early exit.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp:106
 offset = cu_sp->GetNextCompileUnitOffset();
   }
+

Perhaps add a comment explaining what is being done here?



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp:121
+  break;
+}
+  }

I find hard to follow. What do you think about:
```
   while (debug_types_data.ValidOffset(offset)) {
cu_sp = DWARFCompileUnit::Extract(m_dwarf2Data, debug_types_data,
   &offset, true)));
if (!cu_sp)
   return;
m_type_sig_to_cu_index[cu_sp->GetTypeSignature()] =
m_compile_units.size();
m_compile_units.push_back(cu_sp);
offset = cu_sp->GetNextCompileUnitOffset();
  }
```
?

Also, shouldn't the error be propagated?


https://reviews.llvm.org/D32167



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

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Greg Clayton via Phabricator via lldb-commits]

clayborg updated this revision to Diff 135999.
clayborg added a comment.

Address Adrian's comments.


https://reviews.llvm.org/D32167

Files:
  include/lldb/lldb-enumerations.h
  include/lldb/lldb-forward.h
  packages/Python/lldbsuite/test/lldbinline.py
  packages/Python/lldbsuite/test/lldbtest.py
  packages/Python/lldbsuite/test/make/Makefile.rules
  packages/Python/lldbsuite/test/plugins/builder_base.py
  packages/Python/lldbsuite/test/test_categories.py
  source/Core/Section.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  source/Plugins/SymbolFile/DWARF/DIERef.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserGo.cpp
  source/Plugins/SymbolFile/DWARF/DWARFAttribute.cpp
  source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
  source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.h
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Symbol/ObjectFile.cpp

Index: source/Symbol/ObjectFile.cpp
===
--- source/Symbol/ObjectFile.cpp
+++ source/Symbol/ObjectFile.cpp
@@ -360,6 +360,7 @@
   case eSectionTypeDWARFDebugRanges:
   case eSectionTypeDWARFDebugStr:
   case eSectionTypeDWARFDebugStrOffsets:
+  case eSectionTypeDWARFDebugTypes:
   case eSectionTypeDWARFAppleNames:
   case eSectionTypeDWARFAppleTypes:
   case eSectionTypeDWARFAppleNamespaces:
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -246,6 +246,7 @@
   const lldb_private::DWARFDataExtractor &get_debug_ranges_data();
   const lldb_private::DWARFDataExtractor &get_debug_str_data();
   const lldb_private::DWARFDataExtractor &get_debug_str_offsets_data();
+  const lldb_private::DWARFDataExtractor &get_debug_types_data();
   const lldb_private::DWARFDataExtractor &get_apple_names_data();
   const lldb_private::DWARFDataExtractor &get_apple_types_data();
   const lldb_private::DWARFDataExtractor &get_apple_namespaces_data();
@@ -491,6 +492,7 @@
   DWARFDataSegment m_data_debug_ranges;
   DWARFDataSegment m_data_debug_str;
   DWARFDataSegment m_data_debug_str_offsets;
+  DWARFDataSegment m_data_debug_types;
   DWARFDataSegment m_data_apple_names;
   DWARFDataSegment m_data_apple_types;
   DWARFDataSegment m_data_apple_namespaces;
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -501,21 +501,6 @@
 if (section_list == NULL)
   return 0;
 
-// On non Apple platforms we might have .debug_types debug info that
-// is created by using "-fdebug-types-section". LLDB currently will try
-// to load this debug info, but it causes crashes during debugging when
-// types are missing since it doesn't know how to parse the info in
-// the .debug_types type units. This causes all complex debug info
-// types to be unresolved. Because this causes LLDB to crash and since
-// it really doesn't provide a solid debuggiung experience, we should
-// disable trying to debug this kind of DWARF until support gets
-// added or deprecated.
-if (section_list->FindSectionByName(ConstString(".debug_types"))) {
-  m_obj_file->GetModule()->ReportWarning(
-"lldb doesn’t support .debug_types debug info");
-  return 0;
-}
-
 uint64_t debug_abbrev_file_size = 0;
 uint64_t debug_info_file_size = 0;
 uint64_t debug_line_file_size = 0;
@@ -591,8 +576,24 @@
 const DWARFDataExtractor &
 SymbolFileDWARF::GetCachedSectionData(lldb::SectionType sect_type,
   DWARFDataSegment &data_segment) {
-  llvm::call_once(data_segment.m_flag, [this, sect_type, &data_segment] {
+  llvm::call_once(data_segment.m_flag, [&] {
 this->LoadSectionData(sect_type, std::ref(data_segment.m_data));
+if (sect_type == eSectionTypeDWARFDebugTypes) {
+  // To add .debug_types support in DWARF 4 and earlier with minimally
+  // invasive changes to the current DWARF parsing code, we pretend that
+  // any DIEs i

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

Sorry, I forgot about that. This looks fine as far as I am concerned. Thank you 
for flying lldb, and in particular, for creating the gdb-client testing 
framework.


https://reviews.llvm.org/D42145



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

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D32167#1019702, @clayborg wrote:

> I am afraid of the opposite: we test what we think we need to test and our 
> simple tests cases don't adequately test the feature we are adding. I can 
> certainly limit the testing to very simple test cases with a one test for a 
> class and one for a enum, but that wouldn't have caught the issues I ran into 
> with static variables in classes that I fixed before submitting this. My 
> point is it is hard to figure out what a compiler, or debugger might hose up 
> without testing all possible cases. Please chime in with opinions as I will 
> go with the flow on this.


I don't think anyone here is suggesting to test less. The question we are 
asking is if running all 1000 or so tests doesn't just giv a false sense of 
security (and makes the testing process longer). A specially crafted test can 
trigger more corner cases and make it easier to debug future failures that a 
generic test. If the case of a static variable in a class is interesting, then 
maybe a test where you have two static variables from the same class defined in 
two different compilation units (and maybe referenced from a third one) is 
interesting as well. I'm pretty sure we don't have a test like that right now. 
Another interesting case which would not be tested in the "separate flavour" 
mode is the mixed-flags case: have part of your module compiled with type 
units, part without (and maybe a third part with type units and fission)..

Running the entire dotest test suite in -fdebug-types-section is certainly a 
good way to catch problems, but it's not the way to write regression tests. 
FWIW, the way I plan to test the new accelerator tables when I get to the lldb 
part is to run dotest with the new flags locally during development, use the 
failures to identify interesting test vectors, and then write regular tests to 
trigger these.


https://reviews.llvm.org/D32167



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

[Lldb-commits] [PATCH] D43784: Un-XFAIL TestCallStdStringFunction.test for Windows

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth abandoned this revision.
amccarth added a comment.

Abandoning.  This isn't really working on Windows.  The breakpoint fails to set 
for reasons I don't understand yet.


https://reviews.llvm.org/D43784



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

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

[Owen Shaw via Phabricator via lldb-commits]

owenpshaw added a comment.

Thanks!  Just a reminder that I don't have commit access, so someone will need 
to land this for me.  Appreciate all the help.


https://reviews.llvm.org/D42145



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

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Paul Robinson via Phabricator via lldb-commits]

probinson added a comment.

In https://reviews.llvm.org/D32167#1020032, @labath wrote:

> Running the entire dotest test suite in -fdebug-types-section is certainly a 
> good way to catch problems, but it's not the way to write regression tests.


Is there testing in place that is more serious/thorough than the normal 
regression testing?  It might be reasonable to do the full cartesian set at a 
slower pace.  For example LLVM has the concept of "expensive checks" and there 
are a small number of bots out there that enable them.  But it's not something 
anybody runs normally.


https://reviews.llvm.org/D32167



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

Re: [Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

[Zachary Turner via lldb-commits]

Llvm also separates the Integration tests (test-suite) from the regression
tests. That’s another approach to handling the Cartesian product testing
On Mon, Feb 26, 2018 at 6:01 PM Paul Robinson via Phabricator via
lldb-commits  wrote:

> probinson added a comment.
>
> In https://reviews.llvm.org/D32167#1020032, @labath wrote:
>
> > Running the entire dotest test suite in -fdebug-types-section is
> certainly a good way to catch problems, but it's not the way to write
> regression tests.
>
>
> Is there testing in place that is more serious/thorough than the normal
> regression testing?  It might be reasonable to do the full cartesian set at
> a slower pace.  For example LLVM has the concept of "expensive checks" and
> there are a small number of bots out there that enable them.  But it's not
> something anybody runs normally.
>
>
> https://reviews.llvm.org/D32167
>
>
>
> ___
> 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] [lldb] r326166 - Mark test_*int*_t_dwarf as failing on FreeBSD

[Ed Maste via lldb-commits]

Author: emaste
Date: Mon Feb 26 18:54:17 2018
New Revision: 326166

URL: http://llvm.org/viewvc/llvm-project?rev=326166&view=rev
Log:
Mark test_*int*_t_dwarf as failing on FreeBSD

Further investigation required; tests will be enabled on the buildbot
worker soon. Marking failing tests for now in order to start with a
green buildbot while investigation takes place.

This is a recommit of r326134, with the required import added.

llvm.org/pr36527

Modified:

lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/enum_types/TestCPP11EnumTypes.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/enum_types/TestCPP11EnumTypes.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/enum_types/TestCPP11EnumTypes.py?rev=326166&r1=326165&r2=326166&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/enum_types/TestCPP11EnumTypes.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/enum_types/TestCPP11EnumTypes.py
 Mon Feb 26 18:54:17 2018
@@ -6,6 +6,7 @@ from __future__ import print_function
 import os
 import time
 import lldb
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 import lldbsuite.test.lldbutil as lldbutil
 
@@ -14,6 +15,7 @@ class CPP11EnumTypesTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
+@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_int8_t(self):
 """Test C++11 enumeration class types as int8_t types."""
 self.build(
@@ -21,6 +23,7 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=int8_t"'})
 self.image_lookup_for_enum_type()
 
+@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_int16_t(self):
 """Test C++11 enumeration class types as int16_t types."""
 self.build(
@@ -28,6 +31,7 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=int16_t"'})
 self.image_lookup_for_enum_type()
 
+@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_int32_t(self):
 """Test C++11 enumeration class types as int32_t types."""
 self.build(
@@ -35,6 +39,7 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=int32_t"'})
 self.image_lookup_for_enum_type()
 
+@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_int64_t(self):
 """Test C++11 enumeration class types as int64_t types."""
 self.build(
@@ -42,6 +47,7 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=int64_t"'})
 self.image_lookup_for_enum_type()
 
+@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_uint8_t(self):
 """Test C++11 enumeration class types as uint8_t types."""
 self.build(
@@ -49,6 +55,7 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=uint8_t"'})
 self.image_lookup_for_enum_type()
 
+@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_uint16_t(self):
 """Test C++11 enumeration class types as uint16_t types."""
 self.build(
@@ -56,6 +63,7 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=uint16_t"'})
 self.image_lookup_for_enum_type()
 
+@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_uint32_t(self):
 """Test C++11 enumeration class types as uint32_t types."""
 self.build(
@@ -63,6 +71,7 @@ class CPP11EnumTypesTestCase(TestBase):
 'CFLAGS_EXTRAS': '"-DTEST_BLOCK_CAPTURED_VARS=uint32_t"'})
 self.image_lookup_for_enum_type()
 
+@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 def test_uint64_t(self):
 """Test C++11 enumeration class types as uint64_t types."""
 self.build(


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