date:20191115

[Lldb-commits] [PATCH] D70294: [lldb][NFC] Pass a ValueObject to DumpTypeValue instead of bitfield size/offset pair

2019-11-15 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor created this revision.
teemperor added a reviewer: labath.
teemperor added a project: Upstreaming LLDB's downstream patches.
Herald added subscribers: lldb-commits, JDevlieghere, abidh.
Herald added a project: LLDB.
teemperor retitled this revision from "[lldb] Pass a ValueObject to 
DumpTypeValue instead of bitfield size/offset pair" to "[lldb][NFC] Pass a 
ValueObject to DumpTypeValue instead of bitfield size/offset pair".

This makes calling DumpTypeValue less tricky as it's not possible to mix up the 
size/offset
parameters. Also allows us to drop downstream swift patches that try to pass 
even more
ValueObject members as separate parameters to this function.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D70294

Files:
  lldb/include/lldb/Symbol/ClangASTContext.h
  lldb/include/lldb/Symbol/CompilerType.h
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/source/DataFormatters/TypeFormat.cpp
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/source/Symbol/CompilerType.cpp

Index: lldb/source/Symbol/CompilerType.cpp
===
--- lldb/source/Symbol/CompilerType.cpp
+++ lldb/source/Symbol/CompilerType.cpp
@@ -734,14 +734,12 @@
 bool CompilerType::DumpTypeValue(Stream *s, lldb::Format format,
  const DataExtractor &data,
  lldb::offset_t byte_offset, size_t byte_size,
- uint32_t bitfield_bit_size,
- uint32_t bitfield_bit_offset,
+ ValueObject &value_object,
  ExecutionContextScope *exe_scope) {
   if (!IsValid())
 return false;
   return m_type_system->DumpTypeValue(m_type, s, format, data, byte_offset,
-  byte_size, bitfield_bit_size,
-  bitfield_bit_offset, exe_scope);
+  byte_size, value_object, exe_scope);
 }
 
 void CompilerType::DumpSummary(ExecutionContext *exe_ctx, Stream *s,
Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -9390,15 +9390,15 @@
 
 static bool DumpEnumValue(const clang::QualType &qual_type, Stream *s,
   const DataExtractor &data, lldb::offset_t byte_offset,
-  size_t byte_size, uint32_t bitfield_bit_offset,
-  uint32_t bitfield_bit_size) {
+  size_t byte_size, ValueObject &value_object) {
   const clang::EnumType *enutype =
   llvm::cast(qual_type.getTypePtr());
   const clang::EnumDecl *enum_decl = enutype->getDecl();
   assert(enum_decl);
   lldb::offset_t offset = byte_offset;
   const uint64_t enum_svalue = data.GetMaxS64Bitfield(
-  &offset, byte_size, bitfield_bit_size, bitfield_bit_offset);
+  &offset, byte_size, value_object.GetBitfieldBitSize(),
+  value_object.GetBitfieldBitOffset());
   bool can_be_bitfield = true;
   uint64_t covered_bits = 0;
   int num_enumerators = 0;
@@ -9468,11 +9468,12 @@
   return true;
 }
 
-bool ClangASTContext::DumpTypeValue(
-lldb::opaque_compiler_type_t type, Stream *s, lldb::Format format,
-const DataExtractor &data, lldb::offset_t byte_offset, size_t byte_size,
-uint32_t bitfield_bit_size, uint32_t bitfield_bit_offset,
-ExecutionContextScope *exe_scope) {
+bool ClangASTContext::DumpTypeValue(lldb::opaque_compiler_type_t type,
+Stream *s, lldb::Format format,
+const DataExtractor &data,
+lldb::offset_t byte_offset,
+size_t byte_size, ValueObject &value_object,
+ExecutionContextScope *exe_scope) {
   if (!type)
 return false;
   if (IsAggregateType(type)) {
@@ -9484,8 +9485,8 @@
 
 if (type_class == clang::Type::Elaborated) {
   qual_type = llvm::cast(qual_type)->getNamedType();
-  return DumpTypeValue(qual_type.getAsOpaquePtr(), s, format, data, byte_offset, byte_size,
-   bitfield_bit_size, bitfield_bit_offset, exe_scope);
+  return DumpTypeValue(qual_type.getAsOpaquePtr(), s, format, data,
+   byte_offset, byte_size, value_object, exe_scope);
 }
 
 switch (type_class) {
@@ -9507,11 +9508,7 @@
   data,  // Data buffer containing all bytes for this type
   byte_offset,   // Offset into "data" where to grab value from
   typedef_byte_size, // Size of this type in bytes
-  bitfield_bit_size, // Size in bits of a bitfield value, if zero don't
- // treat as a bitfield
-  bitfield_bit_offset, // Offset in bits of a bitfield value if
-   // bit

[Lldb-commits] [PATCH] D70120: [lldb][NFC] Let CompilerType::DumpTypeValue take a option struct instead of a huge list of options

2019-11-15 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor abandoned this revision.
teemperor added a comment.

D70294  is the new thing


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70120/new/

https://reviews.llvm.org/D70120



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

[Lldb-commits] [PATCH] D70272: [-gmodules] Let LLDB log a warning if the Clang module hash mismatches.

2019-11-15 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: lldb/include/lldb/Symbol/SymbolFile.h:283-285
+  /// Return a hash for this symbol file, such as a dwo_id.
+  virtual llvm::Optional GetSymbolHash() { return {}; }
+  

Can we remove this and put a cast in 
`SymbolFileDWARF::UpdateExternalModuleListIfNeeded` instead? It looks like that 
function should check that it has really found a dwarf file (and not a pdb for 
instance) anyway...



Comment at: lldb/source/Host/common/Host.cpp:304
+  if (log)
+log->VAPrintf(format, args);
 }

Are you sure it's legal to recycle the `va_list` this way? Should you maybe 
re-initialize it?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1570
+static uint64_t GetDWOId(DWARFCompileUnit &dwarf_cu,
+ DWARFDebugInfoEntry cu_die) {
+  uint64_t dwo_id =

`DWARFDebugInfoEntry` objects assume that they are living in one big vector and 
do pointer arithmetic on their `this` pointers. I don't think it's wise to copy 
them even if that happens to work in this case...



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1579
+llvm::Optional SymbolFileDWARF::GetSymbolHash() {
+  if (auto comp_unit = GetCompileUnitAtIndex(0)) {
+if (DWARFCompileUnit *cu = llvm::dyn_cast_or_null(

With this implementation, the function will return the dwo id of the first 
skeleton unit in the main module in the dwo scenario. I think this is very 
unexpected and not very useful. I think this should check that the file 
contains a just a single compile unit, at least.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1724-1725
+
+// Verify the DWO hash.
+// FIXME: Technically "0" is a valid hash.
+uint64_t dwo_id = GetDWOId(*dwarf_cu, *die.GetDIE());

Maybe you could have GetDWOId return Optional. That way, the FIXME will 
be inside that function, and not in all of it's callers.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70272/new/

https://reviews.llvm.org/D70272



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

[Lldb-commits] [PATCH] D70303: Fix TestFormatters.py stepping too far

2019-11-15 Thread Diana Picus via Phabricator via lldb-commits

rovka created this revision.
rovka added reviewers: omjavaid, lawrence_danna.
Herald added a subscriber: kristof.beyls.

TestFormatters.py has a sequence of three 'next' commands to get past
all the initializations in the test function. On AArch64 (and
potentially other platforms), this was one 'next' too many and we ended
up outside our frame.

This patch replaces the sequence with a 'thread until ' the line of the
return from the function, so we should stop after all the
initializations but before actually returning.


https://reviews.llvm.org/D70303

Files:
  
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py


Index: 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
===
--- 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
+++ 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
@@ -231,9 +231,8 @@
 0) == 122,
 '*a_ptr = 122')
 
-self.runCmd("n")
-self.runCmd("n")
-self.runCmd("n")
+ret = line_number("main.cpp", "return")
+self.runCmd("thread until " + str(ret))
 
 self.expect("frame variable numbers",
 substrs=['1', '2', '3', '4', '5'])


Index: lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
===
--- lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
+++ lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
@@ -231,9 +231,8 @@
 0) == 122,
 '*a_ptr = 122')
 
-self.runCmd("n")
-self.runCmd("n")
-self.runCmd("n")
+ret = line_number("main.cpp", "return")
+self.runCmd("thread until " + str(ret))
 
 self.expect("frame variable numbers",
 substrs=['1', '2', '3', '4', '5'])
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70294: [lldb][NFC] Pass a ValueObject to DumpTypeValue instead of bitfield size/offset pair

[Lldb-commits] [PATCH] D70120: [lldb][NFC] Let CompilerType::DumpTypeValue take a option struct instead of a huge list of options

[Lldb-commits] [PATCH] D70272: [-gmodules] Let LLDB log a warning if the Clang module hash mismatches.

[Lldb-commits] [PATCH] D70303: Fix TestFormatters.py stepping too far

4 matches

Site Navigation

Mail list logo

Footer information