[Lldb-commits] [PATCH] D53532: [FileSpec] Add VFS support to FileSpec convenience methods.
labath added a comment.
I also think it we should try to keep FileSpec in the Utility module. We should
be able to do that by deleting the "utility" functions in the FileSpec class,
and updating everything to go through the FileSystem class.
After the VFS introduction, something like `file_spec.Exists()` will not even
be well-defined anyway, as one will have to specify which file system are we
checking the existence in. So, it would be more correct to say
`file_spec.Exists(my_vfs)`, at which point it's not too much of a stretch to
change this into `my_vfs.Exists(file_spec)`.
(This is the same argument I was giving earlier, since even before VFS we kinda
were dealing with multiple file systems (host, remote device). In fact, after
this it might be a nice cleanup to change the Platform class from vending a
bunch of FS-like methods to (FileExists, Unlink, CreateSymlink, ...), to have a
single `getRemoteFS` method returning a VFS which does the right thing "under
the hood").
tl;dr: I'd propose to (in two or more patches):
- replace FileSpec "utility" methods with equivalent FileSystem functionality
- do the necessary FileSystem changes to support VFS
Comment at: source/Host/common/FileSystem.cpp:24-29
+ static FileSystem *g_fs;
+ static llvm::once_flag g_once_flag;
+ llvm::call_once(g_once_flag, []() {
+if (g_fs == nullptr)
+ g_fs = new FileSystem();
+ });
replace with `static FileSystem *g_fs = new FileSystem();`
Comment at: source/Host/common/FileSystem.cpp:33-35
+void FileSystem::ChangeFileSystem(IntrusiveRefCntPtr fs) {
+ m_fs = fs;
+}
I'm not sure what's your use case here, but would it make more sense instead of
modifying the singleton object to have more that one FileSystem instance
floating around? It could be local to each `Debugger` (?) object and everyone
would fetch it from there when needed.
https://reviews.llvm.org/D53532
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r345247 - [API] Extend the `SBThreadPlan` interface
Author: aleksandr.urakov
Date: Thu Oct 25 01:27:42 2018
New Revision: 345247
URL: http://llvm.org/viewvc/llvm-project?rev=345247&view=rev
Log:
[API] Extend the `SBThreadPlan` interface
Summary:
This patch extends the `SBThreadPlan` to allow retrieving of thread plans
for scripted steps.
Reviewers: labath, zturner, jingham
Reviewed By: jingham
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D53361
Added:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Makefile
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c
Modified:
lldb/trunk/include/lldb/API/SBThreadPlan.h
lldb/trunk/scripts/interface/SBThreadPlan.i
lldb/trunk/source/API/SBThreadPlan.cpp
Modified: lldb/trunk/include/lldb/API/SBThreadPlan.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBThreadPlan.h?rev=345247&r1=345246&r2=345247&view=diff
==
--- lldb/trunk/include/lldb/API/SBThreadPlan.h (original)
+++ lldb/trunk/include/lldb/API/SBThreadPlan.h Thu Oct 25 01:27:42 2018
@@ -88,6 +88,8 @@ public:
SBThreadPlan QueueThreadPlanForRunToAddress(SBAddress address);
+ SBThreadPlan QueueThreadPlanForStepScripted(const char *script_class_name);
+
#ifndef SWIG
lldb_private::ThreadPlan *get();
#endif
Added:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Makefile
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Makefile?rev=345247&view=auto
==
---
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Makefile
(added)
+++
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Makefile
Thu Oct 25 01:27:42 2018
@@ -0,0 +1,5 @@
+LEVEL = ../../make
+
+C_SOURCES := main.c
+
+include $(LEVEL)/Makefile.rules
Added:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py?rev=345247&view=auto
==
---
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
(added)
+++
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
Thu Oct 25 01:27:42 2018
@@ -0,0 +1,37 @@
+import lldb
+
+class StepWithChild:
+def __init__(self, thread_plan):
+self.thread_plan = thread_plan
+self.child_thread_plan = self.queue_child_thread_plan()
+
+def explains_stop(self, event):
+return False
+
+def should_stop(self, event):
+if not self.child_thread_plan.IsPlanComplete():
+return False
+
+self.thread_plan.SetPlanComplete(True)
+
+return True
+
+def should_step(self):
+return False
+
+def queue_child_thread_plan(self):
+return None
+
+class StepOut(StepWithChild):
+def __init__(self, thread_plan, dict):
+StepWithChild.__init__(self, thread_plan)
+
+def queue_child_thread_plan(self):
+return self.thread_plan.QueueThreadPlanForStepOut(0)
+
+class StepScripted(StepWithChild):
+def __init__(self, thread_plan, dict):
+StepWithChild.__init__(self, thread_plan)
+
+def queue_child_thread_plan(self):
+return self.thread_plan.QueueThreadPlanForStepScripted("Steps.StepOut")
Added:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py?rev=345247&view=auto
==
---
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
(added)
+++
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
Thu Oct 25 01:27:42 2018
@@ -0,0 +1,41 @@
+"""
+Tests stepping with scripted thread plans.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+class StepScriptedTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_standard_step_out(self):
+"""Tests stepping with the scripted thread plan laying over a standard
thread plan for stepping out."""
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.c")
+self.step_out_with_scripted_plan("Steps.StepOut")
+
[Lldb-commits] [PATCH] D53361: [API] Extend the `SBThreadPlan` interface
This revision was automatically updated to reflect the committed changes.
Closed by commit rL345247: [API] Extend the `SBThreadPlan` interface (authored
by aleksandr.urakov, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D53361?vs=170852&id=171048#toc
Repository:
rL LLVM
https://reviews.llvm.org/D53361
Files:
lldb/trunk/include/lldb/API/SBThreadPlan.h
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Makefile
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c
lldb/trunk/scripts/interface/SBThreadPlan.i
lldb/trunk/source/API/SBThreadPlan.cpp
Index: lldb/trunk/scripts/interface/SBThreadPlan.i
===
--- lldb/trunk/scripts/interface/SBThreadPlan.i
+++ lldb/trunk/scripts/interface/SBThreadPlan.i
@@ -106,6 +106,9 @@
SBThreadPlan
QueueThreadPlanForRunToAddress (SBAddress address);
+SBThreadPlan
+QueueThreadPlanForStepScripted(const char *script_class_name);
+
protected:
friend class SBBreakpoint;
Index: lldb/trunk/include/lldb/API/SBThreadPlan.h
===
--- lldb/trunk/include/lldb/API/SBThreadPlan.h
+++ lldb/trunk/include/lldb/API/SBThreadPlan.h
@@ -88,6 +88,8 @@
SBThreadPlan QueueThreadPlanForRunToAddress(SBAddress address);
+ SBThreadPlan QueueThreadPlanForStepScripted(const char *script_class_name);
+
#ifndef SWIG
lldb_private::ThreadPlan *get();
#endif
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c
@@ -0,0 +1,10 @@
+#include
+
+void foo() {
+ printf("Set a breakpoint here.\n");
+}
+
+int main() {
+ foo();
+ return 0;
+}
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
@@ -0,0 +1,37 @@
+import lldb
+
+class StepWithChild:
+def __init__(self, thread_plan):
+self.thread_plan = thread_plan
+self.child_thread_plan = self.queue_child_thread_plan()
+
+def explains_stop(self, event):
+return False
+
+def should_stop(self, event):
+if not self.child_thread_plan.IsPlanComplete():
+return False
+
+self.thread_plan.SetPlanComplete(True)
+
+return True
+
+def should_step(self):
+return False
+
+def queue_child_thread_plan(self):
+return None
+
+class StepOut(StepWithChild):
+def __init__(self, thread_plan, dict):
+StepWithChild.__init__(self, thread_plan)
+
+def queue_child_thread_plan(self):
+return self.thread_plan.QueueThreadPlanForStepOut(0)
+
+class StepScripted(StepWithChild):
+def __init__(self, thread_plan, dict):
+StepWithChild.__init__(self, thread_plan)
+
+def queue_child_thread_plan(self):
+return self.thread_plan.QueueThreadPlanForStepScripted("Steps.StepOut")
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
@@ -0,0 +1,41 @@
+"""
+Tests stepping with scripted thread plans.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+class StepScriptedTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_standard_step_out(self):
+"""Tests stepping with the scripted thread plan laying over a standard thread plan for stepping out."""
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.c")
+self.step_out_with_scripted_plan("Steps.StepOut")
+
+def test_scripted_step_out(self):
+"""Tests stepping with the scripted thread plan laying over an another scripted thread plan for stepping out."""
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.c")
+self.step_out_with_scripted_plan("Steps.StepScripted")
+
+def setUp(self):
+TestBase.setUp(self)
+self.runCmd("command script import Steps.py")
+
+def step_out_with_scripted_plan(self, name):
+(t
[Lldb-commits] [lldb] r345249 - [LLDB] - Parse the DW_LLE_startx_length correctly for DWARF v5 case.
Author: grimar
Date: Thu Oct 25 02:22:26 2018
New Revision: 345249
URL: http://llvm.org/viewvc/llvm-project?rev=345249&view=rev
Log:
[LLDB] - Parse the DW_LLE_startx_length correctly for DWARF v5 case.
Currently, we always parse the length field of DW_LLE_startx_length entry as
U32.
That is correct for pre-standard definition:
https://gcc.gnu.org/wiki/DebugFission - "A start/length entry contains one
unsigned LEB128 number
and a 4-byte unsigned value (as would be represented by the form code
DW_FORM_const4u). The first
number is an index into the .debug_addr section that selects the beginning
offset, and the second
number is the length of the range. ")
But DWARF v5 says: "This is a form of bounded location description that has two
unsigned ULEB operands.
The first value is an address index (into the .debug_addr section) that
indicates the beginning of the address
range over which the location is valid. The second value is the length of the
range."
Fortunately, we can easily handle the difference. No test case because it seems
impossible to test
until we will be ready to use DWARF v5 in tests that need to run the
executables.
Differential revision: https://reviews.llvm.org/D53646
Modified:
lldb/trunk/include/lldb/Expression/DWARFExpression.h
lldb/trunk/source/Expression/DWARFExpression.cpp
Modified: lldb/trunk/include/lldb/Expression/DWARFExpression.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/DWARFExpression.h?rev=345249&r1=345248&r2=345249&view=diff
==
--- lldb/trunk/include/lldb/Expression/DWARFExpression.h (original)
+++ lldb/trunk/include/lldb/Expression/DWARFExpression.h Thu Oct 25 02:22:26
2018
@@ -40,8 +40,10 @@ public:
enum LocationListFormat : uint8_t {
NonLocationList, // Not a location list
RegularLocationList, // Location list format used in non-split dwarf files
-SplitDwarfLocationList, // Location list format used in split dwarf files
-LocLists, // Location list format used in DWARF v5
(.debug_loclists).
+SplitDwarfLocationList, // Location list format used in pre-DWARF v5 split
+// dwarf files (.debug_loc.dwo)
+LocLists, // Location list format used in DWARF v5
+// (.debug_loclists/.debug_loclists.dwo).
};
//--
@@ -153,7 +155,7 @@ public:
lldb::addr_t GetLocation_DW_OP_addr(uint32_t op_addr_idx, bool &error) const;
bool Update_DW_OP_addr(lldb::addr_t file_addr);
-
+
void SetModule(const lldb::ModuleSP &module) { m_module_wp = module; }
bool ContainsThreadLocalStorage() const;
Modified: lldb/trunk/source/Expression/DWARFExpression.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/DWARFExpression.cpp?rev=345249&r1=345248&r2=345249&view=diff
==
--- lldb/trunk/source/Expression/DWARFExpression.cpp (original)
+++ lldb/trunk/source/Expression/DWARFExpression.cpp Thu Oct 25 02:22:26 2018
@@ -3029,7 +3029,9 @@ bool DWARFExpression::AddressRangeForLoc
if (!debug_loc_data.ValidOffset(*offset_ptr))
return false;
- switch (dwarf_cu->GetSymbolFileDWARF()->GetLocationListFormat()) {
+ DWARFExpression::LocationListFormat format =
+ dwarf_cu->GetSymbolFileDWARF()->GetLocationListFormat();
+ switch (format) {
case NonLocationList:
return false;
case RegularLocationList:
@@ -3051,7 +3053,9 @@ bool DWARFExpression::AddressRangeForLoc
case DW_LLE_startx_length: {
uint64_t index = debug_loc_data.GetULEB128(offset_ptr);
low_pc = ReadAddressFromDebugAddrSection(dwarf_cu, index);
- uint32_t length = debug_loc_data.GetU32(offset_ptr);
+ uint64_t length = (format == LocLists)
+? debug_loc_data.GetULEB128(offset_ptr)
+: debug_loc_data.GetU32(offset_ptr);
high_pc = low_pc + length;
return true;
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53646: [LLDB] - Parse the DW_LLE_startx_length correctly for DWARF v5 case.
This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB345249: [LLDB] - Parse the DW_LLE_startx_length correctly
for DWARF v5 case. (authored by grimar, committed by ).
Herald added a subscriber: lldb-commits.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D53646
Files:
include/lldb/Expression/DWARFExpression.h
source/Expression/DWARFExpression.cpp
Index: source/Expression/DWARFExpression.cpp
===
--- source/Expression/DWARFExpression.cpp
+++ source/Expression/DWARFExpression.cpp
@@ -3029,7 +3029,9 @@
if (!debug_loc_data.ValidOffset(*offset_ptr))
return false;
- switch (dwarf_cu->GetSymbolFileDWARF()->GetLocationListFormat()) {
+ DWARFExpression::LocationListFormat format =
+ dwarf_cu->GetSymbolFileDWARF()->GetLocationListFormat();
+ switch (format) {
case NonLocationList:
return false;
case RegularLocationList:
@@ -3051,7 +3053,9 @@
case DW_LLE_startx_length: {
uint64_t index = debug_loc_data.GetULEB128(offset_ptr);
low_pc = ReadAddressFromDebugAddrSection(dwarf_cu, index);
- uint32_t length = debug_loc_data.GetU32(offset_ptr);
+ uint64_t length = (format == LocLists)
+? debug_loc_data.GetULEB128(offset_ptr)
+: debug_loc_data.GetU32(offset_ptr);
high_pc = low_pc + length;
return true;
}
Index: include/lldb/Expression/DWARFExpression.h
===
--- include/lldb/Expression/DWARFExpression.h
+++ include/lldb/Expression/DWARFExpression.h
@@ -40,8 +40,10 @@
enum LocationListFormat : uint8_t {
NonLocationList, // Not a location list
RegularLocationList, // Location list format used in non-split dwarf files
-SplitDwarfLocationList, // Location list format used in split dwarf files
-LocLists, // Location list format used in DWARF v5
(.debug_loclists).
+SplitDwarfLocationList, // Location list format used in pre-DWARF v5 split
+// dwarf files (.debug_loc.dwo)
+LocLists, // Location list format used in DWARF v5
+// (.debug_loclists/.debug_loclists.dwo).
};
//--
@@ -153,7 +155,7 @@
lldb::addr_t GetLocation_DW_OP_addr(uint32_t op_addr_idx, bool &error) const;
bool Update_DW_OP_addr(lldb::addr_t file_addr);
-
+
void SetModule(const lldb::ModuleSP &module) { m_module_wp = module; }
bool ContainsThreadLocalStorage() const;
Index: source/Expression/DWARFExpression.cpp
===
--- source/Expression/DWARFExpression.cpp
+++ source/Expression/DWARFExpression.cpp
@@ -3029,7 +3029,9 @@
if (!debug_loc_data.ValidOffset(*offset_ptr))
return false;
- switch (dwarf_cu->GetSymbolFileDWARF()->GetLocationListFormat()) {
+ DWARFExpression::LocationListFormat format =
+ dwarf_cu->GetSymbolFileDWARF()->GetLocationListFormat();
+ switch (format) {
case NonLocationList:
return false;
case RegularLocationList:
@@ -3051,7 +3053,9 @@
case DW_LLE_startx_length: {
uint64_t index = debug_loc_data.GetULEB128(offset_ptr);
low_pc = ReadAddressFromDebugAddrSection(dwarf_cu, index);
- uint32_t length = debug_loc_data.GetU32(offset_ptr);
+ uint64_t length = (format == LocLists)
+? debug_loc_data.GetULEB128(offset_ptr)
+: debug_loc_data.GetU32(offset_ptr);
high_pc = low_pc + length;
return true;
}
Index: include/lldb/Expression/DWARFExpression.h
===
--- include/lldb/Expression/DWARFExpression.h
+++ include/lldb/Expression/DWARFExpression.h
@@ -40,8 +40,10 @@
enum LocationListFormat : uint8_t {
NonLocationList, // Not a location list
RegularLocationList, // Location list format used in non-split dwarf files
-SplitDwarfLocationList, // Location list format used in split dwarf files
-LocLists, // Location list format used in DWARF v5 (.debug_loclists).
+SplitDwarfLocationList, // Location list format used in pre-DWARF v5 split
+// dwarf files (.debug_loc.dwo)
+LocLists, // Location list format used in DWARF v5
+// (.debug_loclists/.debug_loclists.dwo).
};
//--
@@ -153,7 +155,7 @@
lldb::addr_t GetLocation_DW_OP_addr(uint32_t op_addr_idx, bool &error) const;
bool Update_DW_OP_addr(lldb::addr_t file_addr);
-
+
void SetModule(const lldb::ModuleSP &module) { m_module_wp = module; }
bool ContainsThreadLocalStorage() const;
___
lldb-commits mailing list
l
[Lldb-commits] [PATCH] D53368: [Symbol] Search symbols with name and type in a symbol file
aleksandr.urakov marked 5 inline comments as done.
aleksandr.urakov added a comment.
Ah, yes, sure! It's my mistake. I didn't pay attention to the fact that a
symtab owns symbols. I'll update the patch, thanks!
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:1344-1346
+auto symbol_ptr = GetPublicSymbol(*pub_symbol);
+if (!symbol_ptr)
+ continue;
clayborg wrote:
> Maybe get the file address from "pub_symbol" and avoid converting to a
> SymbolSP that we will never use? See more comments below.
Unfortunately there is no method of `PDBSymbolPublicSymbol` which allows to
retrieve the file address directly. I'll calculate it as section + offset
instead.
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:1353
+
+symtab.AddSymbol(*symbol_ptr);
+ }
clayborg wrote:
> Just make a local symbol and convert it from PDB to lldb_private::Symbol
> here? Something like:
> ```
> symtab.AddSymbol(ConvertPDBSymbol(pdb_symbol));
> ```
>
> So it seems we shouldn't need the m_public_symbols storage in this class
> anymore since "symtab" will now own the symbol we just created.
The problem here is that `ConvertPDBSymbol` can fail e.g. if somehow
`pub_symbol` will contain an invalid section number. We can:
- change the interface of the function to signal about errors;
- just assume that the section number is always valid (because we already
retrieved it before during the file address calculation).
In both cases we will retrieve the section twice. We also can:
- just pass already calculated section and offset to `ConvertPDBSymbol`. But it
leads to a blurred responsibility and a weird interface.
So I think that it would be better just create a symbol right here - it seems
that it doesn't make the code more complex. What do you think about it?
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:1968-1969
+
+lldb_private::Symbol *SymbolFilePDB::GetPublicSymbol(
+const llvm::pdb::PDBSymbolPublicSymbol &pub_symbol) {
+ auto it = m_public_symbols.find(pub_symbol.getSymIndexId());
clayborg wrote:
> Maybe convert this to:
> ```
> lldb_private::Symbol ConvertPDBSymbol(const llvm::pdb::PDBSymbolPublicSymbol
> &pub_symbol)
> ```
> And we only call this if we need to create a symbol we will add to the
> "symtab" in SymbolFilePDB::AddSymbols(...)
See the comment above
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
aleksandr.urakov updated this revision to Diff 171056.
aleksandr.urakov marked 3 inline comments as done.
https://reviews.llvm.org/D53368
Files:
include/lldb/Symbol/SymbolFile.h
include/lldb/Symbol/SymbolVendor.h
source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
source/Symbol/SymbolVendor.cpp
unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
Index: unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
===
--- unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
+++ unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
@@ -616,3 +616,20 @@
EXPECT_EQ(0u, num_results);
EXPECT_EQ(0u, results.GetSize());
}
+
+TEST_F(SymbolFilePDBTests, TestFindSymbolsWithNameAndType) {
+ FileSpec fspec(m_pdb_test_exe.c_str(), false);
+ ArchSpec aspec("i686-pc-windows");
+ lldb::ModuleSP module = std::make_shared(fspec, aspec);
+
+ SymbolContextList sc_list;
+ EXPECT_EQ(1u,
+module->FindSymbolsWithNameAndType(ConstString("?foo@@YAHH@Z"),
+ lldb::eSymbolTypeAny, sc_list));
+ EXPECT_EQ(1u, sc_list.GetSize());
+
+ SymbolContext sc;
+ EXPECT_TRUE(sc_list.GetContextAtIndex(0, sc));
+ EXPECT_STREQ("int foo(int)",
+ sc.GetFunctionName(Mangled::ePreferDemangled).AsCString());
+}
Index: source/Symbol/SymbolVendor.cpp
===
--- source/Symbol/SymbolVendor.cpp
+++ source/Symbol/SymbolVendor.cpp
@@ -61,7 +61,7 @@
//--
SymbolVendor::SymbolVendor(const lldb::ModuleSP &module_sp)
: ModuleChild(module_sp), m_type_list(), m_compile_units(),
- m_sym_file_ap() {}
+ m_sym_file_ap(), m_symtab() {}
//--
// Destructor
@@ -438,14 +438,23 @@
Symtab *SymbolVendor::GetSymtab() {
ModuleSP module_sp(GetModule());
- if (module_sp) {
-ObjectFile *objfile = module_sp->GetObjectFile();
-if (objfile) {
- // Get symbol table from unified section list.
- return objfile->GetSymtab();
-}
- }
- return nullptr;
+ if (!module_sp)
+return nullptr;
+
+ std::lock_guard guard(module_sp->GetMutex());
+
+ if (m_symtab)
+return m_symtab;
+
+ ObjectFile *objfile = module_sp->GetObjectFile();
+ if (!objfile)
+return nullptr;
+
+ m_symtab = objfile->GetSymtab();
+ if (m_symtab && m_sym_file_ap)
+m_sym_file_ap->AddSymbols(*m_symtab);
+
+ return m_symtab;
}
void SymbolVendor::ClearSymtab() {
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -132,6 +132,8 @@
const std::string &scope_qualified_name,
std::vector &mangled_names) override;
+ void AddSymbols(lldb_private::Symtab &symtab) override;
+
uint32_t
FindTypes(const lldb_private::SymbolContext &sc,
const lldb_private::ConstString &name,
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -1331,6 +1331,58 @@
const std::string &scope_qualified_name,
std::vector &mangled_names) {}
+void SymbolFilePDB::AddSymbols(lldb_private::Symtab &symtab) {
+ std::set sym_addresses;
+ for (size_t i = 0; i < symtab.GetNumSymbols(); i++)
+sym_addresses.insert(symtab.SymbolAtIndex(i)->GetFileAddress());
+
+ auto results = m_global_scope_up->findAllChildren();
+ if (!results)
+return;
+
+ auto section_list = m_obj_file->GetSectionList();
+ if (!section_list)
+return;
+
+ while (auto pub_symbol = results->getNext()) {
+auto section_idx = pub_symbol->getAddressSection() - 1;
+if (section_idx >= section_list->GetSize())
+ continue;
+
+auto section = section_list->GetSectionAtIndex(section_idx);
+if (!section)
+ continue;
+
+auto offset = pub_symbol->getAddressOffset();
+
+auto file_addr = section->GetFileAddress() + offset;
+if (sym_addresses.find(file_addr) != sym_addresses.end())
+ continue;
+sym_addresses.insert(file_addr);
+
+auto size = pub_symbol->getLength();
+symtab.AddSymbol(
+Symbol(pub_symbol->getSymIndexId(), // symID
+ pub_symbol->getName().c_str(), // name
+ true, // name_is_mangled
+ pub_symbol->isCode() ? eSymbolTypeCode : eSymbolTypeData, // type
+ true, // external
+ false, // is_debug
+ false, // is_trampoline
+ false, // is_artificial
+ section, // section_sp
+ offset,// value
+ size, // size
+
[Lldb-commits] [lldb] r345251 - Recommit r345127 "[LLDB] - Add support for DW_RLE_base_address and DW_RLE_offset_pair entries (.debug_rnglists)"
Author: grimar
Date: Thu Oct 25 03:25:45 2018
New Revision: 345251
URL: http://llvm.org/viewvc/llvm-project?rev=345251&view=rev
Log:
Recommit r345127 "[LLDB] - Add support for DW_RLE_base_address and
DW_RLE_offset_pair entries (.debug_rnglists)"
With the fix: do not forget to hanlde the DW_RLE_start_end, which seems was
omited/forgotten/removed by mistake.
Original commit message:
The patch implements the support for DW_RLE_base_address and DW_RLE_offset_pair
.debug_rnglists entries
Differential revision: https://reviews.llvm.org/D53140
Added : /lldb/trunk/lit/Breakpoint/Inputs/debug_rnglist_offset_pair.yaml
Added : /lldb/trunk/lit/Breakpoint/debug_rnglist_offset_pair.test
Modified : /lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
Modified : /lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
Modified : /lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
Modified : /lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Modified : /lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
Added:
lldb/trunk/lit/Breakpoint/Inputs/debug_rnglist_offset_pair.yaml
- copied unchanged from r345156,
lldb/trunk/lit/Breakpoint/Inputs/debug_rnglist_offset_pair.yaml
lldb/trunk/lit/Breakpoint/debug_rnglist_offset_pair.test
- copied unchanged from r345156,
lldb/trunk/lit/Breakpoint/debug_rnglist_offset_pair.test
Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp?rev=345251&r1=345250&r2=345251&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp Thu Oct
25 03:25:45 2018
@@ -456,20 +456,15 @@ bool DWARFDebugInfoEntry::GetDIENamesAnd
break;
case DW_AT_ranges: {
- const DWARFDebugRanges *debug_ranges = dwarf2Data->DebugRanges();
- if (debug_ranges) {
-debug_ranges->FindRanges(cu->GetRangesBase(),
form_value.Unsigned(), ranges);
-// All DW_AT_ranges are relative to the base address of the compile
-// unit. We add the compile unit base address to make sure all the
-// addresses are properly fixed up.
-ranges.Slide(cu->GetBaseAddress());
- } else {
+ const DWARFDebugRangesBase *debug_ranges = dwarf2Data->DebugRanges();
+ if (debug_ranges)
+debug_ranges->FindRanges(cu, form_value.Unsigned(), ranges);
+ else
cu->GetSymbolFileDWARF()->GetObjectFile()->GetModule()->ReportError(
"{0x%8.8x}: DIE has DW_AT_ranges(0x%" PRIx64
") attribute yet DWARF has no .debug_ranges, please file a bug
"
"and attach the file at the start of this error message",
m_offset, form_value.Unsigned());
- }
} break;
case DW_AT_name:
@@ -1065,10 +1060,8 @@ size_t DWARFDebugInfoEntry::GetAttribute
dwarf2Data, cu, DW_AT_ranges, DW_INVALID_OFFSET,
check_specification_or_abstract_origin);
if (debug_ranges_offset != DW_INVALID_OFFSET) {
-if (DWARFDebugRanges *debug_ranges = dwarf2Data->DebugRanges())
- debug_ranges->FindRanges(cu->GetRangesBase(), debug_ranges_offset,
- ranges);
-ranges.Slide(cu->GetBaseAddress());
+if (DWARFDebugRangesBase *debug_ranges = dwarf2Data->DebugRanges())
+ debug_ranges->FindRanges(cu, debug_ranges_offset, ranges);
} else if (check_hi_lo_pc) {
dw_addr_t lo_pc = LLDB_INVALID_ADDRESS;
dw_addr_t hi_pc = LLDB_INVALID_ADDRESS;
@@ -1723,12 +1716,9 @@ bool DWARFDebugInfoEntry::LookupAddress(
dwarf2Data, cu, DW_AT_ranges, DW_INVALID_OFFSET);
if (debug_ranges_offset != DW_INVALID_OFFSET) {
DWARFRangeList ranges;
- DWARFDebugRanges *debug_ranges = dwarf2Data->DebugRanges();
- debug_ranges->FindRanges(cu->GetRangesBase(), debug_ranges_offset,
ranges);
- // All DW_AT_ranges are relative to the base address of the compile
- // unit. We add the compile unit base address to make sure all the
- // addresses are properly fixed up.
- ranges.Slide(cu->GetBaseAddress());
+ DWARFDebugRangesBase *debug_ranges = dwarf2Data->DebugRanges();
+ debug_ranges->FindRanges(cu, debug_ranges_offset, ranges);
+
if (ranges.FindEntryThatContains(address)) {
fo
[Lldb-commits] [lldb] r345274 - Fix a bug PlatformDarwin::SDKSupportsModule.
Author: adrian
Date: Thu Oct 25 08:30:43 2018
New Revision: 345274
URL: http://llvm.org/viewvc/llvm-project?rev=345274&view=rev
Log:
Fix a bug PlatformDarwin::SDKSupportsModule.
This fixes a bug PlatformDarwin::SDKSupportsModule introduced by
https://reviews.llvm.org/D47889. VersionTuple::tryParse() can deal
with an optional third (micro) component, but the parse will fail when
there are extra characters after the version number (e.g.: trying to
parse the substring "12.0.sdk" out of "iPhoneSimulator12.0.sdk" fails
after that patch). Fixed here by stripping the ".sdk" suffix first.
(Part of) rdar://problem/45041492
Differential Revision https://reviews.llvm.org/D53677
Modified:
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
lldb/trunk/source/Utility/ArchSpec.cpp
lldb/trunk/unittests/Platform/PlatformDarwinTest.cpp
Modified: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp?rev=345274&r1=345273&r2=345274&view=diff
==
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp Thu Oct 25
08:30:43 2018
@@ -1402,14 +1402,11 @@ bool PlatformDarwin::SDKSupportsModules(
if (last_path_component) {
const llvm::StringRef sdk_name = last_path_component.GetStringRef();
-llvm::StringRef version_part;
-
-if (sdk_name.startswith(sdk_strings[(int)desired_type])) {
- version_part =
- sdk_name.drop_front(strlen(sdk_strings[(int)desired_type]));
-} else {
+if (!sdk_name.startswith(sdk_strings[(int)desired_type]))
return false;
-}
+auto version_part =
+sdk_name.drop_front(strlen(sdk_strings[(int)desired_type]));
+version_part.consume_back(".sdk");
llvm::VersionTuple version;
if (version.tryParse(version_part))
Modified: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h?rev=345274&r1=345273&r2=345274&view=diff
==
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h (original)
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h Thu Oct 25
08:30:43 2018
@@ -85,6 +85,12 @@ public:
static std::tuple
ParseVersionBuildDir(llvm::StringRef str);
+ enum class SDKType {
+MacOSX = 0,
+iPhoneSimulator,
+iPhoneOS,
+ };
+
protected:
void ReadLibdispatchOffsetsAddress(lldb_private::Process *process);
@@ -95,12 +101,6 @@ protected:
const lldb_private::FileSpecList *module_search_paths_ptr,
lldb::ModuleSP *old_module_sp_ptr, bool *did_create_ptr);
- enum class SDKType {
-MacOSX = 0,
-iPhoneSimulator,
-iPhoneOS,
- };
-
static bool SDKSupportsModules(SDKType sdk_type, llvm::VersionTuple version);
static bool SDKSupportsModules(SDKType desired_type,
Modified: lldb/trunk/source/Utility/ArchSpec.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/ArchSpec.cpp?rev=345274&r1=345273&r2=345274&view=diff
==
--- lldb/trunk/source/Utility/ArchSpec.cpp (original)
+++ lldb/trunk/source/Utility/ArchSpec.cpp Thu Oct 25 08:30:43 2018
@@ -1029,6 +1029,11 @@ static bool isCompatibleEnvironment(llvm
rhs == llvm::Triple::UnknownEnvironment)
return true;
+ // If any of the environment is unknown then they are compatible
+ if (lhs == llvm::Triple::UnknownEnvironment ||
+ rhs == llvm::Triple::UnknownEnvironment)
+return true;
+
// If one of the environment is Android and the other one is EABI then they
// are considered to be compatible. This is required as a workaround for
// shared libraries compiled for Android without the NOTE section indicating
Modified: lldb/trunk/unittests/Platform/PlatformDarwinTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Platform/PlatformDarwinTest.cpp?rev=345274&r1=345273&r2=345274&view=diff
==
--- lldb/trunk/unittests/Platform/PlatformDarwinTest.cpp (original)
+++ lldb/trunk/unittests/Platform/PlatformDarwinTest.cpp Thu Oct 25 08:30:43
2018
@@ -18,6 +18,13 @@
using namespace lldb;
using namespace lldb_private;
+struct PlatformDarwinTester : public PlatformDarwin {
+ static bool SDKSupportsModules(SDKType desired_type,
+ const lldb_private::FileSpec &sdk_path) {
+return PlatformDarwin::SDKSupportsModules(desired_type, sdk_path);
+ }
+};
+
TEST(PlatformDarwinTest, TestParseVersionBuildDir) {
llvm::VersionTuple V;
llvm::StringRef D;
@@ -44,4 +51,23 @@ TES
[Lldb-commits] [PATCH] D53677: Fix a bug PlatformDarwin::SDKSupportsModule
shafik added inline comments.
Comment at: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1405
-llvm::StringRef version_part;
-
-if (sdk_name.startswith(sdk_strings[(int)desired_type])) {
- version_part =
- sdk_name.drop_front(strlen(sdk_strings[(int)desired_type]));
-} else {
+if (!sdk_name.startswith(sdk_strings[(int)desired_type]))
return false;
Could we avoid C-style casts, and in this case use `static_cast` although in
this case it is obvious, using explicit casts avoids bugs in the long-term due
to changes in type and cv-qualifiers etc... and makes your intent clear.
I personally would advocate for -Wold-style-cast being a hard error in C++ code.
Comment at: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1408
+auto version_part =
+sdk_name.drop_front(strlen(sdk_strings[(int)desired_type]));
+version_part.consume_back(".sdk");
Same comment here.
https://reviews.llvm.org/D53677
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53709: Get rid of C-style cast.
aprantl created this revision.
aprantl added a reviewer: shafik.
This addresses review feedback from https://reviews.llvm.org/D53677.
https://reviews.llvm.org/D53709
Files:
source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
source/Plugins/Platform/MacOSX/PlatformDarwin.h
Index: source/Plugins/Platform/MacOSX/PlatformDarwin.h
===
--- source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -85,7 +85,7 @@
static std::tuple
ParseVersionBuildDir(llvm::StringRef str);
- enum class SDKType {
+ enum SDKType : unsigned {
MacOSX = 0,
iPhoneSimulator,
iPhoneOS,
Index: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1402,10 +1402,10 @@
if (last_path_component) {
const llvm::StringRef sdk_name = last_path_component.GetStringRef();
-if (!sdk_name.startswith(sdk_strings[(int)desired_type]))
+if (!sdk_name.startswith(sdk_strings[desired_type]))
return false;
auto version_part =
-sdk_name.drop_front(strlen(sdk_strings[(int)desired_type]));
+sdk_name.drop_front(strlen(sdk_strings[desired_type]));
version_part.consume_back(".sdk");
llvm::VersionTuple version;
Index: source/Plugins/Platform/MacOSX/PlatformDarwin.h
===
--- source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -85,7 +85,7 @@
static std::tuple
ParseVersionBuildDir(llvm::StringRef str);
- enum class SDKType {
+ enum SDKType : unsigned {
MacOSX = 0,
iPhoneSimulator,
iPhoneOS,
Index: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1402,10 +1402,10 @@
if (last_path_component) {
const llvm::StringRef sdk_name = last_path_component.GetStringRef();
-if (!sdk_name.startswith(sdk_strings[(int)desired_type]))
+if (!sdk_name.startswith(sdk_strings[desired_type]))
return false;
auto version_part =
-sdk_name.drop_front(strlen(sdk_strings[(int)desired_type]));
+sdk_name.drop_front(strlen(sdk_strings[desired_type]));
version_part.consume_back(".sdk");
llvm::VersionTuple version;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53709: Get rid of C-style cast.
shafik added a comment. LGTM https://reviews.llvm.org/D53709 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r345278 - Get rid of casts. (NFC)
Author: adrian
Date: Thu Oct 25 09:15:17 2018
New Revision: 345278
URL: http://llvm.org/viewvc/llvm-project?rev=345278&view=rev
Log:
Get rid of casts. (NFC)
Differential Revision: https://reviews.llvm.org/D53709
Modified:
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
Modified: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp?rev=345278&r1=345277&r2=345278&view=diff
==
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp Thu Oct 25
09:15:17 2018
@@ -1402,10 +1402,10 @@ bool PlatformDarwin::SDKSupportsModules(
if (last_path_component) {
const llvm::StringRef sdk_name = last_path_component.GetStringRef();
-if (!sdk_name.startswith(sdk_strings[(int)desired_type]))
+if (!sdk_name.startswith(sdk_strings[desired_type]))
return false;
auto version_part =
-sdk_name.drop_front(strlen(sdk_strings[(int)desired_type]));
+sdk_name.drop_front(strlen(sdk_strings[desired_type]));
version_part.consume_back(".sdk");
llvm::VersionTuple version;
Modified: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h?rev=345278&r1=345277&r2=345278&view=diff
==
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h (original)
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h Thu Oct 25
09:15:17 2018
@@ -85,7 +85,7 @@ public:
static std::tuple
ParseVersionBuildDir(llvm::StringRef str);
- enum class SDKType {
+ enum SDKType : unsigned {
MacOSX = 0,
iPhoneSimulator,
iPhoneOS,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53709: Get rid of C-style cast.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL345278: Get rid of casts. (NFC) (authored by adrian,
committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D53709?vs=171117&id=171119#toc
Repository:
rL LLVM
https://reviews.llvm.org/D53709
Files:
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
Index: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
===
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -85,7 +85,7 @@
static std::tuple
ParseVersionBuildDir(llvm::StringRef str);
- enum class SDKType {
+ enum SDKType : unsigned {
MacOSX = 0,
iPhoneSimulator,
iPhoneOS,
Index: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1402,10 +1402,10 @@
if (last_path_component) {
const llvm::StringRef sdk_name = last_path_component.GetStringRef();
-if (!sdk_name.startswith(sdk_strings[(int)desired_type]))
+if (!sdk_name.startswith(sdk_strings[desired_type]))
return false;
auto version_part =
-sdk_name.drop_front(strlen(sdk_strings[(int)desired_type]));
+sdk_name.drop_front(strlen(sdk_strings[desired_type]));
version_part.consume_back(".sdk");
llvm::VersionTuple version;
Index: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
===
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -85,7 +85,7 @@
static std::tuple
ParseVersionBuildDir(llvm::StringRef str);
- enum class SDKType {
+ enum SDKType : unsigned {
MacOSX = 0,
iPhoneSimulator,
iPhoneOS,
Index: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1402,10 +1402,10 @@
if (last_path_component) {
const llvm::StringRef sdk_name = last_path_component.GetStringRef();
-if (!sdk_name.startswith(sdk_strings[(int)desired_type]))
+if (!sdk_name.startswith(sdk_strings[desired_type]))
return false;
auto version_part =
-sdk_name.drop_front(strlen(sdk_strings[(int)desired_type]));
+sdk_name.drop_front(strlen(sdk_strings[desired_type]));
version_part.consume_back(".sdk");
llvm::VersionTuple version;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
zturner updated this revision to Diff 171134.
zturner added a comment.
Fixed issues pointed out by @jingham and added some test coverage for this.
https://reviews.llvm.org/D53662
Files:
lldb/include/lldb/Core/Module.h
lldb/include/lldb/Symbol/SymbolFile.h
lldb/include/lldb/Symbol/SymbolVendor.h
lldb/include/lldb/Symbol/Type.h
lldb/include/lldb/Symbol/TypeMap.h
lldb/lit/SymbolFile/NativePDB/Inputs/tag-types.lldbinit
lldb/lit/SymbolFile/NativePDB/tag-types.cpp
lldb/source/Core/Module.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
lldb/source/Symbol/SymbolFile.cpp
lldb/source/Symbol/SymbolVendor.cpp
lldb/source/Symbol/Type.cpp
lldb/source/Symbol/TypeList.cpp
lldb/source/Symbol/TypeMap.cpp
lldb/tools/lldb-test/lldb-test.cpp
lldb/unittests/Symbol/TestType.cpp
Index: lldb/unittests/Symbol/TestType.cpp
===
--- lldb/unittests/Symbol/TestType.cpp
+++ lldb/unittests/Symbol/TestType.cpp
@@ -21,9 +21,7 @@
const char *expected_scope,
const char *expected_name) {
llvm::StringRef scope, name;
- lldb::TypeClass type_class;
- bool is_scoped =
- Type::GetTypeScopeAndBasename(full_type, scope, name, type_class);
+ bool is_scoped = Type::GetTypeScopeAndBasename(full_type, scope, name);
EXPECT_EQ(is_scoped, expected_is_scoped);
if (expected_is_scoped) {
EXPECT_EQ(scope, expected_scope);
Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -456,8 +456,8 @@
SymbolContext SC;
DenseSet SearchedFiles;
TypeMap Map;
- Vendor.FindTypes(SC, ConstString(Name), ContextPtr, true, UINT32_MAX,
- SearchedFiles, Map);
+ Vendor.FindTypes(SC, Name, ContextPtr, false, true, UINT32_MAX, SearchedFiles,
+ Map);
outs() << formatv("Found {0} types:\n", Map.GetSize());
StreamString Stream;
Index: lldb/source/Symbol/TypeMap.cpp
===
--- lldb/source/Symbol/TypeMap.cpp
+++ lldb/source/Symbol/TypeMap.cpp
@@ -153,18 +153,35 @@
void TypeMap::RemoveMismatchedTypes(const char *qualified_typename,
bool exact_match) {
+ llvm::StringRef name(qualified_typename);
llvm::StringRef type_scope;
llvm::StringRef type_basename;
- TypeClass type_class = eTypeClassAny;
+ TypeClass type_class = Type::ConsumeTypeClass(name);
if (!Type::GetTypeScopeAndBasename(qualified_typename, type_scope,
- type_basename, type_class)) {
+ type_basename)) {
type_basename = qualified_typename;
type_scope = "";
}
return RemoveMismatchedTypes(type_scope, type_basename, type_class,
exact_match);
}
+void TypeMap::OnlyKeepTypeClass(lldb::TypeClass type_class) {
+ if (type_class == eTypeClassAny)
+return;
+
+ collection matching_types;
+
+ iterator pos, end = m_types.end();
+
+ for (const auto &t : m_types) {
+TypeClass tc = t.second->GetForwardCompilerType().GetTypeClass();
+if (tc & type_class)
+ matching_types.insert(t);
+ }
+ m_types.swap(matching_types);
+}
+
void TypeMap::RemoveMismatchedTypes(const std::string &type_scope,
const std::string &type_basename,
TypeClass type_class, bool exact_match) {
@@ -193,8 +210,7 @@
llvm::StringRef match_type_scope;
llvm::StringRef match_type_basename;
if (Type::GetTypeScopeAndBasename(match_type_name, match_type_scope,
-match_type_basename,
-match_type_class)) {
+match_type_basename)) {
if (match_type_basename == type_basename) {
const size_t type_scope_size = type_scope.size();
const size_t match_type_scope_size = match_type_scope.size();
Index: lldb/source/Symbol/TypeList.cpp
===
--- lldb/source/Symbol/TypeList.cpp
+++ lldb/source/Symbol/TypeList.cpp
@@ -112,9 +112,10 @@
bool exact_match) {
llvm::StringRef type_scope;
llvm::StringRef type_basename;
- TypeClass type_class = eTypeClassAny;
+ llvm::StringRef name(qualified_typ
[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
clayborg added a comment. As long as Swig is happy and the ABI doesn't change I am ok with this. Will we see the variables better when debugging? Or is this solely so the SymbolContextItem type doesn't disappear from the debug info? https://reviews.llvm.org/D53597 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r345287 - Remove accidentally committed duplicate code
Author: adrian Date: Thu Oct 25 10:36:05 2018 New Revision: 345287 URL: http://llvm.org/viewvc/llvm-project?rev=345287&view=rev Log: Remove accidentally committed duplicate code Modified: lldb/trunk/source/Utility/ArchSpec.cpp Modified: lldb/trunk/source/Utility/ArchSpec.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/ArchSpec.cpp?rev=345287&r1=345286&r2=345287&view=diff == --- lldb/trunk/source/Utility/ArchSpec.cpp (original) +++ lldb/trunk/source/Utility/ArchSpec.cpp Thu Oct 25 10:36:05 2018 @@ -1029,11 +1029,6 @@ static bool isCompatibleEnvironment(llvm rhs == llvm::Triple::UnknownEnvironment) return true; - // If any of the environment is unknown then they are compatible - if (lhs == llvm::Triple::UnknownEnvironment || - rhs == llvm::Triple::UnknownEnvironment) -return true; - // If one of the environment is Android and the other one is EABI then they // are considered to be compatible. This is required as a workaround for // shared libraries compiled for Android without the NOTE section indicating ___ 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. Very close, just down to making the SymbolVendor::GetSymtab() call symtab.CalculateSymbolSizes() and symtab.Finalize(). Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:1382-1383 + + symtab.CalculateSymbolSizes(); + symtab.Finalize(); +} Seems like these two lines should be done in the symbol vendor? Maybe this function should return the number of symbols added and the symbol vendor could see if AddSymbols returns a positive number, and if so, call symtab.CalculateSymbolSizes() and symtab.Finalize(). We should also see who else is calling these and remove any calls and only do it in the SymbolVendor one time. 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] D53597: Don't type-erase the SymbolContextItem enum
zturner added a comment. In https://reviews.llvm.org/D53597#1276086, @clayborg wrote: > As long as Swig is happy and the ABI doesn't change I am ok with this. Will > we see the variables better when debugging? Or is this solely so the > SymbolContextItem type doesn't disappear from the debug info? We will see variables better when debugging too. https://reviews.llvm.org/D53597 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
zturner added a comment. In https://reviews.llvm.org/D53597#1276110, @zturner wrote: > In https://reviews.llvm.org/D53597#1276086, @clayborg wrote: > > > As long as Swig is happy and the ABI doesn't change I am ok with this. Will > > we see the variables better when debugging? Or is this solely so the > > SymbolContextItem type doesn't disappear from the debug info? > > > We will see variables better when debugging too. (That was actually my primary motivation) https://reviews.llvm.org/D53597 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This looks good to me. Looking at the addition of Type::ConsumeTypeClass makes it really clear that both this function and Type::GetTypeScopeAndBasename need to dispatch to the CompilerSystem to do this work. The actual code is way too C-family-ish to be in the generic Type.cpp. That will require passing in a language because I don't think you know enough from just the symfile and name to know which language the user was looking up types for. But this change doesn't make it harder to get that right, so fixing it doesn't need to be part of this patch. So far as I can tell, you can't do an O(1) lookup of an exact name in DWARF from the dwarf_names table (or the proceeding apple tables). The tables are organized by base name (or really whatever in the DW_AT_name attribute of the die). So you will always have to pull the base name out, find all the base name matches and then run up the parent dies to build up the fully qualified name. Each name entry has the parent DIE, so building up the full name is pretty efficient. But still, the best you can do O(number_of_occurances_of_name) and knowing the name is exact doesn't help the search speed. If I'm right about that (Greg will surely know) then we should remove the FIXME comment in SymbolFileDWARF since it refers to an unfixable fix. https://reviews.llvm.org/D53662 ___ 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
davide added inline comments.
Comment at:
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp:213-229
+ // Handle variables with incomplete array types.
+ auto *type = in_value.GetCompilerType().GetOpaqueQualType();
+ auto qual_type = clang::QualType::getFromOpaquePtr(type).getCanonicalType();
+ if (qual_type->getTypeClass() == clang::Type::IncompleteArray) {
+if (auto variable = in_value.GetVariable()) {
+ auto *lldb_type = variable->GetType();
+ auto *symbol_file = lldb_type->GetSymbolFile();
clayborg wrote:
> We must abstract this via the TypeSystem stuff and make this happen via the
> CompilerType interface. What happens when "in_value" is a swift type or other
> type from the type system? Crash
My understanding is that it is not possible today to get here with a swift
type. In fact, `GetDynamicTypeAndAddress` in swift will be always called on a
`SwiftLanguageRuntime`.
That said, I'm not necessarily advocating against the abstraction, just
pointing out that this is just a theoretical concern, and given this is an
internal API in the debugger, it could be possibly enforced by an assertion to
make sure people don't change the status quo accidentally.
https://reviews.llvm.org/D53530
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
So far as I can tell, this patch will make lookup of exact types faster for PDB, but because of the way DWARF debug_names tables are constructed, I don't think there's any way we can do the same thing for DWARF. But unless I'm misunderstanding the patch, this doesn't change correctness of the lookup (except for fixing "type lookup 'struct Foo'"). Did I miss something? Jim > On Oct 25, 2018, at 10:42 AM, Zachary Turner via Phabricator > wrote: > > zturner added a comment. > > In https://reviews.llvm.org/D53597#1276086, @clayborg wrote: > >> As long as Swig is happy and the ABI doesn't change I am ok with this. Will >> we see the variables better when debugging? Or is this solely so the >> SymbolContextItem type doesn't disappear from the debug info? > > > We will see variables better when debugging too. > > > https://reviews.llvm.org/D53597 > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
jingham added subscribers: clayborg, zturner. jingham added a comment. So far as I can tell, this patch will make lookup of exact types faster for PDB, but because of the way DWARF debug_names tables are constructed, I don't think there's any way we can do the same thing for DWARF. But unless I'm misunderstanding the patch, this doesn't change correctness of the lookup (except for fixing "type lookup 'struct Foo'"). Did I miss something? Jim https://reviews.llvm.org/D53597 ___ 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
aleksandr.urakov added a comment. Yes, I'll implement it tomorrow (I'm already OOO now), thanks. But is it really necessary to check the number of symbols added if we must to calculate / finalize the symtab after getting it from object file anyway? May be just always do it after creation and processing by the symbol file? For each symtab it will be done just once because of caching. 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] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
clayborg added a comment.
The current SymbolFile::FindTypes(...) in was designed with type base name only
due to how DWARF stores it variables. It has a "const CompilerDeclContext
*parent_decl_ctx" which can be specified in order to limit what we find. So we
might be able to think of this as a type lookup by basename. It might be handy
to add another type lookup that does lookups by fully qualified name only. and
leave the current infrastructure alone? A default implementation can be added
that returns false for all symbol file unless they support name lookup. Also
the current implementation allows you to under specify a type. For code like:
struct Pt { char x; char y; };
namespace a {
struct Pt { short x; short y; };
namespace b {
struct Pt { int x; int y; };
}
}
We can find all of them using:
(lldb) type lookup Pt
struct Pt {
short x;
short y;
}
struct Pt {
int x;
int y;
}
struct Pt {
char x;
char y;
}
Or each one individually using:
(lldb) type lookup a::b::Pt
struct Pt {
int x;
int y;
}
(lldb) type lookup a::Pt
struct Pt {
short x;
short y;
}
(lldb) type lookup ::Pt
struct Pt {
char x;
char y;
}
Or under specify the namespace:
(lldb) type lookup b::Pt
struct Pt {
int x;
int y;
}
That is the current behavior. Not saying it is right. Note we also don't
display the decl context for a type when dumping it which would be nice to fix.
So we probably need to make sure there are tests for all of the cases above
once we hone in on the approach we want all symbol plug-ins to use.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2443
+
+ // FIXME: When exact_match is true, we should do an optimized O(1) lookup.
+ llvm::StringRef scope;
There is no O(1) lookup for types in DWARF. Accelerator tables do not have
fully qualified type names, only the identifier for the type basename only.
https://reviews.llvm.org/D53662
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
zturner added a comment. In https://reviews.llvm.org/D53597#1276145, @jingham wrote: > So far as I can tell, this patch will make lookup of exact types faster for > PDB, but because of the way DWARF debug_names tables are constructed, I don't > think there's any way we can do the same thing for DWARF. > > But unless I'm misunderstanding the patch, this doesn't change correctness of > the lookup (except for fixing "type lookup 'struct Foo'"). Did I miss > something? > > Jim That's the other patch. This patch is NFC and just makes debugging nicer because you can see enum values in your debugger as rich enumerators. But for the other patch, if what you said is correct, then I suppose that's correct. I asked Eric Christopher and he said he thought (but wasn't 100% sure) that types were hashed in the accelerator tables by their full name and not their base name. If that is true then it could make exact matches faster. But if it's incorrect then yes, it wouldn't be able to make exact matches faster in DWARF. https://reviews.llvm.org/D53597 ___ 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 added a comment. In https://reviews.llvm.org/D53368#1276152, @aleksandr.urakov wrote: > Yes, I'll implement it tomorrow (I'm already OOO now), thanks. But is it > really necessary to check the number of symbols added if we must to calculate > / finalize the symtab after getting it from object file anyway? May be just > always do it after creation and processing by the symbol file? For each > symtab it will be done just once because of caching. yes, fine to still have void and always call symtab.CalculateSymbolSizes(); and symtab.Finalize() only in the symbol vendor. Find the other places this is called in the ObjectFile plug-ins and remove them and do them once in Symbol vendor when we fetch the symtab for the first time https://reviews.llvm.org/D53368 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
Ah, right... Too many patches (a good problem!)
The standard as I read it says that the name entry points into the general
string table, but doesn't specify which entry it points to. However, the
current DWARF debug_info doesn't ever emit a string for the fully qualified
name, so you would have to construct it specially to have an exact name to
refer to. I also couldn't see anything that said specifically whether the
DW_AT_name for a type has to be the full name or a base name, it just says:
If a name has been given to the structure, union, or class in the source
program, then the corresponding structure type, union type, or class type entry
has a DW_AT_name attribute whose value is a null-terminated string containing
the type name.
But it would bloat the name tables to use the full name, and since you can
reconstruct it from the context it's not needed... So I've only seen base
names in the name entry for types in the debug_info.
Anyway, current clang for -gdwarf-5 produces:
Bucket 1 [
Name 2 {
Hash: 0xB887389
String: 0x00c3 "Foo"
Entry @ 0x92 {
Abbrev: 0x39
Tag: DW_TAG_namespace
DW_IDX_die_offset: 0x004c
}
}
]
Bucket 2 [
Name 3 {
Hash: 0xB8860BA
String: 0x00c7 "Bar"
Entry @ 0x9b {
Abbrev: 0x13
Tag: DW_TAG_structure_type
DW_IDX_die_offset: 0x004e
}
}
Name 4 {
Hash: 0x7C9A7F6A
String: 0x00b5 "main"
Entry @ 0xa4 {
Abbrev: 0x2E
Tag: DW_TAG_subprogram
DW_IDX_die_offset: 0x0026
}
}
For:
namespace Foo
{
struct Bar
{
int First;
};
}
int
main()
{
Foo::Bar mine = {10};
return mine.First;
}
Jim
> On Oct 25, 2018, at 11:12 AM, Zachary Turner via Phabricator
> wrote:
>
> zturner added a comment.
>
> In https://reviews.llvm.org/D53597#1276145, @jingham wrote:
>
>> So far as I can tell, this patch will make lookup of exact types faster for
>> PDB, but because of the way DWARF debug_names tables are constructed, I
>> don't think there's any way we can do the same thing for DWARF.
>>
>> But unless I'm misunderstanding the patch, this doesn't change correctness
>> of the lookup (except for fixing "type lookup 'struct Foo'"). Did I miss
>> something?
>>
>> Jim
>
>
> That's the other patch. This patch is NFC and just makes debugging nicer
> because you can see enum values in your debugger as rich enumerators. But
> for the other patch, if what you said is correct, then I suppose that's
> correct. I asked Eric Christopher and he said he thought (but wasn't 100%
> sure) that types were hashed in the accelerator tables by their full name and
> not their base name. If that is true then it could make exact matches
> faster. But if it's incorrect then yes, it wouldn't be able to make exact
> matches faster in DWARF.
>
>
> https://reviews.llvm.org/D53597
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
jingham added a comment.
Ah, right... Too many patches (a good problem!)
The standard as I read it says that the name entry points into the general
string table, but doesn't specify which entry it points to. However, the
current DWARF debug_info doesn't ever emit a string for the fully qualified
name, so you would have to construct it specially to have an exact name to
refer to. I also couldn't see anything that said specifically whether the
DW_AT_name for a type has to be the full name or a base name, it just says:
If a name has been given to the structure, union, or class in the source
program, then the corresponding structure type, union type, or class type entry
has a DW_AT_name attribute whose value is a null-terminated string containing
the type name.
But it would bloat the name tables to use the full name, and since you can
reconstruct it from the context it's not needed... So I've only seen base
names in the name entry for types in the debug_info.
Anyway, current clang for -gdwarf-5 produces:
Bucket 1 [
Name 2 {
Hash: 0xB887389
String: 0x00c3 "Foo"
Entry @ 0x92 {
Abbrev: 0x39
Tag: DW_TAG_namespace
DW_IDX_die_offset: 0x004c
}
}
]
Bucket 2 [
Name 3 {
Hash: 0xB8860BA
String: 0x00c7 "Bar"
Entry @ 0x9b {
Abbrev: 0x13
Tag: DW_TAG_structure_type
DW_IDX_die_offset: 0x004e
}
}
Name 4 {
Hash: 0x7C9A7F6A
String: 0x00b5 "main"
Entry @ 0xa4 {
Abbrev: 0x2E
Tag: DW_TAG_subprogram
DW_IDX_die_offset: 0x0026
}
}
For:
namespace Foo
{
struct Bar
{
int First;
};
}
int
main()
{
Foo::Bar mine = {10};
return mine.First;
}
Jim
https://reviews.llvm.org/D53597
___
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 to FileSpec convenience methods.
JDevlieghere added a comment. While I don't disagree with the proposed approach, it needs to be highlighted that extracting all file system operations would be very intrusive. - Almost all logic in FileSpec deals with the filesystem. You can't even resolve a path without it. How do you imagine this would work? - This will significantly increase the verbosity of working with FileSpec. Depending on the solution to the previous bullet point, you might have to do something like `FileSpec spec(path);FS.Resolve(spec);FS.Exists(spec)`. Additionally, if we make the FileSystem a member of the debugger, we'd have to pass it everywhere we pass a FileSpec. I'm not convinced that's something we want. We could put the FileSpec and the FileSystem in a wrapper object but then we're essentially where we are today. Furthermore, for the reproducers, we need the FileSystem to live above the debugger because of the shared module cache. I can't really imagine a scenario where we'd need two debuggers with a different VFS. https://reviews.llvm.org/D53532 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
zturner added a subscriber: jingham. zturner added a comment. I guess the question is, How is that hash and the bucket computed? If it's based on the full name, then you should be able to get fast exact lookup. If it's based on the based name, then it will indeed be slow. https://reviews.llvm.org/D53597 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
I guess the question is, How is that hash and the bucket computed? If it's
based on the full name, then you should be able to get fast exact lookup.
If it's based on the based name, then it will indeed be slow.
On Thu, Oct 25, 2018 at 11:33 AM Jim Ingham via Phabricator via
lldb-commits wrote:
> jingham added a comment.
>
> Ah, right... Too many patches (a good problem!)
>
> The standard as I read it says that the name entry points into the general
> string table, but doesn't specify which entry it points to. However, the
> current DWARF debug_info doesn't ever emit a string for the fully qualified
> name, so you would have to construct it specially to have an exact name to
> refer to. I also couldn't see anything that said specifically whether the
> DW_AT_name for a type has to be the full name or a base name, it just says:
>
> If a name has been given to the structure, union, or class in the source
> program, then the corresponding structure type, union type, or class type
> entry has a DW_AT_name attribute whose value is a null-terminated string
> containing the type name.
>
> But it would bloat the name tables to use the full name, and since you can
> reconstruct it from the context it's not needed... So I've only seen base
> names in the name entry for types in the debug_info.
>
> Anyway, current clang for -gdwarf-5 produces:
>
> Bucket 1 [
> Name 2 {
> Hash: 0xB887389
> String: 0x00c3 "Foo"
> Entry @ 0x92 {
> Abbrev: 0x39
> Tag: DW_TAG_namespace
> DW_IDX_die_offset: 0x004c
> }
> }
> ]
> Bucket 2 [
> Name 3 {
> Hash: 0xB8860BA
> String: 0x00c7 "Bar"
> Entry @ 0x9b {
> Abbrev: 0x13
> Tag: DW_TAG_structure_type
> DW_IDX_die_offset: 0x004e
> }
> }
> Name 4 {
> Hash: 0x7C9A7F6A
> String: 0x00b5 "main"
> Entry @ 0xa4 {
> Abbrev: 0x2E
> Tag: DW_TAG_subprogram
> DW_IDX_die_offset: 0x0026
> }
> }
>
> For:
>
> namespace Foo
> {
>
> struct Bar
> {
> int First;
> };
>
> }
>
> int
> main()
> {
>
> Foo::Bar mine = {10};
> return mine.First;
>
> }
>
> Jim
>
>
> https://reviews.llvm.org/D53597
>
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
jingham added a comment. So the current approach relies on the ability to sniff the name to determine the context in which the user intends to find it. It does (and always did) use the presence of an initial "::" to tell whether a symbol is exact. That's obviously also inappropriate for a generic Type method. But OTOH, there needs to be a funnel point where you take in a string you know nothing about (from the user either in type lookup or in an expression) and find it as best you can. I don't think we want to force command line users to say "type lookup --exact " so we've got to figure it out internally. Internally, it might be helpful to do some initial analysis of the input type string, and then dispatch it to an exact or inexact search. But given I don't think you can get away without a FindTypes that takes a string that you don't know whether it is exact or not, I don't feel strongly about that. We should abstract "is exact" and ask the various type systems to handle that request, and we also need to abstract "remove type class" and again ask the various type systems to handle that. That seems to me the main ugliness that this patch reveals. https://reviews.llvm.org/D53662 ___ 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 to FileSpec convenience methods.
zturner added a comment. I envision `FileSpec` as being more of a `PathSpec`. It should be able manipulate paths as strings and nothing more. There is indeed some logic that still remains that resolves paths, but it manages to do this using LLVM's file system APIs and the only reason it's still there is because it was baked in and a bit hard to remove. One idea for removing it would be to have `FileSpec FileSystem::Resolve(const FileSpec &)`. Then instead of saying `FileSpec foo(path, /*resolve = */ true);`, they could say `FileSpec foo = FileSystem::Resolve(FileSpec(path));` or something. https://reviews.llvm.org/D53532 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
I am pretty sure the has is computed from the name string. And BTW, having the
base name in the quick lookup tables (either with or alongside the full name)
is a really good thing. People using the debugger really don't want to type
out fully qualified names as a general rule. So you have to quickly support
incomplete name searches, and the best way to do that is grab the base name,
and start matching up to where the input stopped specifying the name. So if
you're only going to have one entry per type, having the base name be the quick
lookup is more flexible, though it does have the cost that fully specified
names are slower to lookup. But the slowness is not O(number of symbols) which
would be horrible but O(number of symbols with this base name). That can get
bad for all the commonly used type names in C++ templates - the STL seems to
produce boatloads of types of the same base name. But still isn't that bad.
Jim
> On Oct 25, 2018, at 11:40 AM, Zachary Turner wrote:
>
> I guess the question is, How is that hash and the bucket computed? If it's
> based on the full name, then you should be able to get fast exact lookup. If
> it's based on the based name, then it will indeed be slow.
>
> On Thu, Oct 25, 2018 at 11:33 AM Jim Ingham via Phabricator via lldb-commits
> wrote:
> jingham added a comment.
>
> Ah, right... Too many patches (a good problem!)
>
> The standard as I read it says that the name entry points into the general
> string table, but doesn't specify which entry it points to. However, the
> current DWARF debug_info doesn't ever emit a string for the fully qualified
> name, so you would have to construct it specially to have an exact name to
> refer to. I also couldn't see anything that said specifically whether the
> DW_AT_name for a type has to be the full name or a base name, it just says:
>
> If a name has been given to the structure, union, or class in the source
> program, then the corresponding structure type, union type, or class type
> entry has a DW_AT_name attribute whose value is a null-terminated string
> containing the type name.
>
> But it would bloat the name tables to use the full name, and since you can
> reconstruct it from the context it's not needed... So I've only seen base
> names in the name entry for types in the debug_info.
>
> Anyway, current clang for -gdwarf-5 produces:
>
> Bucket 1 [
> Name 2 {
> Hash: 0xB887389
> String: 0x00c3 "Foo"
> Entry @ 0x92 {
> Abbrev: 0x39
> Tag: DW_TAG_namespace
> DW_IDX_die_offset: 0x004c
> }
> }
> ]
> Bucket 2 [
> Name 3 {
> Hash: 0xB8860BA
> String: 0x00c7 "Bar"
> Entry @ 0x9b {
> Abbrev: 0x13
> Tag: DW_TAG_structure_type
> DW_IDX_die_offset: 0x004e
> }
> }
> Name 4 {
> Hash: 0x7C9A7F6A
> String: 0x00b5 "main"
> Entry @ 0xa4 {
> Abbrev: 0x2E
> Tag: DW_TAG_subprogram
> DW_IDX_die_offset: 0x0026
> }
> }
>
> For:
>
> namespace Foo
> {
>
> struct Bar
> {
> int First;
> };
>
> }
>
> int
> main()
> {
>
> Foo::Bar mine = {10};
> return mine.First;
>
> }
>
> Jim
>
>
> https://reviews.llvm.org/D53597
>
>
>
> ___
> 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
Re: [Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
Yes, it's an interesting dichotomy how the two formats have evolved. PDB
was designed with IDEs in mind so it's optimized around exact matches. For
example, you press a key on a specific line of code. Nobody ever is
entering commands to do fuzzy lookups based on base names, so this was
never really considered a use case. And if you go and try to use Windows
command line debuggers, it's indeed slow.
On Thu, Oct 25, 2018 at 11:51 AM Jim Ingham wrote:
> I am pretty sure the has is computed from the name string. And BTW,
> having the base name in the quick lookup tables (either with or alongside
> the full name) is a really good thing. People using the debugger really
> don't want to type out fully qualified names as a general rule. So you
> have to quickly support incomplete name searches, and the best way to do
> that is grab the base name, and start matching up to where the input
> stopped specifying the name. So if you're only going to have one entry per
> type, having the base name be the quick lookup is more flexible, though it
> does have the cost that fully specified names are slower to lookup. But
> the slowness is not O(number of symbols) which would be horrible but
> O(number of symbols with this base name). That can get bad for all the
> commonly used type names in C++ templates - the STL seems to produce
> boatloads of types of the same base name. But still isn't that bad.
>
> Jim
>
>
> > On Oct 25, 2018, at 11:40 AM, Zachary Turner wrote:
> >
> > I guess the question is, How is that hash and the bucket computed? If
> it's based on the full name, then you should be able to get fast exact
> lookup. If it's based on the based name, then it will indeed be slow.
> >
> > On Thu, Oct 25, 2018 at 11:33 AM Jim Ingham via Phabricator via
> lldb-commits wrote:
> > jingham added a comment.
> >
> > Ah, right... Too many patches (a good problem!)
> >
> > The standard as I read it says that the name entry points into the
> general string table, but doesn't specify which entry it points to.
> However, the current DWARF debug_info doesn't ever emit a string for the
> fully qualified name, so you would have to construct it specially to have
> an exact name to refer to. I also couldn't see anything that said
> specifically whether the DW_AT_name for a type has to be the full name or a
> base name, it just says:
> >
> > If a name has been given to the structure, union, or class in the source
> program, then the corresponding structure type, union type, or class type
> entry has a DW_AT_name attribute whose value is a null-terminated string
> containing the type name.
> >
> > But it would bloat the name tables to use the full name, and since you
> can reconstruct it from the context it's not needed... So I've only seen
> base names in the name entry for types in the debug_info.
> >
> > Anyway, current clang for -gdwarf-5 produces:
> >
> > Bucket 1 [
> > Name 2 {
> > Hash: 0xB887389
> > String: 0x00c3 "Foo"
> > Entry @ 0x92 {
> > Abbrev: 0x39
> > Tag: DW_TAG_namespace
> > DW_IDX_die_offset: 0x004c
> > }
> > }
> > ]
> > Bucket 2 [
> > Name 3 {
> > Hash: 0xB8860BA
> > String: 0x00c7 "Bar"
> > Entry @ 0x9b {
> > Abbrev: 0x13
> > Tag: DW_TAG_structure_type
> > DW_IDX_die_offset: 0x004e
> > }
> > }
> > Name 4 {
> > Hash: 0x7C9A7F6A
> > String: 0x00b5 "main"
> > Entry @ 0xa4 {
> > Abbrev: 0x2E
> > Tag: DW_TAG_subprogram
> > DW_IDX_die_offset: 0x0026
> > }
> > }
> >
> > For:
> >
> > namespace Foo
> > {
> >
> > struct Bar
> > {
> > int First;
> > };
> >
> > }
> >
> > int
> > main()
> > {
> >
> > Foo::Bar mine = {10};
> > return mine.First;
> >
> > }
> >
> > Jim
> >
> >
> > https://reviews.llvm.org/D53597
> >
> >
> >
> > ___
> > 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
Re: [Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
gdb's expression parser went out of it's way to allow you to type as little as
possible, and to do things that made no sense in the current context (use
variables and types in expressions that aren't actually visible in the current
context, etc). People's workflows came to really depend on these features, and
the whole system grew up to support that. There's a lot of complexity in lldb
to support this kind of investigation, but I don't think we can do without it
w/o facing a riot. It's a bit of a "give a mouse a cookie" type situation.
Jim
> On Oct 25, 2018, at 12:09 PM, Zachary Turner wrote:
>
> Yes, it's an interesting dichotomy how the two formats have evolved. PDB was
> designed with IDEs in mind so it's optimized around exact matches. For
> example, you press a key on a specific line of code. Nobody ever is entering
> commands to do fuzzy lookups based on base names, so this was never really
> considered a use case. And if you go and try to use Windows command line
> debuggers, it's indeed slow.
>
> On Thu, Oct 25, 2018 at 11:51 AM Jim Ingham wrote:
> I am pretty sure the has is computed from the name string. And BTW, having
> the base name in the quick lookup tables (either with or alongside the full
> name) is a really good thing. People using the debugger really don't want to
> type out fully qualified names as a general rule. So you have to quickly
> support incomplete name searches, and the best way to do that is grab the
> base name, and start matching up to where the input stopped specifying the
> name. So if you're only going to have one entry per type, having the base
> name be the quick lookup is more flexible, though it does have the cost that
> fully specified names are slower to lookup. But the slowness is not O(number
> of symbols) which would be horrible but O(number of symbols with this base
> name). That can get bad for all the commonly used type names in C++
> templates - the STL seems to produce boatloads of types of the same base
> name. But still isn't that bad.
>
> Jim
>
>
> > On Oct 25, 2018, at 11:40 AM, Zachary Turner wrote:
> >
> > I guess the question is, How is that hash and the bucket computed? If it's
> > based on the full name, then you should be able to get fast exact lookup.
> > If it's based on the based name, then it will indeed be slow.
> >
> > On Thu, Oct 25, 2018 at 11:33 AM Jim Ingham via Phabricator via
> > lldb-commits wrote:
> > jingham added a comment.
> >
> > Ah, right... Too many patches (a good problem!)
> >
> > The standard as I read it says that the name entry points into the general
> > string table, but doesn't specify which entry it points to. However, the
> > current DWARF debug_info doesn't ever emit a string for the fully qualified
> > name, so you would have to construct it specially to have an exact name to
> > refer to. I also couldn't see anything that said specifically whether the
> > DW_AT_name for a type has to be the full name or a base name, it just says:
> >
> > If a name has been given to the structure, union, or class in the source
> > program, then the corresponding structure type, union type, or class type
> > entry has a DW_AT_name attribute whose value is a null-terminated string
> > containing the type name.
> >
> > But it would bloat the name tables to use the full name, and since you can
> > reconstruct it from the context it's not needed... So I've only seen base
> > names in the name entry for types in the debug_info.
> >
> > Anyway, current clang for -gdwarf-5 produces:
> >
> > Bucket 1 [
> > Name 2 {
> > Hash: 0xB887389
> > String: 0x00c3 "Foo"
> > Entry @ 0x92 {
> > Abbrev: 0x39
> > Tag: DW_TAG_namespace
> > DW_IDX_die_offset: 0x004c
> > }
> > }
> > ]
> > Bucket 2 [
> > Name 3 {
> > Hash: 0xB8860BA
> > String: 0x00c7 "Bar"
> > Entry @ 0x9b {
> > Abbrev: 0x13
> > Tag: DW_TAG_structure_type
> > DW_IDX_die_offset: 0x004e
> > }
> > }
> > Name 4 {
> > Hash: 0x7C9A7F6A
> > String: 0x00b5 "main"
> > Entry @ 0xa4 {
> > Abbrev: 0x2E
> > Tag: DW_TAG_subprogram
> > DW_IDX_die_offset: 0x0026
> > }
> > }
> >
> > For:
> >
> > namespace Foo
> > {
> >
> > struct Bar
> > {
> > int First;
> > };
> >
> > }
> >
> > int
> > main()
> > {
> >
> > Foo::Bar mine = {10};
> > return mine.First;
> >
> > }
> >
> > Jim
> >
> >
> > https://reviews.llvm.org/D53597
> >
> >
> >
> > ___
> > lldb-commits mailing list
> > lldb-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53532: [FileSpec] Add VFS support to FileSpec convenience methods.
JDevlieghere added a comment. Thanks for the feedback! I totally agree it's a good solution and it was one of the things I considered. It didn't make it to the top of the list because it is very intrusive and changes the semantics of FileSpec quite a bit (i.e. it becomes a PathSpec as Zachary noted). Anyway, I'm happy to do this and I'll split this up in two patches as Pavel suggested. I'll extend the FileSystem first, so that we can do the VFS stuff straight away instead of implementing these methods twice. https://reviews.llvm.org/D53532 ___ 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 to FileSpec convenience methods.
zturner added a comment. In https://reviews.llvm.org/D53532#1276463, @JDevlieghere wrote: > Thanks for the feedback! I totally agree it's a good solution and it was one > of the things I considered. It didn't make it to the top of the list because > it is very intrusive and changes the semantics of FileSpec quite a bit (i.e. > it becomes a PathSpec as Zachary noted). Anyway, I'm happy to do this and > I'll split this up in two patches as Pavel suggested. I'll extend the > FileSystem first, so that we can do the VFS stuff straight away instead of > implementing these methods twice. It does change them, but that's always been the sort of "end goal" of `FileSpec` anyway, it just hasn't been reached yet. Hopefully we can get there with your help :) https://reviews.llvm.org/D53532 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53590: Refactor SetBaseClassesForType and DeleteBaseClasses to be more C++'y
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB345312: [NFC] Refactor SetBaseClasses and
DeleteBaseClasses. (authored by zturner, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D53590?vs=170700&id=171185#toc
Repository:
rLLDB LLDB
https://reviews.llvm.org/D53590
Files:
include/lldb/Symbol/ClangASTContext.h
source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
source/Symbol/ClangASTContext.cpp
unittests/Symbol/TestClangASTContext.cpp
Index: unittests/Symbol/TestClangASTContext.cpp
===
--- unittests/Symbol/TestClangASTContext.cpp
+++ unittests/Symbol/TestClangASTContext.cpp
@@ -337,15 +337,19 @@
EXPECT_NE(nullptr, non_empty_base_field_decl);
EXPECT_TRUE(ClangASTContext::RecordHasFields(non_empty_base_decl));
+ std::vector> bases;
+
// Test that a record with no direct fields, but fields in a base returns true
CompilerType empty_derived = m_ast->CreateRecordType(
nullptr, lldb::eAccessPublic, "EmptyDerived", clang::TTK_Struct,
lldb::eLanguageTypeC_plus_plus, nullptr);
ClangASTContext::StartTagDeclarationDefinition(empty_derived);
- CXXBaseSpecifier *non_empty_base_spec = m_ast->CreateBaseClassSpecifier(
- non_empty_base.GetOpaqueQualType(), lldb::eAccessPublic, false, false);
- bool result = m_ast->SetBaseClassesForClassType(
- empty_derived.GetOpaqueQualType(), &non_empty_base_spec, 1);
+ std::unique_ptr non_empty_base_spec =
+ m_ast->CreateBaseClassSpecifier(non_empty_base.GetOpaqueQualType(),
+ lldb::eAccessPublic, false, false);
+ bases.push_back(std::move(non_empty_base_spec));
+ bool result = m_ast->TransferBaseClasses(empty_derived.GetOpaqueQualType(),
+ std::move(bases));
ClangASTContext::CompleteTagDeclarationDefinition(empty_derived);
EXPECT_TRUE(result);
CXXRecordDecl *empty_derived_non_empty_base_cxx_decl =
@@ -363,10 +367,12 @@
nullptr, lldb::eAccessPublic, "EmptyDerived2", clang::TTK_Struct,
lldb::eLanguageTypeC_plus_plus, nullptr);
ClangASTContext::StartTagDeclarationDefinition(empty_derived2);
- CXXBaseSpecifier *non_empty_vbase_spec = m_ast->CreateBaseClassSpecifier(
- non_empty_base.GetOpaqueQualType(), lldb::eAccessPublic, true, false);
- result = m_ast->SetBaseClassesForClassType(empty_derived2.GetOpaqueQualType(),
- &non_empty_vbase_spec, 1);
+ std::unique_ptr non_empty_vbase_spec =
+ m_ast->CreateBaseClassSpecifier(non_empty_base.GetOpaqueQualType(),
+ lldb::eAccessPublic, true, false);
+ bases.push_back(std::move(non_empty_vbase_spec));
+ result = m_ast->TransferBaseClasses(empty_derived2.GetOpaqueQualType(),
+ std::move(bases));
ClangASTContext::CompleteTagDeclarationDefinition(empty_derived2);
EXPECT_TRUE(result);
CXXRecordDecl *empty_derived_non_empty_vbase_cxx_decl =
@@ -377,9 +383,6 @@
empty_derived_non_empty_vbase_cxx_decl, false));
EXPECT_TRUE(
ClangASTContext::RecordHasFields(empty_derived_non_empty_vbase_decl));
-
- delete non_empty_base_spec;
- delete non_empty_vbase_spec;
}
TEST_F(TestClangASTContext, TemplateArguments) {
Index: source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
===
--- source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -59,12 +59,12 @@
CVType udt_cvt = m_symbol_file.m_index->tpi().getType(ti);
lldb::opaque_compiler_type_t base_qt = base_ct.GetOpaqueQualType();
- clang::CXXBaseSpecifier *base_spec =
+ std::unique_ptr base_spec =
m_symbol_file.GetASTContext().CreateBaseClassSpecifier(
base_qt, TranslateMemberAccess(access), false,
udt_cvt.kind() == LF_CLASS);
-
- m_bases.push_back(base_spec);
+ lldbassert(base_spec);
+ m_bases.push_back(std::move(base_spec));
return base_qt;
}
@@ -172,9 +172,8 @@
void UdtRecordCompleter::complete() {
ClangASTContext &clang = m_symbol_file.GetASTContext();
- clang.SetBaseClassesForClassType(m_derived_ct.GetOpaqueQualType(),
- m_bases.data(), m_bases.size());
- ClangASTContext::DeleteBaseClassSpecifiers(m_bases.data(), m_bases.size());
+ clang.TransferBaseClasses(m_derived_ct.GetOpaqueQualType(),
+std::move(m_bases));
clang.AddMethodOverridesForCXXRecordType(m_derived_ct.GetOpaqueQual
[Lldb-commits] [lldb] r345312 - [NFC] Refactor SetBaseClasses and DeleteBaseClasses.
Author: zturner
Date: Thu Oct 25 13:44:56 2018
New Revision: 345312
URL: http://llvm.org/viewvc/llvm-project?rev=345312&view=rev
Log:
[NFC] Refactor SetBaseClasses and DeleteBaseClasses.
We currently had a 2-step process where we had to call
SetBaseClassesForType and DeleteBaseClasses. Every single caller
followed this exact 2-step process, and there was manual memory
management going on with raw pointers. We can do better than this
by storing a vector of unique_ptrs and passing this around.
This makes for a cleaner API, and we only need to call one method
so there is no possibility of a user forgetting to call
DeleteBaseClassSpecifiers.
In addition to this, it also makes for a *simpler* API. Part of
why I wanted to do this is because when I was implementing the native
PDB interface I had to spend some time understanding exactly what I
was deleting and why. ClangAST has significant mental overhead
associated with it, and reducing the API surface can go along
way to making it simpler for people to understand.
Differential Revision: https://reviews.llvm.org/D53590
Modified:
lldb/trunk/include/lldb/Symbol/ClangASTContext.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
lldb/trunk/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
lldb/trunk/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
lldb/trunk/source/Symbol/ClangASTContext.cpp
lldb/trunk/unittests/Symbol/TestClangASTContext.cpp
Modified: lldb/trunk/include/lldb/Symbol/ClangASTContext.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTContext.h?rev=345312&r1=345311&r2=345312&view=diff
==
--- lldb/trunk/include/lldb/Symbol/ClangASTContext.h (original)
+++ lldb/trunk/include/lldb/Symbol/ClangASTContext.h Thu Oct 25 13:44:56 2018
@@ -858,18 +858,14 @@ public:
void AddMethodOverridesForCXXRecordType(lldb::opaque_compiler_type_t type);
// C++ Base Classes
- clang::CXXBaseSpecifier *
+ std::unique_ptr
CreateBaseClassSpecifier(lldb::opaque_compiler_type_t type,
lldb::AccessType access, bool is_virtual,
bool base_of_class);
- static void DeleteBaseClassSpecifiers(clang::CXXBaseSpecifier **base_classes,
-unsigned num_base_classes);
-
- bool
- SetBaseClassesForClassType(lldb::opaque_compiler_type_t type,
- clang::CXXBaseSpecifier const *const
*base_classes,
- unsigned num_base_classes);
+ bool TransferBaseClasses(
+ lldb::opaque_compiler_type_t type,
+ std::vector> bases);
static bool SetObjCSuperClass(const CompilerType &type,
const CompilerType &superclass_compiler_type);
Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=345312&r1=345311&r2=345312&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Thu Oct
25 13:44:56 2018
@@ -2184,14 +2184,14 @@ bool DWARFASTParserClang::CompleteTypeFr
}
SymbolContext sc(die.GetLLDBCompileUnit());
-std::vector base_classes;
+std::vector> bases;
std::vector member_accessibilities;
bool is_a_class = false;
// Parse members and base classes first
DWARFDIECollection member_function_dies;
DelayedPropertyList delayed_properties;
-ParseChildMembers(sc, die, clang_type, class_language, base_classes,
+ParseChildMembers(sc, die, clang_type, class_language, bases,
member_accessibilities, member_function_dies,
delayed_properties, default_accessibility,
is_a_class,
layout_info);
@@ -2255,11 +2255,11 @@ bool DWARFASTParserClang::CompleteTypeFr
&member_accessibilities.front(), member_accessibilities.size());
}
-if (!base_classes.empty()) {
+if (!bases.empty()) {
// Make sure all base classes refer to complete types and not forward
// declarations. If we don't do this, clang will crash with an
- // assertion in the call to clang_type.SetBaseClassesForClassType()
- for (auto &base_class : base_classes) {
+ // assertion in the call to clang_type.TransferBaseClasses()
+ for (const auto &base_class : bases) {
clang::TypeSourceInfo *type_source_info =
base_class->getTypeSourceInfo();
i
[Lldb-commits] [lldb] r345313 - Don't type-erase the SymbolContextItem enumeration.
Author: zturner Date: Thu Oct 25 13:45:19 2018 New Revision: 345313 URL: http://llvm.org/viewvc/llvm-project?rev=345313&view=rev Log: Don't type-erase the SymbolContextItem enumeration. When we get the `resolve_scope` parameter from the SB API, it's a `uint32_t`. We then pass it through all of LLDB this way, as a uint32. This is unfortunate, because it means the user of an API never actually knows what they're dealing with. We can call it something like `resolve_scope` and have comments saying "this is a value from the `SymbolContextItem` enumeration, but it makes more sense to just have it actually *be* the correct type in the actual C++ type system to begin with. This way the person reading the code just knows what it is. The reason to use integers instead of enumerations for flags is because when you do bitwise operations on enumerations they get promoted to integers, so it makes it tedious to constantly be casting them back to the enumeration types, so I've introduced a macro to make this happen magically. By writing LLDB_MARK_AS_BITMASK_ENUM after defining an enumeration, it will define overloaded operators so that the returned type will be the original enum. This should address all the mechanical issues surrounding using rich enum types directly. This way, we get a better debugger experience, and new users to the codebase can get more easily acquainted with the codebase because their IDE features can help them understand what the types mean. Differential Revision: https://reviews.llvm.org/D53597 Modified: lldb/trunk/include/lldb/Core/Address.h lldb/trunk/include/lldb/Core/Module.h lldb/trunk/include/lldb/Core/ModuleList.h lldb/trunk/include/lldb/Symbol/CompileUnit.h lldb/trunk/include/lldb/Symbol/SymbolFile.h lldb/trunk/include/lldb/Symbol/SymbolVendor.h lldb/trunk/include/lldb/Target/StackFrame.h lldb/trunk/include/lldb/lldb-enumerations.h lldb/trunk/source/API/SBAddress.cpp lldb/trunk/source/API/SBFrame.cpp lldb/trunk/source/API/SBModule.cpp lldb/trunk/source/API/SBTarget.cpp lldb/trunk/source/Commands/CommandObjectSource.cpp lldb/trunk/source/Core/Address.cpp lldb/trunk/source/Core/Disassembler.cpp lldb/trunk/source/Core/Module.cpp lldb/trunk/source/Core/ModuleList.cpp lldb/trunk/source/Core/SourceManager.cpp lldb/trunk/source/Plugins/Architecture/Mips/ArchitectureMips.cpp lldb/trunk/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp lldb/trunk/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h lldb/trunk/source/Symbol/CompileUnit.cpp lldb/trunk/source/Symbol/SymbolFile.cpp lldb/trunk/source/Symbol/SymbolVendor.cpp lldb/trunk/source/Target/StackFrame.cpp Modified: lldb/trunk/include/lldb/Core/Address.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Address.h?rev=345313&r1=345312&r2=345313&view=diff == --- lldb/trunk/include/lldb/Core/Address.h (original) +++ lldb/trunk/include/lldb/Core/Address.h Thu Oct 25 13:45:19 2018 @@ -508,9 +508,9 @@ public: /// /// @see SymbolContextScope::CalculateSymbolContext(SymbolContext*) //-- - uint32_t CalculateSymbolContext( - SymbolContext *sc, - uint32_t resolve_scope = lldb::eSymbolContextEverything) const; + uint32_t CalculateSymbolContext(SymbolContext *sc, + lldb::SymbolContextItem resolve_scope = + lldb::eSymbolContextEverything) const; lldb::ModuleSP CalculateSymbolContextModule() const; Modified: lldb/trunk/include/lldb/Core/Module.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Module.h?rev=345313&r1=345312&r2=345313&view=diff == --- lldb/trunk/include/lldb/Core/Module.h (original) +++ lldb/trunk/include/lldb/Core/Module.h Thu Oct 25 13:45:19 2018 @@ -816,10 +816,9 @@ public: /// /// @see SymbolContext::Scope //---
[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB345313: Don't type-erase the SymbolContextItem
enumeration. (authored by zturner, committed by ).
Herald added a subscriber: abidh.
Changed prior to commit:
https://reviews.llvm.org/D53597?vs=170737&id=171187#toc
Repository:
rLLDB LLDB
https://reviews.llvm.org/D53597
Files:
include/lldb/Core/Address.h
include/lldb/Core/Module.h
include/lldb/Core/ModuleList.h
include/lldb/Symbol/CompileUnit.h
include/lldb/Symbol/SymbolFile.h
include/lldb/Symbol/SymbolVendor.h
include/lldb/Target/StackFrame.h
include/lldb/lldb-enumerations.h
source/API/SBAddress.cpp
source/API/SBFrame.cpp
source/API/SBModule.cpp
source/API/SBTarget.cpp
source/Commands/CommandObjectSource.cpp
source/Core/Address.cpp
source/Core/Disassembler.cpp
source/Core/Module.cpp
source/Core/ModuleList.cpp
source/Core/SourceManager.cpp
source/Plugins/Architecture/Mips/ArchitectureMips.cpp
source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
source/Plugins/Process/Utility/RegisterContextLLDB.cpp
source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp
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.cpp
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/CompileUnit.cpp
source/Symbol/SymbolFile.cpp
source/Symbol/SymbolVendor.cpp
source/Target/StackFrame.cpp
Index: source/API/SBModule.cpp
===
--- source/API/SBModule.cpp
+++ source/API/SBModule.cpp
@@ -217,9 +217,9 @@
uint32_t resolve_scope) {
SBSymbolContext sb_sc;
ModuleSP module_sp(GetSP());
+ SymbolContextItem scope = static_cast(resolve_scope);
if (module_sp && addr.IsValid())
-module_sp->ResolveSymbolContextForAddress(addr.ref(), resolve_scope,
- *sb_sc);
+module_sp->ResolveSymbolContextForAddress(addr.ref(), scope, *sb_sc);
return sb_sc;
}
Index: source/API/SBTarget.cpp
===
--- source/API/SBTarget.cpp
+++ source/API/SBTarget.cpp
@@ -660,11 +660,12 @@
SBTarget::ResolveSymbolContextForAddress(const SBAddress &addr,
uint32_t resolve_scope) {
SBSymbolContext sc;
+ SymbolContextItem scope = static_cast(resolve_scope);
if (addr.IsValid()) {
TargetSP target_sp(GetSP());
if (target_sp)
- target_sp->GetImages().ResolveSymbolContextForAddress(
- addr.ref(), resolve_scope, sc.ref());
+ target_sp->GetImages().ResolveSymbolContextForAddress(addr.ref(), scope,
+sc.ref());
}
return sc;
}
Index: source/API/SBAddress.cpp
===
--- source/API/SBAddress.cpp
+++ source/API/SBAddress.cpp
@@ -198,8 +198,9 @@
SBSymbolContext SBAddress::GetSymbolContext(uint32_t resolve_scope) {
SBSymbolContext sb_sc;
+ SymbolContextItem scope = static_cast(resolve_scope);
if (m_opaque_ap->IsValid())
-m_opaque_ap->CalculateSymbolContext(&sb_sc.ref(), resolve_scope);
+m_opaque_ap->CalculateSymbolContext(&sb_sc.ref(), scope);
return sb_sc;
}
Index: source/API/SBFrame.cpp
===
--- source/API/SBFrame.cpp
+++ source/API/SBFrame.cpp
@@ -112,16 +112,16 @@
SBSymbolContext sb_sym_ctx;
std::unique_lock lock;
ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
-
+ SymbolContextItem scope = static_cast(resolve_scope);
StackFrame *frame = nullptr;
Target *target = exe_ctx.GetTargetPtr();
Process *process = exe_ctx.GetProcessPtr();
if (target && process) {
Process::StopLocker stop_locker;
if (stop_locker.TryLock(&process->GetRunLock())) {
frame = exe_ctx.GetFramePtr();
if (frame) {
-sb_sym_ctx.SetSymbolContext(&frame->GetSymbolContext(resolve_scope));
+sb_sym_ctx.SetSymbolContext(&frame->GetSymbolContext(scope));
} else {
if (log)
log->Printf("SBFrame::GetVariables () => error: could not "
Index: source/Core/SourceManager.cpp
===
--- source/Core/SourceManager.cpp
+++ source/Core/SourceManager
[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL345313: Don't type-erase the SymbolContextItem
enumeration. (authored by zturner, committed by ).
Herald added subscribers: llvm-commits, jrtc27.
Changed prior to commit:
https://reviews.llvm.org/D53597?vs=170737&id=171186#toc
Repository:
rL LLVM
https://reviews.llvm.org/D53597
Files:
lldb/trunk/include/lldb/Core/Address.h
lldb/trunk/include/lldb/Core/Module.h
lldb/trunk/include/lldb/Core/ModuleList.h
lldb/trunk/include/lldb/Symbol/CompileUnit.h
lldb/trunk/include/lldb/Symbol/SymbolFile.h
lldb/trunk/include/lldb/Symbol/SymbolVendor.h
lldb/trunk/include/lldb/Target/StackFrame.h
lldb/trunk/include/lldb/lldb-enumerations.h
lldb/trunk/source/API/SBAddress.cpp
lldb/trunk/source/API/SBFrame.cpp
lldb/trunk/source/API/SBModule.cpp
lldb/trunk/source/API/SBTarget.cpp
lldb/trunk/source/Commands/CommandObjectSource.cpp
lldb/trunk/source/Core/Address.cpp
lldb/trunk/source/Core/Disassembler.cpp
lldb/trunk/source/Core/Module.cpp
lldb/trunk/source/Core/ModuleList.cpp
lldb/trunk/source/Core/SourceManager.cpp
lldb/trunk/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
lldb/trunk/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
lldb/trunk/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
lldb/trunk/source/Symbol/CompileUnit.cpp
lldb/trunk/source/Symbol/SymbolFile.cpp
lldb/trunk/source/Symbol/SymbolVendor.cpp
lldb/trunk/source/Target/StackFrame.cpp
Index: lldb/trunk/source/Symbol/CompileUnit.cpp
===
--- lldb/trunk/source/Symbol/CompileUnit.cpp
+++ lldb/trunk/source/Symbol/CompileUnit.cpp
@@ -278,7 +278,8 @@
uint32_t CompileUnit::ResolveSymbolContext(const FileSpec &file_spec,
uint32_t line, bool check_inlines,
- bool exact, uint32_t resolve_scope,
+ bool exact,
+ SymbolContextItem resolve_scope,
SymbolContextList &sc_list) {
// First find all of the file indexes that match our "file_spec". If
// "file_spec" has an empty directory, then only compare the basenames when
Index: lldb/trunk/source/Symbol/SymbolVendor.cpp
===
--- lldb/trunk/source/Symbol/SymbolVendor.cpp
+++ lldb/trunk/source/Symbol/SymbolVendor.cpp
@@ -235,7 +235,7 @@
}
uint32_t SymbolVendor::ResolveSymbolContext(const Address &so_addr,
-uint32_t resolve_scope,
+SymbolContextItem resolve_scope,
SymbolContext &sc) {
ModuleSP module_sp(GetModule());
if (module_sp) {
@@ -248,7 +248,7 @@
uint32_t SymbolVendor::ResolveSymbolContext(const FileSpec &file_spec,
uint32_t line, bool check_inlines,
-uint32_t resolve_scope,
+SymbolContextItem resolve_scope,
SymbolContextList &sc_list) {
ModuleSP module_sp(GetModule());
if (module_sp) {
Index: lldb/trunk/source/Symbol/SymbolFile.cpp
===
--- lldb/trunk/source/Symbol/SymbolFile.cpp
+++ lldb/trunk/source/Symbol/SymbolFile.cpp
@@ -97,7 +97,7 @@
uint32_t SymbolFile::ResolveSymbolContext(const FileSpec &file_spec,
uint32_t line, bool check_inlines,
- uint32_t resolve_scope,
+ lldb::SymbolContextItem resolve_scope,
SymbolContextList &sc_list) {
return 0;
}
Index: lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.
[Lldb-commits] [lldb] r345314 - Don't type-erase the FunctionNameType or TypeClass enums.
Author: zturner
Date: Thu Oct 25 13:45:40 2018
New Revision: 345314
URL: http://llvm.org/viewvc/llvm-project?rev=345314&view=rev
Log:
Don't type-erase the FunctionNameType or TypeClass enums.
This is similar to D53597, but following up with 2 more enums.
After this, all flag enums should be strongly typed all the way
through to the symbol files plugins.
Differential Revision: https://reviews.llvm.org/D53616
Modified:
lldb/trunk/include/lldb/Breakpoint/BreakpointResolverName.h
lldb/trunk/include/lldb/Core/Module.h
lldb/trunk/include/lldb/Core/ModuleList.h
lldb/trunk/include/lldb/Symbol/SymbolFile.h
lldb/trunk/include/lldb/Symbol/SymbolVendor.h
lldb/trunk/include/lldb/Target/Target.h
lldb/trunk/include/lldb/lldb-enumerations.h
lldb/trunk/source/API/SBCompileUnit.cpp
lldb/trunk/source/API/SBModule.cpp
lldb/trunk/source/API/SBTarget.cpp
lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp
lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp
lldb/trunk/source/Core/Module.cpp
lldb/trunk/source/Core/ModuleList.cpp
lldb/trunk/source/Expression/IRExecutionUnit.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
lldb/trunk/source/Symbol/SymbolFile.cpp
lldb/trunk/source/Symbol/SymbolVendor.cpp
lldb/trunk/source/Target/Target.cpp
Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointResolverName.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointResolverName.h?rev=345314&r1=345313&r2=345314&view=diff
==
--- lldb/trunk/include/lldb/Breakpoint/BreakpointResolverName.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/BreakpointResolverName.h Thu Oct 25
13:45:40 2018
@@ -31,20 +31,23 @@ namespace lldb_private {
class BreakpointResolverName : public BreakpointResolver {
public:
BreakpointResolverName(Breakpoint *bkpt, const char *name,
- uint32_t name_type_mask, lldb::LanguageType language,
+ lldb::FunctionNameType name_type_mask,
+ lldb::LanguageType language,
Breakpoint::MatchType type, lldb::addr_t offset,
bool skip_prologue);
// This one takes an array of names. It is always MatchType = Exact.
BreakpointResolverName(Breakpoint *bkpt, const char *names[],
- size_t num_names, uint32_t name_type_mask,
+ size_t num_names,
+ lldb::FunctionNameType name_type_mask,
lldb::LanguageType language, lldb::addr_t offset,
bool skip_prologue);
// This one takes a C++ array of names. It is always MatchType = Exact.
BreakpointResolverName(Breakpoint *bkpt, std::vector names,
- uint32_t name_type_mask, lldb::LanguageType language,
- lldb::addr_t offset, bool skip_prologue);
+ lldb::FunctionNameType name_type_mask,
+ lldb::LanguageType language, lldb::addr_t offset,
+ bool skip_prologue);
// Creates a function breakpoint by regular expression. Takes over control
// of the lifespan of func_regex.
@@ -89,7 +92,8 @@ protected:
lldb::LanguageType m_language;
bool m_skip_prologue;
- void AddNameLookup(const ConstString &name, uint32_t name_type_mask);
+ void AddNameLookup(const ConstString &name,
+ lldb::FunctionNameType name_type_mask);
};
} // namespace lldb_private
Modified: lldb/trunk/include/lldb/Core/Module.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Module.h?rev=345314&r1=345313&r2=345314&view=diff
==
--- lldb/trunk/include/lldb/Core/Module.h (original)
+++ lldb/trunk/include/lldb/Core/Module.h Thu Oct 25 13:45:40 2018
@@ -380,7 +380,7 @@ public:
//--
size_t FindFunctions(const ConstString &name,
const CompilerDeclContext *parent_decl_ctx,
- uint32_t name_type_mask, bool symbols_ok,
+ lldb::FunctionNameType name_type_mask, bool symbols_ok,
[Lldb-commits] [PATCH] D53616: Don't type-erase the FunctionNameType or TypeClass enums
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL345314: Don't type-erase the FunctionNameType or
TypeClass enums. (authored by zturner, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D53616?vs=170762&id=171188#toc
Repository:
rL LLVM
https://reviews.llvm.org/D53616
Files:
lldb/trunk/include/lldb/Breakpoint/BreakpointResolverName.h
lldb/trunk/include/lldb/Core/Module.h
lldb/trunk/include/lldb/Core/ModuleList.h
lldb/trunk/include/lldb/Symbol/SymbolFile.h
lldb/trunk/include/lldb/Symbol/SymbolVendor.h
lldb/trunk/include/lldb/Target/Target.h
lldb/trunk/include/lldb/lldb-enumerations.h
lldb/trunk/source/API/SBCompileUnit.cpp
lldb/trunk/source/API/SBModule.cpp
lldb/trunk/source/API/SBTarget.cpp
lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp
lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp
lldb/trunk/source/Core/Module.cpp
lldb/trunk/source/Core/ModuleList.cpp
lldb/trunk/source/Expression/IRExecutionUnit.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
lldb/trunk/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
lldb/trunk/source/Symbol/SymbolFile.cpp
lldb/trunk/source/Symbol/SymbolVendor.cpp
lldb/trunk/source/Target/Target.cpp
Index: lldb/trunk/source/Expression/IRExecutionUnit.cpp
===
--- lldb/trunk/source/Expression/IRExecutionUnit.cpp
+++ lldb/trunk/source/Expression/IRExecutionUnit.cpp
@@ -709,9 +709,10 @@
struct IRExecutionUnit::SearchSpec {
ConstString name;
- uint32_t mask;
+ lldb::FunctionNameType mask;
- SearchSpec(ConstString n, uint32_t m = lldb::eFunctionNameTypeFull)
+ SearchSpec(ConstString n,
+ lldb::FunctionNameType m = lldb::eFunctionNameTypeFull)
: name(n), mask(m) {}
};
Index: lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp
===
--- lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp
+++ lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp
@@ -30,7 +30,7 @@
using namespace lldb_private;
BreakpointResolverName::BreakpointResolverName(
-Breakpoint *bkpt, const char *name_cstr, uint32_t name_type_mask,
+Breakpoint *bkpt, const char *name_cstr, FunctionNameType name_type_mask,
LanguageType language, Breakpoint::MatchType type, lldb::addr_t offset,
bool skip_prologue)
: BreakpointResolver(bkpt, BreakpointResolver::NameResolver, offset),
@@ -51,7 +51,7 @@
BreakpointResolverName::BreakpointResolverName(
Breakpoint *bkpt, const char *names[], size_t num_names,
-uint32_t name_type_mask, LanguageType language, lldb::addr_t offset,
+FunctionNameType name_type_mask, LanguageType language, lldb::addr_t offset,
bool skip_prologue)
: BreakpointResolver(bkpt, BreakpointResolver::NameResolver, offset),
m_match_type(Breakpoint::Exact), m_language(language),
@@ -61,9 +61,12 @@
}
}
-BreakpointResolverName::BreakpointResolverName(
-Breakpoint *bkpt, std::vector names, uint32_t name_type_mask,
-LanguageType language, lldb::addr_t offset, bool skip_prologue)
+BreakpointResolverName::BreakpointResolverName(Breakpoint *bkpt,
+ std::vector names,
+ FunctionNameType name_type_mask,
+ LanguageType language,
+ lldb::addr_t offset,
+ bool skip_prologue)
: BreakpointResolver(bkpt, BreakpointResolver::NameResolver, offset),
m_match_type(Breakpoint::Exact), m_language(language),
m_skip_prologue(skip_prologue) {
@@ -161,23 +164,23 @@
return nullptr;
}
std::vector names;
-std::vector name_masks;
+std::vector name_masks;
for (size_t i = 0; i < num_elem; i++) {
- uint32_t name_mask;
llvm::StringRef name;
success = names_array->GetItemAtIndexAsString(i, name);
if (!success) {
error.SetErrorString("BRN::CFSD: name entry is not a string.");
return nullptr;
}
- success = names_mask_array->GetItemAtIndexAsInteger(i, name_mask);
+ std::underlying_type::type fnt
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
zturner created this revision.
zturner added reviewers: lemo, aleksandr.urakov, stella.stamenova.
LLDB has the ability to display global variables, even without a running
process, via the `target variable` command. This is because global variables
are linker initialized, so their values are embedded directly into the
executables. This gives us great power for testing native PDB functionality in
a cross-platform manner, because we don't actually need a running process. We
can just create a target using an EXE file, and display global variables. And
global variables can have arbitrarily complex types, so in theory we can fully
exercise the type system, record layout, and data formatters for native PDB
files and PE/COFF executables on any host platform, as long as our type does
not require a dynamic initializer.
This patch adds basic support for finding variables by name, and adds an
exhaustive test for fundamental data types and pointers / references to
fundamental data types.
Subsequent patches will extend this to typedefs, classes, pointers to
functions, and other cases.
https://reviews.llvm.org/D53731
Files:
lldb/lit/SymbolFile/NativePDB/Inputs/globals-fundamental.lldbinit
lldb/lit/SymbolFile/NativePDB/globals-fundamental.cpp
lldb/source/Plugins/SymbolFile/NativePDB/PdbSymUid.h
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
===
--- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
+++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
@@ -103,6 +103,11 @@
size_t ParseFunctionBlocks(const SymbolContext &sc) override;
+ uint32_t FindGlobalVariables(const ConstString &name,
+ const CompilerDeclContext *parent_decl_ctx,
+ uint32_t max_matches,
+ VariableList &variables) override;
+
size_t ParseTypes(const SymbolContext &sc) override;
size_t ParseVariablesForContext(const SymbolContext &sc) override {
return 0;
@@ -175,11 +180,13 @@
lldb::CompUnitSP GetOrCreateCompileUnit(const CompilandIndexItem &cci);
lldb::TypeSP GetOrCreateType(PdbSymUid type_uid);
lldb::TypeSP GetOrCreateType(llvm::codeview::TypeIndex ti);
+ lldb::VariableSP GetOrCreateGlobalVariable(PdbSymUid var_uid);
lldb::FunctionSP CreateFunction(PdbSymUid func_uid, const SymbolContext &sc);
lldb::CompUnitSP CreateCompileUnit(const CompilandIndexItem &cci);
lldb::TypeSP CreateType(PdbSymUid type_uid);
lldb::TypeSP CreateAndCacheType(PdbSymUid type_uid);
+ lldb::VariableSP CreateGlobalVariable(PdbSymUid var_uid);
llvm::BumpPtrAllocator m_allocator;
@@ -193,6 +200,7 @@
llvm::DenseMap m_uid_to_decl;
+ llvm::DenseMap m_global_vars;
llvm::DenseMap m_functions;
llvm::DenseMap m_compilands;
llvm::DenseMap m_types;
Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -16,14 +16,17 @@
#include "lldb/Core/Module.h"
#include "lldb/Core/PluginManager.h"
+#include "lldb/Core/StreamBuffer.h"
#include "lldb/Symbol/ClangASTContext.h"
#include "lldb/Symbol/ClangASTImporter.h"
#include "lldb/Symbol/ClangExternalASTSourceCommon.h"
#include "lldb/Symbol/CompileUnit.h"
#include "lldb/Symbol/LineTable.h"
#include "lldb/Symbol/ObjectFile.h"
#include "lldb/Symbol/SymbolContext.h"
#include "lldb/Symbol/SymbolVendor.h"
+#include "lldb/Symbol/Variable.h"
+#include "lldb/Symbol/VariableList.h"
#include "llvm/DebugInfo/CodeView/CVRecord.h"
#include "llvm/DebugInfo/CodeView/CVTypeVisitor.h"
@@ -886,6 +889,128 @@
return CreateAndCacheType(type_uid);
}
+static DWARFExpression MakeGlobalLocationExpression(uint16_t section,
+uint32_t offset,
+ModuleSP module) {
+ if (!module)
+return DWARFExpression(nullptr);
+
+ const ArchSpec &architecture = module->GetArchitecture();
+ ByteOrder byte_order = architecture.GetByteOrder();
+ uint32_t address_size = architecture.GetAddressByteSize();
+ uint32_t byte_size = architecture.GetDataByteSize();
+ if (byte_order == eByteOrderInvalid || address_size == 0)
+return DWARFExpression(nullptr);
+
+ RegisterKind register_kind = eRegisterKindDWARF;
+ StreamBuffer<32> stream(Stream::eBinary, address_size, byte_order);
+ stream.PutHex8(DW_OP_addr);
+
+ SectionList *section_list = module->GetSectionList();
+ if (!section_list)
+return DWARFExpression(nullptr);
+
+ uint32_t section_idx = section - 1;
+ if (section_idx >= section_list->GetSize())
+return DWA
[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
zturner added a comment. In https://reviews.llvm.org/D53662#1276253, @jingham wrote: > So the current approach relies on the ability to sniff the name to determine > the context in which the user intends to find it. It does (and always did) > use the presence of an initial "::" to tell whether a symbol is exact. > That's obviously also inappropriate for a generic Type method. But OTOH, > there needs to be a funnel point where you take in a string you know nothing > about (from the user either in type lookup or in an expression) and find it > as best you can. I don't think we want to force command line users to say > "type lookup --exact " so we've got to figure it out internally. > > Internally, it might be helpful to do some initial analysis of the input type > string, and then dispatch it to an exact or inexact search. But given I > don't think you can get away without a FindTypes that takes a string that you > don't know whether it is exact or not, I don't feel strongly about that. > > We should abstract "is exact" and ask the various type systems to handle that > request, and we also need to abstract "remove type class" and again ask the > various type systems to handle that. That seems to me the main ugliness that > this patch reveals. Just to clarify, is the consensus then that I can submit this as is? Or that it needs some modification? Greg's suggestion of making a separate method called `FindExactType` could work, but it doesn't seem that much different than passing a boolean call `exact_match`, which is what I've done here. On the extreme end, we could just make `Module::FindTypes` do absolutely nothing except call the SymbolFile plugin.I don't feel too strongly either way. The current patch seems NFC for DWARF and strictly better for PDB, but I'm willing to make changes if anyone feels like there's an architecturally more sound approach. https://reviews.llvm.org/D53662 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
jingham added a comment. Is there anything PDB specific about the test you've added? If not, it might be good to use this as a generic SymbolFile test. https://reviews.llvm.org/D53731 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
zturner added a comment. which generic SymbolFile test do you mean? The lit ones are the only ones that are set up to run in this particular manner (run lines, check lines, etc), and currently we don't have a way to run different / multiple command line invocations. I came up with this test in order to test the new changes I introduced in `SymbolFileNativePdb.cpp` that are also in this patch, so I definitely want to make sure all of that code still gets exercised with whatever test strategy I end up using. I guess the way I see it, there's two interesting things this test could exercise. 1) the debug info, and 2) the formatters. If we want to test the formatters, we could probably brainstorm a way to do it generically with 1 test (or set of tests) that omits the SymbolFile from the picture entirely and is generic enough that it should be applicable to any platform/compiler/host. If we want to test the SymbolFile plugin though, then it may need to be specific to the debug info type. I think what you're getting at though is that we probably need some notion of "debug info variants" for these lit tests like we have in the dotest suite. I actually had an idea for something that might work like that a while back, which I described here: https://reviews.llvm.org/D52618#1252906 But the idea would basically be that instead of hardcoding a command line like I've done with `clang-cl /Z7 etc etc` we could write something more generic that would evaluate to different (or perhaps even multiple) things, so we could run it different ways. https://reviews.llvm.org/D53731 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
jingham added a comment. It seemed to me like Greg thought you were changing the behavior of lookups, which this patch doesn't do, it just makes it more efficient. I don't know if that alters his objections or not. The Module and higher layer of FindTypes calls are awkward. For instance Module::FindTypes takes an "exact_match" parameter, but it doesn't actually mean "exact match". If exact_match is passed in as true, then it does mean exactly match the name. But if it is false it means "check if this name is fully qualified and if so make it an exact match otherwise do a partial match". The header comment for the function is wrong, too. It says: /// @param[in] exact_match /// If \b true, \a type_name is fully qualified and must match /// exactly. If \b false, \a type_name is a partially qualified /// name where the leading namespaces or classes can be /// omitted to make finding types that a user may type /// easier. So we should change the variable name (or even just the docs) at this level to better reflect what the parameter actually does. You still need to turn off the guessing because we don't have a 100% certain way to tell whether a type name is fully qualified, and the alternative of prepending a "::" to force the guessing algorithm's hand is gross (and very C++ specific). But with that proviso, I don't think passing this as a parameter is any better/worse than having Module:FindTypesExact and Module::FindTypesAutoDetect or some better name that I can't think up right now. But I don't think you need the third "FindTypesPartial" that forces a partial match is useful. I don't see a use for "this name really looks fully qualified but I want you to treat it as a partial name". So expressing this as a bool parameter seems fine to me. OTOH, I don't think it is desirable to have different versions of the Symfile and lower calls for Exact and not Exact, passing the exact_match parameter through there is fine IMO. After all, in all these calls exact_match means exactly what it says, you are requiring an exact match. So you don't gain clarity by having different function names. BTW, you should remove the O(1) FIXME comment so you don't send some future eager contributor on a wild goose chase to figure out how to do this thing you can't do in DWARF. https://reviews.llvm.org/D53662 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
jingham added a comment. Well, what's really going on is that I'm not familiar enough with lit to know that it doesn't have the ability to run different commands to produce the input file... But as you guessed, my point is that you have written a bunch of tests that would be valuable to test against any symfile format, so it is a shame to only run them against one format. OTOH, if that's not possible right now, I'm content to wait for some enhancements to lit that make it possible. https://reviews.llvm.org/D53731 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
zturner added a comment. In https://reviews.llvm.org/D53731#1276633, @jingham wrote: > Well, what's really going on is that I'm not familiar enough with lit to know > that it doesn't have the ability to run different commands to produce the > input file... But as you guessed, my point is that you have written a bunch > of tests that would be valuable to test against any symfile format, so it is > a shame to only run them against one format. OTOH, if that's not possible > right now, I'm content to wait for some enhancements to lit that make it > possible. I can maybe hardcode multiple runlines, one for each possible target triple. But what's really going on is that I'm not familiar enough with other platforms to know how to generate their binaries :) But I'll actually give it a try. Just to explain what's going on here, I've got these two lines: // RUN: clang-cl /Z7 /GS- /GR- /c /Fo%t.obj -- %s // RUN: lld-link /DEBUG /nodefaultlib /entry:main /OUT:%t.exe /PDB:%t.pdb -- %t.obj Those two run the compiler and linker and write the output to a temporary file, but don't actually check anything yet. The next two lines (which are actually one command broken with a continuation character) // RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb -f %t.exe -s \ // RUN: %p/Inputs/globals-fundamental.lldbinit | FileCheck %s runs lldb and pipes the result to FileCheck, using this file as the check file. So what I could *try* doing is having multiple compiler / linker invocations. One that produces a Windows executable / PDB file. One that produces a Linux executable / DWARF. One that produces some kind of OSX binary (I think lld doesn't work with MachO yet, so I'm not sure about this one), writing the linker outputs to different files each time. Then we could maybe run lldb 3 times, once against each input, but using the same check file each time. I'll give it a try and see what happens. https://reviews.llvm.org/D53731 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
jingham added a comment. Could you also use Vedant's new FileCheck dotest test class? That should allow you to write the tests exactly as you are, but use the dotest mechanism to build and run the example. https://reviews.llvm.org/D53731 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
lemo accepted this revision.
lemo added a comment.
This revision is now accepted and ready to land.
looks good. a few comments inline.
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbSymUid.h:46
+ // due to the debug magic at the beginning of the
stream.
+ uint64_t global : 1; // True if this is from the globals stream.
+ uint64_t modi : 16; // For non-global, this is the 0-based index of module.
30 + 1 != 32 - what's going on?
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbSymUid.h:135
+ static PdbSymUid makeGlobalVariableUid(uint32_t offset) {
+PdbSymUid uid;
+uid.m_uid.cu_sym.modi = 0;
= {}; ? (to avoid uninitialized bits)
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:892
+static DWARFExpression MakeGlobalLocationExpression(uint16_t section,
+uint32_t offset,
assert that section > 0 ? (as precondition)
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:913
+
+ uint32_t section_idx = section - 1;
+ if (section_idx >= section_list->GetSize())
comment explaining the - 1 ?
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:936
+ lldb::ValueType scope = eValueTypeInvalid;
+ TypeIndex ti;
+ llvm::StringRef name;
pls explicitly initialize all the locals
Comment at:
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:1406
+ TypeSP type_sp = CreateAndCacheType(uid);
+ return &*type_sp;
}
type_sp->get() seems cleaner / more readable
https://reviews.llvm.org/D53731
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
zturner added reviewers: vsk, labath. zturner added a comment. In https://reviews.llvm.org/D53731#1276660, @jingham wrote: > Could you also use Vedant's new FileCheck dotest test class? That should > allow you to write the tests exactly as you are, but use the dotest mechanism > to build and run the example. Adding Vedant here. My understanding is that the work he did there is like the inverse of what I'm doing. It allows you to use FileCheck from inside of dotest tests, but I was trying to bypass dotest here. I don't necessarily think "dotest for all things" should be an explicit goal (i actually think long term we should move away from it, but that's for another day). Aside from that though, I don't think this particular test needs any of the functionality that dotest offers. It offers building in multiple configurations, but we can get that from this test by just specifying mulitple run lines (I'm testing this out locally and it seems like I can get it to work). Eventually, using some kind of configuration / builder type system like I brainstormed in the review thread I linked previously, I think we could have all the advantages of dotest's builder even with this style of test. FWIW, I was also under the impression that Vedant's solution with FileCheck in dotest was only intended as sort of an immediate solution to a problem, but not necessary the long term desired end-state. (I could be wrong about this though). https://reviews.llvm.org/D53731 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
vsk added a comment.
In https://reviews.llvm.org/D53731#1276732, @zturner wrote:
> In https://reviews.llvm.org/D53731#1276660, @jingham wrote:
>
> > Could you also use Vedant's new FileCheck dotest test class? That should
> > allow you to write the tests exactly as you are, but use the dotest
> > mechanism to build and run the example.
>
>
> Adding Vedant here. My understanding is that the work he did there is like
> the inverse of what I'm doing. It allows you to use FileCheck from inside of
> dotest tests, but I was trying to bypass dotest here. I don't necessarily
> think "dotest for all things" should be an explicit goal (i actually think
> long term we should move away from it, but that's for another day). Aside
> from that though, I don't think this particular test needs any of the
> functionality that dotest offers. It offers building in multiple
> configurations, but we can get that from this test by just specifying
> mulitple run lines (I'm testing this out locally and it seems like I can get
> it to work). Eventually, using some kind of configuration / builder type
> system like I brainstormed in the review thread I linked previously, I think
> we could have all the advantages of dotest's builder even with this style of
> test.
>
> FWIW, I was also under the impression that Vedant's solution with FileCheck
> in dotest was only intended as sort of an immediate solution to a problem,
> but not necessary the long term desired end-state. (I could be wrong about
> this though).
The tests as-written seem to exercise the new functionality. I think it'd be
fine to use the FileCheck-in-{dotest,lldbinline} support if you need to set a
breakpoint, run a command, and then validate its output (though it looks like
you don't)
https://reviews.llvm.org/D53731
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
zturner added a comment.
In https://reviews.llvm.org/D53731#1276743, @vsk wrote:
> In https://reviews.llvm.org/D53731#1276732, @zturner wrote:
>
> > In https://reviews.llvm.org/D53731#1276660, @jingham wrote:
> >
> > > Could you also use Vedant's new FileCheck dotest test class? That should
> > > allow you to write the tests exactly as you are, but use the dotest
> > > mechanism to build and run the example.
> >
> >
> > Adding Vedant here. My understanding is that the work he did there is like
> > the inverse of what I'm doing. It allows you to use FileCheck from inside
> > of dotest tests, but I was trying to bypass dotest here. I don't
> > necessarily think "dotest for all things" should be an explicit goal (i
> > actually think long term we should move away from it, but that's for
> > another day). Aside from that though, I don't think this particular test
> > needs any of the functionality that dotest offers. It offers building in
> > multiple configurations, but we can get that from this test by just
> > specifying mulitple run lines (I'm testing this out locally and it seems
> > like I can get it to work). Eventually, using some kind of configuration /
> > builder type system like I brainstormed in the review thread I linked
> > previously, I think we could have all the advantages of dotest's builder
> > even with this style of test.
> >
> > FWIW, I was also under the impression that Vedant's solution with FileCheck
> > in dotest was only intended as sort of an immediate solution to a problem,
> > but not necessary the long term desired end-state. (I could be wrong about
> > this though).
>
>
> The tests as-written seem to exercise the new functionality. I think it'd be
> fine to use the FileCheck-in-{dotest,lldbinline} support if you need to set a
> breakpoint, run a command, and then validate its output (though it looks like
> you don't)
Yea, once I need to actually run a process I expect I'll need to start using
it. For the time being, I'm trying see how much ground I can cover without a
process. The reasoning is two-fold.
First, it means the tests can run anywhere. If there's no process, my test
coverage isn't limited to a specific host OS.
Second, from a layering perspective, if I know that there's a ton of test
coverage for "everything that happens before a process is spawned", then once
you get to the point that you are spawning process, it limits the scope of
where you have to search when something does go wrong.
So that's my thinking.
https://reviews.llvm.org/D53731
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r345346 - Add functionality to export settings
Author: jdevlieghere Date: Thu Oct 25 17:00:17 2018 New Revision: 345346 URL: http://llvm.org/viewvc/llvm-project?rev=345346&view=rev Log: Add functionality to export settings For the reproducer feature I need to be able to export and import the current LLDB configuration. To realize this I've extended the existing functionality to print settings. With the help of a new formatting option, we can now write the settings and their values to a file structured as regular commands. Concretely the functionality works as follows: (lldb) settings export -f /path/to/file This file contains a bunch of settings set commands, followed by the setting's name and value. ... settings set use-external-editor false settings set use-color true settings set auto-one-line-summaries true settings set auto-indent true ... You can import the settings again by either sourcing the file or using the settings read command. (lldb) settings read -f /path/to/file Differential revision: https://reviews.llvm.org/D52651 Added: lldb/trunk/lit/Settings/TestExport.test Modified: lldb/trunk/include/lldb/Interpreter/OptionValue.h lldb/trunk/source/Commands/CommandObjectSettings.cpp lldb/trunk/source/Interpreter/OptionValueArray.cpp lldb/trunk/source/Interpreter/OptionValueDictionary.cpp lldb/trunk/source/Interpreter/OptionValueFileSpecLIst.cpp lldb/trunk/source/Interpreter/OptionValueFormatEntity.cpp lldb/trunk/source/Interpreter/OptionValueLanguage.cpp lldb/trunk/source/Interpreter/Property.cpp Modified: lldb/trunk/include/lldb/Interpreter/OptionValue.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/OptionValue.h?rev=345346&r1=345345&r2=345346&view=diff == --- lldb/trunk/include/lldb/Interpreter/OptionValue.h (original) +++ lldb/trunk/include/lldb/Interpreter/OptionValue.h Thu Oct 25 17:00:17 2018 @@ -58,9 +58,11 @@ public: eDumpOptionValue = (1u << 2), eDumpOptionDescription = (1u << 3), eDumpOptionRaw = (1u << 4), +eDumpOptionCommand = (1u << 5), eDumpGroupValue = (eDumpOptionName | eDumpOptionType | eDumpOptionValue), eDumpGroupHelp = -(eDumpOptionName | eDumpOptionType | eDumpOptionDescription) +(eDumpOptionName | eDumpOptionType | eDumpOptionDescription), +eDumpGroupExport = (eDumpOptionCommand | eDumpOptionName | eDumpOptionValue) }; OptionValue() Added: lldb/trunk/lit/Settings/TestExport.test URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Settings/TestExport.test?rev=345346&view=auto == --- lldb/trunk/lit/Settings/TestExport.test (added) +++ lldb/trunk/lit/Settings/TestExport.test Thu Oct 25 17:00:17 2018 @@ -0,0 +1,32 @@ +# This tests writing and reading settings from LLDB. + +# Check that the settings can be written to file and read again without +# altering the values. +# RUN: %lldb -b -o 'settings write -f %t.foo' -o 'settings read -f %t.foo' -o 'settings export %t.bar' -o 'settings read -f %t.bar' 2>&1 | FileCheck %s --check-prefix SUCCESS +# RUN: diff -w %t.foo %t.bar +# SUCCESS-NOT: error: + +# Check that exporting target settings only export target settings and nothing else. +# RUN: %lldb -b -o 'settings write -f %t.target target' 2>&1 | FileCheck %s --check-prefix SUCCESS +# RUN: cat %t.target | FileCheck %s --check-prefix TARGET +# TARGET: settings set target +# TARGET-NOT: settings set platform +# TARGET-NOT: settings set symbols +# TARGET-NOT: settings set interpreter +# TARGET-NOT: settings set plugin + +# Check that settings appear twice when appending. +# RUN: %lldb -b -o 'settings write -a -f %t.append target' -o 'settings write -a -f %t.append target' 2>&1 | FileCheck %s --check-prefix SUCCESS +# RUN: cat %t.append | FileCheck %s --check-prefix APPEND +# APPEND: settings set target.language +# APPEND: settings set target.language + +# Check that an error is printed for non-existing setting. +# RUN: echo "settings set bogus" > %t.bogus_setting +# RUN: %lldb -b -o 'settings read -f %t.bogus_setting' 2>&1 | FileCheck %s --check-prefix BOGUS-SETTING +# BOGUS-SETTING: error: invalid value path + +# Check that an error is printed for invalid value. +# RUN: echo "settings set target.language bogus" > %t.bogus_value +# RUN: %lldb -b -o 'settings read -f %t.bogus_value' 2>&1 | FileCheck %s --check-prefix BOGUS-VALUE +# BOGUS-VALUE: error: invalid language type Modified: lldb/trunk/source/Commands/CommandObjectSettings.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectSettings.cpp?rev=345346&r1=345345&r2=345346&view=diff == --- lldb/trunk/source/Commands/CommandObjectSettings.cpp (original) +++ lldb/trunk/source/Commands/CommandObjectSettings.cpp Thu Oct 25 17:00:17 2018 @@ -321,6 +321,207 @@ protected:
[Lldb-commits] [PATCH] D52651: Add functionality to export settings
This revision was automatically updated to reflect the committed changes.
Closed by commit rL345346: Add functionality to export settings (authored by
JDevlieghere, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D52651?vs=170258&id=171232#toc
Repository:
rL LLVM
https://reviews.llvm.org/D52651
Files:
lldb/trunk/include/lldb/Interpreter/OptionValue.h
lldb/trunk/lit/Settings/TestExport.test
lldb/trunk/source/Commands/CommandObjectSettings.cpp
lldb/trunk/source/Interpreter/OptionValueArray.cpp
lldb/trunk/source/Interpreter/OptionValueDictionary.cpp
lldb/trunk/source/Interpreter/OptionValueFileSpecLIst.cpp
lldb/trunk/source/Interpreter/OptionValueFormatEntity.cpp
lldb/trunk/source/Interpreter/OptionValueLanguage.cpp
lldb/trunk/source/Interpreter/Property.cpp
Index: lldb/trunk/source/Interpreter/OptionValueFileSpecLIst.cpp
===
--- lldb/trunk/source/Interpreter/OptionValueFileSpecLIst.cpp
+++ lldb/trunk/source/Interpreter/OptionValueFileSpecLIst.cpp
@@ -25,16 +25,24 @@
if (dump_mask & eDumpOptionType)
strm.Printf("(%s)", GetTypeAsCString());
if (dump_mask & eDumpOptionValue) {
-if (dump_mask & eDumpOptionType)
- strm.Printf(" =%s", m_current_value.GetSize() > 0 ? "\n" : "");
-strm.IndentMore();
+const bool one_line = dump_mask & eDumpOptionCommand;
const uint32_t size = m_current_value.GetSize();
+if (dump_mask & eDumpOptionType)
+ strm.Printf(" =%s",
+ (m_current_value.GetSize() > 0 && !one_line) ? "\n" : "");
+if (!one_line)
+ strm.IndentMore();
for (uint32_t i = 0; i < size; ++i) {
- strm.Indent();
- strm.Printf("[%u]: ", i);
+ if (!one_line) {
+strm.Indent();
+strm.Printf("[%u]: ", i);
+ }
m_current_value.GetFileSpecAtIndex(i).Dump(&strm);
+ if (one_line)
+strm << ' ';
}
-strm.IndentLess();
+if (!one_line)
+ strm.IndentLess();
}
}
Index: lldb/trunk/source/Interpreter/OptionValueFormatEntity.cpp
===
--- lldb/trunk/source/Interpreter/OptionValueFormatEntity.cpp
+++ lldb/trunk/source/Interpreter/OptionValueFormatEntity.cpp
@@ -61,10 +61,10 @@
strm.Printf("(%s)", GetTypeAsCString());
if (dump_mask & eDumpOptionValue) {
if (dump_mask & eDumpOptionType)
- strm.PutCString(" = \"");
+ strm.PutCString(" = ");
std::string escaped;
EscapeBackticks(m_current_format, escaped);
-strm << escaped << '"';
+strm << '"' << escaped << '"';
}
}
Index: lldb/trunk/source/Interpreter/OptionValueLanguage.cpp
===
--- lldb/trunk/source/Interpreter/OptionValueLanguage.cpp
+++ lldb/trunk/source/Interpreter/OptionValueLanguage.cpp
@@ -28,7 +28,8 @@
if (dump_mask & eDumpOptionValue) {
if (dump_mask & eDumpOptionType)
strm.PutCString(" = ");
-strm.PutCString(Language::GetNameForLanguageType(m_current_value));
+if (m_current_value != eLanguageTypeUnknown)
+ strm.PutCString(Language::GetNameForLanguageType(m_current_value));
}
}
Index: lldb/trunk/source/Interpreter/OptionValueArray.cpp
===
--- lldb/trunk/source/Interpreter/OptionValueArray.cpp
+++ lldb/trunk/source/Interpreter/OptionValueArray.cpp
@@ -31,13 +31,17 @@
strm.Printf("(%s)", GetTypeAsCString());
}
if (dump_mask & eDumpOptionValue) {
-if (dump_mask & eDumpOptionType)
- strm.Printf(" =%s", (m_values.size() > 0) ? "\n" : "");
-strm.IndentMore();
+const bool one_line = dump_mask & eDumpOptionCommand;
const uint32_t size = m_values.size();
+if (dump_mask & eDumpOptionType)
+ strm.Printf(" =%s", (m_values.size() > 0 && !one_line) ? "\n" : "");
+if (!one_line)
+ strm.IndentMore();
for (uint32_t i = 0; i < size; ++i) {
- strm.Indent();
- strm.Printf("[%u]: ", i);
+ if (!one_line) {
+strm.Indent();
+strm.Printf("[%u]: ", i);
+ }
const uint32_t extra_dump_options = m_raw_value_dump ? eDumpOptionRaw : 0;
switch (array_element_type) {
default:
@@ -63,10 +67,16 @@
extra_dump_options);
break;
}
- if (i < (size - 1))
-strm.EOL();
+
+ if (!one_line) {
+if (i < (size - 1))
+ strm.EOL();
+ } else {
+strm << ' ';
+ }
}
-strm.IndentLess();
+if (!one_line)
+ strm.IndentLess();
}
}
Index: lldb/trunk/source/Interpreter/OptionValueDictionary.cpp
===
--- lldb/trunk/source/Interpreter/OptionValueDictionary.cpp
+++ lldb/trunk/source/Interpreter/OptionValueDictionary.cpp
@@ -33,16 +33,23 @@
strm.Printf("(%s)", GetTyp
[Lldb-commits] [PATCH] D52651: Add functionality to export settings
This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB345346: Add functionality to export settings (authored by
JDevlieghere, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52651?vs=170258&id=171231#toc
Repository:
rLLDB LLDB
https://reviews.llvm.org/D52651
Files:
include/lldb/Interpreter/OptionValue.h
lit/Settings/TestExport.test
source/Commands/CommandObjectSettings.cpp
source/Interpreter/OptionValueArray.cpp
source/Interpreter/OptionValueDictionary.cpp
source/Interpreter/OptionValueFileSpecLIst.cpp
source/Interpreter/OptionValueFormatEntity.cpp
source/Interpreter/OptionValueLanguage.cpp
source/Interpreter/Property.cpp
Index: source/Commands/CommandObjectSettings.cpp
===
--- source/Commands/CommandObjectSettings.cpp
+++ source/Commands/CommandObjectSettings.cpp
@@ -321,6 +321,207 @@
};
//-
+// CommandObjectSettingsWrite -- Write settings to file
+//-
+
+static constexpr OptionDefinition g_settings_write_options[] = {
+// clang-format off
+ { LLDB_OPT_SET_ALL, true, "file", 'f', OptionParser::eRequiredArgument, nullptr, {}, CommandCompletions::eDiskFileCompletion, eArgTypeFilename,"The file into which to write the settings." },
+ { LLDB_OPT_SET_ALL, false, "append",'a', OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone,"Append to saved settings file if it exists."},
+// clang-format on
+};
+
+class CommandObjectSettingsWrite : public CommandObjectParsed {
+public:
+ CommandObjectSettingsWrite(CommandInterpreter &interpreter)
+ : CommandObjectParsed(
+interpreter, "settings export",
+"Write matching debugger settings and their "
+"current values to a file that can be read in with "
+"\"settings read\". Defaults to writing all settings.",
+nullptr),
+m_options() {
+CommandArgumentEntry arg1;
+CommandArgumentData var_name_arg;
+
+// Define the first (and only) variant of this arg.
+var_name_arg.arg_type = eArgTypeSettingVariableName;
+var_name_arg.arg_repetition = eArgRepeatOptional;
+
+// There is only one variant this argument could be; put it into the
+// argument entry.
+arg1.push_back(var_name_arg);
+
+// Push the data for the first argument into the m_arguments vector.
+m_arguments.push_back(arg1);
+ }
+
+ ~CommandObjectSettingsWrite() override = default;
+
+ Options *GetOptions() override { return &m_options; }
+
+ class CommandOptions : public Options {
+ public:
+CommandOptions() : Options() {}
+
+~CommandOptions() override = default;
+
+Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
+ ExecutionContext *execution_context) override {
+ Status error;
+ const int short_option = m_getopt_table[option_idx].val;
+
+ switch (short_option) {
+ case 'f':
+m_filename.assign(option_arg);
+break;
+ case 'a':
+m_append = true;
+break;
+ default:
+error.SetErrorStringWithFormat("unrecognized option '%c'",
+ short_option);
+break;
+ }
+
+ return error;
+}
+
+void OptionParsingStarting(ExecutionContext *execution_context) override {
+ m_filename.clear();
+ m_append = false;
+}
+
+llvm::ArrayRef GetDefinitions() override {
+ return llvm::makeArrayRef(g_settings_write_options);
+}
+
+// Instance variables to hold the values for command options.
+std::string m_filename;
+bool m_append = false;
+ };
+
+protected:
+ bool DoExecute(Args &args, CommandReturnObject &result) override {
+std::string path(FileSpec(m_options.m_filename, true).GetPath());
+uint32_t options = File::OpenOptions::eOpenOptionWrite |
+ File::OpenOptions::eOpenOptionCanCreate;
+if (m_options.m_append)
+ options |= File::OpenOptions::eOpenOptionAppend;
+else
+ options |= File::OpenOptions::eOpenOptionTruncate;
+
+StreamFile out_file(path.c_str(), options,
+lldb::eFilePermissionsFileDefault);
+
+if (!out_file.GetFile().IsValid()) {
+ result.AppendErrorWithFormat("%s: unable to write to file", path.c_str());
+ result.SetStatus(eReturnStatusFailed);
+ return false;
+}
+
+// Exporting should not be context sensitive.
+ExecutionContext clean_ctx;
+
+if (args.empty()) {
+ m_interpreter.GetDebugger().DumpAllPropertyValues(
+ &clean_ctx, out_file, OptionValue::eDumpGroupExport);
+ return result.Succeeded();
+}
+
+for (const auto &arg : args) {
+ Status error(m_interpreter.GetDebugger().DumpPropertyValu
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
jingham added a comment. dotest tests don't require a process. Presumably dotest knows how to build windows targeted PDB debug flavor files (to go along with dwarf/dsym/etc.). So it would be straightforward to make a test that had your input sources, built and made a target out of it and then poked at static variables and their types. That would straightaway run with all the different symbol file formats we support. That was why using Vedant's FileCheck thing made sense to me. You would use that to specify the test cases, since you like that way of writing tests and anyway already have them written out in that form, but use the dotest machinery to build it for whatever symfile format and target architecture/s the person who was running the test dialed up. But if you are interesting in also getting this to work with the straight FileCheck style test, your time is your own... BTW, I would use dotest tests specifically for the kind of thing you are doing here because then you can test against the SBType and SBTypeMembers from the debug info you've ingested, which would give you bit and byte offsets and sizes for free. But if your differing tastes end up getting you to add that info to "type lookup" - which we really should do - then I guess we win either way... https://reviews.llvm.org/D53731 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
zturner added a comment. In https://reviews.llvm.org/D53731#1276818, @jingham wrote: > dotest tests don't require a process. Presumably dotest knows how to build > windows targeted PDB debug flavor files (to go along with dwarf/dsym/etc.). > So it would be straightforward to make a test that had your input sources, > built and made a target out of it and then poked at static variables and > their types. That would straightaway run with all the different symbol file > formats we support. > > That was why using Vedant's FileCheck thing made sense to me. You would use > that to specify the test cases, since you like that way of writing tests and > anyway already have them written out in that form, but use the dotest > machinery to build it for whatever symfile format and target architecture/s > the person who was running the test dialed up. But if you are interesting in > also getting this to work with the straight FileCheck style test, your time > is your own... > > BTW, I would use dotest tests specifically for the kind of thing you are > doing here because then you can test against the SBType and SBTypeMembers > from the debug info you've ingested, which would give you bit and byte > offsets and sizes for free. But if your differing tastes end up getting you > to add that info to "type lookup" - which we really should do - then I guess > we win either way... Yea, so one of the commands I'm used to on the Windows command line debugger WinDbg is the `dt` (dump type) command. It's basically the equivalent of `type lookup` - more powerful in some ways, less in others . The output format looks like this: 0:000> dt _EXCEPTION_RECORD ntdll!_EXCEPTION_RECORD +0x000 ExceptionCode: Int4B +0x004 ExceptionFlags : Uint4B +0x008 ExceptionRecord : Ptr64 _EXCEPTION_RECORD +0x010 ExceptionAddress : Ptr64 Void +0x018 NumberParameters : Uint4B +0x020 ExceptionInformation : [15] Uint8B So you can see the field offsets and such. (Less powerful because that's about it, no methods, for example). But you can also pass it an address, which it then formats the block of memory as that type. e.g. 0:000> dt ntdll!_TEB32 007ed000 +0x000 NtTib: _NT_TIB32 +0x01c EnvironmentPointer : 0 +0x020 ClientId : _CLIENT_ID32 +0x028 ActiveRpcHandle : 0 +0x02c ThreadLocalStoragePointer : 0 +0x030 ProcessEnvironmentBlock : 0x7ed000 +0x034 LastErrorValue : 0 +0x038 CountOfOwnedCriticalSections : 0 +0x03c CsrClientThread : 0 +0x040 Win32ThreadInfo : 0x4d18 +0x044 User32Reserved : [26] 0 +0x0ac UserReserved : [5] 0 So up on my list of things to implement is these two features. https://reviews.llvm.org/D53731 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r345350 - Remove test that checks auto-completion for settings set.
Author: jdevlieghere
Date: Thu Oct 25 17:39:27 2018
New Revision: 345350
URL: http://llvm.org/viewvc/llvm-project?rev=345350&view=rev
Log:
Remove test that checks auto-completion for settings set.
With the new `-f` option for `settings set`, `-` (dash) no longer
auto-complete to `-g`.
Modified:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
Modified:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py?rev=345350&r1=345349&r2=345350&view=diff
==
---
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
(original)
+++
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
Thu Oct 25 17:39:27 2018
@@ -175,11 +175,6 @@ class CommandLineCompletionTestCase(Test
self.complete_from_to('settings set thread-f', 'settings set
thread-format')
@skipIfFreeBSD # timing out on the FreeBSD buildbot
-def test_settings_s_dash(self):
-"""Test that 'settings set -' completes to 'settings set -g'."""
-self.complete_from_to('settings set -', 'settings set -g')
-
-@skipIfFreeBSD # timing out on the FreeBSD buildbot
def test_settings_clear_th(self):
"""Test that 'settings clear thread-f' completes to 'settings clear
thread-format'."""
self.complete_from_to(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
jingham added a comment.
The first of the commands you showed presents info we definitely should add to
type lookup. I actually have a bug around to do that, but it hasn't risen to
the top of my queue because it's trivial to do with the SB API's so every time
I've needed that info I get it from script. Selfish me...
The second command is done in lldb with "memory read -t TYPE", and you can also
use the "-c COUNT" argument to treat the memory as an array of COUNT elements
of the TYPE:
(lldb) fr v myFoo
(foo *) myFoo = 0x00010040
(lldb) memory read -t foo 0x00010040
(foo) 0x10040 = {
First = 0
Second = 0
}
(lldb) memory read -t foo 0x00010040 -c 10
(foo) 0x10040 = {
First = 0
Second = 0
}
(foo) 0x10048 = {
First = 1
Second = 10
}
(foo) 0x100400010 = {
First = 2
Second = 20
}
if you want to also see the types of all the subelements add the -T flag, and
if you want to see all the memory locations of the subelements, add the -L flag:
(lldb) memory read -t foo 0x00010040 -c 10 -T -L
0x00010040: (foo) 0x10040 = {
0x00010040: (int) First = 0
0x00010044: (int) Second = 0
}
0x00010048: (foo) 0x10048 = {
0x00010048: (int) First = 1
0x0001004c: (int) Second = 10
}
BTW, the latter two flags have the same meaning pretty much wherever we print
values (expression, frame var...)
https://reviews.llvm.org/D53731
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r345350 - Remove test that checks auto-completion for settings set.
Thanks for fixing the bots!
On Thu, Oct 25, 2018 at 5:41 PM Jonas Devlieghere via lldb-commits
wrote:
>
> Author: jdevlieghere
> Date: Thu Oct 25 17:39:27 2018
> New Revision: 345350
>
> URL: http://llvm.org/viewvc/llvm-project?rev=345350&view=rev
> Log:
> Remove test that checks auto-completion for settings set.
>
> With the new `-f` option for `settings set`, `-` (dash) no longer
> auto-complete to `-g`.
>
> Modified:
>
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
>
> Modified:
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py?rev=345350&r1=345349&r2=345350&view=diff
> ==
> ---
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
> (original)
> +++
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
> Thu Oct 25 17:39:27 2018
> @@ -175,11 +175,6 @@ class CommandLineCompletionTestCase(Test
> self.complete_from_to('settings set thread-f', 'settings set
> thread-format')
>
> @skipIfFreeBSD # timing out on the FreeBSD buildbot
> -def test_settings_s_dash(self):
> -"""Test that 'settings set -' completes to 'settings set -g'."""
> -self.complete_from_to('settings set -', 'settings set -g')
> -
> -@skipIfFreeBSD # timing out on the FreeBSD buildbot
> def test_settings_clear_th(self):
> """Test that 'settings clear thread-f' completes to 'settings clear
> thread-format'."""
> self.complete_from_to(
>
>
> ___
> 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] r345351 - Update test that checks auto-completion for settings set.
Author: jdevlieghere
Date: Thu Oct 25 17:50:54 2018
New Revision: 345351
URL: http://llvm.org/viewvc/llvm-project?rev=345351&view=rev
Log:
Update test that checks auto-completion for settings set.
This reverts r345350 and updates the test rather than removing it. Now
we check that `--g` auto-completes to `--global`.
Modified:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
Modified:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py?rev=345351&r1=345350&r2=345351&view=diff
==
---
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
(original)
+++
lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
Thu Oct 25 17:50:54 2018
@@ -175,6 +175,11 @@ class CommandLineCompletionTestCase(Test
self.complete_from_to('settings set thread-f', 'settings set
thread-format')
@skipIfFreeBSD # timing out on the FreeBSD buildbot
+def test_settings_s_dash(self):
+"""Test that 'settings set --g' completes to 'settings set
--global'."""
+self.complete_from_to('settings set --g', 'settings set --global')
+
+@skipIfFreeBSD # timing out on the FreeBSD buildbot
def test_settings_clear_th(self):
"""Test that 'settings clear thread-f' completes to 'settings clear
thread-format'."""
self.complete_from_to(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
