[Lldb-commits] [PATCH] D124110: Refactor protected virtual functions from SymbolFile into new SymbolFileActual class.
yinghuitan updated this revision to Diff 424794.
yinghuitan added a comment.
Address review comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124110/new/
https://reviews.llvm.org/D124110
Files:
lldb/include/lldb/Symbol/SymbolFile.h
lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
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/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
lldb/source/Symbol/SymbolFile.cpp
Index: lldb/source/Symbol/SymbolFile.cpp
===
--- lldb/source/Symbol/SymbolFile.cpp
+++ lldb/source/Symbol/SymbolFile.cpp
@@ -25,6 +25,7 @@
using namespace lldb;
char SymbolFile::ID;
+char SymbolFileCommon::ID;
void SymbolFile::PreloadSymbols() {
// No-op for most implementations.
@@ -33,9 +34,6 @@
std::recursive_mutex &SymbolFile::GetModuleMutex() const {
return GetObjectFile()->GetModule()->GetMutex();
}
-ObjectFile *SymbolFile::GetMainObjectFile() {
- return m_objfile_sp->GetModule()->GetObjectFile();
-}
SymbolFile *SymbolFile::FindPlugin(ObjectFileSP objfile_sp) {
std::unique_ptr best_symfile_up;
@@ -87,16 +85,6 @@
return best_symfile_up.release();
}
-llvm::Expected
-SymbolFile::GetTypeSystemForLanguage(lldb::LanguageType language) {
- auto type_system_or_err =
- m_objfile_sp->GetModule()->GetTypeSystemForLanguage(language);
- if (type_system_or_err) {
-type_system_or_err->SetSymbolFile(this);
- }
- return type_system_or_err;
-}
-
uint32_t
SymbolFile::ResolveSymbolContext(const SourceLocationSpec &src_location_spec,
lldb::SymbolContextItem resolve_scope,
@@ -154,7 +142,37 @@
#endif
}
-uint32_t SymbolFile::GetNumCompileUnits() {
+SymbolFile::RegisterInfoResolver::~RegisterInfoResolver() = default;
+
+Symtab *SymbolFileCommon::GetSymtab() {
+ std::lock_guard guard(GetModuleMutex());
+ if (m_symtab)
+return m_symtab;
+
+ // Fetch the symtab from the main object file.
+ m_symtab = GetMainObjectFile()->GetSymtab();
+
+ // Then add our symbols to it.
+ if (m_symtab)
+AddSymbols(*m_symtab);
+
+ return m_symtab;
+}
+
+ObjectFile *SymbolFileCommon::GetMainObjectFile() {
+ return m_objfile_sp->GetModule()->GetObjectFile();
+}
+
+void SymbolFileCommon::SectionFileAddressesChanged() {
+ ObjectFile *module_objfile = GetMainObjectFile();
+ ObjectFile *symfile_objfile = GetObjectFile();
+ if (symfile_objfile != module_objfile)
+symfile_objfile->SectionFileAddressesChanged();
+ if (m_symtab)
+m_symtab->SectionFileAddressesChanged();
+}
+
+uint32_t SymbolFileCommon::GetNumCompileUnits() {
std::lock_guard guard(GetModuleMutex());
if (!m_compile_units) {
// Create an array of compile unit shared pointers -- which will each
@@ -164,7 +182,7 @@
return m_compile_units->size();
}
-CompUnitSP SymbolFile::GetCompileUnitAtIndex(uint32_t idx) {
+CompUnitSP SymbolFileCommon::GetCompileUnitAtIndex(uint32_t idx) {
std::lock_guard guard(GetModuleMutex());
uint32_t num = GetNumCompileUnits();
if (idx >= num)
@@ -175,7 +193,8 @@
return cu_sp;
}
-void SymbolFile::SetCompileUnitAtIndex(uint32_t idx, const CompUnitSP &cu_sp) {
+void SymbolFileCommon::SetCompileUnitAtIndex(uint32_t idx,
+ const CompUnitSP &cu_sp) {
std::lock_guard guard(GetModuleMutex());
const size_t num_compile_units = GetNumCompileUnits();
assert(idx < num_compile_units);
@@ -190,31 +209,29 @@
(*m_compile_units)[idx] = cu_sp;
}
-Symtab *SymbolFile::GetSymtab() {
- std::lock_guard guard(GetModuleMutex());
- if (m_symtab)
-return m_symtab;
-
- // Fetch the symtab from the main object file.
- m_symtab = GetMainObjectFile()->GetSymtab();
-
- // Then add our symbols to it.
- if (m_symtab)
-AddSymbols(*m_symtab);
-
- return m_symtab;
+llvm::Expected
+SymbolFileCommon::GetTypeSystemForLanguage(lldb::LanguageType language) {
+ auto type_system_or_err =
+ m_objfile_sp->GetModule()->GetTypeSystemForLanguage(language);
+ if (type_system_or_err) {
+type_system_or_err->SetSymbolFile(this);
+ }
+ return type_system_or_err;
}
-void SymbolFile::SectionFileAddressesChanged() {
- ObjectFile *module_objfile = GetMainObjectFile();
- ObjectFile *symfile_objfile = GetObjectFile();
- if (symfile_objfile != module_objfile)
-symfile_objfile->SectionFileAddressesChanged();
- if (m_symtab)
-m_symtab->SectionFileAddr
[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info
yinghuitan marked 8 inline comments as done.
yinghuitan added a comment.
Thanks for all the feedback. I will address the comments and add @clayborg's
follow-up suggestions into summary.
Comment at: lldb/source/Symbol/SymbolFileOnDemand.cpp:47
+lldb::LanguageType SymbolFileOnDemand::ParseLanguage(CompileUnit &comp_unit) {
+ if (!this->m_debug_info_enabled) {
+Log *log = GetLog();
DavidSpickett wrote:
> Is there a specific reason to use "this" explicitly instead of just
> `!m_debug_info_enabled`?
>
> Something to do with the wrapping of the underlying SymbolFile perhaps.
I will remove from this diff. I guess I just got this habit from working in
other languages which may explicitly require `this` otherwise can fail.
Comment at: lldb/source/Symbol/SymbolFileOnDemand.cpp:110
+ __FUNCTION__);
+// Not early exit.
+return false;
DavidSpickett wrote:
> What's the meaning of this comment?
Return false from `ForEachExternalModule` tells the caller to not exit early.
This is the default value used by `SymbolFile::ForEachExternalModule()`. Let me
make it more obvious.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121631/new/
https://reviews.llvm.org/D121631
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info
yinghuitan updated this revision to Diff 424808.
yinghuitan marked 3 inline comments as done.
yinghuitan added a comment.
Incorporate feedback and update summary with follow-up todos.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121631/new/
https://reviews.llvm.org/D121631
Files:
lldb/docs/use/ondemand.rst
lldb/include/lldb/Core/ModuleList.h
lldb/include/lldb/Symbol/SymbolFile.h
lldb/include/lldb/Symbol/SymbolFileOnDemand.h
lldb/include/lldb/Target/Statistics.h
lldb/include/lldb/Utility/LLDBLog.h
lldb/source/Core/CoreProperties.td
lldb/source/Core/Module.cpp
lldb/source/Core/ModuleList.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
lldb/source/Symbol/CMakeLists.txt
lldb/source/Symbol/CompileUnit.cpp
lldb/source/Symbol/SymbolFile.cpp
lldb/source/Symbol/SymbolFileOnDemand.cpp
lldb/source/Target/Statistics.cpp
lldb/source/Utility/LLDBLog.cpp
lldb/test/API/symbol_ondemand/breakpoint_language/Makefile
lldb/test/API/symbol_ondemand/breakpoint_language/TestBreakpointLanguageOnDemand.py
lldb/test/API/symbol_ondemand/breakpoint_language/c_lang.c
lldb/test/API/symbol_ondemand/breakpoint_language/cpp_lang.cpp
lldb/test/API/symbol_ondemand/breakpoint_language/main.cpp
lldb/test/API/symbol_ondemand/breakpoint_source_regex/Makefile
lldb/test/API/symbol_ondemand/breakpoint_source_regex/TestSourceTextRegexBreakpoint.py
lldb/test/API/symbol_ondemand/breakpoint_source_regex/main.cpp
lldb/test/API/symbol_ondemand/shared_library/Makefile
lldb/test/API/symbol_ondemand/shared_library/TestSharedLib.py
lldb/test/API/symbol_ondemand/shared_library/foo.c
lldb/test/API/symbol_ondemand/shared_library/foo.h
lldb/test/API/symbol_ondemand/shared_library/shared.c
lldb/test/Shell/SymbolFile/OnDemand/Inputs/basic.cpp
lldb/test/Shell/SymbolFile/OnDemand/source-breakpoint.test
lldb/test/Shell/SymbolFile/OnDemand/symbolic-breakpoint.test
Index: lldb/test/Shell/SymbolFile/OnDemand/symbolic-breakpoint.test
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/OnDemand/symbolic-breakpoint.test
@@ -0,0 +1,23 @@
+# Test shows that symbolic function breakpoint works with LLDB on demand symbol loading.
+
+# RUN: mkdir -p %t
+# RUN: cd %t
+# RUN: %build %p/Inputs/basic.cpp -o basic.out
+# RUN: %lldb -b -O "settings set symbols.load-on-demand true" -s %s basic.out | FileCheck %s
+
+breakpoint list
+# CHECK: No breakpoints currently set
+
+b bar
+# CHECK: where = {{.*}}`bar(int, int) + {{.*}} at basic.cpp:1
+
+breakpoint list
+# CHECK: where = {{.*}}`bar(int, int) + {{.*}} at basic.cpp:1
+
+run
+# CHECK: stop reason = breakpoint
+
+bt
+# CHECK: {{.*}}`bar(x=33, y=78) at basic.cpp:1
+# CHECK: {{.*}}`foo(x=33, y=78) at basic.cpp:3
+# CHECK: {{.*}}`main(argc=1, argv={{.*}}) at basic.cpp:5
Index: lldb/test/Shell/SymbolFile/OnDemand/source-breakpoint.test
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/OnDemand/source-breakpoint.test
@@ -0,0 +1,23 @@
+# Test shows that source line breakpoint works with LLDB on demand symbol loading.
+
+# RUN: mkdir -p %t
+# RUN: cd %t
+# RUN: %build %p/Inputs/basic.cpp -o basic.out
+# RUN: %lldb -b -O "settings set symbols.load-on-demand true" -s %s basic.out | FileCheck %s
+
+breakpoint list
+# CHECK: No breakpoints currently set
+
+breakpoint set -f basic.cpp -l 1
+# CHECK: where = {{.*}}`bar(int, int) + {{.*}} at basic.cpp:1
+
+breakpoint list
+# CHECK: file = 'basic.cpp'
+
+run
+# CHECK: stop reason = breakpoint
+
+bt
+# CHECK: {{.*}}`bar(x=33, y=78) at basic.cpp:1
+# CHECK: {{.*}}`foo(x=33, y=78) at basic.cpp:3
+# CHECK: {{.*}}`main(argc=1, argv={{.*}}) at basic.cpp:5
Index: lldb/test/Shell/SymbolFile/OnDemand/Inputs/basic.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/OnDemand/Inputs/basic.cpp
@@ -0,0 +1,5 @@
+int bar(int x, int y) { return x + y + 5; }
+
+int foo(int x, int y) { return bar(x, y) + 12; }
+
+int main(int argc, char **argv) { return foo(33, 78); }
Index: lldb/test/API/symbol_ondemand/shared_library/shared.c
===
--- /dev/null
+++ lldb/test/API/symbol_ondemand/shared_library/shared.c
@@ -0,0 +1,9 @@
+#include "foo.h"
+#include
+
+int global_shared = 897;
+int main(void) {
+ puts("This is a shared library test...");
+ foo(); // Set breakpoint 0 here.
+ return 0;
+}
Index: lldb/test/API/symbol_ondemand/shared_library/foo.h
===
--- /dev/null
+++ lldb/test/API/symbol_ondemand/shared_library/foo.h
@@ -0,0 +1,6 @@
+#ifndef foo_h__
+#define foo_h__
+
+extern void foo(void);
+
+#endif // foo_h
