[Lldb-commits] [PATCH] D53375: [PDB] Improve performance of the PDB DIA plugin
aleksandr.urakov added a comment. Thank you! Repository: rLLDB LLDB https://reviews.llvm.org/D53375 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53361: [API] Extend the `SBThreadPlan` interface
aleksandr.urakov added a comment. Thanks! Yes, I've added this exactly to allow usage of a scripted plan from another scripted plan. Sure, I will add a test for this :) But I can't figure out how this part is tested, can you explain me this a little, please? Repository: rLLDB LLDB https://reviews.llvm.org/D53361 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53086: [PDB] Fix flaky `variables-locations.test` after PR38857
aleksandr.urakov added a comment. Thanks for explanations! I completely agree with you that it were better (and simpler) to implement it with the help of debug info. And if I will improve it later, I'll also choose the way you have described. But the problem is that this sketch was made a couple of months ago, and it seems work, so may be it is not a so bad idea to use it (and not redo it now to work with a debug info)? Besides, in ideal implementation we would still need something like this to cover that rare cases with prologues and epilogues. I've been stashing a lot of changes for past few months, and now the stash is big enough, so I'm trying to send it all on review. Now I understand that it's not a good strategy :) The such approach makes inconvenient situations like this, so I think it is better to send reviews as soon as possible. Sorry for that. I've brushed the sketch and have sent it for review last week, here it is: https://reviews.llvm.org/D53435 https://reviews.llvm.org/D53086 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53506: [ClangASTContext] Extract VTable pointers from C++ objects
aleksandr.urakov created this revision.
aleksandr.urakov added reviewers: clayborg, labath, granata.enrico.
aleksandr.urakov added a project: LLDB.
Herald added subscribers: lldb-commits, teemperor, abidh.
This patch processes the case of retrieving a virtual base when the object is
already read from the debuggee memory.
To achieve that `ValueObject::GetCPPVTableAddress` was removed (btw, it really
returned not a C++ VTable address but an object's address, which is a C++
VTable **pointer** address for Itanium, but have nothing to do with VTable
address for MSVC) and was reimplemented in `ClangASTContext` (because access to
the process is needed to retrieve the VTable pointer in general, and because
this is the only place that used old version of
`ValueObject::GetCPPVTableAddress`).
This patch allows to use real object's VTable instead of searching virtual
bases by offsets restored by `MicrosoftRecordLayoutBuilder`. PDB has no enough
info to restore VBase offsets properly, so we have to read real VTable instead.
This patch depends on https://reviews.llvm.org/D53497
Repository:
rLLDB LLDB
https://reviews.llvm.org/D53506
Files:
include/lldb/Core/ValueObject.h
lit/SymbolFile/PDB/Inputs/VBases.cpp
lit/SymbolFile/PDB/Inputs/VBases.script
lit/SymbolFile/PDB/vbases.test
source/Core/ValueObject.cpp
source/Symbol/ClangASTContext.cpp
Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -204,6 +204,122 @@
}
}
+static lldb::addr_t GetVTableAddress(Process &process,
+ VTableContextBase &vtable_ctx,
+ ValueObject &valobj,
+ const ASTRecordLayout &record_layout) {
+ // Retrieve type info
+ CompilerType pointee_type;
+ CompilerType this_type(valobj.GetCompilerType());
+ uint32_t type_info = this_type.GetTypeInfo(&pointee_type);
+ if (!type_info)
+return LLDB_INVALID_ADDRESS;
+
+ // Check if it's a pointer or reference
+ bool ptr_or_ref = false;
+ if (type_info & (eTypeIsPointer | eTypeIsReference)) {
+ptr_or_ref = true;
+type_info = pointee_type.GetTypeInfo();
+ }
+
+ // We process only C++ classes
+ const uint32_t cpp_class = eTypeIsClass | eTypeIsCPlusPlus;
+ if ((type_info & cpp_class) != cpp_class)
+return LLDB_INVALID_ADDRESS;
+
+ // Calculate offset to VTable pointer
+ lldb::offset_t vbtable_ptr_offset =
+ vtable_ctx.isMicrosoft() ? record_layout.getVBPtrOffset().getQuantity()
+ : 0;
+
+ if (ptr_or_ref) {
+// We have a pointer / ref to object, so read
+// VTable pointer from process memory
+
+if (valobj.GetAddressTypeOfChildren() != eAddressTypeLoad)
+ return LLDB_INVALID_ADDRESS;
+
+auto vbtable_ptr_addr = valobj.GetValueAsUnsigned(LLDB_INVALID_ADDRESS);
+if (vbtable_ptr_addr == LLDB_INVALID_ADDRESS)
+ return LLDB_INVALID_ADDRESS;
+
+vbtable_ptr_addr += vbtable_ptr_offset;
+
+Status err;
+return process.ReadPointerFromMemory(vbtable_ptr_addr, err);
+ }
+
+ // We have an object already read from process memory,
+ // so just extract VTable pointer from it
+
+ DataExtractor data;
+ Status err;
+ auto size = valobj.GetData(data, err);
+ if (err.Fail() || vbtable_ptr_offset + data.GetAddressByteSize() > size)
+return LLDB_INVALID_ADDRESS;
+
+ return data.GetPointer(&vbtable_ptr_offset);
+}
+
+static int64_t ReadVBaseOffsetFromVTable(Process &process,
+ VTableContextBase &vtable_ctx,
+ lldb::addr_t vtable_ptr,
+ const CXXRecordDecl *cxx_record_decl,
+ const CXXRecordDecl *base_class_decl) {
+ if (vtable_ctx.isMicrosoft()) {
+clang::MicrosoftVTableContext &msoft_vtable_ctx =
+static_cast(vtable_ctx);
+
+// Get the index into the virtual base table. The
+// index is the index in uint32_t from vbtable_ptr
+const unsigned vbtable_index =
+msoft_vtable_ctx.getVBTableIndex(cxx_record_decl, base_class_decl);
+const lldb::addr_t base_offset_addr = vtable_ptr + vbtable_index * 4;
+Status err;
+return process.ReadSignedIntegerFromMemory(base_offset_addr, 4, INT64_MAX,
+ err);
+ }
+
+ clang::ItaniumVTableContext &itanium_vtable_ctx =
+ static_cast(vtable_ctx);
+
+ clang::CharUnits base_offset_offset =
+ itanium_vtable_ctx.getVirtualBaseOffsetOffset(cxx_record_decl,
+base_class_decl);
+ const lldb::addr_t base_offset_addr =
+ vtable_ptr + base_offset_offset.getQuantity();
+ const uint32_t base_offset_size = process.GetAddressByteSize();
+ Status err;
+ return process.ReadSignedIntegerFromMemory(base_offset_addr, base_offset
[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected
clayborg added a comment. Since this is duplicated in so many spots, can we make a helper function to do this to ensure no one gets it wrong. We might be able to pass in the "guard" and "stop_locker" as reference variables and modify them in the helper function. Repository: rLLDB LLDB https://reviews.llvm.org/D53412 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53506: [ClangASTContext] Extract VTable pointers from C++ objects
zturner added a comment. > PDB has no enough info to restore VBase offsets properly, so we have to read > real VTable instead. What's missing that you're unable to restore the VBase offset properly? Repository: rLLDB LLDB https://reviews.llvm.org/D53506 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r344913 - Some cleanups to the native pdb plugin [NFC].
Author: zturner
Date: Mon Oct 22 09:19:07 2018
New Revision: 344913
URL: http://llvm.org/viewvc/llvm-project?rev=344913&view=rev
Log:
Some cleanups to the native pdb plugin [NFC].
This is mostly some cleanup done in the process of implementing
some basic support for types. I tried to split up the patch a
bit to get some of the NFC portion of the patch out into a separate
commit, and this is the result of that. It moves some code around,
deletes some spurious namespace qualifications, removes some
unnecessary header includes, forward declarations, etc.
Modified:
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbSymUid.h
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbUtil.h
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
Modified: lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbSymUid.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbSymUid.h?rev=344913&r1=344912&r2=344913&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbSymUid.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbSymUid.h Mon Oct 22
09:19:07 2018
@@ -18,6 +18,7 @@
#ifndef LLDB_PLUGINS_SYMBOLFILENATIVEPDB_PDBSYMUID_H
#define LLDB_PLUGINS_SYMBOLFILENATIVEPDB_PDBSYMUID_H
+#include "llvm/DebugInfo/CodeView/SymbolRecord.h"
#include "llvm/DebugInfo/PDB/PDBTypes.h"
#include "llvm/Support/Compiler.h"
Modified: lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp?rev=344913&r1=344912&r2=344913&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp Mon Oct 22
09:19:07 2018
@@ -10,16 +10,18 @@
#include "PdbUtil.h"
#include "llvm/DebugInfo/CodeView/SymbolDeserializer.h"
+#include "llvm/DebugInfo/CodeView/TypeDeserializer.h"
#include "lldb/Utility/LLDBAssert.h"
+#include "lldb/lldb-enumerations.h"
+
using namespace lldb_private;
using namespace lldb_private::npdb;
using namespace llvm::codeview;
using namespace llvm::pdb;
-llvm::pdb::PDB_SymType
-lldb_private::npdb::CVSymToPDBSym(llvm::codeview::SymbolKind kind) {
+PDB_SymType lldb_private::npdb::CVSymToPDBSym(SymbolKind kind) {
switch (kind) {
case S_COMPILE3:
case S_OBJNAME:
@@ -71,7 +73,32 @@ lldb_private::npdb::CVSymToPDBSym(llvm::
return PDB_SymType::None;
}
-bool lldb_private::npdb::SymbolHasAddress(const llvm::codeview::CVSymbol &sym)
{
+PDB_SymType lldb_private::npdb::CVTypeToPDBType(TypeLeafKind kind) {
+ switch (kind) {
+ case LF_ARRAY:
+return PDB_SymType::ArrayType;
+ case LF_ARGLIST:
+return PDB_SymType::FunctionSig;
+ case LF_BCLASS:
+return PDB_SymType::BaseClass;
+ case LF_BINTERFACE:
+return PDB_SymType::BaseInterface;
+ case LF_CLASS:
+ case LF_STRUCTURE:
+ case LF_INTERFACE:
+ case LF_UNION:
+return PDB_SymType::UDT;
+ case LF_POINTER:
+return PDB_SymType::PointerType;
+ case LF_ENUM:
+return PDB_SymType::Enum;
+ default:
+lldbassert(false && "Invalid type record kind!");
+ }
+ return PDB_SymType::None;
+}
+
+bool lldb_private::npdb::SymbolHasAddress(const CVSymbol &sym) {
switch (sym.kind()) {
case S_GPROC32:
case S_LPROC32:
@@ -98,7 +125,7 @@ bool lldb_private::npdb::SymbolHasAddres
}
}
-bool lldb_private::npdb::SymbolIsCode(const llvm::codeview::CVSymbol &sym) {
+bool lldb_private::npdb::SymbolIsCode(const CVSymbol &sym) {
switch (sym.kind()) {
case S_GPROC32:
case S_LPROC32:
@@ -156,8 +183,7 @@ SegmentOffset GetSegmentAndOffset(
return SegmentOffsetLength{record.Segment, record.Offset, record.Size};
}
-SegmentOffsetLength lldb_private::npdb::GetSegmentOffsetAndLength(
-const llvm::codeview::CVSymbol &sym) {
+SegmentOffsetLength
+lldb_private::npdb::GetSegmentOffsetAndLength(const CVSymbol &sym) {
switch (sym.kind()) {
case S_GPROC32:
case S_LPROC32:
@@ -256,3 +282,76 @@ SegmentOffsetLength lldb_private::npdb::
}
return {0, 0, 0};
}
+
+bool lldb_private::npdb::IsForwardRefUdt(CVType cvt) {
+ ClassRecord cr;
+ UnionRecord ur;
+ EnumRecord er;
+ switch (cvt.kind()) {
+ case LF_CLASS:
+ case LF_STRUCTURE:
+ case LF_INTERFACE:
+llvm::cantFail(TypeDeserializer::deserializeAs(cvt, cr));
+return cr.isForwardRef();
+ case LF_UNION:
+llvm::cantFail(TypeDeserializer::deserializeAs(cvt, ur));
+return ur.isForwardRef();
+ case LF_ENUM:
+llvm::cantFail(TypeDeserializer::deserializeAs(cvt, er));
+return er.isForwardRef();
+ default:
+return false;
+ }
+}
+
+lldb::AccessType
+lldb_private::npdb::TranslateMemberAccess(Member
[Lldb-commits] [PATCH] D53506: [ClangASTContext] Extract VTable pointers from C++ objects
aleksandr.urakov added a comment. In https://reviews.llvm.org/D53506#1270893, @zturner wrote: > What's missing that you're unable to restore the VBase offset properly? If I understand correctly, in the PDB there is only info about offset to VTablePtr and index in VTable, so there is enough info to retrieve VBase offset fairly, and we do it in that way. But there's no info in PDB about offset to VBase directly from object. This info is used when the "fair" doesn't work (e.g. at line 6640). This patch just makes the "fair" way to work in more cases. Repository: rLLDB LLDB https://reviews.llvm.org/D53506 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53506: [ClangASTContext] Extract VTable pointers from C++ objects
zturner added a comment.
In https://reviews.llvm.org/D53506#1270919, @aleksandr.urakov wrote:
> In https://reviews.llvm.org/D53506#1270893, @zturner wrote:
>
> > What's missing that you're unable to restore the VBase offset properly?
>
>
> If I understand correctly, in the PDB there is only info about offset to
> VTablePtr and index in VTable, so there is enough info to retrieve VBase
> offset fairly, and we do it in that way. But there's no info in PDB about
> offset to VBase directly from object. This info is used when the "fair"
> doesn't work (e.g. at line 6640). This patch just makes the "fair" way to
> work in more cases.
My understanding of record layout with virtual bases is still sketchy (it's
very confusing), and it's even worse with DIA because the API is so general and
poorly documented, so let's go to the low-level CodeView records.
typedef struct lfVBClass {
unsigned short leaf; // LF_VBCLASS (virtual base) |
LV_IVBCLASS (indirect virtual base)
CV_fldattr_tattr; // attribute
CV_typ_tindex; // type index of direct virtual base class
CV_typ_tvbptr; // type index of virtual base pointer
unsigned char vbpoff[CV_ZEROLEN]; // virtual base pointer offset
from address point
// followed by virtual base offset from
vbtable
} lfVBClass;
This is what we have access to reading directly from the raw pdb file, which is
sometimes more information than what we have access to using DIA. Of course,
we also have to interpret whether this actually means what we think it means by
inspecting the bytes of a C++ object in a debugger and comparing the layout to
what the debug info tells us.
So, the point is, just because we don't have access to the info via DIA doesn't
mean we won't have access to the info once the native pdb plugin is complete.
Just something to think about.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D53506
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50155: Delete MacOSXFrameBackchain unwind logic (NFC)
jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Sorry for not replying to this. Yes the code is unused - it was the initial unwinder implementation when lldb is very young, before we had our current one. The main reason to keep it around is (as you say) an example of the simplest unwinder someone might use when porting lldb to a new platform. I don't feel strongly about it one way or the other, I'm fine with removing it if that's what you want to do. https://reviews.llvm.org/D50155 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53511: [NativePDB] Support type lookup by name
zturner created this revision.
zturner added reviewers: labath, lemo, aleksandr.urakov, stella.stamenova,
asmith.
Herald added subscribers: arphaman, mgorny.
This is the minimal set of functionality necessary to support type lookup by
name in the native PDB plugin.
For the purposes of testing I decided to bypass `lldb-test` and go with a
scripted lldbinit file. I'm sticking with this approach wherever possible to
prove that this is "true" cross-platform debugger functionality. However,
there are definite limitations. For example, the output format of `type
lookup` has some limitations. It doesn't print the layout of the type in any
kind of detail (e.g. field offsets)`, it doesn't support lookup by regex, it
doesn't print the underlying type of an enumeration, it doesn't support
limiting the scope of the search to specific kinds of types, so there is
definitely room for `lldb-test` to come into the picture here to expand the
coverage since we have full control over the output format.
I tried to think of some interesting test cases to exercise some edge cases
here, but I welcome some ideas for other interesting cases. I consciously
avoided things like bit fields, member functions, pointers to members, and
virtual bases because there was a balancing act between implementing a useful
piece of functionality and keeping the patch small enough that it can be
understandable enough to review meaningfully.
Welcome any feedback. Hopefully this is getting to the point that maybe it's
hackable by others as well, although I'm definitely going to continue working
on it.
https://reviews.llvm.org/D53511
Files:
lldb/lit/SymbolFile/NativePDB/Inputs/tag-types.lldbinit
lldb/lit/SymbolFile/NativePDB/tag-types.cpp
lldb/source/Plugins/SymbolFile/NativePDB/CMakeLists.txt
lldb/source/Plugins/SymbolFile/NativePDB/PdbIndex.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
Index: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
===
--- /dev/null
+++ lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
@@ -0,0 +1,68 @@
+//===-- SymbolFileNativePDB.h ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLDB_PLUGINS_SYMBOLFILE_NATIVEPDB_UDTRECORDCOMPLETER_H
+#define LLDB_PLUGINS_SYMBOLFILE_NATIVEPDB_UDTRECORDCOMPLETER_H
+
+#include "lldb/Symbol/ClangASTImporter.h"
+#include "llvm/DebugInfo/CodeView/CVRecord.h"
+#include "llvm/DebugInfo/CodeView/TypeRecord.h"
+#include "llvm/DebugInfo/CodeView/TypeVisitorCallbacks.h"
+
+#include "PdbSymUid.h"
+
+namespace clang {
+class CXXBaseSpecifier;
+class TagDecl;
+} // namespace clang
+
+namespace lldb_private {
+class Type;
+class CompilerType;
+namespace npdb {
+class SymbolFileNativePDB;
+
+class UdtRecordCompleter : public llvm::codeview::TypeVisitorCallbacks {
+ union UdtTagRecord {
+UdtTagRecord() {}
+llvm::codeview::UnionRecord ur;
+llvm::codeview::ClassRecord cr;
+llvm::codeview::EnumRecord er;
+ } m_cvr;
+
+ PdbSymUid m_uid;
+ CompilerType &m_derived_ct;
+ clang::TagDecl &m_tag_decl;
+ SymbolFileNativePDB &m_symbol_file;
+ std::vector m_bases;
+ ClangASTImporter::LayoutInfo m_layout;
+
+public:
+ UdtRecordCompleter(PdbSymUid uid, CompilerType &derived_ct,
+ clang::TagDecl &tag_decl,
+ SymbolFileNativePDB &symbol_file);
+
+#define MEMBER_RECORD(EnumName, EnumVal, Name) \
+ llvm::Error visitKnownMember(llvm::codeview::CVMemberRecord &CVR,\
+ llvm::codeview::Name##Record &Record) override;
+#define MEMBER_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
+#include "llvm/DebugInfo/CodeView/CodeViewTypes.def"
+
+ void complete();
+
+private:
+ lldb::opaque_compiler_type_t
+ AddBaseClassForTypeIndex(llvm::codeview::TypeIndex ti,
+ llvm::codeview::MemberAccess access);
+};
+
+} // namespace npdb
+} // namespace lldb_private
+
+#endif // LLDB_PLUGINS_SYMBOLFILE_NATIVEPDB_UDTRECORDCOMPLETER_H
Index: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
===
--- /dev/null
+++ lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -0,0 +1,188 @@
+#include "UdtRecordCompleter.h"
+
+#include "PdbIndex.h"
+#include "PdbSymUid.h"
+#include "PdbUtil.h"
+#include "SymbolFileNativePDB.h"
+
+#include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Symbol
[Lldb-commits] [PATCH] D53361: [API] Extend the `SBThreadPlan` interface
jingham added a comment. This should be easy to test with the python testsuite (lldbtest.) Start with the sample_test in packages/Python/lldbsuite/test - don't use the inline one, that won't be flexible enough. Then you can just make a scripted thread plan that just pushes a "step over" or "step in" plan, and step with it and make sure it landed where it should. Then to test your addition, make another scripted plan that calls your trivial stepping one, and make sure that stops where expected as well. Repository: rLLDB LLDB https://reviews.llvm.org/D53361 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52543: [DWARFSymbolFile] Add the module lock where necessary and assert that we own it.
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
See inlined comments
Comment at: include/lldb/Symbol/SymbolFile.h:98
+ virtual std::recursive_mutex &GetModuleMutex() const;
+
Might add a comment here stating something like "Symbols file subclasses should
override this to return the Module that owns the TypeSystem that this symbol
file modifies type information in".
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2022-2025
+ if (m_debug_map_module_wp.expired())
+return GetObjectFile()->GetModule()->GetMutex();
+ lldb::ModuleSP module_sp(m_debug_map_module_wp.lock());
+ return module_sp->GetMutex();
Possible race condition and crasher here. Best coded as:
```
lldb::ModuleSP module_sp(m_debug_map_module_wp.lock());
if (module_sp)
return module_sp->GetMutex();
return GetObjectFile()->GetModule()->GetMutex();
```
Otherwise some other thread could clear "m_debug_map_module_wp" after
"m_debug_map_module_wp.expired()" is called, but before
"m_debug_map_module_wp.lock()" is called and we would crash.
Comment at: source/Symbol/SymbolFile.cpp:160-168
+void SymbolFile::AssertModuleLock() {
+ // We assert that we have to module lock by trying to acquire the lock from a
+ // different thread. Note that we must abort if the result is true to
+ // guarantee correctness.
+ lldbassert(std::async(std::launch::async,
+[this] { return this->GetModuleMutex().try_lock(); })
+ .get() == false &&
We should make this entire thing and all uses of it into a macro so we can
enable it for Debug builds, but disable it for Release builds and have no extra
code in release builds. We really shouldn't be starting threads up all the time
for every function in SymbolFile subclasses for Release builds. It is ok for
Debug builds or maybe manually enable this when we run into issues, but this is
too expensive to leave in for all builds.
https://reviews.llvm.org/D52543
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53368: [Symbol] Search symbols with name and type in a symbol file
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. All symbol tables are currently extracted from the object files via ObjectFile::GetSymtab(). Are symbols only in the PDB file? If so I would vote to add a "virtual void SymbolVendor::AddSymbols(Symtab *symtab)" and a "virtual void SymbolFile::AddSymbols(Symtab *symtab)" where we take the symbol table that comes from the object file and we can add symbols to it if the symbol file has symbols it wants to add to the object file's symbol table. All symbol queries go through the lldb_private::Symtab class anyway. Care must be taken to watch out for symbols that might already exist from an ObjectFile's symbol table to ensure don't have duplicates. So I would: - Add "virtual void SymbolVendor::AddSymbols(Symtab *symtab);" to SymbolVendor that just calls through to its SymbolFile to do the work - Add "virtual void SymbolFile::AddSymbols(Symtab *symtab)" to SymbolFile with default implementation that does nothing - Override SymbolFile::AddSymbols() for SymbolFilePDB and add symbols to the provided symbol table - Modify *SymbolVendor::GetSymtab()" to get the object file symbol table, then pass that along to any symbol file instances it owns to allow each symbol file to augment the symbol table - Remove all "FindPublicSymbols()" code from patch - Revert all symbol searching code to just use the Symtab class now that it contains all needed symbols Repository: rLLDB LLDB https://reviews.llvm.org/D53368 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53436: [LLDB] - Implement the support for the .debug_loclists section.
clayborg added a comment. Just fix the assert to use lldbassert so we don't crash as mentioned in the inline comment and this is good to go. Comment at: source/Expression/DWARFExpression.cpp:3070 // Not supported entry type + assert(false && "Not supported location list type"); return false; use lldbassert so this doesn't crash a release debugger. https://reviews.llvm.org/D53436 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52543: [DWARFSymbolFile] Add the module lock where necessary and assert that we own it.
JDevlieghere updated this revision to Diff 170465.
JDevlieghere marked 3 inline comments as done.
JDevlieghere added a comment.
Address Greg's feedback.
https://reviews.llvm.org/D52543
Files:
include/lldb/Symbol/SymbolFile.h
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
source/Symbol/SymbolFile.cpp
Index: source/Symbol/SymbolFile.cpp
===
--- source/Symbol/SymbolFile.cpp
+++ source/Symbol/SymbolFile.cpp
@@ -19,12 +19,18 @@
#include "lldb/Utility/StreamString.h"
#include "lldb/lldb-private.h"
+#include
+
using namespace lldb_private;
void SymbolFile::PreloadSymbols() {
// No-op for most implementations.
}
+std::recursive_mutex &SymbolFile::GetModuleMutex() const {
+ return GetObjectFile()->GetModule()->GetMutex();
+}
+
SymbolFile *SymbolFile::FindPlugin(ObjectFile *obj_file) {
std::unique_ptr best_symfile_ap;
if (obj_file != nullptr) {
@@ -150,3 +156,17 @@
types.Clear();
return 0;
}
+
+void SymbolFile::AssertModuleLock() {
+ // The code below is too expensive to leave enabled in release builds. It's
+ // enabled in debug builds or when the correct macro is set.
+#if defined(LLDB_CONFIGURATION_DEBUG)
+ // We assert that we have to module lock by trying to acquire the lock from a
+ // different thread. Note that we must abort if the result is true to
+ // guarantee correctness.
+ assert(std::async(std::launch::async,
+[this] { return this->GetModuleMutex().try_lock(); })
+ .get() == false &&
+ "Module is not locked");
+#endif
+}
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -228,6 +228,8 @@
void PreloadSymbols() override;
+ std::recursive_mutex &GetModuleMutex() const override;
+
//--
// PluginInterface protocol
//--
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -260,6 +260,9 @@
}
TypeList *SymbolFileDWARF::GetTypeList() {
+ // This method can be called without going through the symbol vendor so we
+ // need to lock the module.
+ std::lock_guard guard(GetModuleMutex());
SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
if (debug_map_symfile)
return debug_map_symfile->GetTypeList();
@@ -341,6 +344,7 @@
uint32_t type_mask, TypeList &type_list)
{
+ ASSERT_MODULE_LOCK(this);
TypeSet type_set;
CompileUnit *comp_unit = NULL;
@@ -860,6 +864,7 @@
}
CompUnitSP SymbolFileDWARF::ParseCompileUnitAtIndex(uint32_t cu_idx) {
+ ASSERT_MODULE_LOCK(this);
CompUnitSP cu_sp;
DWARFDebugInfo *info = DebugInfo();
if (info) {
@@ -872,6 +877,7 @@
Function *SymbolFileDWARF::ParseCompileUnitFunction(const SymbolContext &sc,
const DWARFDIE &die) {
+ ASSERT_MODULE_LOCK(this);
if (die.IsValid()) {
TypeSystem *type_system =
GetTypeSystemForLanguage(die.GetCU()->GetLanguageType());
@@ -895,6 +901,7 @@
}
lldb::LanguageType
SymbolFileDWARF::ParseCompileUnitLanguage(const SymbolContext &sc) {
+ ASSERT_MODULE_LOCK(this);
assert(sc.comp_unit);
DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
if (dwarf_cu)
@@ -904,6 +911,7 @@
}
size_t SymbolFileDWARF::ParseCompileUnitFunctions(const SymbolContext &sc) {
+ ASSERT_MODULE_LOCK(this);
assert(sc.comp_unit);
size_t functions_added = 0;
DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
@@ -926,6 +934,7 @@
bool SymbolFileDWARF::ParseCompileUnitSupportFiles(
const SymbolContext &sc, FileSpecList &support_files) {
+ ASSERT_MODULE_LOCK(this);
assert(sc.comp_unit);
DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
if (dwarf_cu) {
@@ -951,6 +960,7 @@
bool SymbolFileDWARF::ParseCompileUnitIsOptimized(
const lldb_private::SymbolContext &sc) {
+ ASSERT_MODULE_LOCK(this);
DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
if (dwarf_cu)
return dwarf_cu->GetIsOptimized();
@@ -960,6 +970,7 @@
bool SymbolFileDWARF::ParseImportedModules(
const lldb_private::SymbolContext &sc,
std::vector &imported_modules) {
+ ASSERT_MODULE_LOCK(this);
assert(sc.comp_unit);
DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
if (dwarf_cu) {
@@ -1037,6 +1048,7 @@
}
bool SymbolFileDWARF::ParseCompileUnitLineTable(const SymbolContext &sc) {
+ ASSERT_MODULE_LOCK(this);
assert(sc.comp_unit);
if (sc.comp_unit->GetLin
[Lldb-commits] [PATCH] D53436: [LLDB] - Implement the support for the .debug_loclists section.
grimar updated this revision to Diff 170468.
grimar added a comment.
- Used lldbassert as suggested.
https://reviews.llvm.org/D53436
Files:
include/lldb/Expression/DWARFExpression.h
include/lldb/lldb-enumerations.h
source/Core/Section.cpp
source/Expression/DWARFExpression.cpp
source/Expression/IRExecutionUnit.cpp
source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
source/Symbol/ObjectFile.cpp
Index: source/Symbol/ObjectFile.cpp
===
--- source/Symbol/ObjectFile.cpp
+++ source/Symbol/ObjectFile.cpp
@@ -352,6 +352,7 @@
case eSectionTypeDWARFDebugLine:
case eSectionTypeDWARFDebugLineStr:
case eSectionTypeDWARFDebugLoc:
+ case eSectionTypeDWARFDebugLocLists:
case eSectionTypeDWARFDebugMacInfo:
case eSectionTypeDWARFDebugMacro:
case eSectionTypeDWARFDebugNames:
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp
@@ -30,7 +30,7 @@
case lldb::eSectionTypeDWARFDebugLine:
return llvm::DW_SECT_LINE;
case lldb::eSectionTypeDWARFDebugLoc:
-return llvm::DW_SECT_LOC;
+return llvm::DW_SECT_LOC;
case lldb::eSectionTypeDWARFDebugStrOffsets:
return llvm::DW_SECT_STR_OFFSETS;
// case lldb::eSectionTypeDWARFDebugMacinfo:
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -244,6 +244,7 @@
const lldb_private::DWARFDataExtractor &get_debug_line_str_data();
const lldb_private::DWARFDataExtractor &get_debug_macro_data();
const lldb_private::DWARFDataExtractor &get_debug_loc_data();
+ const lldb_private::DWARFDataExtractor &get_debug_loclists_data();
const lldb_private::DWARFDataExtractor &get_debug_ranges_data();
const lldb_private::DWARFDataExtractor &get_debug_rnglists_data();
const lldb_private::DWARFDataExtractor &get_debug_str_data();
@@ -267,6 +268,8 @@
const DWARFDebugRanges *DebugRanges() const;
+ const lldb_private::DWARFDataExtractor &DebugLocData();
+
static bool SupportedVersion(uint16_t version);
DWARFDIE
@@ -475,6 +478,7 @@
DWARFDataSegment m_data_debug_line_str;
DWARFDataSegment m_data_debug_macro;
DWARFDataSegment m_data_debug_loc;
+ DWARFDataSegment m_data_debug_loclists;
DWARFDataSegment m_data_debug_ranges;
DWARFDataSegment m_data_debug_rnglists;
DWARFDataSegment m_data_debug_str;
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -650,10 +650,22 @@
return GetCachedSectionData(eSectionTypeDWARFDebugMacro, m_data_debug_macro);
}
+const DWARFDataExtractor &SymbolFileDWARF::DebugLocData() {
+ const DWARFDataExtractor &debugLocData = get_debug_loc_data();
+ if (debugLocData.GetByteSize() > 0)
+return debugLocData;
+ return get_debug_loclists_data();
+}
+
const DWARFDataExtractor &SymbolFileDWARF::get_debug_loc_data() {
return GetCachedSectionData(eSectionTypeDWARFDebugLoc, m_data_debug_loc);
}
+const DWARFDataExtractor &SymbolFileDWARF::get_debug_loclists_data() {
+ return GetCachedSectionData(eSectionTypeDWARFDebugLocLists,
+ m_data_debug_loclists);
+}
+
const DWARFDataExtractor &SymbolFileDWARF::get_debug_ranges_data() {
return GetCachedSectionData(eSectionTypeDWARFDebugRanges,
m_data_debug_ranges);
@@ -3307,7 +3319,7 @@
uint32_t block_length = form_value.Unsigned();
location.CopyOpcodeData(module, data, block_offset, block_length);
} else {
- const DWARFDataExtractor &debug_loc_data = get_debug_loc_data();
+ const DWARFDataExtractor &debug_loc_data = DebugLocData();
const dw_offset_t debug_loc_offset = form_value.Unsigned();
size_t loc_list_length = DWARFExpression::LocationListSize(
@@ -3821,6 +3833,8 @@
DWARFExpression::LocationListFormat
SymbolFileDWARF::GetLocationListFormat() const {
+ if (m_data_debug_loclists.m_data.GetByteSize() > 0)
+return DWARFExpression::LocLists;
return DWARFExpression::RegularLocationList;
}
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
Re: [Lldb-commits] [PATCH] D53368: [Symbol] Search symbols with name and type in a symbol file
To answer your question, PE/COFF executable symbol tables are basically empty On Mon, Oct 22, 2018 at 10:44 AM Greg Clayton via Phabricator < revi...@reviews.llvm.org> wrote: > clayborg requested changes to this revision. > clayborg added a comment. > This revision now requires changes to proceed. > > All symbol tables are currently extracted from the object files via > ObjectFile::GetSymtab(). Are symbols only in the PDB file? If so I would > vote to add a "virtual void SymbolVendor::AddSymbols(Symtab *symtab)" and a > "virtual void SymbolFile::AddSymbols(Symtab *symtab)" where we take the > symbol table that comes from the object file and we can add symbols to it > if the symbol file has symbols it wants to add to the object file's symbol > table. All symbol queries go through the lldb_private::Symtab class anyway. > Care must be taken to watch out for symbols that might already exist from > an ObjectFile's symbol table to ensure don't have duplicates. > > So I would: > > - Add "virtual void SymbolVendor::AddSymbols(Symtab *symtab);" to > SymbolVendor that just calls through to its SymbolFile to do the work > - Add "virtual void SymbolFile::AddSymbols(Symtab *symtab)" to SymbolFile > with default implementation that does nothing > - Override SymbolFile::AddSymbols() for SymbolFilePDB and add symbols to > the provided symbol table > - Modify *SymbolVendor::GetSymtab()" to get the object file symbol table, > then pass that along to any symbol file instances it owns to allow each > symbol file to augment the symbol table > - Remove all "FindPublicSymbols()" code from patch > - Revert all symbol searching code to just use the Symtab class now that > it contains all needed symbols > > > Repository: > rLLDB LLDB > > https://reviews.llvm.org/D53368 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53368: [Symbol] Search symbols with name and type in a symbol file
zturner added a subscriber: aleksandr.urakov. zturner added a comment. To answer your question, PE/COFF executable symbol tables are basically empty Repository: rLLDB LLDB https://reviews.llvm.org/D53368 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52543: [DWARFSymbolFile] Add the module lock where necessary and assert that we own it.
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D52543 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected
cameron314 added a comment. I suppose we could but there's a few places outside of SBProcess that also use the run lock and API mutex; personally, I prefer it to be explicit which mutex is being taken first. Repository: rLLDB LLDB https://reviews.llvm.org/D53412 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r344945 - [SymbolFile] Add the module lock where necessary and assert that we own it.
Author: jdevlieghere
Date: Mon Oct 22 13:14:36 2018
New Revision: 344945
URL: http://llvm.org/viewvc/llvm-project?rev=344945&view=rev
Log:
[SymbolFile] Add the module lock where necessary and assert that we own it.
As discussed with Greg at the dev meeting, we need to ensure we have the
module lock in the SymbolFile. Usually the symbol file is accessed
through the symbol vendor which ensures that the necessary locks are
taken. However, there are a few methods that are accessed by the
expression parser and were lacking the lock.
This patch adds the locking where necessary and everywhere else asserts
that we actually already own the lock.
Differential revision: https://reviews.llvm.org/D52543
Modified:
lldb/trunk/include/lldb/Symbol/SymbolFile.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/trunk/source/Symbol/SymbolFile.cpp
Modified: lldb/trunk/include/lldb/Symbol/SymbolFile.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/SymbolFile.h?rev=344945&r1=344944&r2=344945&view=diff
==
--- lldb/trunk/include/lldb/Symbol/SymbolFile.h (original)
+++ lldb/trunk/include/lldb/Symbol/SymbolFile.h Mon Oct 22 13:14:36 2018
@@ -20,6 +20,14 @@
#include "llvm/ADT/DenseSet.h"
+#include
+
+#if defined(LLDB_CONFIGURATION_DEBUG)
+#define ASSERT_MODULE_LOCK(expr) (expr->AssertModuleLock();)
+#else
+#define ASSERT_MODULE_LOCK(expr) ((void)0)
+#endif
+
namespace lldb_private {
class SymbolFile : public PluginInterface {
@@ -94,6 +102,12 @@ public:
virtual uint32_t CalculateAbilities() = 0;
//--
+ /// Symbols file subclasses should override this to return the Module that
+ /// owns the TypeSystem that this symbol file modifies type information in.
+ //--
+ virtual std::recursive_mutex &GetModuleMutex() const;
+
+ //--
/// Initialize the SymbolFile object.
///
/// The SymbolFile object with the best set of abilities (detected
@@ -208,6 +222,8 @@ public:
virtual void Dump(Stream &s) {}
protected:
+ void AssertModuleLock();
+
ObjectFile *m_obj_file; // The object file that symbols can be extracted
from.
uint32_t m_abilities;
bool m_calculated_abilities;
Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp?rev=344945&r1=344944&r2=344945&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Mon Oct 22
13:14:36 2018
@@ -260,6 +260,9 @@ SymbolFile *SymbolFileDWARF::CreateInsta
}
TypeList *SymbolFileDWARF::GetTypeList() {
+ // This method can be called without going through the symbol vendor so we
+ // need to lock the module.
+ std::lock_guard guard(GetModuleMutex());
SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
if (debug_map_symfile)
return debug_map_symfile->GetTypeList();
@@ -341,6 +344,7 @@ size_t SymbolFileDWARF::GetTypes(SymbolC
uint32_t type_mask, TypeList &type_list)
{
+ ASSERT_MODULE_LOCK(this);
TypeSet type_set;
CompileUnit *comp_unit = NULL;
@@ -860,6 +864,7 @@ uint32_t SymbolFileDWARF::GetNumCompileU
}
CompUnitSP SymbolFileDWARF::ParseCompileUnitAtIndex(uint32_t cu_idx) {
+ ASSERT_MODULE_LOCK(this);
CompUnitSP cu_sp;
DWARFDebugInfo *info = DebugInfo();
if (info) {
@@ -872,6 +877,7 @@ CompUnitSP SymbolFileDWARF::ParseCompile
Function *SymbolFileDWARF::ParseCompileUnitFunction(const SymbolContext &sc,
const DWARFDIE &die) {
+ ASSERT_MODULE_LOCK(this);
if (die.IsValid()) {
TypeSystem *type_system =
GetTypeSystemForLanguage(die.GetCU()->GetLanguageType());
@@ -895,6 +901,7 @@ bool SymbolFileDWARF::FixupAddress(Addre
}
lldb::LanguageType
SymbolFileDWARF::ParseCompileUnitLanguage(const SymbolContext &sc) {
+ ASSERT_MODULE_LOCK(this);
assert(sc.comp_unit);
DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
if (dwarf_cu)
@@ -904,6 +911,7 @@ SymbolFileDWARF::ParseCompileUnitLanguag
}
size_t SymbolFileDWARF::ParseCompileUnitFunctions(const SymbolContext &sc) {
+ ASSERT_MODULE_LOCK(this);
assert(sc.comp_unit);
size_t functions_added = 0;
DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
@@ -926,6 +934,7 @@ size_t SymbolFileDWARF::ParseCompileUnit
bool SymbolFileDWARF::ParseCompileUnitSupportFiles(
const SymbolContext &sc, FileSpecList &support_files) {
+ ASSERT_MODULE_LOCK(this);
[Lldb-commits] [PATCH] D52543: [DWARFSymbolFile] Add the module lock where necessary and assert that we own it.
This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB344945: [SymbolFile] Add the module lock where necessary
and assert that we own it. (authored by JDevlieghere, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52543?vs=170465&id=170482#toc
Repository:
rLLDB LLDB
https://reviews.llvm.org/D52543
Files:
include/lldb/Symbol/SymbolFile.h
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
source/Symbol/SymbolFile.cpp
Index: source/Symbol/SymbolFile.cpp
===
--- source/Symbol/SymbolFile.cpp
+++ source/Symbol/SymbolFile.cpp
@@ -19,12 +19,18 @@
#include "lldb/Utility/StreamString.h"
#include "lldb/lldb-private.h"
+#include
+
using namespace lldb_private;
void SymbolFile::PreloadSymbols() {
// No-op for most implementations.
}
+std::recursive_mutex &SymbolFile::GetModuleMutex() const {
+ return GetObjectFile()->GetModule()->GetMutex();
+}
+
SymbolFile *SymbolFile::FindPlugin(ObjectFile *obj_file) {
std::unique_ptr best_symfile_ap;
if (obj_file != nullptr) {
@@ -150,3 +156,17 @@
types.Clear();
return 0;
}
+
+void SymbolFile::AssertModuleLock() {
+ // The code below is too expensive to leave enabled in release builds. It's
+ // enabled in debug builds or when the correct macro is set.
+#if defined(LLDB_CONFIGURATION_DEBUG)
+ // We assert that we have to module lock by trying to acquire the lock from a
+ // different thread. Note that we must abort if the result is true to
+ // guarantee correctness.
+ assert(std::async(std::launch::async,
+[this] { return this->GetModuleMutex().try_lock(); })
+ .get() == false &&
+ "Module is not locked");
+#endif
+}
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -260,6 +260,9 @@
}
TypeList *SymbolFileDWARF::GetTypeList() {
+ // This method can be called without going through the symbol vendor so we
+ // need to lock the module.
+ std::lock_guard guard(GetModuleMutex());
SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
if (debug_map_symfile)
return debug_map_symfile->GetTypeList();
@@ -341,6 +344,7 @@
uint32_t type_mask, TypeList &type_list)
{
+ ASSERT_MODULE_LOCK(this);
TypeSet type_set;
CompileUnit *comp_unit = NULL;
@@ -860,6 +864,7 @@
}
CompUnitSP SymbolFileDWARF::ParseCompileUnitAtIndex(uint32_t cu_idx) {
+ ASSERT_MODULE_LOCK(this);
CompUnitSP cu_sp;
DWARFDebugInfo *info = DebugInfo();
if (info) {
@@ -872,6 +877,7 @@
Function *SymbolFileDWARF::ParseCompileUnitFunction(const SymbolContext &sc,
const DWARFDIE &die) {
+ ASSERT_MODULE_LOCK(this);
if (die.IsValid()) {
TypeSystem *type_system =
GetTypeSystemForLanguage(die.GetCU()->GetLanguageType());
@@ -895,6 +901,7 @@
}
lldb::LanguageType
SymbolFileDWARF::ParseCompileUnitLanguage(const SymbolContext &sc) {
+ ASSERT_MODULE_LOCK(this);
assert(sc.comp_unit);
DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
if (dwarf_cu)
@@ -904,6 +911,7 @@
}
size_t SymbolFileDWARF::ParseCompileUnitFunctions(const SymbolContext &sc) {
+ ASSERT_MODULE_LOCK(this);
assert(sc.comp_unit);
size_t functions_added = 0;
DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
@@ -926,6 +934,7 @@
bool SymbolFileDWARF::ParseCompileUnitSupportFiles(
const SymbolContext &sc, FileSpecList &support_files) {
+ ASSERT_MODULE_LOCK(this);
assert(sc.comp_unit);
DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
if (dwarf_cu) {
@@ -951,6 +960,7 @@
bool SymbolFileDWARF::ParseCompileUnitIsOptimized(
const lldb_private::SymbolContext &sc) {
+ ASSERT_MODULE_LOCK(this);
DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
if (dwarf_cu)
return dwarf_cu->GetIsOptimized();
@@ -960,6 +970,7 @@
bool SymbolFileDWARF::ParseImportedModules(
const lldb_private::SymbolContext &sc,
std::vector &imported_modules) {
+ ASSERT_MODULE_LOCK(this);
assert(sc.comp_unit);
DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
if (dwarf_cu) {
@@ -1037,6 +1048,7 @@
}
bool SymbolFileDWARF::ParseCompileUnitLineTable(const SymbolContext &sc) {
+ ASSERT_MODULE_LOCK(this);
assert(sc.comp_unit);
if (sc.comp_unit->GetLineTable() != NULL)
return true;
@@ -1122,6 +1134,7 @@
}
bool SymbolFileDWARF::ParseCompileUnitDebugMacros(const SymbolContext &sc) {
+ ASSERT_MODULE_LOCK(this);
assert(sc.comp_unit);
DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
@@ -1301,6 +1314,9 @@
}
SymbolFileDWARF *SymbolFileDWARF::GetDWARFForUID(lldb::user_id_
[Lldb-commits] [PATCH] D53375: [PDB] Improve performance of the PDB DIA plugin
Hui added a comment. In https://reviews.llvm.org/D53375#1269504, @asmith wrote: > Thanks for adding the cache. We noticed the slowdown also and were discussing > the same thing. This LGTM if the other reviewers done have any comments. I think parsing types e.g. SymbolFilePDB::ParseTypes also has speed bumps. Will be good to have them cached too. Repository: rLLDB LLDB https://reviews.llvm.org/D53375 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53511: [NativePDB] Support type lookup by name
lemo added a comment.
Nice :)
Comment at: lldb/lit/SymbolFile/NativePDB/tag-types.cpp:87
+// Test single inheritance.
+class Derived : public Class {
+public:
- at least one virtual function + override?
- at least one method returning void?
Comment at: lldb/lit/SymbolFile/NativePDB/tag-types.cpp:94
+ // infinite cycle.
+ Derived &Reference;
+
pointer to itself too?
Comment at: lldb/lit/SymbolFile/NativePDB/tag-types.cpp:110
+
+// Test multiple inheritance, as well as protected and private inheritance.
+class Derived2 : protected Class, private Struct {
just an idea - add a virtual inheritance variation?
Comment at: lldb/lit/SymbolFile/NativePDB/tag-types.cpp:131
+
+int main(int argc, char **argv) {
+ Struct S;
a few suggestions for additional things to cover:
- local classes
- lambdas
- instantiating class and function templates
- explicit specializations
- for classes, partial specializations
- namespaces
- anonymous namespace
https://reviews.llvm.org/D53511
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53511: [NativePDB] Support type lookup by name
zturner added inline comments.
Comment at: lldb/lit/SymbolFile/NativePDB/tag-types.cpp:87
+// Test single inheritance.
+class Derived : public Class {
+public:
lemo wrote:
> - at least one virtual function + override?
> - at least one method returning void?
The reason I didn't add any tests for methods is because I didn't add anything
in the implementation to parse method records either. As I said in the patch
description, it was a balance between providing some piece of useful
functionality while keeping the patch size down. So I tried to limit the scope
to just fields minus bitfields, since there's some extra complexity there that
I wanted to punt on for now. The reason I have the constructor is because it
was required in order to initialize the reference member, otherwise it wouldn't
compile.
Comment at: lldb/lit/SymbolFile/NativePDB/tag-types.cpp:110
+
+// Test multiple inheritance, as well as protected and private inheritance.
+class Derived2 : protected Class, private Struct {
lemo wrote:
> just an idea - add a virtual inheritance variation?
Virtual inheritance is a good followup, but if you look at
`UdtRecordCompleter.cpp` and Ctrl+F for `VirtualBaseClassRecord`, I basically
treat them as regular base classes and I put a fixme to handle the virtual
aspects of the base. So that's a known problem right now.
Comment at: lldb/lit/SymbolFile/NativePDB/tag-types.cpp:131
+
+int main(int argc, char **argv) {
+ Struct S;
lemo wrote:
> a few suggestions for additional things to cover:
> - local classes
> - lambdas
> - instantiating class and function templates
>- explicit specializations
>- for classes, partial specializations
> - namespaces
> - anonymous namespace
Some of these I could probably handle right now. I tried to keep the scope of
the patch to "types which would be named", because it makes it easy to write
tests (the test can just do lookup by name, basically a WinDbg `dt` test.).
That makes local classes, lambdas, anonymous namespace, and anything to do with
functions out of scope.
https://reviews.llvm.org/D53511
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types
aprantl created this revision.
aprantl added reviewers: jingham, clayborg.
aprantl added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Clang recently improved its DWARF support for C VLA types. The DWARF now looks
like this:
0x0051: DW_TAG_variable [4]
DW_AT_location( fbreg -32 )
DW_AT_name( "__vla_expr" )
DW_AT_type( {0x00d3} ( long unsigned int ) )
DW_AT_artificial( true )
...
0x00da: DW_TAG_array_type [10] *
DW_AT_type( {0x00cc} ( int ) )
0x00df: DW_TAG_subrange_type [11]
DW_AT_type( {0x00e9} ( __ARRAY_SIZE_TYPE__ ) )
DW_AT_count( {0x0051} )
Without this patch LLDB will naively interpret the DIE offset 0x51 as the
static size of the array, which is clearly wrong.
This patch uses LLDB's dynamic type mechanism to re-parse VLA types with an
optional execution context, to dynamically resolve the size of the array
correctly. These dynamic types are not being cached, since they are only valid
in a single execution context.
See the testcase for an example:
4 int foo(int a) {
5 int vla[a];
6for (int i = 0; i < a; ++i)
7 vla[i] = i;
8
-> 9pause(); // break here
10 return vla[a-1];
11 }
12
(lldb) fr v vla
(int [4]) vla = ([0] = 0, [1] = 1, [2] = 2, [3] = 3)
(lldb) quit
https://reviews.llvm.org/D53530
Files:
include/lldb/Symbol/SymbolFile.h
include/lldb/Symbol/Variable.h
packages/Python/lldbsuite/test/lang/c/vla/Makefile
packages/Python/lldbsuite/test/lang/c/vla/TestVLA.py
packages/Python/lldbsuite/test/lang/c/vla/main.c
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
source/Plugins/SymbolFile/DWARF/DWARFASTParserGo.cpp
source/Plugins/SymbolFile/DWARF/DWARFASTParserGo.h
source/Plugins/SymbolFile/DWARF/DWARFASTParserJava.cpp
source/Plugins/SymbolFile/DWARF/DWARFASTParserJava.h
source/Plugins/SymbolFile/DWARF/DWARFASTParserOCaml.cpp
source/Plugins/SymbolFile/DWARF/DWARFASTParserOCaml.h
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
source/Symbol/ClangASTContext.cpp
Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -3696,6 +3696,9 @@
return IsPossibleDynamicType(
llvm::cast(qual_type)->desugar().getAsOpaquePtr(),
dynamic_pointee_type, check_cplusplus, check_objc);
+
+case clang::Type::IncompleteArray:
+ return true;
default:
break;
}
Index: source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
===
--- source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
+++ source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
@@ -79,7 +79,9 @@
size_t
ParseVariablesForContext(const lldb_private::SymbolContext &sc) override;
- lldb_private::Type *ResolveTypeUID(lldb::user_id_t type_uid) override;
+ lldb_private::Type *
+ ResolveTypeUID(lldb::user_id_t type_uid,
+ const lldb_private::ExecutionContextRef exe_ctx = {}) override;
bool CompleteType(lldb_private::CompilerType &compiler_type) override;
Index: source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
===
--- source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
+++ source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
@@ -235,7 +235,8 @@
return 0;
}
-Type *SymbolFileSymtab::ResolveTypeUID(lldb::user_id_t type_uid) {
+Type *SymbolFileSymtab::ResolveTypeUID(
+lldb::user_id_t type_uid, const lldb_private::ExecutionContextRef exe_ctx) {
return NULL;
}
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -84,7 +84,9 @@
size_t
ParseVariablesForContext(const lldb_private::SymbolContext &sc) override;
- lldb_private::Type *ResolveTypeUID(lldb::user_id_t type_uid) override;
+ lldb_private::Type *
+ Resolv
[Lldb-commits] [lldb] r344960 - [DWARF] Use a function-local offset for AT_call_return_pc
Author: vedantk
Date: Mon Oct 22 14:44:21 2018
New Revision: 344960
URL: http://llvm.org/viewvc/llvm-project?rev=344960&view=rev
Log:
[DWARF] Use a function-local offset for AT_call_return_pc
Logs provided by @stella.stamenova indicate that on Linux, lldb adds a
spurious slide offset to the return PC it loads from AT_call_return_pc
attributes (see the list thread: "[PATCH] D50478: Add support for
artificial tail call frames").
This patch side-steps the issue by getting rid of the load address
calculation in lldb's CallEdge::GetReturnPCAddress.
The idea is to have the DWARF writer emit function-local offsets to the
instruction after a call. I.e. return-pc = label-after-call-insn -
function-entry. LLDB can simply add this offset to the base address of a
function to get the return PC.
Differential Revision: https://reviews.llvm.org/D53469
Modified:
lldb/trunk/include/lldb/Symbol/Function.h
lldb/trunk/source/Symbol/Function.cpp
Modified: lldb/trunk/include/lldb/Symbol/Function.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/Function.h?rev=344960&r1=344959&r2=344960&view=diff
==
--- lldb/trunk/include/lldb/Symbol/Function.h (original)
+++ lldb/trunk/include/lldb/Symbol/Function.h Mon Oct 22 14:44:21 2018
@@ -324,7 +324,8 @@ public:
/// made the call.
lldb::addr_t GetReturnPCAddress(Function &caller, Target &target) const;
- /// Like \ref GetReturnPCAddress, but returns an unresolved file address.
+ /// Like \ref GetReturnPCAddress, but returns an unslid function-local PC
+ /// offset.
lldb::addr_t GetUnresolvedReturnPCAddress() const { return return_pc; }
private:
@@ -337,8 +338,9 @@ private:
Function *def;
} lazy_callee;
- /// An invalid address if this is a tail call. Otherwise, the return PC for
- /// the call. Note that this is a file address which must be resolved.
+ /// An invalid address if this is a tail call. Otherwise, the function-local
+ /// PC offset. Adding this PC offset to the function's base load address
+ /// gives the return PC for the call.
lldb::addr_t return_pc;
/// Whether or not an attempt was made to find the callee's definition.
Modified: lldb/trunk/source/Symbol/Function.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/Function.cpp?rev=344960&r1=344959&r2=344960&view=diff
==
--- lldb/trunk/source/Symbol/Function.cpp (original)
+++ lldb/trunk/source/Symbol/Function.cpp Mon Oct 22 14:44:21 2018
@@ -180,8 +180,7 @@ Function *CallEdge::GetCallee(ModuleList
lldb::addr_t CallEdge::GetReturnPCAddress(Function &caller,
Target &target) const {
const Address &base = caller.GetAddressRange().GetBaseAddress();
- Address return_pc_addr{base.GetSection(), return_pc};
- return return_pc_addr.GetLoadAddress(&target);
+ return base.GetLoadAddress(&target) + return_pc;
}
//--
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53532: [FileSpec] Add VFS support
JDevlieghere created this revision.
JDevlieghere added reviewers: zturner, labath, clayborg.
JDevlieghere added a project: LLDB.
This patch adds support for virtual file systems by delegating relevant
operations. By default the real file system is used so this should be NFC for
all intents and purposes.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D53532
Files:
include/lldb/Utility/FileSpec.h
source/API/SBFileSpec.cpp
source/Utility/FileSpec.cpp
unittests/Utility/FileSpecTest.cpp
Index: unittests/Utility/FileSpecTest.cpp
===
--- unittests/Utility/FileSpecTest.cpp
+++ unittests/Utility/FileSpecTest.cpp
@@ -10,8 +10,11 @@
#include "gtest/gtest.h"
#include "lldb/Utility/FileSpec.h"
+#include "llvm/Support/Errc.h"
using namespace lldb_private;
+using namespace llvm;
+using llvm::sys::fs::UniqueID;
TEST(FileSpecTest, FileAndDirectoryComponents) {
FileSpec fs_posix("/foo/bar", false, FileSpec::Style::posix);
@@ -370,3 +373,180 @@
EXPECT_FALSE(fs_windows.RemoveLastPathComponent());
EXPECT_STREQ("C:", fs_windows.GetCString());
}
+
+// Copied verbatim from unittests/Support/VirtualFileSystemTest.cpp
+namespace {
+struct DummyFile : public vfs::File {
+ vfs::Status S;
+ explicit DummyFile(vfs::Status S) : S(S) {}
+ llvm::ErrorOr status() override { return S; }
+ llvm::ErrorOr>
+ getBuffer(const Twine &Name, int64_t FileSize, bool RequiresNullTerminator,
+bool IsVolatile) override {
+llvm_unreachable("unimplemented");
+ }
+ std::error_code close() override { return std::error_code(); }
+};
+
+class DummyFileSystem : public vfs::FileSystem {
+ int FSID; // used to produce UniqueIDs
+ int FileID; // used to produce UniqueIDs
+ std::map FilesAndDirs;
+
+ static int getNextFSID() {
+static int Count = 0;
+return Count++;
+ }
+
+public:
+ DummyFileSystem() : FSID(getNextFSID()), FileID(0) {}
+
+ ErrorOr status(const Twine &Path) override {
+std::map::iterator I =
+FilesAndDirs.find(Path.str());
+if (I == FilesAndDirs.end())
+ return make_error_code(llvm::errc::no_such_file_or_directory);
+return I->second;
+ }
+ ErrorOr>
+ openFileForRead(const Twine &Path) override {
+auto S = status(Path);
+if (S)
+ return std::unique_ptr(new DummyFile{*S});
+return S.getError();
+ }
+ llvm::ErrorOr getCurrentWorkingDirectory() const override {
+return std::string();
+ }
+ std::error_code setCurrentWorkingDirectory(const Twine &Path) override {
+return std::error_code();
+ }
+ // Map any symlink to "/symlink".
+ std::error_code getRealPath(const Twine &Path,
+ SmallVectorImpl &Output) const override {
+auto I = FilesAndDirs.find(Path.str());
+if (I == FilesAndDirs.end())
+ return make_error_code(llvm::errc::no_such_file_or_directory);
+if (I->second.isSymlink()) {
+ Output.clear();
+ Twine("/symlink").toVector(Output);
+ return std::error_code();
+}
+Output.clear();
+Path.toVector(Output);
+return std::error_code();
+ }
+
+ struct DirIterImpl : public llvm::vfs::detail::DirIterImpl {
+std::map &FilesAndDirs;
+std::map::iterator I;
+std::string Path;
+bool isInPath(StringRef S) {
+ if (Path.size() < S.size() && S.find(Path) == 0) {
+auto LastSep = S.find_last_of('/');
+if (LastSep == Path.size() || LastSep == Path.size() - 1)
+ return true;
+ }
+ return false;
+}
+DirIterImpl(std::map &FilesAndDirs,
+const Twine &_Path)
+: FilesAndDirs(FilesAndDirs), I(FilesAndDirs.begin()),
+ Path(_Path.str()) {
+ for (; I != FilesAndDirs.end(); ++I) {
+if (isInPath(I->first)) {
+ CurrentEntry =
+ vfs::directory_entry(I->second.getName(), I->second.getType());
+ break;
+}
+ }
+}
+std::error_code increment() override {
+ ++I;
+ for (; I != FilesAndDirs.end(); ++I) {
+if (isInPath(I->first)) {
+ CurrentEntry =
+ vfs::directory_entry(I->second.getName(), I->second.getType());
+ break;
+}
+ }
+ if (I == FilesAndDirs.end())
+CurrentEntry = vfs::directory_entry();
+ return std::error_code();
+}
+ };
+
+ vfs::directory_iterator dir_begin(const Twine &Dir,
+std::error_code &EC) override {
+return vfs::directory_iterator(
+std::make_shared(FilesAndDirs, Dir));
+ }
+
+ void addEntry(StringRef Path, const vfs::Status &Status) {
+FilesAndDirs[Path] = Status;
+ }
+
+ void addRegularFile(StringRef Path, sys::fs::perms Perms = sys::fs::all_all) {
+vfs::Status S(Path, UniqueID(FSID, FileID++),
+ std::chrono::system_clock::now(), 0, 0, 1024,
+ sys::fs::file_type::regular_file, Perms);
+addEntry(Path, S);
+ }
+
+ void addDirectory(StringRef Path, sys:
[Lldb-commits] [PATCH] D53511: [NativePDB] Support type lookup by name
lemo accepted this revision.
lemo added a comment.
This revision is now accepted and ready to land.
Looks good to me, minor notes inline.
Comment at: lldb/lit/SymbolFile/NativePDB/tag-types.cpp:131
+
+int main(int argc, char **argv) {
+ Struct S;
zturner wrote:
> lemo wrote:
> > a few suggestions for additional things to cover:
> > - local classes
> > - lambdas
> > - instantiating class and function templates
> >- explicit specializations
> >- for classes, partial specializations
> > - namespaces
> > - anonymous namespace
> Some of these I could probably handle right now. I tried to keep the scope
> of the patch to "types which would be named", because it makes it easy to
> write tests (the test can just do lookup by name, basically a WinDbg `dt`
> test.). That makes local classes, lambdas, anonymous namespace, and anything
> to do with functions out of scope.
k. how about more basic stuff like?
- pointer to pointer
- const/volatile
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:313
+ case SimpleTypeKind::SByte:
+return "signed chr";
+ case SimpleTypeKind::Character16:
char
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:335
+ case SimpleTypeKind::Int64Quad:
+return "__int64";
+ case SimpleTypeKind::Int32:
int64_t ?
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:344
+ case SimpleTypeKind::UInt64Quad:
+return "unsigned __int64";
+ case SimpleTypeKind::HResult:
unsigned int64_t ?
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:660
+ct = ct.GetPointerType();
+uint32_t pointer_size = 4;
+switch (ti.getSimpleMode()) {
minor: = 0; ?
(to make any potential mistakes more obvious)
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:687
+ CompilerType ct = m_clang->GetBasicType(bt);
+ size_t size = GetTypeSizeForSimpleKind(ti.getSimpleKind());
+
assert size > 0 ?
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:189
+}
\ No newline at end of file
nit: add empty line at the end of file
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h:26
+namespace lldb_private {
+class Type;
+class CompilerType;
nit: vertical space between { and the next line
https://reviews.llvm.org/D53511
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types
jingham added a comment. This seems good to me, but Greg is more expert than I so I'd wait on his okay. Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1901-1907 +dwarf->GetTypeList()->Insert(type_sp); +// Cache the type if it isn't context-specific. +auto &cache = dwarf->GetDIEToType(); +if (exe_ctx.GetFrameSP()) + cache.erase(die.GetDIE()); +else + cache[die.GetDIE()] = type_sp.get(); You have to do this twice, maybe there should be a method to add the results to the DIE to Type map? https://reviews.llvm.org/D53530 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types
aprantl updated this revision to Diff 170526.
aprantl added a comment.
Factor out caching of types as suggested. Thanks!
https://reviews.llvm.org/D53530
Files:
include/lldb/Symbol/SymbolFile.h
include/lldb/Symbol/Variable.h
packages/Python/lldbsuite/test/lang/c/vla/Makefile
packages/Python/lldbsuite/test/lang/c/vla/TestVLA.py
packages/Python/lldbsuite/test/lang/c/vla/main.c
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
source/Plugins/SymbolFile/DWARF/DWARFASTParserGo.cpp
source/Plugins/SymbolFile/DWARF/DWARFASTParserGo.h
source/Plugins/SymbolFile/DWARF/DWARFASTParserJava.cpp
source/Plugins/SymbolFile/DWARF/DWARFASTParserJava.h
source/Plugins/SymbolFile/DWARF/DWARFASTParserOCaml.cpp
source/Plugins/SymbolFile/DWARF/DWARFASTParserOCaml.h
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
source/Symbol/ClangASTContext.cpp
Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -3696,6 +3696,9 @@
return IsPossibleDynamicType(
llvm::cast(qual_type)->desugar().getAsOpaquePtr(),
dynamic_pointee_type, check_cplusplus, check_objc);
+
+case clang::Type::IncompleteArray:
+ return true;
default:
break;
}
Index: source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
===
--- source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
+++ source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
@@ -79,7 +79,9 @@
size_t
ParseVariablesForContext(const lldb_private::SymbolContext &sc) override;
- lldb_private::Type *ResolveTypeUID(lldb::user_id_t type_uid) override;
+ lldb_private::Type *
+ ResolveTypeUID(lldb::user_id_t type_uid,
+ const lldb_private::ExecutionContextRef exe_ctx = {}) override;
bool CompleteType(lldb_private::CompilerType &compiler_type) override;
Index: source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
===
--- source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
+++ source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
@@ -235,7 +235,8 @@
return 0;
}
-Type *SymbolFileSymtab::ResolveTypeUID(lldb::user_id_t type_uid) {
+Type *SymbolFileSymtab::ResolveTypeUID(
+lldb::user_id_t type_uid, const lldb_private::ExecutionContextRef exe_ctx) {
return NULL;
}
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -84,7 +84,9 @@
size_t
ParseVariablesForContext(const lldb_private::SymbolContext &sc) override;
- lldb_private::Type *ResolveTypeUID(lldb::user_id_t type_uid) override;
+ lldb_private::Type *
+ ResolveTypeUID(lldb::user_id_t type_uid,
+ const lldb_private::ExecutionContextRef exe_ctx = {}) override;
bool CompleteType(lldb_private::CompilerType &compiler_type) override;
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -558,7 +558,9 @@
return num_added;
}
-lldb_private::Type *SymbolFilePDB::ResolveTypeUID(lldb::user_id_t type_uid) {
+lldb_private::Type *
+SymbolFilePDB::ResolveTypeUID(lldb::user_id_t type_uid,
+ const lldb_private::ExecutionContextRef exe_ctx) {
auto find_result = m_types.find(type_uid);
if (find_result != m_types.end())
return find_result->second.get();
Index: source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
===
--- source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
+++ source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
@@ -116,7 +116,9 @@
ParseVariablesForContext(const lldb_private::SymbolContext &sc) override {
return 0;
}
- lldb_private::Type *ResolveTypeUID(lldb::user_id_t type_uid) override {
+ lldb_private::Type *
+ ResolveTypeUID(lldb::user_id_t type_uid,
+ const lldb_private::ExecutionContextRef exe_ctx) override {
return nullptr;
[Lldb-commits] [lldb] r344979 - Fix typo in ASSERT_MODULE_LOCK macro definition
Author: jdevlieghere Date: Mon Oct 22 17:18:27 2018 New Revision: 344979 URL: http://llvm.org/viewvc/llvm-project?rev=344979&view=rev Log: Fix typo in ASSERT_MODULE_LOCK macro definition Modified: lldb/trunk/include/lldb/Symbol/SymbolFile.h Modified: lldb/trunk/include/lldb/Symbol/SymbolFile.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/SymbolFile.h?rev=344979&r1=344978&r2=344979&view=diff == --- lldb/trunk/include/lldb/Symbol/SymbolFile.h (original) +++ lldb/trunk/include/lldb/Symbol/SymbolFile.h Mon Oct 22 17:18:27 2018 @@ -23,7 +23,7 @@ #include #if defined(LLDB_CONFIGURATION_DEBUG) -#define ASSERT_MODULE_LOCK(expr) (expr->AssertModuleLock();) +#define ASSERT_MODULE_LOCK(expr) (expr->AssertModuleLock()) #else #define ASSERT_MODULE_LOCK(expr) ((void)0) #endif ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r344982 - [ValueObject] Stop assuming types are non-zero sized.
Author: davide
Date: Mon Oct 22 17:31:46 2018
New Revision: 344982
URL: http://llvm.org/viewvc/llvm-project?rev=344982&view=rev
Log:
[ValueObject] Stop assuming types are non-zero sized.
Some backends might violate this assumption. No test case
upstream unfortunately as this is not the case with C++,
but I'm going to add a test in swift language support.
Modified:
lldb/trunk/source/Core/ValueObjectConstResultImpl.cpp
Modified: lldb/trunk/source/Core/ValueObjectConstResultImpl.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ValueObjectConstResultImpl.cpp?rev=344982&r1=344981&r2=344982&view=diff
==
--- lldb/trunk/source/Core/ValueObjectConstResultImpl.cpp (original)
+++ lldb/trunk/source/Core/ValueObjectConstResultImpl.cpp Mon Oct 22 17:31:46
2018
@@ -77,7 +77,13 @@ ValueObject *ValueObjectConstResultImpl:
ignore_array_bounds, child_name_str, child_byte_size, child_byte_offset,
child_bitfield_bit_size, child_bitfield_bit_offset, child_is_base_class,
child_is_deref_of_parent, m_impl_backend, language_flags);
- if (child_compiler_type && child_byte_size) {
+
+ // One might think we should check that the size of the children
+ // is always strictly positive, hence we could avoid creating a
+ // ValueObject if that's not the case, but it turns out there
+ // are languages out there which allow zero-size types with
+ // children (e.g. Swift).
+ if (child_compiler_type) {
if (synthetic_index)
child_byte_offset += child_byte_size * synthetic_index;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
