[Lldb-commits] [PATCH] D54241: Fix bug in printing ValueObjects and in PE/COFF Plugin

[Zachary Turner via Phabricator via lldb-commits]

zturner created this revision.
zturner added a reviewer: clayborg.
Herald added subscribers: JDevlieghere, aprantl.

The first bug is that the implementation of `GetCompilerType` for `ValueObjectVariable` only requests the forward type.  This is insufficient for printing enum decls.  I suspect this works with the DWARF plugin because the DWARF plugin probably completes enums immediately.  The PDB plugin implements enums similarly to other tag records though, and only completes them lazily.  `ValueObjectVariable` then requests the forward compiler type, and makes no attempt to complete it, resulting in insufficient information to print out the value of the enum.

Note that in general, you need a full compiler type to print out the variable anyway.  If it's a class type you need it to print out its children, and this already happens when the `ValueObject` calls `GetNumChildren`.  So this particular change isn't actually invoking any additional work or parsing that wasn't already going to happen anyway, it just allows the case where a symbol file plugin parses enums lazily.

The second bug fixed here is in `ObjectFilePECOFF`.  Both MSVC and clang-cl emit their bss section as a magic section called `.data` with a raw size of 0 and a file offset of 0.    These 3 properties together are sufficient to indicate "this is actually a BSS section", but we had no mechanism of reading from such sections.  So we would just try the default read algorithm, which looks at the size, sees that it's too small (0 bytes), and gives up.  So we need to add support for this in `ObjectFilePECOFF`.

This fixes all of the outstanding printing bugs in the native pdb reading tests.


https://reviews.llvm.org/D54241

Files:
  lldb/lit/SymbolFile/NativePDB/ast-reconstruction.cpp
  lldb/source/Core/ValueObjectVariable.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h


Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===================================================================
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -125,6 +125,10 @@
 
   ObjectFile::Strata CalculateStrata() override;
 
+  size_t ReadSectionData(lldb_private::Section *section,
+                         lldb::offset_t section_offset, void *dst,
+                         size_t dst_len) override;
+
   //------------------------------------------------------------------
   // PluginInterface protocol
   //------------------------------------------------------------------
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -1053,6 +1053,32 @@
 }
 
 ObjectFile::Strata ObjectFilePECOFF::CalculateStrata() { return eStrataUser; }
+
+static bool IsCompressedBss(const Section &section) {
+  if (section.GetName() != ConstString(".data"))
+    return false;
+  if (section.GetFileSize() != 0)
+    return false;
+  if (section.GetFileOffset() != 0)
+    return false;
+  return true;
+}
+
+size_t ObjectFilePECOFF::ReadSectionData(Section *section,
+                                         lldb::offset_t section_offset,
+                                         void *dst, size_t dst_len) {
+  if (!IsCompressedBss(*section))
+    return ObjectFile::ReadSectionData(section, section_offset, dst, dst_len);
+
+  if (section_offset >= section->GetByteSize())
+    return 0;
+
+  size_t bytes_avail = section->GetByteSize() - section_offset;
+  size_t read_size = std::min(dst_len, bytes_avail);
+  ::memset(dst, 0, read_size);
+  return read_size;
+}
+
 //------------------------------------------------------------------
 // PluginInterface protocol
 //------------------------------------------------------------------
Index: lldb/source/Core/ValueObjectVariable.cpp
===================================================================
--- lldb/source/Core/ValueObjectVariable.cpp
+++ lldb/source/Core/ValueObjectVariable.cpp
@@ -67,7 +67,7 @@
 CompilerType ValueObjectVariable::GetCompilerTypeImpl() {
   Type *var_type = m_variable_sp->GetType();
   if (var_type)
-    return var_type->GetForwardCompilerType();
+    return var_type->GetFullCompilerType();
   return CompilerType();
 }
 
Index: lldb/lit/SymbolFile/NativePDB/ast-reconstruction.cpp
===================================================================
--- lldb/lit/SymbolFile/NativePDB/ast-reconstruction.cpp
+++ lldb/lit/SymbolFile/NativePDB/ast-reconstruction.cpp
@@ -89,16 +89,16 @@
 // CHECK: (TrivialC) TC = {}
 // CHECK: (TrivialS) TS = {}
 // CHECK: (TrivialU) TU = {}
-// CHECK: (TrivialE) TE = <Unable to determine byte size.>
-// CHECK: (A::B::C<int>) ABCInt = (ABCMember = <read memory from {{.*}} failed>)
-// CHECK: (A::B::C<float>) ABCFloat = (ABCMember = <read memory from {{.*}} failed>)
-// CHECK: (A::B::C<void>) ABCVoid = (ABCSpecializationMember = <read memory from {{.*}} failed>)
+// CHECK: (TrivialE) TE = TE_A
+// CHECK: (A::B::C<int>) ABCInt = (ABCMember = 0)
+// CHECK: (A::B::C<float>) ABCFloat = (ABCMember = 0)
+// CHECK: (A::B::C<void>) ABCVoid = (ABCSpecializationMember = 0x0000000000000000)
 // CHECK: (A::C<0>) AC0 = {}
 // CHECK: (A::C<-1>) ACNeg1 = {}
-// CHECK: (A::C<0>::D) AC0D = (ACDMember = <read memory from {{.*}} failed>, CPtr = <read memory from {{.*}} failed>)
-// CHECK: (A::C<-1>::D) ACNeg1D = (ACDMember = <read memory from {{.*}} failed>, CPtr = <read memory from {{.*}} failed>)
+// CHECK: (A::C<0>::D) AC0D = (ACDMember = 0, CPtr = 0x0000000000000000)
+// CHECK: (A::C<-1>::D) ACNeg1D = (ACDMember = 0, CPtr = 0x0000000000000000)
 // CHECK: (A::D) AD = {}
-// CHECK: (A::D::E) ADE = (ADDMember = <read memory from {{.*}} failed>)
+// CHECK: (A::D::E) ADE = (ADDMember = 0)
 // CHECK: Dumping clang ast for 1 modules.
 // CHECK: TranslationUnitDecl {{.*}}
 // CHECK: |-CXXRecordDecl {{.*}} class TrivialC definition


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D54241.173089.patch
Type: text/x-patch
Size: 4155 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20181108/16e7ef37/attachment.bin>