[Lldb-commits] [PATCH] D39733: Simplify NativeProcessProtocol::GetArchitecture/GetByteOrder
eugene added inline comments. Comment at: include/lldb/Host/common/NativeProcessProtocol.h:104 - virtual bool GetArchitecture(ArchSpec &arch) const = 0; + virtual const ArchSpec &GetArchitecture() const = 0; Why return reference instead of a value? https://reviews.llvm.org/D39733 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39733: Simplify NativeProcessProtocol::GetArchitecture/GetByteOrder
eugene added inline comments. Comment at: include/lldb/Host/common/NativeProcessProtocol.h:104 - virtual bool GetArchitecture(ArchSpec &arch) const = 0; + virtual const ArchSpec &GetArchitecture() const = 0; labath wrote: > eugene wrote: > > Why return reference instead of a value? > I'd actually reverse that: Why value instead of a reference ? :D > > Returning by reference allows us to potentially avoid a copy. It does mean > that the process object has to hold the archspec as a member, but that is > currently not an issue, and we can always change that. > Why value instead of a reference ? :D IMO returning by value is a default option, for 2 reasons: 1. References and pointers introduce implicit lifetime dependencies (prone to use-after-free) 2. Value can change over time, and client might not expect it to happen. Potentially avoid a copy in place where a copy was made previously and didn't seem to hurt anybody seems like a bad trade-off for one more place where we can get a dangling pointer. https://reviews.llvm.org/D39733 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41533: Advanced guessing of rendezvous breakpoint
eugene updated this revision to Diff 129002.
eugene edited the summary of this revision.
eugene added a reviewer: tberghammer.
eugene added a project: LLDB.
eugene added a subscriber: lldb-commits.
eugene added a comment.
Fix tests.
https://reviews.llvm.org/D41533
Files:
packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py
source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
Index: source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
===
--- source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
+++ source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
@@ -85,13 +85,17 @@
/// mapped to the address space
lldb::addr_t m_vdso_base;
+ /// Contains AT_BASE, which means a dynamic loader has been
+ /// mapped to the address space
+ lldb::addr_t m_interpreter_base;
+
/// Loaded module list. (link map for each module)
std::map>
m_loaded_modules;
- /// Enables a breakpoint on a function called by the runtime
+ /// If possible sets a breakpoint on a function called by the runtime
/// linker each time a module is loaded or unloaded.
- virtual void SetRendezvousBreakpoint();
+ bool SetRendezvousBreakpoint();
/// Callback routine which updates the current list of loaded modules based
/// on the information supplied by the runtime linker.
@@ -138,7 +142,11 @@
/// of all dependent modules.
virtual void LoadAllCurrentModules();
- void LoadVDSO(lldb_private::ModuleList &modules);
+ void LoadVDSO();
+
+ // Loading an interpreter module (if present) assumming m_interpreter_base
+ // already points to its base address.
+ lldb::ModuleSP LoadInterpreterModule();
/// Computes a value for m_load_offset returning the computed address on
/// success and LLDB_INVALID_ADDRESS on failure.
@@ -148,9 +156,10 @@
/// success and LLDB_INVALID_ADDRESS on failure.
lldb::addr_t GetEntryPoint();
- /// Evaluate if Aux vectors contain vDSO information
+ /// Evaluate if Aux vectors contain vDSO and LD information
/// in case they do, read and assign the address to m_vdso_base
- void EvalVdsoStatus();
+ /// and m_interpreter_base.
+ void EvalSpecialModulesStatus();
/// Loads Module from inferior process.
void ResolveExecutableModule(lldb::ModuleSP &module_sp);
Index: source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -79,7 +79,8 @@
: DynamicLoader(process), m_rendezvous(process),
m_load_offset(LLDB_INVALID_ADDRESS), m_entry_point(LLDB_INVALID_ADDRESS),
m_auxv(), m_dyld_bid(LLDB_INVALID_BREAK_ID),
- m_vdso_base(LLDB_INVALID_ADDRESS) {}
+ m_vdso_base(LLDB_INVALID_ADDRESS),
+ m_interpreter_base(LLDB_INVALID_ADDRESS) {}
DynamicLoaderPOSIXDYLD::~DynamicLoaderPOSIXDYLD() {
if (m_dyld_bid != LLDB_INVALID_BREAK_ID) {
@@ -117,7 +118,7 @@
: "",
load_offset);
- EvalVdsoStatus();
+ EvalSpecialModulesStatus();
// if we dont have a load address we cant re-base
bool rebase_exec = (load_offset == LLDB_INVALID_ADDRESS) ? false : true;
@@ -207,7 +208,7 @@
executable = GetTargetExecutable();
load_offset = ComputeLoadOffset();
- EvalVdsoStatus();
+ EvalSpecialModulesStatus();
if (executable.get() && load_offset != LLDB_INVALID_ADDRESS) {
ModuleList module_list;
@@ -217,7 +218,12 @@
if (log)
log->Printf("DynamicLoaderPOSIXDYLD::%s about to call ProbeEntry()",
__FUNCTION__);
-ProbeEntry();
+
+if (!SetRendezvousBreakpoint()) {
+ // If we cannot establish rendezvous breakpoint right now
+ // we'll try again at enty point.
+ ProbeEntry();
+}
m_process->GetTarget().ModulesDidLoad(module_list);
}
@@ -329,38 +335,72 @@
return false; // Continue running.
}
-void DynamicLoaderPOSIXDYLD::SetRendezvousBreakpoint() {
+bool DynamicLoaderPOSIXDYLD::SetRendezvousBreakpoint() {
Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
+ if (m_dyld_bid != LLDB_INVALID_BREAK_ID) {
+LLDB_LOG(log,
+ "Rendezvous breakpoint breakpoint id {0} for pid {1}"
+ "is already set.",
+ m_dyld_bid,
+ m_process ? m_process->GetID() : LLDB_INVALID_PROCESS_ID);
+return true;
+ }
- addr_t break_addr = m_rendezvous.GetBreakAddress();
+ addr_t break_addr;
Target &target = m_process->GetTarget();
- if (m_dyld_bid == LLDB_INVALID_BREAK_ID) {
-if (log)
- log->Printf("DynamicLoaderPOSIXDYLD::%s pid %" PRI
[Lldb-commits] [PATCH] D41533: Advanced guessing of rendezvous breakpoint
eugene marked 5 inline comments as done.
eugene added inline comments.
Comment at:
source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:365-368
+static const char *DebugStateCandidates[] = {
+"_dl_debug_state", "rtld_db_dlactivity", "__dl_rtld_db_dlactivity",
+"r_debug_state", "_r_debug_state", "_rtld_debug_state",
+};
clayborg wrote:
> Will only one of these ever be present?
Yes. These are the names of rendezvous functions in different known linkers
(Android, Linux, BSD) None of them should have more than one function from this
list.
Comment at:
source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:379
+}
+break_addr = symbol->GetLoadAddress(&m_process->GetTarget());
+if (break_addr == LLDB_INVALID_ADDRESS) {
clayborg wrote:
> labath wrote:
> > It should be possible to create the breakpoint via
> > `Target::CreateBreakpoint` (the overload that lets you specify a list of
> > function names) instead of manually resolving the symbols... Have you tried
> > doing that?
> Agreed, I would use the breakpoint setting function in target that allows you
> to specify one or more names as labath suggested. One question about this
> code though: how would we ever get a load address from a module if the
> dynamic loader doesn't load it itself? Are these symbols in the "[vdso]"
> module and is the [vdso]" module loaded differently?
OS loads the dynamic loader, if it is requested by the executable, and passes
its base address via AT_BASE value in the auxiliary vector
(https://refspecs.linuxfoundation.org/LSB_1.3.0/IA64/spec/auxiliaryvector.html).
That's how we know where to look for it. (see EvalSpecialModulesStatus)
vdso is yet another special module, but it is not related to the dynamic loader.
https://reviews.llvm.org/D41533
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41533: Advanced guessing of rendezvous breakpoint
eugene updated this revision to Diff 129216.
eugene marked an inline comment as done.
eugene added a comment.
Addressing code review comments. Switching from manual symbol resolution to
the appropriate overload of CreateBreakpoint.
https://reviews.llvm.org/D41533
Files:
packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py
source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
Index: source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
===
--- source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
+++ source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
@@ -85,13 +85,17 @@
/// mapped to the address space
lldb::addr_t m_vdso_base;
+ /// Contains AT_BASE, which means a dynamic loader has been
+ /// mapped to the address space
+ lldb::addr_t m_interpreter_base;
+
/// Loaded module list. (link map for each module)
std::map>
m_loaded_modules;
- /// Enables a breakpoint on a function called by the runtime
+ /// If possible sets a breakpoint on a function called by the runtime
/// linker each time a module is loaded or unloaded.
- virtual void SetRendezvousBreakpoint();
+ bool SetRendezvousBreakpoint();
/// Callback routine which updates the current list of loaded modules based
/// on the information supplied by the runtime linker.
@@ -138,7 +142,11 @@
/// of all dependent modules.
virtual void LoadAllCurrentModules();
- void LoadVDSO(lldb_private::ModuleList &modules);
+ void LoadVDSO();
+
+ // Loading an interpreter module (if present) assumming m_interpreter_base
+ // already points to its base address.
+ lldb::ModuleSP LoadInterpreterModule();
/// Computes a value for m_load_offset returning the computed address on
/// success and LLDB_INVALID_ADDRESS on failure.
@@ -148,9 +156,10 @@
/// success and LLDB_INVALID_ADDRESS on failure.
lldb::addr_t GetEntryPoint();
- /// Evaluate if Aux vectors contain vDSO information
+ /// Evaluate if Aux vectors contain vDSO and LD information
/// in case they do, read and assign the address to m_vdso_base
- void EvalVdsoStatus();
+ /// and m_interpreter_base.
+ void EvalSpecialModulesStatus();
/// Loads Module from inferior process.
void ResolveExecutableModule(lldb::ModuleSP &module_sp);
Index: source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -79,7 +79,8 @@
: DynamicLoader(process), m_rendezvous(process),
m_load_offset(LLDB_INVALID_ADDRESS), m_entry_point(LLDB_INVALID_ADDRESS),
m_auxv(), m_dyld_bid(LLDB_INVALID_BREAK_ID),
- m_vdso_base(LLDB_INVALID_ADDRESS) {}
+ m_vdso_base(LLDB_INVALID_ADDRESS),
+ m_interpreter_base(LLDB_INVALID_ADDRESS) {}
DynamicLoaderPOSIXDYLD::~DynamicLoaderPOSIXDYLD() {
if (m_dyld_bid != LLDB_INVALID_BREAK_ID) {
@@ -117,7 +118,7 @@
: "",
load_offset);
- EvalVdsoStatus();
+ EvalSpecialModulesStatus();
// if we dont have a load address we cant re-base
bool rebase_exec = (load_offset == LLDB_INVALID_ADDRESS) ? false : true;
@@ -207,7 +208,7 @@
executable = GetTargetExecutable();
load_offset = ComputeLoadOffset();
- EvalVdsoStatus();
+ EvalSpecialModulesStatus();
if (executable.get() && load_offset != LLDB_INVALID_ADDRESS) {
ModuleList module_list;
@@ -217,7 +218,12 @@
if (log)
log->Printf("DynamicLoaderPOSIXDYLD::%s about to call ProbeEntry()",
__FUNCTION__);
-ProbeEntry();
+
+if (!SetRendezvousBreakpoint()) {
+ // If we cannot establish rendezvous breakpoint right now
+ // we'll try again at entry point.
+ ProbeEntry();
+}
m_process->GetTarget().ModulesDidLoad(module_list);
}
@@ -329,38 +335,75 @@
return false; // Continue running.
}
-void DynamicLoaderPOSIXDYLD::SetRendezvousBreakpoint() {
+bool DynamicLoaderPOSIXDYLD::SetRendezvousBreakpoint() {
Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
+ if (m_dyld_bid != LLDB_INVALID_BREAK_ID) {
+LLDB_LOG(log,
+ "Rendezvous breakpoint breakpoint id {0} for pid {1}"
+ "is already set.",
+ m_dyld_bid,
+ m_process ? m_process->GetID() : LLDB_INVALID_PROCESS_ID);
+return true;
+ }
- addr_t break_addr = m_rendezvous.GetBreakAddress();
+ addr_t break_addr;
Target &target = m_process->GetTarget();
-
- if (m_dyld_bid == LLDB_INVALID_BREAK_ID) {
-if (log)
- log->Printf("DynamicLoaderPOSIXDYLD::%s pid %"
[Lldb-commits] [PATCH] D41533: Advanced guessing of rendezvous breakpoint
This revision was automatically updated to reflect the committed changes.
Closed by commit rL322209: Advanced guessing of rendezvous breakpoint (authored
by eugene, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D41533?vs=129216&id=129309#toc
Repository:
rL LLVM
https://reviews.llvm.org/D41533
Files:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py
lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
Index: lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/trunk/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -79,7 +79,8 @@
: DynamicLoader(process), m_rendezvous(process),
m_load_offset(LLDB_INVALID_ADDRESS), m_entry_point(LLDB_INVALID_ADDRESS),
m_auxv(), m_dyld_bid(LLDB_INVALID_BREAK_ID),
- m_vdso_base(LLDB_INVALID_ADDRESS) {}
+ m_vdso_base(LLDB_INVALID_ADDRESS),
+ m_interpreter_base(LLDB_INVALID_ADDRESS) {}
DynamicLoaderPOSIXDYLD::~DynamicLoaderPOSIXDYLD() {
if (m_dyld_bid != LLDB_INVALID_BREAK_ID) {
@@ -117,7 +118,7 @@
: "",
load_offset);
- EvalVdsoStatus();
+ EvalSpecialModulesStatus();
// if we dont have a load address we cant re-base
bool rebase_exec = (load_offset == LLDB_INVALID_ADDRESS) ? false : true;
@@ -207,7 +208,7 @@
executable = GetTargetExecutable();
load_offset = ComputeLoadOffset();
- EvalVdsoStatus();
+ EvalSpecialModulesStatus();
if (executable.get() && load_offset != LLDB_INVALID_ADDRESS) {
ModuleList module_list;
@@ -217,7 +218,12 @@
if (log)
log->Printf("DynamicLoaderPOSIXDYLD::%s about to call ProbeEntry()",
__FUNCTION__);
-ProbeEntry();
+
+if (!SetRendezvousBreakpoint()) {
+ // If we cannot establish rendezvous breakpoint right now
+ // we'll try again at entry point.
+ ProbeEntry();
+}
m_process->GetTarget().ModulesDidLoad(module_list);
}
@@ -329,38 +335,77 @@
return false; // Continue running.
}
-void DynamicLoaderPOSIXDYLD::SetRendezvousBreakpoint() {
+bool DynamicLoaderPOSIXDYLD::SetRendezvousBreakpoint() {
Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
+ if (m_dyld_bid != LLDB_INVALID_BREAK_ID) {
+LLDB_LOG(log,
+ "Rendezvous breakpoint breakpoint id {0} for pid {1}"
+ "is already set.",
+ m_dyld_bid,
+ m_process ? m_process->GetID() : LLDB_INVALID_PROCESS_ID);
+return true;
+ }
- addr_t break_addr = m_rendezvous.GetBreakAddress();
+ addr_t break_addr;
Target &target = m_process->GetTarget();
-
- if (m_dyld_bid == LLDB_INVALID_BREAK_ID) {
-if (log)
- log->Printf("DynamicLoaderPOSIXDYLD::%s pid %" PRIu64
- " setting rendezvous break address at 0x%" PRIx64,
- __FUNCTION__,
- m_process ? m_process->GetID() : LLDB_INVALID_PROCESS_ID,
- break_addr);
-Breakpoint *dyld_break =
-target.CreateBreakpoint(break_addr, true, false).get();
-dyld_break->SetCallback(RendezvousBreakpointHit, this, true);
-dyld_break->SetBreakpointKind("shared-library-event");
-m_dyld_bid = dyld_break->GetID();
+ BreakpointSP dyld_break;
+ if (m_rendezvous.IsValid()) {
+break_addr = m_rendezvous.GetBreakAddress();
+LLDB_LOG(log, "Setting rendezvous break address for pid {0} at {1:x}",
+ m_process ? m_process->GetID() : LLDB_INVALID_PROCESS_ID,
+ break_addr);
+dyld_break = target.CreateBreakpoint(break_addr, true, false);
} else {
-if (log)
- log->Printf("DynamicLoaderPOSIXDYLD::%s pid %" PRIu64
- " reusing break id %" PRIu32 ", address at 0x%" PRIx64,
- __FUNCTION__,
- m_process ? m_process->GetID() : LLDB_INVALID_PROCESS_ID,
- m_dyld_bid, break_addr);
+LLDB_LOG(log, "Rendezvous structure is not set up yet. "
+ "Trying to locate rendezvous breakpoint in the interpreter "
+ "by symbol name.");
+ModuleSP interpreter = LoadInterpreterModule();
+if (!interpreter) {
+ LLDB_LOG(log, "Can't find interpreter, rendezvous breakpoint isn't set.");
+ return false;
+}
+
+// Function names from different dynamic loaders that are known
+// to be used as rendezvous between the loader and debuggers.
+static std::vector DebugStateCandidates{
+"_dl_debug_state", "rtld_db_dlactivity", "__dl_rtld_db_dlactivity",
+"
[Lldb-commits] [PATCH] D42488: Remove ObjectFile usage from HostLinux::GetProcessInfo
eugene added inline comments. Comment at: source/Host/linux/Host.cpp:129 + auto buffer_sp = DataBufferLLVM::CreateSliceFromPath(exe_path, 0x20, 0); + if (buffer_sp) +return ArchSpec(); Shouldn't it be ``` if (!buffer_sp) ``` ? https://reviews.llvm.org/D42488 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31451: New C++ function name parsing logic
eugene added a comment. Greg, this name is amazing. My c++filt refuses to demangle it. We can probably give up on parsing C++ names if they're longer than 1000 characters or something. Repository: rL LLVM https://reviews.llvm.org/D31451 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42939: More correct handling of error cases C++ name parser
eugene created this revision.
eugene added a reviewer: labath.
Now incorrect type argument that looks like T doesn't cause an assert,
but just a parsing error.
Bug: 36224
https://reviews.llvm.org/D42939
Files:
source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
Index: unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -160,4 +160,6 @@
"selector:otherField:", context, basename));
EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
"abc::", context, basename));
+EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
+ "f>", context, basename));
}
Index: source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
===
--- source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
+++ source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
@@ -242,8 +242,7 @@
}
}
- assert(template_counter >= 0);
- if (template_counter > 0) {
+ if (template_counter != 0) {
return false;
}
start_position.Remove();
Index: unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -160,4 +160,6 @@
"selector:otherField:", context, basename));
EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
"abc::", context, basename));
+EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
+ "f>", context, basename));
}
Index: source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
===
--- source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
+++ source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
@@ -242,8 +242,7 @@
}
}
- assert(template_counter >= 0);
- if (template_counter > 0) {
+ if (template_counter != 0) {
return false;
}
start_position.Remove();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42939: More correct handling of error cases C++ name parser
eugene updated this revision to Diff 132916.
eugene added a comment.
fix formating
https://reviews.llvm.org/D42939
Files:
source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
Index: unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -160,4 +160,6 @@
"selector:otherField:", context, basename));
EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
"abc::", context, basename));
+ EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
+ "f>", context, basename));
}
Index: source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
===
--- source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
+++ source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
@@ -242,8 +242,7 @@
}
}
- assert(template_counter >= 0);
- if (template_counter > 0) {
+ if (template_counter != 0) {
return false;
}
start_position.Remove();
Index: unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -160,4 +160,6 @@
"selector:otherField:", context, basename));
EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
"abc::", context, basename));
+ EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
+ "f>", context, basename));
}
Index: source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
===
--- source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
+++ source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
@@ -242,8 +242,7 @@
}
}
- assert(template_counter >= 0);
- if (template_counter > 0) {
+ if (template_counter != 0) {
return false;
}
start_position.Remove();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42939: More correct handling of error cases C++ name parser
eugene added inline comments. Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp:199 case tok::greatergreater: template_counter -= 2; can_open_template = false; labath wrote: > While looking at the bug, this part here struck me as dubious. > > Can you check that this properly handles a name like `F<(3)>>(1)> f<3, 1>()` > (which is the demangled form of _Z1fILi3ELi1EE1FIXrsT_T0_EEv). oh my, you're right. this code doesn't account for shifts in the name. I'm going to submit this code anyway and then think about it. https://reviews.llvm.org/D42939 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42939: More correct handling of error cases C++ name parser
This revision was automatically updated to reflect the committed changes.
Closed by commit rL324380: More correct handling of error cases C++ name parser
(authored by eugene, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D42939?vs=132916&id=133052#toc
Repository:
rL LLVM
https://reviews.llvm.org/D42939
Files:
lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
lldb/trunk/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
Index: lldb/trunk/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- lldb/trunk/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/trunk/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -160,4 +160,6 @@
"selector:otherField:", context, basename));
EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
"abc::", context, basename));
+ EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
+ "f>", context, basename));
}
Index: lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
===
--- lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
@@ -242,8 +242,7 @@
}
}
- assert(template_counter >= 0);
- if (template_counter > 0) {
+ if (template_counter != 0) {
return false;
}
start_position.Remove();
Index: lldb/trunk/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- lldb/trunk/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/trunk/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -160,4 +160,6 @@
"selector:otherField:", context, basename));
EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
"abc::", context, basename));
+ EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
+ "f>", context, basename));
}
Index: lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
===
--- lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
@@ -242,8 +242,7 @@
}
}
- assert(template_counter >= 0);
- if (template_counter > 0) {
+ if (template_counter != 0) {
return false;
}
start_position.Remove();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45554: Make sure deleting all breakpoints clears their sites first
eugene created this revision.
eugene added reviewers: labath, jingham.
Fixing crash after "breakpoint delete". Bug:
https://bugs.llvm.org/show_bug.cgi?id=36430
https://reviews.llvm.org/D45554
Files:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
source/Breakpoint/BreakpointList.cpp
Index: source/Breakpoint/BreakpointList.cpp
===
--- source/Breakpoint/BreakpointList.cpp
+++ source/Breakpoint/BreakpointList.cpp
@@ -99,7 +99,7 @@
void BreakpointList::RemoveAllowed(bool notify) {
std::lock_guard guard(m_mutex);
-
+
bp_collection::iterator pos, end = m_breakpoints.end();
if (notify) {
for (pos = m_breakpoints.begin(); pos != end; ++pos) {
@@ -116,10 +116,12 @@
}
pos = m_breakpoints.begin();
while ( pos != end) {
- if((*pos)->AllowDelete())
-pos = m_breakpoints.erase(pos);
- else
-pos++;
+auto bp = *pos;
+if (bp->AllowDelete()) {
+ bp->ClearAllBreakpointSites();
+ pos = m_breakpoints.erase(pos);
+} else
+ pos++;
}
}
Index:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
===
---
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
+++
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
@@ -6,8 +6,10 @@
// License. See LICENSE.TXT for details.
//
//===--===//
+#include
int main (int argc, char const *argv[])
{
+printf("Observable side effect\n");
return 0; // Set break point at this line.
}
Index:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
===
---
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -45,6 +45,25 @@
self.addTearDownHook(
lambda: self.runCmd("settings clear auto-confirm"))
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
+def test_delete_all_breakpoints(self):
+"""Test that deleting all breakpoints works."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_symbol(self, "main")
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1,
loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.runCmd("breakpoint delete")
+self.runCmd("process continue")
+self.expect("process status", PROCESS_STOPPED,
+patterns=['Process .* exited with status = 0'])
+
+
def breakpoint_command_sequence(self):
"""Test a sequence of breakpoint command add, list, and delete."""
exe = self.getBuildArtifact("a.out")
Index: source/Breakpoint/BreakpointList.cpp
===
--- source/Breakpoint/BreakpointList.cpp
+++ source/Breakpoint/BreakpointList.cpp
@@ -99,7 +99,7 @@
void BreakpointList::RemoveAllowed(bool notify) {
std::lock_guard guard(m_mutex);
-
+
bp_collection::iterator pos, end = m_breakpoints.end();
if (notify) {
for (pos = m_breakpoints.begin(); pos != end; ++pos) {
@@ -116,10 +116,12 @@
}
pos = m_breakpoints.begin();
while ( pos != end) {
- if((*pos)->AllowDelete())
-pos = m_breakpoints.erase(pos);
- else
-pos++;
+auto bp = *pos;
+if (bp->AllowDelete()) {
+ bp->ClearAllBreakpointSites();
+ pos = m_breakpoints.erase(pos);
+} else
+ pos++;
}
}
Index: packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
===
--- packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
+++ packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
@@ -6,8 +6,10 @@
// License. See LICENSE.TXT for details.
//
//===--===//
+#include
int main (int argc, char const *argv[])
{
+printf("Observable side effect\n");
return 0; // Set break point at this line.
}
Index: packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
===
--- packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
[Lldb-commits] [PATCH] D45554: Make sure deleting all breakpoints clears their sites first
eugene updated this revision to Diff 142209.
eugene marked 2 inline comments as done.
eugene added a comment.
add comment to the test
https://reviews.llvm.org/D45554
Files:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
source/Breakpoint/BreakpointList.cpp
Index: source/Breakpoint/BreakpointList.cpp
===
--- source/Breakpoint/BreakpointList.cpp
+++ source/Breakpoint/BreakpointList.cpp
@@ -99,7 +99,7 @@
void BreakpointList::RemoveAllowed(bool notify) {
std::lock_guard guard(m_mutex);
-
+
bp_collection::iterator pos, end = m_breakpoints.end();
if (notify) {
for (pos = m_breakpoints.begin(); pos != end; ++pos) {
@@ -116,10 +116,12 @@
}
pos = m_breakpoints.begin();
while ( pos != end) {
- if((*pos)->AllowDelete())
-pos = m_breakpoints.erase(pos);
- else
-pos++;
+auto bp = *pos;
+if (bp->AllowDelete()) {
+ bp->ClearAllBreakpointSites();
+ pos = m_breakpoints.erase(pos);
+} else
+ pos++;
}
}
Index:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
===
---
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
+++
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
@@ -6,8 +6,12 @@
// License. See LICENSE.TXT for details.
//
//===--===//
+#include
int main (int argc, char const *argv[])
{
+// This line adds a real body to the function, so we can set more than one
+// breakpoint in it.
+printf("Observable side effect\n");
return 0; // Set break point at this line.
}
Index:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
===
---
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -45,6 +45,25 @@
self.addTearDownHook(
lambda: self.runCmd("settings clear auto-confirm"))
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
+def test_delete_all_breakpoints(self):
+"""Test that deleting all breakpoints works."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_symbol(self, "main")
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1,
loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.runCmd("breakpoint delete")
+self.runCmd("process continue")
+self.expect("process status", PROCESS_STOPPED,
+patterns=['Process .* exited with status = 0'])
+
+
def breakpoint_command_sequence(self):
"""Test a sequence of breakpoint command add, list, and delete."""
exe = self.getBuildArtifact("a.out")
Index: source/Breakpoint/BreakpointList.cpp
===
--- source/Breakpoint/BreakpointList.cpp
+++ source/Breakpoint/BreakpointList.cpp
@@ -99,7 +99,7 @@
void BreakpointList::RemoveAllowed(bool notify) {
std::lock_guard guard(m_mutex);
-
+
bp_collection::iterator pos, end = m_breakpoints.end();
if (notify) {
for (pos = m_breakpoints.begin(); pos != end; ++pos) {
@@ -116,10 +116,12 @@
}
pos = m_breakpoints.begin();
while ( pos != end) {
- if((*pos)->AllowDelete())
-pos = m_breakpoints.erase(pos);
- else
-pos++;
+auto bp = *pos;
+if (bp->AllowDelete()) {
+ bp->ClearAllBreakpointSites();
+ pos = m_breakpoints.erase(pos);
+} else
+ pos++;
}
}
Index: packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
===
--- packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
+++ packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
@@ -6,8 +6,12 @@
// License. See LICENSE.TXT for details.
//
//===--===//
+#include
int main (int argc, char const *argv[])
{
+// This line adds a real body to the function, so we can set more than one
+// breakpoint in it.
+printf("Observable side effect\n");
return 0; // Set break point at this line.
}
Index: packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCo
[Lldb-commits] [PATCH] D45554: Make sure deleting all breakpoints clears their sites first
eugene added inline comments.
Comment at:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c:13
{
+printf("Observable side effect\n");
return 0; // Set break point at this line.
labath wrote:
> Why did you need to add this? This seems like something that could easily be
> removed/reshuffled in the future (breaking your test, if it depends on it).
I just need more than one bp location in the function. "bp main" and a
breakpoint on the line 14 were the same thing before that.
I'll add a comment.
Comment at: source/Breakpoint/BreakpointList.cpp:120-121
+auto bp = *pos;
+if (bp->AllowDelete()) {
+ bp->ClearAllBreakpointSites();
+ pos = m_breakpoints.erase(pos);
labath wrote:
> Don't we need to do the same thing in the other "remove" functions (`Remove`,
> `RemoveAll`).
RemoveAll does the same at the line 83.
With Remove it's a more complicated. Target removes sites manually by disabling
breakpoint, but it has some extra logic that I don't fully understand and not
sure if I should touch.
https://reviews.llvm.org/D45554
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45554: Make sure deleting all breakpoints clears their sites first
eugene updated this revision to Diff 142319.
eugene added a comment.
Got rid of the printf in the test
https://reviews.llvm.org/D45554
Files:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
source/Breakpoint/BreakpointList.cpp
Index: source/Breakpoint/BreakpointList.cpp
===
--- source/Breakpoint/BreakpointList.cpp
+++ source/Breakpoint/BreakpointList.cpp
@@ -99,7 +99,7 @@
void BreakpointList::RemoveAllowed(bool notify) {
std::lock_guard guard(m_mutex);
-
+
bp_collection::iterator pos, end = m_breakpoints.end();
if (notify) {
for (pos = m_breakpoints.begin(); pos != end; ++pos) {
@@ -116,10 +116,12 @@
}
pos = m_breakpoints.begin();
while ( pos != end) {
- if((*pos)->AllowDelete())
-pos = m_breakpoints.erase(pos);
- else
-pos++;
+auto bp = *pos;
+if (bp->AllowDelete()) {
+ bp->ClearAllBreakpointSites();
+ pos = m_breakpoints.erase(pos);
+} else
+ pos++;
}
}
Index:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
===
---
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
+++
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
@@ -9,5 +9,9 @@
int main (int argc, char const *argv[])
{
+// Add a body to the function, so we can set more than one
+// breakpoint in it.
+static volatile int var = 0;
+var++;
return 0; // Set break point at this line.
}
Index:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
===
---
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -45,6 +45,25 @@
self.addTearDownHook(
lambda: self.runCmd("settings clear auto-confirm"))
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
+def test_delete_all_breakpoints(self):
+"""Test that deleting all breakpoints works."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_symbol(self, "main")
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1,
loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.runCmd("breakpoint delete")
+self.runCmd("process continue")
+self.expect("process status", PROCESS_STOPPED,
+patterns=['Process .* exited with status = 0'])
+
+
def breakpoint_command_sequence(self):
"""Test a sequence of breakpoint command add, list, and delete."""
exe = self.getBuildArtifact("a.out")
Index: source/Breakpoint/BreakpointList.cpp
===
--- source/Breakpoint/BreakpointList.cpp
+++ source/Breakpoint/BreakpointList.cpp
@@ -99,7 +99,7 @@
void BreakpointList::RemoveAllowed(bool notify) {
std::lock_guard guard(m_mutex);
-
+
bp_collection::iterator pos, end = m_breakpoints.end();
if (notify) {
for (pos = m_breakpoints.begin(); pos != end; ++pos) {
@@ -116,10 +116,12 @@
}
pos = m_breakpoints.begin();
while ( pos != end) {
- if((*pos)->AllowDelete())
-pos = m_breakpoints.erase(pos);
- else
-pos++;
+auto bp = *pos;
+if (bp->AllowDelete()) {
+ bp->ClearAllBreakpointSites();
+ pos = m_breakpoints.erase(pos);
+} else
+ pos++;
}
}
Index: packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
===
--- packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
+++ packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
@@ -9,5 +9,9 @@
int main (int argc, char const *argv[])
{
+// Add a body to the function, so we can set more than one
+// breakpoint in it.
+static volatile int var = 0;
+var++;
return 0; // Set break point at this line.
}
Index: packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
===
--- packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++ packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -45,6 +45,25 @@
self.addTearDownH
[Lldb-commits] [PATCH] D45554: Make sure deleting all breakpoints clears their sites first
eugene marked 2 inline comments as done. eugene added a comment. There is an ownership cycle between BreakpointSite::m_owners and BreakpointLocation::m_bp_site_sp. We should probably make m_owners a collection of weak references. But currently most of the code just works it around by calling Breakpoint::ClearAllBreakpointSites() before deleting a breakpoint. https://reviews.llvm.org/D45554 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45554: Make sure deleting all breakpoints clears their sites first
eugene added a comment. Well, I agree that breakpoints, locations and sites could benefit from ownership refactoring. shared_ptr cycles are bad. Let's discuss it at lldb-dev. Meanwhile I think it's still ok to fix this bug right now, by doing what has already been done in other places. https://reviews.llvm.org/D45554 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45554: Make sure deleting all breakpoints clears their sites first
eugene added a comment. If nobody minds, I'd appreciate if somebody would accept this patch. https://reviews.llvm.org/D45554 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45554: Make sure deleting all breakpoints clears their sites first
This revision was automatically updated to reflect the committed changes.
Closed by commit rL330163: Make sure deleting all breakpoints clears their
sites first (authored by eugene, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D45554?vs=142319&id=142710#toc
Repository:
rL LLVM
https://reviews.llvm.org/D45554
Files:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
lldb/trunk/source/Breakpoint/BreakpointList.cpp
Index:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
===
---
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
+++
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
@@ -9,5 +9,9 @@
int main (int argc, char const *argv[])
{
+// Add a body to the function, so we can set more than one
+// breakpoint in it.
+static volatile int var = 0;
+var++;
return 0; // Set break point at this line.
}
Index:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
===
---
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -45,6 +45,25 @@
self.addTearDownHook(
lambda: self.runCmd("settings clear auto-confirm"))
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
+def test_delete_all_breakpoints(self):
+"""Test that deleting all breakpoints works."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_symbol(self, "main")
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1,
loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.runCmd("breakpoint delete")
+self.runCmd("process continue")
+self.expect("process status", PROCESS_STOPPED,
+patterns=['Process .* exited with status = 0'])
+
+
def breakpoint_command_sequence(self):
"""Test a sequence of breakpoint command add, list, and delete."""
exe = self.getBuildArtifact("a.out")
Index: lldb/trunk/source/Breakpoint/BreakpointList.cpp
===
--- lldb/trunk/source/Breakpoint/BreakpointList.cpp
+++ lldb/trunk/source/Breakpoint/BreakpointList.cpp
@@ -99,7 +99,7 @@
void BreakpointList::RemoveAllowed(bool notify) {
std::lock_guard guard(m_mutex);
-
+
bp_collection::iterator pos, end = m_breakpoints.end();
if (notify) {
for (pos = m_breakpoints.begin(); pos != end; ++pos) {
@@ -116,10 +116,12 @@
}
pos = m_breakpoints.begin();
while ( pos != end) {
- if((*pos)->AllowDelete())
-pos = m_breakpoints.erase(pos);
- else
-pos++;
+auto bp = *pos;
+if (bp->AllowDelete()) {
+ bp->ClearAllBreakpointSites();
+ pos = m_breakpoints.erase(pos);
+} else
+ pos++;
}
}
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c
@@ -9,5 +9,9 @@
int main (int argc, char const *argv[])
{
+// Add a body to the function, so we can set more than one
+// breakpoint in it.
+static volatile int var = 0;
+var++;
return 0; // Set break point at this line.
}
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -45,6 +45,25 @@
self.addTearDownHook(
lambda: self.runCmd("settings clear auto-confirm"))
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
+def test_delete_all_breakpoints(self):
+"""Test that deleting all breakpoints works."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe,
[Lldb-commits] [PATCH] D31129: Remove remaining platform specific code from FileSpec
eugene accepted this revision. eugene added a comment. This revision is now accepted and ready to land. I like how much code you deleted. https://reviews.llvm.org/D31129 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31451: New C++ function name parsing logic
eugene updated this revision to Diff 93438.
eugene added reviewers: labath, zturner, jingham.
eugene changed the visibility from "eugene (Eugene Zemtsov)" to "Public (No
Login Required)".
eugene changed the edit policy from "eugene (Eugene Zemtsov)" to "All Users".
eugene added a subscriber: lldb-commits.
eugene added a comment.
Herald added a subscriber: mgorny.
Improve template method parsing accuracy.
https://reviews.llvm.org/D31451
Files:
source/Plugins/Language/CPlusPlus/CMakeLists.txt
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
Index: unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -13,28 +13,132 @@
using namespace lldb_private;
-TEST(CPlusPlusLanguage, MethodName) {
+TEST(CPlusPlusLanguage, MethodNameParsing) {
struct TestCase {
std::string input;
std::string context, basename, arguments, qualifiers, scope_qualified_name;
};
TestCase test_cases[] = {
+ {"void f(int)", "", "f", "(int)", "", "f"},
+ {"main(int, char *[]) ", "", "main", "(int, char *[])", "", "main"},
{"foo::bar(baz)", "foo", "bar", "(baz)", "", "foo::bar"},
+ {"foo::~bar(baz)", "foo", "~bar", "(baz)", "", "foo::~bar"},
+
+ // Operators
{"std::basic_ostream >& "
"std::operator<< >"
"(std::basic_ostream >&, char const*)",
"std", "operator<< >",
"(std::basic_ostream >&, char const*)", "",
- "std::operator<< >"}};
+ "std::operator<< >"},
+ {"operator delete[](void*, clang::ASTContext const&, unsigned long)", "",
+ "operator delete[]", "(void*, clang::ASTContext const&, unsigned long)",
+ "", "operator delete[]"},
+ {"llvm::Optional::operator bool() const",
+ "llvm::Optional", "operator bool", "()", "const",
+ "llvm::Optional::operator bool"},
+ {"(anonymous namespace)::FactManager::operator[](unsigned short)",
+ "(anonymous namespace)::FactManager", "operator[]", "(unsigned short)",
+ "", "(anonymous namespace)::FactManager::operator[]"},
+ {"const int& std::map>::operator[](short) const",
+ "std::map>", "operator[]", "(short)", "const",
+ "std::map>::operator[]"},
+ {"CompareInsn::operator()(llvm::StringRef, InsnMatchEntry const&)",
+ "CompareInsn", "operator()", "(llvm::StringRef, InsnMatchEntry const&)",
+ "", "CompareInsn::operator()"},
+ {"llvm::Optional::operator*() const &",
+ "llvm::Optional", "operator*", "()", "const &",
+ "llvm::Optional::operator*"},
+ // Internal classes
+ {"operator<<(Cls, Cls)::Subclass::function()",
+ "operator<<(Cls, Cls)::Subclass", "function", "()", "",
+ "operator<<(Cls, Cls)::Subclass::function"},
+ {"SAEC::checkFunction(context&) const::CallBack::CallBack(int)",
+ "SAEC::checkFunction(context&) const::CallBack", "CallBack", "(int)", "",
+ "SAEC::checkFunction(context&) const::CallBack::CallBack"},
+ // Anonymous namespace
+ {"XX::(anonymous namespace)::anon_class::anon_func() const",
+ "XX::(anonymous namespace)::anon_class", "anon_func", "()", "const",
+ "XX::(anonymous namespace)::anon_class::anon_func"},
+
+ // Function pointers
+ {"string (*f(vector&&))(float)", "", "f", "(vector&&)", "",
+ "f"},
+ {"void (*&std::_Any_data::_M_access())()", "std::_Any_data",
+ "_M_access", "()", "",
+ "std::_Any_data::_M_access"},
+ {"void (*(*(*(*(*(*(*(* const&func1(int))())())())())())())())()", "",
+ "func1", "(int)", "", "func1"},
+
+ // Templates
+ {"void llvm::PM>::"
+ "addPass(llvm::VP)",
+ "llvm::PM>", "addPass",
+ "(llvm::VP)", "",
+ "llvm::PM>::"
+ "addPass"},
+ {"void std::vector >"
+ "::_M_emplace_back_aux(Class const&)",
+ "std::vector >",
+ "_M_emplace_back_aux", "(Class const&)", "",
+ "std::vector >::"
+ "_M_emplace_back_aux"},
+ {"unsigned long llvm::countTrailingOnes"
+ "(unsigned int, llvm::ZeroBehavior)",
+ "llvm", "countTrailingOnes",
+ "(unsigned int, llvm::ZeroBehavior)", "",
+ "llvm::countTrailingOnes"},
+ {"std::enable_if<(10u)<(64), bool>::type llvm::isUInt<10u>(unsigned "
+ "long)",
+ "llvm", "isUInt<10u>", "(unsigned long)", "", "llvm::isUInt<10u>"},
+ {"f, sizeof(B)()", "",
+ "f, sizeof(B)", "()", "",
+ "f, sizeof(B)"}};
for (const auto &test : test_cases) {
CPlusPlusLanguage::MethodName method(ConstString(test.input));
-EXPECT_TRUE(method.IsValid());
-EXPECT_EQ(test.context, method.Ge
[Lldb-commits] [PATCH] D31451: New C++ function name parsing logic
eugene updated this revision to Diff 93572.
eugene added a comment.
Addressing code-review comments.
Most notable change: MethodName::Parse() tries simple version of name parser,
before invoking full power of CPlusPlusNameParser. It really helps with the
perf.
https://reviews.llvm.org/D31451
Files:
source/Plugins/Language/CPlusPlus/CMakeLists.txt
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
Index: unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -6,35 +6,139 @@
// License. See LICENSE.TXT for details.
//
//===--===//
-
#include "gtest/gtest.h"
#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
using namespace lldb_private;
-TEST(CPlusPlusLanguage, MethodName) {
+TEST(CPlusPlusLanguage, MethodNameParsing) {
struct TestCase {
std::string input;
std::string context, basename, arguments, qualifiers, scope_qualified_name;
};
TestCase test_cases[] = {
- {"foo::bar(baz)", "foo", "bar", "(baz)", "", "foo::bar"},
+ {"main(int, char *[]) ", "", "main", "(int, char *[])", "", "main"},
+ {"foo::bar(baz) const", "foo", "bar", "(baz)", "const", "foo::bar"},
+ {"foo::~bar(baz)", "foo", "~bar", "(baz)", "", "foo::~bar"},
+ {"a::b::c::d(e,f)", "a::b::c", "d", "(e,f)", "", "a::b::c::d"},
+ {"void f(int)", "", "f", "(int)", "", "f"},
+
+ // Operators
{"std::basic_ostream >& "
"std::operator<< >"
"(std::basic_ostream >&, char const*)",
"std", "operator<< >",
"(std::basic_ostream >&, char const*)", "",
- "std::operator<< >"}};
+ "std::operator<< >"},
+ {"operator delete[](void*, clang::ASTContext const&, unsigned long)", "",
+ "operator delete[]", "(void*, clang::ASTContext const&, unsigned long)",
+ "", "operator delete[]"},
+ {"llvm::Optional::operator bool() const",
+ "llvm::Optional", "operator bool", "()", "const",
+ "llvm::Optional::operator bool"},
+ {"(anonymous namespace)::FactManager::operator[](unsigned short)",
+ "(anonymous namespace)::FactManager", "operator[]", "(unsigned short)",
+ "", "(anonymous namespace)::FactManager::operator[]"},
+ {"const int& std::map>::operator[](short) const",
+ "std::map>", "operator[]", "(short)", "const",
+ "std::map>::operator[]"},
+ {"CompareInsn::operator()(llvm::StringRef, InsnMatchEntry const&)",
+ "CompareInsn", "operator()", "(llvm::StringRef, InsnMatchEntry const&)",
+ "", "CompareInsn::operator()"},
+ {"llvm::Optional::operator*() const &",
+ "llvm::Optional", "operator*", "()", "const &",
+ "llvm::Optional::operator*"},
+ // Internal classes
+ {"operator<<(Cls, Cls)::Subclass::function()",
+ "operator<<(Cls, Cls)::Subclass", "function", "()", "",
+ "operator<<(Cls, Cls)::Subclass::function"},
+ {"SAEC::checkFunction(context&) const::CallBack::CallBack(int)",
+ "SAEC::checkFunction(context&) const::CallBack", "CallBack", "(int)", "",
+ "SAEC::checkFunction(context&) const::CallBack::CallBack"},
+ // Anonymous namespace
+ {"XX::(anonymous namespace)::anon_class::anon_func() const",
+ "XX::(anonymous namespace)::anon_class", "anon_func", "()", "const",
+ "XX::(anonymous namespace)::anon_class::anon_func"},
+
+ // Function pointers
+ {"string (*f(vector&&))(float)", "", "f", "(vector&&)", "",
+ "f"},
+ {"void (*&std::_Any_data::_M_access())()", "std::_Any_data",
+ "_M_access", "()", "",
+ "std::_Any_data::_M_access"},
+ {"void (*(*(*(*(*(*(*(* const&func1(int))())())())())())())())()", "",
+ "func1", "(int)", "", "func1"},
+
+ // Templates
+ {"void llvm::PM>::"
+ "addPass(llvm::VP)",
+ "llvm::PM>", "addPass",
+ "(llvm::VP)", "",
+ "llvm::PM>::"
+ "addPass"},
+ {"void std::vector >"
+ "::_M_emplace_back_aux(Class const&)",
+ "std::vector >",
+ "_M_emplace_back_aux", "(Class const&)", "",
+ "std::vector >::"
+ "_M_emplace_back_aux"},
+ {"unsigned long llvm::countTrailingOnes"
+ "(unsigned int, llvm::ZeroBehavior)",
+ "llvm", "countTrailingOnes",
+ "(unsigned int, llvm::ZeroBehavior)", "",
+ "llvm::countTrailingOnes"},
+ {"std::enable_if<(10u)<(64), bool>::type llvm::isUInt<10u>(unsigned "
+ "long)",
+ "llvm", "isUInt<10u>", "(unsigned long)", "", "llvm::isUInt<10u>"},
+ {"f, sizeof(B)()", "",
+ "f, sizeof(B)", "()"
[Lldb-commits] [PATCH] D31451: New C++ function name parsing logic
eugene marked 4 inline comments as done.
eugene added a comment.
> I think we should do some performance measurements to see whether this needs
> more optimising or it's fine as is.
>
> I propose the following benchmark:
>
> bin/lldb bin/clang
>
> make sure clang is statically linked
> breakpoint set --name non_exisiting_function
>
> Setting the breakpoint on a nonexistent function avoids us timing the
> breakpoint setting machinery, while still getting every symbol in the
> executable parsed.
I did some micro-benchmarking and on average new parser is ~3 time slower than
the old one. (new parser - ~200k string/s, old parser - ~700k string/s)
clang::Lexer appears to be the slowest part of it.
On the clang breakpoint benchmark you proposed, it is hard to notice much of a
difference. clang binary has about 500k functions.
With new parser it takes about 11s to try to set a breakpoint, on the old one
it's about 10s. (release version of LLDB, debug static version of clang)
I decided to use a hybrid approach, when we use old parsing code for simple
cases and call new parser only when it fails.
80% of clang functions are simple enough that we don't really need the new
parser, so it helped to bring clang breakpoint test back to 10s.
I think it's reasonable to assume that similar distribution is true for most
programs, and most of their functions can be parsed with the old code.
I don't think we can really improve performance of a new parser without giving
up on clang::Lexer, and I'm reluctant to do it.
So I propose to keep hybrid approach and call a new parser only for complicated
cases.
Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:94
if (!m_parsed && m_full) {
-//ConstString mangled;
-//m_full.GetMangledCounterpart(mangled);
-//printf ("\n parsing = '%s'\n", m_full.GetCString());
-//if (mangled)
-//printf (" mangled = '%s'\n", mangled.GetCString());
-m_parse_error = false;
-m_parsed = true;
-llvm::StringRef full(m_full.GetCString());
-
-size_t arg_start, arg_end;
-llvm::StringRef parens("()", 2);
-if (ReverseFindMatchingChars(full, parens, arg_start, arg_end)) {
- m_arguments = full.substr(arg_start, arg_end - arg_start + 1);
- if (arg_end + 1 < full.size())
-m_qualifiers = full.substr(arg_end + 1);
- if (arg_start > 0) {
-size_t basename_end = arg_start;
-size_t context_start = 0;
-size_t context_end = llvm::StringRef::npos;
-if (basename_end > 0 && full[basename_end - 1] == '>') {
- // TODO: handle template junk...
- // Templated function
- size_t template_start, template_end;
- llvm::StringRef lt_gt("<>", 2);
- if (ReverseFindMatchingChars(full, lt_gt, template_start,
- template_end, basename_end)) {
-// Check for templated functions that include return type like:
-// 'void foo()'
-context_start = full.rfind(' ', template_start);
-if (context_start == llvm::StringRef::npos)
- context_start = 0;
-else
- ++context_start;
-
-context_end = full.rfind(':', template_start);
-if (context_end == llvm::StringRef::npos ||
-context_end < context_start)
- context_end = context_start;
- } else {
-context_end = full.rfind(':', basename_end);
- }
-} else if (context_end == llvm::StringRef::npos) {
- context_end = full.rfind(':', basename_end);
-}
-
-if (context_end == llvm::StringRef::npos)
- m_basename = full.substr(0, basename_end);
-else {
- if (context_start < context_end)
-m_context =
-full.substr(context_start, context_end - 1 - context_start);
- const size_t basename_begin = context_end + 1;
- m_basename =
- full.substr(basename_begin, basename_end - basename_begin);
-}
-m_type = eTypeUnknownMethod;
- } else {
-m_parse_error = true;
-return;
- }
-
- if (!IsValidBasename(m_basename)) {
-// The C++ basename doesn't match our regular expressions so this can't
-// be a valid C++ method, clear everything out and indicate an error
-m_context = llvm::StringRef();
-m_basename = llvm::StringRef();
-m_arguments = llvm::StringRef();
-m_qualifiers = llvm::StringRef();
-m_parse_error = true;
- }
+CPlusPlusNameParser parser(m_full.GetStringRef());
+auto function = parser.ParseAsFunctionDefinition();
labath wrote:
> How about the following api:
> ```
> if (auto function =
> CPlusPlusNameParser::ParseAsFunctionDefinition(m_full.GetStringRef())) {
> ...
> ```
If you don't mind I'll leave it as it is.
I understand that i
[Lldb-commits] [PATCH] D31451: New C++ function name parsing logic
eugene updated this revision to Diff 93693.
eugene marked 3 inline comments as done.
eugene added a comment.
Addressing review commnets
https://reviews.llvm.org/D31451
Files:
source/Plugins/Language/CPlusPlus/CMakeLists.txt
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
Index: unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -6,35 +6,139 @@
// License. See LICENSE.TXT for details.
//
//===--===//
-
#include "gtest/gtest.h"
#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
using namespace lldb_private;
-TEST(CPlusPlusLanguage, MethodName) {
+TEST(CPlusPlusLanguage, MethodNameParsing) {
struct TestCase {
std::string input;
std::string context, basename, arguments, qualifiers, scope_qualified_name;
};
TestCase test_cases[] = {
- {"foo::bar(baz)", "foo", "bar", "(baz)", "", "foo::bar"},
+ {"main(int, char *[]) ", "", "main", "(int, char *[])", "", "main"},
+ {"foo::bar(baz) const", "foo", "bar", "(baz)", "const", "foo::bar"},
+ {"foo::~bar(baz)", "foo", "~bar", "(baz)", "", "foo::~bar"},
+ {"a::b::c::d(e,f)", "a::b::c", "d", "(e,f)", "", "a::b::c::d"},
+ {"void f(int)", "", "f", "(int)", "", "f"},
+
+ // Operators
{"std::basic_ostream >& "
"std::operator<< >"
"(std::basic_ostream >&, char const*)",
"std", "operator<< >",
"(std::basic_ostream >&, char const*)", "",
- "std::operator<< >"}};
+ "std::operator<< >"},
+ {"operator delete[](void*, clang::ASTContext const&, unsigned long)", "",
+ "operator delete[]", "(void*, clang::ASTContext const&, unsigned long)",
+ "", "operator delete[]"},
+ {"llvm::Optional::operator bool() const",
+ "llvm::Optional", "operator bool", "()", "const",
+ "llvm::Optional::operator bool"},
+ {"(anonymous namespace)::FactManager::operator[](unsigned short)",
+ "(anonymous namespace)::FactManager", "operator[]", "(unsigned short)",
+ "", "(anonymous namespace)::FactManager::operator[]"},
+ {"const int& std::map>::operator[](short) const",
+ "std::map>", "operator[]", "(short)", "const",
+ "std::map>::operator[]"},
+ {"CompareInsn::operator()(llvm::StringRef, InsnMatchEntry const&)",
+ "CompareInsn", "operator()", "(llvm::StringRef, InsnMatchEntry const&)",
+ "", "CompareInsn::operator()"},
+ {"llvm::Optional::operator*() const &",
+ "llvm::Optional", "operator*", "()", "const &",
+ "llvm::Optional::operator*"},
+ // Internal classes
+ {"operator<<(Cls, Cls)::Subclass::function()",
+ "operator<<(Cls, Cls)::Subclass", "function", "()", "",
+ "operator<<(Cls, Cls)::Subclass::function"},
+ {"SAEC::checkFunction(context&) const::CallBack::CallBack(int)",
+ "SAEC::checkFunction(context&) const::CallBack", "CallBack", "(int)", "",
+ "SAEC::checkFunction(context&) const::CallBack::CallBack"},
+ // Anonymous namespace
+ {"XX::(anonymous namespace)::anon_class::anon_func() const",
+ "XX::(anonymous namespace)::anon_class", "anon_func", "()", "const",
+ "XX::(anonymous namespace)::anon_class::anon_func"},
+
+ // Function pointers
+ {"string (*f(vector&&))(float)", "", "f", "(vector&&)", "",
+ "f"},
+ {"void (*&std::_Any_data::_M_access())()", "std::_Any_data",
+ "_M_access", "()", "",
+ "std::_Any_data::_M_access"},
+ {"void (*(*(*(*(*(*(*(* const&func1(int))())())())())())())())()", "",
+ "func1", "(int)", "", "func1"},
+
+ // Templates
+ {"void llvm::PM>::"
+ "addPass(llvm::VP)",
+ "llvm::PM>", "addPass",
+ "(llvm::VP)", "",
+ "llvm::PM>::"
+ "addPass"},
+ {"void std::vector >"
+ "::_M_emplace_back_aux(Class const&)",
+ "std::vector >",
+ "_M_emplace_back_aux", "(Class const&)", "",
+ "std::vector >::"
+ "_M_emplace_back_aux"},
+ {"unsigned long llvm::countTrailingOnes"
+ "(unsigned int, llvm::ZeroBehavior)",
+ "llvm", "countTrailingOnes",
+ "(unsigned int, llvm::ZeroBehavior)", "",
+ "llvm::countTrailingOnes"},
+ {"std::enable_if<(10u)<(64), bool>::type llvm::isUInt<10u>(unsigned "
+ "long)",
+ "llvm", "isUInt<10u>", "(unsigned long)", "", "llvm::isUInt<10u>"},
+ {"f, sizeof(B)()", "",
+ "f, sizeof(B)", "()", "",
+ "f, sizeof(B)"}};
for (const auto &test : test_cases) {
CPlusPlusLanguage::MethodName method(ConstString
[Lldb-commits] [PATCH] D31451: New C++ function name parsing logic
eugene updated this revision to Diff 93694.
eugene marked an inline comment as done.
https://reviews.llvm.org/D31451
Files:
source/Plugins/Language/CPlusPlus/CMakeLists.txt
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
Index: unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -6,35 +6,139 @@
// License. See LICENSE.TXT for details.
//
//===--===//
-
#include "gtest/gtest.h"
#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
using namespace lldb_private;
-TEST(CPlusPlusLanguage, MethodName) {
+TEST(CPlusPlusLanguage, MethodNameParsing) {
struct TestCase {
std::string input;
std::string context, basename, arguments, qualifiers, scope_qualified_name;
};
TestCase test_cases[] = {
- {"foo::bar(baz)", "foo", "bar", "(baz)", "", "foo::bar"},
+ {"main(int, char *[]) ", "", "main", "(int, char *[])", "", "main"},
+ {"foo::bar(baz) const", "foo", "bar", "(baz)", "const", "foo::bar"},
+ {"foo::~bar(baz)", "foo", "~bar", "(baz)", "", "foo::~bar"},
+ {"a::b::c::d(e,f)", "a::b::c", "d", "(e,f)", "", "a::b::c::d"},
+ {"void f(int)", "", "f", "(int)", "", "f"},
+
+ // Operators
{"std::basic_ostream >& "
"std::operator<< >"
"(std::basic_ostream >&, char const*)",
"std", "operator<< >",
"(std::basic_ostream >&, char const*)", "",
- "std::operator<< >"}};
+ "std::operator<< >"},
+ {"operator delete[](void*, clang::ASTContext const&, unsigned long)", "",
+ "operator delete[]", "(void*, clang::ASTContext const&, unsigned long)",
+ "", "operator delete[]"},
+ {"llvm::Optional::operator bool() const",
+ "llvm::Optional", "operator bool", "()", "const",
+ "llvm::Optional::operator bool"},
+ {"(anonymous namespace)::FactManager::operator[](unsigned short)",
+ "(anonymous namespace)::FactManager", "operator[]", "(unsigned short)",
+ "", "(anonymous namespace)::FactManager::operator[]"},
+ {"const int& std::map>::operator[](short) const",
+ "std::map>", "operator[]", "(short)", "const",
+ "std::map>::operator[]"},
+ {"CompareInsn::operator()(llvm::StringRef, InsnMatchEntry const&)",
+ "CompareInsn", "operator()", "(llvm::StringRef, InsnMatchEntry const&)",
+ "", "CompareInsn::operator()"},
+ {"llvm::Optional::operator*() const &",
+ "llvm::Optional", "operator*", "()", "const &",
+ "llvm::Optional::operator*"},
+ // Internal classes
+ {"operator<<(Cls, Cls)::Subclass::function()",
+ "operator<<(Cls, Cls)::Subclass", "function", "()", "",
+ "operator<<(Cls, Cls)::Subclass::function"},
+ {"SAEC::checkFunction(context&) const::CallBack::CallBack(int)",
+ "SAEC::checkFunction(context&) const::CallBack", "CallBack", "(int)", "",
+ "SAEC::checkFunction(context&) const::CallBack::CallBack"},
+ // Anonymous namespace
+ {"XX::(anonymous namespace)::anon_class::anon_func() const",
+ "XX::(anonymous namespace)::anon_class", "anon_func", "()", "const",
+ "XX::(anonymous namespace)::anon_class::anon_func"},
+
+ // Function pointers
+ {"string (*f(vector&&))(float)", "", "f", "(vector&&)", "",
+ "f"},
+ {"void (*&std::_Any_data::_M_access())()", "std::_Any_data",
+ "_M_access", "()", "",
+ "std::_Any_data::_M_access"},
+ {"void (*(*(*(*(*(*(*(* const&func1(int))())())())())())())())()", "",
+ "func1", "(int)", "", "func1"},
+
+ // Templates
+ {"void llvm::PM>::"
+ "addPass(llvm::VP)",
+ "llvm::PM>", "addPass",
+ "(llvm::VP)", "",
+ "llvm::PM>::"
+ "addPass"},
+ {"void std::vector >"
+ "::_M_emplace_back_aux(Class const&)",
+ "std::vector >",
+ "_M_emplace_back_aux", "(Class const&)", "",
+ "std::vector >::"
+ "_M_emplace_back_aux"},
+ {"unsigned long llvm::countTrailingOnes"
+ "(unsigned int, llvm::ZeroBehavior)",
+ "llvm", "countTrailingOnes",
+ "(unsigned int, llvm::ZeroBehavior)", "",
+ "llvm::countTrailingOnes"},
+ {"std::enable_if<(10u)<(64), bool>::type llvm::isUInt<10u>(unsigned "
+ "long)",
+ "llvm", "isUInt<10u>", "(unsigned long)", "", "llvm::isUInt<10u>"},
+ {"f, sizeof(B)()", "",
+ "f, sizeof(B)", "()", "",
+ "f, sizeof(B)"}};
for (const auto &test : test_cases) {
CPlusPlusLanguage::MethodName method(ConstString(test.input));
-EXPECT_TRUE(method.IsValid());
-
[Lldb-commits] [PATCH] D31451: New C++ function name parsing logic
eugene added a subscriber: labath. eugene marked 2 inline comments as done. eugene added a comment. In https://reviews.llvm.org/D31451#715649, @tberghammer wrote: > Because of this I think some targeted micro benchmark will be much more > useful to measure the performance of this code then an end-to-end test as an > e2e test would have low signal to noise ratio. I did some micro-benchmarking and on average new parser is ~3 time slower than the old one. (new parser - ~200k string/s, old parser - ~700k string/s) clang::Lexer appears to be the slowest part of it. I mitigate this performance loss, by calling simplified parsing code for simple cases and calling new parser only when the old one fails. Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp:18 +using namespace lldb_private; +using llvm::Optional; +using llvm::None; labath wrote: > Are these necessary? You seem to prefix every occurence of Optional and None > anyway... Well, I used None. Now I use Optional as well. https://reviews.llvm.org/D31451 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31451: New C++ function name parsing logic
This revision was automatically updated to reflect the committed changes.
Closed by commit rL299374: New C++ function name parsing logic (authored by
eugene).
Changed prior to commit:
https://reviews.llvm.org/D31451?vs=93694&id=93910#toc
Repository:
rL LLVM
https://reviews.llvm.org/D31451
Files:
lldb/trunk/source/Plugins/Language/CPlusPlus/CMakeLists.txt
lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.h
lldb/trunk/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
Index: lldb/trunk/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- lldb/trunk/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/trunk/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -6,35 +6,139 @@
// License. See LICENSE.TXT for details.
//
//===--===//
-
#include "gtest/gtest.h"
#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
using namespace lldb_private;
-TEST(CPlusPlusLanguage, MethodName) {
+TEST(CPlusPlusLanguage, MethodNameParsing) {
struct TestCase {
std::string input;
std::string context, basename, arguments, qualifiers, scope_qualified_name;
};
TestCase test_cases[] = {
- {"foo::bar(baz)", "foo", "bar", "(baz)", "", "foo::bar"},
+ {"main(int, char *[]) ", "", "main", "(int, char *[])", "", "main"},
+ {"foo::bar(baz) const", "foo", "bar", "(baz)", "const", "foo::bar"},
+ {"foo::~bar(baz)", "foo", "~bar", "(baz)", "", "foo::~bar"},
+ {"a::b::c::d(e,f)", "a::b::c", "d", "(e,f)", "", "a::b::c::d"},
+ {"void f(int)", "", "f", "(int)", "", "f"},
+
+ // Operators
{"std::basic_ostream >& "
"std::operator<< >"
"(std::basic_ostream >&, char const*)",
"std", "operator<< >",
"(std::basic_ostream >&, char const*)", "",
- "std::operator<< >"}};
+ "std::operator<< >"},
+ {"operator delete[](void*, clang::ASTContext const&, unsigned long)", "",
+ "operator delete[]", "(void*, clang::ASTContext const&, unsigned long)",
+ "", "operator delete[]"},
+ {"llvm::Optional::operator bool() const",
+ "llvm::Optional", "operator bool", "()", "const",
+ "llvm::Optional::operator bool"},
+ {"(anonymous namespace)::FactManager::operator[](unsigned short)",
+ "(anonymous namespace)::FactManager", "operator[]", "(unsigned short)",
+ "", "(anonymous namespace)::FactManager::operator[]"},
+ {"const int& std::map>::operator[](short) const",
+ "std::map>", "operator[]", "(short)", "const",
+ "std::map>::operator[]"},
+ {"CompareInsn::operator()(llvm::StringRef, InsnMatchEntry const&)",
+ "CompareInsn", "operator()", "(llvm::StringRef, InsnMatchEntry const&)",
+ "", "CompareInsn::operator()"},
+ {"llvm::Optional::operator*() const &",
+ "llvm::Optional", "operator*", "()", "const &",
+ "llvm::Optional::operator*"},
+ // Internal classes
+ {"operator<<(Cls, Cls)::Subclass::function()",
+ "operator<<(Cls, Cls)::Subclass", "function", "()", "",
+ "operator<<(Cls, Cls)::Subclass::function"},
+ {"SAEC::checkFunction(context&) const::CallBack::CallBack(int)",
+ "SAEC::checkFunction(context&) const::CallBack", "CallBack", "(int)", "",
+ "SAEC::checkFunction(context&) const::CallBack::CallBack"},
+ // Anonymous namespace
+ {"XX::(anonymous namespace)::anon_class::anon_func() const",
+ "XX::(anonymous namespace)::anon_class", "anon_func", "()", "const",
+ "XX::(anonymous namespace)::anon_class::anon_func"},
+
+ // Function pointers
+ {"string (*f(vector&&))(float)", "", "f", "(vector&&)", "",
+ "f"},
+ {"void (*&std::_Any_data::_M_access())()", "std::_Any_data",
+ "_M_access", "()", "",
+ "std::_Any_data::_M_access"},
+ {"void (*(*(*(*(*(*(*(* const&func1(int))())())())())())())())()", "",
+ "func1", "(int)", "", "func1"},
+
+ // Templates
+ {"void llvm::PM>::"
+ "addPass(llvm::VP)",
+ "llvm::PM>", "addPass",
+ "(llvm::VP)", "",
+ "llvm::PM>::"
+ "addPass"},
+ {"void std::vector >"
+ "::_M_emplace_back_aux(Class const&)",
+ "std::vector >",
+ "_M_emplace_back_aux", "(Class const&)", "",
+ "std::vector >::"
+ "_M_emplace_back_aux"},
+ {"unsigned long llvm::countTrailingOnes"
+ "(unsigned int, llvm::ZeroBehavior)",
+ "llvm", "countTrailingOnes",
+ "(unsigned int, llvm::ZeroBehavior)", "",
+ "llvm::countTrailingOnes"},
+ {"std::enable_if<(10u)<(64), bool>::type llvm::isUInt<10u>(unsigned "
+ "long)",
+ "llvm", "isUInt<10u>", "(uns
[Lldb-commits] [PATCH] D31877: Remove Plugins/Process/POSIX from include_directories
eugene accepted this revision. eugene added a comment. This revision is now accepted and ready to land. Thanks for doing it. IMO we should always strive to use "absolute" path for all headers. https://reviews.llvm.org/D31877 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32434: ObjectFileELF: Fix symbol lookup in bss section
eugene added a comment. Is it really necessary to check in binary ELF files? I understand that we don't always have a C compiler capable of producing ELF files, but maybe it's ok to skip this test on those platforms. https://reviews.llvm.org/D32434 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32434: ObjectFileELF: Fix symbol lookup in bss section
eugene accepted this revision. eugene added a comment. This revision is now accepted and ready to land. Neatly done! https://reviews.llvm.org/D32434 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32600: Resurrect pselect MainLoop implementation
eugene added inline comments. Comment at: source/Host/common/MainLoop.cpp:82 + int queue_id; + std::vector events; + struct kevent event_list[4]; here and below struct seems to be redundant https://reviews.llvm.org/D32600 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32753: MainLoop: Add unit tests
eugene accepted this revision.
eugene added inline comments.
This revision is now accepted and ready to land.
Comment at: source/Host/common/MainLoop.cpp:66
class MainLoop::RunImpl {
public:
Could you please add here a comment describing when kqueue/pselect/ppoll are
used.
https://reviews.llvm.org/D32753
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33778: Add a NativeProcessProtocol Factory class
eugene added inline comments. Comment at: source/Plugins/Process/Linux/NativeProcessLinux.h:148 - ::pid_t Attach(lldb::pid_t pid, Status &error); + static llvm::Expected> Attach(::pid_t pid); labath wrote: > zturner wrote: > > Before it was only returning 1, now it's returning a vector. Any reason? > I've refactored the function a bit. It now returns a list of threads that it > has attached to. Previously it stored them in the object itself, but now it > can't as I don't construct a process object until I know that the attach has > succeeded. > > I should probably document the return value though. This method would certainly benefit from a comment that explains the nature of its return value. https://reviews.llvm.org/D33778 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33998: Add pretty-printer for wait(2) statuses and modernize the code handling them
eugene added inline comments.
Comment at: source/Host/common/Host.cpp:1010
+static constexpr char type[] = "WXS";
+OS << formatv("{0}{1:x-2}", type[WS.type], WS.status);
+return;
type[WS.type] seems to be a somewhat unnecessary hack. I would use a simple
switch here.
Comment at: source/Host/common/Host.cpp:1017
+ {"Exited with status"}, {"Killed by signal"}, {"Stopped by signal"}};
+ OS << desc[WS.type] << " " << int(WS.status);
+}
Same as above.
https://reviews.llvm.org/D33998
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D34683: [unittests] Add a helper function for getting an input file
eugene accepted this revision. eugene added a comment. This revision is now accepted and ready to land. Great change. The only comment I have is about location of TestUtilities.cpp. Utility\Helpers kinda implies that these are helpers for Utility tests which is not true. I would move it up one level up lldb/unittests/TestUtilities.cpp https://reviews.llvm.org/D34683 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D34911: Enable parsing C++ names generated by lambda functions.
eugene accepted this revision. eugene added a comment. This revision is now accepted and ready to land. Great change! Thanks making it. https://reviews.llvm.org/D34911 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D34911: Enable parsing C++ names generated by lambda functions.
eugene added a comment. In https://reviews.llvm.org/D34911#808059, @wengxt wrote: > I don't have commit access. May any one help me to commit? I'll do it for you. https://reviews.llvm.org/D34911 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D34911: Enable parsing C++ names generated by lambda functions.
eugene added a comment. Jim already did it. Thanks Jim. https://reviews.llvm.org/D34911 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D35607: Extend 'target symbols add' to load symbols from a given file by UUID.
eugene created this revision.
eugene added a project: LLDB.
Currently on Linux there is now way to force LLDB to use symbols from a certain
file unless there is a UUID match established by build-id or debuglink.
This change somewhat extends ''target symbols add', making it possible to
specify symbol file path and inferior module UUID at the same time.
This way even users can load symbols for stripped binaries even if build-id or
debuglink sections are missing.
https://reviews.llvm.org/D35607
Files:
packages/Python/lldbsuite/test/linux/add-symbols/Makefile
packages/Python/lldbsuite/test/linux/add-symbols/TestTargetSymbolsAddCommand.py
packages/Python/lldbsuite/test/linux/add-symbols/main.c
source/Commands/CommandObjectTarget.cpp
source/Host/common/Symbols.cpp
Index: source/Host/common/Symbols.cpp
===
--- source/Host/common/Symbols.cpp
+++ source/Host/common/Symbols.cpp
@@ -308,8 +308,12 @@
bool Symbols::DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
bool force_lookup) {
- // Fill in the module_spec.GetFileSpec() for the object file and/or the
- // module_spec.GetSymbolFileSpec() for the debug symbols file.
+ // Unlike on Apple there is no smart way to download symbol files.
+ // Simply try using module file spec if it is specified and exists.
+ if (module_spec.GetFileSpec() && module_spec.GetFileSpec().Exists()) {
+module_spec.GetSymbolFileSpec() = module_spec.GetFileSpec();
+return true;
+ }
return false;
}
Index: source/Commands/CommandObjectTarget.cpp
===
--- source/Commands/CommandObjectTarget.cpp
+++ source/Commands/CommandObjectTarget.cpp
@@ -4236,26 +4236,30 @@
error_set = true;
}
} else {
- if (uuid_option_set) {
-module_spec.GetUUID() =
-m_uuid_option_group.GetOptionValue().GetCurrentValue();
-success |= module_spec.GetUUID().IsValid();
- } else if (file_option_set) {
+ if (file_option_set) {
module_spec.GetFileSpec() =
m_file_option.GetOptionValue().GetCurrentValue();
-ModuleSP module_sp(
-target->GetImages().FindFirstModule(module_spec));
-if (module_sp) {
- module_spec.GetFileSpec() = module_sp->GetFileSpec();
- module_spec.GetPlatformFileSpec() =
- module_sp->GetPlatformFileSpec();
- module_spec.GetUUID() = module_sp->GetUUID();
- module_spec.GetArchitecture() = module_sp->GetArchitecture();
-} else {
+if (uuid_option_set) {
+ module_spec.GetUUID() =
+ m_uuid_option_group.GetOptionValue().GetCurrentValue();
module_spec.GetArchitecture() = target->GetArchitecture();
+} else {
+ ModuleSP module_sp(
+ target->GetImages().FindFirstModule(module_spec));
+ if (module_sp) {
+module_spec.GetFileSpec() = module_sp->GetFileSpec();
+module_spec.GetPlatformFileSpec() =
+module_sp->GetPlatformFileSpec();
+module_spec.GetUUID() = module_sp->GetUUID();
+module_spec.GetArchitecture() = module_sp->GetArchitecture();
+ }
}
success |= module_spec.GetUUID().IsValid() ||
module_spec.GetFileSpec().Exists();
+ } else if (uuid_option_set) {
+module_spec.GetUUID() =
+m_uuid_option_group.GetOptionValue().GetCurrentValue();
+success |= module_spec.GetUUID().IsValid();
}
}
Index: packages/Python/lldbsuite/test/linux/add-symbols/main.c
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/add-symbols/main.c
@@ -0,0 +1,6 @@
+#include
+static int var = 5;
+int main() {
+ printf("%p is %d\n", &var, var);
+ return ++var;
+}
Index: packages/Python/lldbsuite/test/linux/add-symbols/TestTargetSymbolsAddCommand.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/add-symbols/TestTargetSymbolsAddCommand.py
@@ -0,0 +1,52 @@
+""" Testing explicit symbol loading via target symbols add. """
+import os
+import time
+import lldb
+import sys
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TargetSymbolsAddCommand(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+self.source = 'main.c'
+
+@no_debug_info_test # Prevent the genaration of the dwarf version of this test
+@skipUnlessPlatform(['linux'])
+def test_target_symbols_add(self):
+"""Test th
[Lldb-commits] [PATCH] D35607: Extend 'target symbols add' to load symbols from a given file by UUID.
eugene added a comment. In https://reviews.llvm.org/D35607#814560, @labath wrote: > Btw, will this work correctly if we are cross-debugging from a mac? (because > then we will use the fancy DownloadObjectAndSymbolFile implementation) Yeah, you're right. I'll move this logic to the CommandObjectTarget.cpp itself. https://reviews.llvm.org/D35607 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D35607: Extend 'target symbols add' to load symbols from a given file by UUID.
eugene added a comment. In https://reviews.llvm.org/D35607#814982, @clayborg wrote: > So we should just make sure this works: > > (lldb) target symbols add --shlib a.out debug_binaries/a.out > I like this, and I'm going to implement it. Pavel is right and the way UUID is set for modules without build-id is kinda strange, but changing it seems like a big and risky change. Actually "target symbols add" already has logic that if UUID doesn't match it solely uses filenames to match debug symbols, so being able to separately set name for both executable and debug modules seems very logical. https://reviews.llvm.org/D35607 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D35607: Extend 'target symbols add' to set symbol file for a given module
eugene updated this revision to Diff 107628.
eugene retitled this revision from "Extend 'target symbols add' to load symbols
from a given file by UUID." to "Extend 'target symbols add' to set symbol file
for a given module".
eugene edited the summary of this revision.
eugene added a comment.
Found a way to implement what I need much easily and switched to using file
names instead of UUID.
Please take another look.
https://reviews.llvm.org/D35607
Files:
packages/Python/lldbsuite/test/linux/add-symbols/Makefile
packages/Python/lldbsuite/test/linux/add-symbols/TestTargetSymbolsAddCommand.py
packages/Python/lldbsuite/test/linux/add-symbols/main.c
source/Commands/CommandObjectTarget.cpp
source/Interpreter/CommandObject.cpp
Index: source/Interpreter/CommandObject.cpp
===
--- source/Interpreter/CommandObject.cpp
+++ source/Interpreter/CommandObject.cpp
@@ -57,7 +57,7 @@
llvm::StringRef CommandObject::GetHelpLong() { return m_cmd_help_long; }
llvm::StringRef CommandObject::GetSyntax() {
- if (m_cmd_syntax.empty())
+ if (!m_cmd_syntax.empty())
return m_cmd_syntax;
StreamString syntax_str;
Index: source/Commands/CommandObjectTarget.cpp
===
--- source/Commands/CommandObjectTarget.cpp
+++ source/Commands/CommandObjectTarget.cpp
@@ -3969,7 +3969,8 @@
"Add a debug symbol file to one of the target's current modules by "
"specifying a path to a debug symbols file, or using the options "
"to specify a module to download symbols for.",
-"target symbols add []", eCommandRequiresTarget),
+"target symbols add []",
+eCommandRequiresTarget),
m_option_group(),
m_file_option(
LLDB_OPT_SET_1, false, "shlib", 's',
@@ -4289,9 +4290,6 @@
if (uuid_option_set) {
result.AppendError("specify either one or more paths to symbol files "
"or use the --uuid option without arguments");
- } else if (file_option_set) {
-result.AppendError("specify either one or more paths to symbol files "
- "or use the --file option without arguments");
} else if (frame_option_set) {
result.AppendError("specify either one or more paths to symbol files "
"or use the --frame option without arguments");
@@ -4301,6 +4299,10 @@
for (auto &entry : args.entries()) {
if (!entry.ref.empty()) {
module_spec.GetSymbolFileSpec().SetFile(entry.ref, true);
+if (file_option_set) {
+ module_spec.GetFileSpec() =
+ m_file_option.GetOptionValue().GetCurrentValue();
+}
if (platform_sp) {
FileSpec symfile_spec;
if (platform_sp
Index: packages/Python/lldbsuite/test/linux/add-symbols/main.c
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/add-symbols/main.c
@@ -0,0 +1,6 @@
+#include
+static int var = 5;
+int main() {
+ printf("%p is %d\n", &var, var);
+ return ++var;
+}
Index: packages/Python/lldbsuite/test/linux/add-symbols/TestTargetSymbolsAddCommand.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/add-symbols/TestTargetSymbolsAddCommand.py
@@ -0,0 +1,52 @@
+""" Testing explicit symbol loading via target symbols add. """
+import os
+import time
+import lldb
+import sys
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TargetSymbolsAddCommand(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+self.source = 'main.c'
+
+@no_debug_info_test # Prevent the genaration of the dwarf version of this test
+@skipUnlessPlatform(['linux'])
+def test_target_symbols_add(self):
+"""Test that 'target symbols add' can load the symbols
+even if gnu.build-id and gnu_debuglink are not present in the module.
+Similar to test_add_dsym_mid_execution test for macos."""
+self.build(clean=True)
+exe = os.path.join(os.getcwd(), "stripped.out")
+
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+
+main_bp = self.target.BreakpointCreateByName("main", "stripped.out")
+self.assertTrue(main_bp, VALID_BREAKPOINT)
+
+self.process = self.target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertTrue(self.process, PROCESS_IS_VALID)
+
+# The stop reason of the thread should be breakpoint.
+self.assertTrue(self.process.GetState() == lldb.eStateStopped,
+STOPPED_DUE_TO_BREAKPOINT)
+
+exe_m
[Lldb-commits] [PATCH] D35607: Extend 'target symbols add' to set symbol file for a given module
eugene updated this revision to Diff 107725.
eugene added a comment.
Added error message when more than one symbol file is provided.
https://reviews.llvm.org/D35607
Files:
packages/Python/lldbsuite/test/linux/add-symbols/Makefile
packages/Python/lldbsuite/test/linux/add-symbols/TestTargetSymbolsAddCommand.py
packages/Python/lldbsuite/test/linux/add-symbols/main.c
source/Commands/CommandObjectTarget.cpp
source/Interpreter/CommandObject.cpp
Index: source/Interpreter/CommandObject.cpp
===
--- source/Interpreter/CommandObject.cpp
+++ source/Interpreter/CommandObject.cpp
@@ -57,7 +57,7 @@
llvm::StringRef CommandObject::GetHelpLong() { return m_cmd_help_long; }
llvm::StringRef CommandObject::GetSyntax() {
- if (m_cmd_syntax.empty())
+ if (!m_cmd_syntax.empty())
return m_cmd_syntax;
StreamString syntax_str;
Index: source/Commands/CommandObjectTarget.cpp
===
--- source/Commands/CommandObjectTarget.cpp
+++ source/Commands/CommandObjectTarget.cpp
@@ -3969,7 +3969,8 @@
"Add a debug symbol file to one of the target's current modules by "
"specifying a path to a debug symbols file, or using the options "
"to specify a module to download symbols for.",
-"target symbols add []", eCommandRequiresTarget),
+"target symbols add []",
+eCommandRequiresTarget),
m_option_group(),
m_file_option(
LLDB_OPT_SET_1, false, "shlib", 's',
@@ -4289,18 +4290,22 @@
if (uuid_option_set) {
result.AppendError("specify either one or more paths to symbol files "
"or use the --uuid option without arguments");
- } else if (file_option_set) {
-result.AppendError("specify either one or more paths to symbol files "
- "or use the --file option without arguments");
} else if (frame_option_set) {
result.AppendError("specify either one or more paths to symbol files "
"or use the --frame option without arguments");
+ } else if (file_option_set && argc > 1) {
+result.AppendError("specify at most one symbol file path when "
+ "--shlib option is set");
} else {
PlatformSP platform_sp(target->GetPlatform());
for (auto &entry : args.entries()) {
if (!entry.ref.empty()) {
module_spec.GetSymbolFileSpec().SetFile(entry.ref, true);
+if (file_option_set) {
+ module_spec.GetFileSpec() =
+ m_file_option.GetOptionValue().GetCurrentValue();
+}
if (platform_sp) {
FileSpec symfile_spec;
if (platform_sp
Index: packages/Python/lldbsuite/test/linux/add-symbols/main.c
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/add-symbols/main.c
@@ -0,0 +1,6 @@
+#include
+static int var = 5;
+int main() {
+ printf("%p is %d\n", &var, var);
+ return ++var;
+}
Index: packages/Python/lldbsuite/test/linux/add-symbols/TestTargetSymbolsAddCommand.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/add-symbols/TestTargetSymbolsAddCommand.py
@@ -0,0 +1,52 @@
+""" Testing explicit symbol loading via target symbols add. """
+import os
+import time
+import lldb
+import sys
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TargetSymbolsAddCommand(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+self.source = 'main.c'
+
+@no_debug_info_test # Prevent the genaration of the dwarf version of this test
+@skipUnlessPlatform(['linux'])
+def test_target_symbols_add(self):
+"""Test that 'target symbols add' can load the symbols
+even if gnu.build-id and gnu_debuglink are not present in the module.
+Similar to test_add_dsym_mid_execution test for macos."""
+self.build(clean=True)
+exe = os.path.join(os.getcwd(), "stripped.out")
+
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+
+main_bp = self.target.BreakpointCreateByName("main", "stripped.out")
+self.assertTrue(main_bp, VALID_BREAKPOINT)
+
+self.process = self.target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertTrue(self.process, PROCESS_IS_VALID)
+
+# The stop reason of the thread should be breakpoint.
+self.assertTrue(self.process.GetState() == lldb.eStateStopped,
+STOPPED_DUE_TO_BREAKPOINT)
+
+exe_module = self.target.GetModuleAtIndex(0)
+
+# Check t
[Lldb-commits] [PATCH] D35607: Extend 'target symbols add' to set symbol file for a given module
eugene added a comment. Greg, are you with me checking this in? https://reviews.llvm.org/D35607 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D35607: Extend 'target symbols add' to set symbol file for a given module
This revision was automatically updated to reflect the committed changes.
Closed by commit rL308933: Extend 'target symbols add' to load symbols from a
given module (authored by eugene).
Changed prior to commit:
https://reviews.llvm.org/D35607?vs=107725&id=107983#toc
Repository:
rL LLVM
https://reviews.llvm.org/D35607
Files:
lldb/trunk/packages/Python/lldbsuite/test/linux/add-symbols/Makefile
lldb/trunk/packages/Python/lldbsuite/test/linux/add-symbols/TestTargetSymbolsAddCommand.py
lldb/trunk/packages/Python/lldbsuite/test/linux/add-symbols/main.c
lldb/trunk/source/Commands/CommandObjectTarget.cpp
lldb/trunk/source/Interpreter/CommandObject.cpp
Index: lldb/trunk/source/Interpreter/CommandObject.cpp
===
--- lldb/trunk/source/Interpreter/CommandObject.cpp
+++ lldb/trunk/source/Interpreter/CommandObject.cpp
@@ -57,7 +57,7 @@
llvm::StringRef CommandObject::GetHelpLong() { return m_cmd_help_long; }
llvm::StringRef CommandObject::GetSyntax() {
- if (m_cmd_syntax.empty())
+ if (!m_cmd_syntax.empty())
return m_cmd_syntax;
StreamString syntax_str;
Index: lldb/trunk/source/Commands/CommandObjectTarget.cpp
===
--- lldb/trunk/source/Commands/CommandObjectTarget.cpp
+++ lldb/trunk/source/Commands/CommandObjectTarget.cpp
@@ -3969,7 +3969,8 @@
"Add a debug symbol file to one of the target's current modules by "
"specifying a path to a debug symbols file, or using the options "
"to specify a module to download symbols for.",
-"target symbols add []", eCommandRequiresTarget),
+"target symbols add []",
+eCommandRequiresTarget),
m_option_group(),
m_file_option(
LLDB_OPT_SET_1, false, "shlib", 's',
@@ -4289,18 +4290,22 @@
if (uuid_option_set) {
result.AppendError("specify either one or more paths to symbol files "
"or use the --uuid option without arguments");
- } else if (file_option_set) {
-result.AppendError("specify either one or more paths to symbol files "
- "or use the --file option without arguments");
} else if (frame_option_set) {
result.AppendError("specify either one or more paths to symbol files "
"or use the --frame option without arguments");
+ } else if (file_option_set && argc > 1) {
+result.AppendError("specify at most one symbol file path when "
+ "--shlib option is set");
} else {
PlatformSP platform_sp(target->GetPlatform());
for (auto &entry : args.entries()) {
if (!entry.ref.empty()) {
module_spec.GetSymbolFileSpec().SetFile(entry.ref, true);
+if (file_option_set) {
+ module_spec.GetFileSpec() =
+ m_file_option.GetOptionValue().GetCurrentValue();
+}
if (platform_sp) {
FileSpec symfile_spec;
if (platform_sp
Index: lldb/trunk/packages/Python/lldbsuite/test/linux/add-symbols/main.c
===
--- lldb/trunk/packages/Python/lldbsuite/test/linux/add-symbols/main.c
+++ lldb/trunk/packages/Python/lldbsuite/test/linux/add-symbols/main.c
@@ -0,0 +1,6 @@
+#include
+static int var = 5;
+int main() {
+ printf("%p is %d\n", &var, var);
+ return ++var;
+}
Index: lldb/trunk/packages/Python/lldbsuite/test/linux/add-symbols/TestTargetSymbolsAddCommand.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/linux/add-symbols/TestTargetSymbolsAddCommand.py
+++ lldb/trunk/packages/Python/lldbsuite/test/linux/add-symbols/TestTargetSymbolsAddCommand.py
@@ -0,0 +1,52 @@
+""" Testing explicit symbol loading via target symbols add. """
+import os
+import time
+import lldb
+import sys
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TargetSymbolsAddCommand(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+self.source = 'main.c'
+
+@no_debug_info_test # Prevent the genaration of the dwarf version of this test
+@skipUnlessPlatform(['linux'])
+def test_target_symbols_add(self):
+"""Test that 'target symbols add' can load the symbols
+even if gnu.build-id and gnu_debuglink are not present in the module.
+Similar to test_add_dsym_mid_execution test for macos."""
+self.build(clean=True)
+exe = os.path.join(os.getcwd(), "stripped.out")
+
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+
+main_bp = self.target.BreakpointCreateByName("main", "stripped.out")
+self.assertTrue(m
[Lldb-commits] [PATCH] D36126: Fix incorrect use of std::unique
eugene created this revision.
eugene added a project: LLDB.
https://reviews.llvm.org/D36126
Files:
source/Symbol/Symtab.cpp
Index: source/Symbol/Symtab.cpp
===
--- source/Symbol/Symtab.cpp
+++ source/Symbol/Symtab.cpp
@@ -616,8 +616,10 @@
std::stable_sort(indexes.begin(), indexes.end(), comparator);
// Remove any duplicates if requested
- if (remove_duplicates)
-std::unique(indexes.begin(), indexes.end());
+ if (remove_duplicates) {
+auto last = std::unique(indexes.begin(), indexes.end());
+indexes.erase(last, indexes.end());
+ }
}
uint32_t Symtab::AppendSymbolIndexesWithName(const ConstString &symbol_name,
Index: source/Symbol/Symtab.cpp
===
--- source/Symbol/Symtab.cpp
+++ source/Symbol/Symtab.cpp
@@ -616,8 +616,10 @@
std::stable_sort(indexes.begin(), indexes.end(), comparator);
// Remove any duplicates if requested
- if (remove_duplicates)
-std::unique(indexes.begin(), indexes.end());
+ if (remove_duplicates) {
+auto last = std::unique(indexes.begin(), indexes.end());
+indexes.erase(last, indexes.end());
+ }
}
uint32_t Symtab::AppendSymbolIndexesWithName(const ConstString &symbol_name,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D36126: Fix incorrect use of std::unique
eugene added a comment. clang drew my attention to it: Symtab.cpp:620:5: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result] std::unique(indexes.begin(), indexes.end()); ^~~ ~~ https://reviews.llvm.org/D36126 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D36126: Fix incorrect use of std::unique
eugene closed this revision. eugene added a comment. Checked in as r309648 https://reviews.llvm.org/D36126 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33213: Use socketpair on all Unix platforms
eugene accepted this revision. eugene added a comment. LGTM. Test run on Linux is clear. I also see a bit of perf bump. Before checking in please remove outdated comment from ProcessGDBRemote::LaunchAndConnectToDebugserver. // Use a socketpair on Apple for now until other platforms can verify it // works and is fast enough https://reviews.llvm.org/D33213 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D36804: Add initial support to PowerPC64 little endian (POWER8)
This revision was automatically updated to reflect the committed changes.
Closed by commit rL312151: Now a ppc64le binary is correctly detected:
(authored by eugene).
Repository:
rL LLVM
https://reviews.llvm.org/D36804
Files:
lldb/trunk/include/lldb/Core/ArchSpec.h
lldb/trunk/source/Core/ArchSpec.cpp
Index: lldb/trunk/source/Core/ArchSpec.cpp
===
--- lldb/trunk/source/Core/ArchSpec.cpp
+++ lldb/trunk/source/Core/ArchSpec.cpp
@@ -188,6 +188,8 @@
{eByteOrderBig, 4, 4, 4, llvm::Triple::ppc, ArchSpec::eCore_ppc_ppc970,
"ppc970"},
+{eByteOrderLittle, 8, 4, 4, llvm::Triple::ppc64le,
+ ArchSpec::eCore_ppc64le_generic, "powerpc64le"},
{eByteOrderBig, 8, 4, 4, llvm::Triple::ppc64,
ArchSpec::eCore_ppc64_generic,
"powerpc64"},
{eByteOrderBig, 8, 4, 4, llvm::Triple::ppc64,
@@ -414,6 +416,8 @@
0xu, 0xu}, // Intel MCU // FIXME: is this correct?
{ArchSpec::eCore_ppc_generic, llvm::ELF::EM_PPC, LLDB_INVALID_CPUTYPE,
0xu, 0xu}, // PowerPC
+{ArchSpec::eCore_ppc64le_generic, llvm::ELF::EM_PPC64,
LLDB_INVALID_CPUTYPE,
+ 0xu, 0xu}, // PowerPC64le
{ArchSpec::eCore_ppc64_generic, llvm::ELF::EM_PPC64, LLDB_INVALID_CPUTYPE,
0xu, 0xu}, // PowerPC64
{ArchSpec::eCore_arm_generic, llvm::ELF::EM_ARM, LLDB_INVALID_CPUTYPE,
Index: lldb/trunk/include/lldb/Core/ArchSpec.h
===
--- lldb/trunk/include/lldb/Core/ArchSpec.h
+++ lldb/trunk/include/lldb/Core/ArchSpec.h
@@ -177,6 +177,7 @@
eCore_ppc_ppc7450,
eCore_ppc_ppc970,
+eCore_ppc64le_generic,
eCore_ppc64_generic,
eCore_ppc64_ppc970_64,
Index: lldb/trunk/source/Core/ArchSpec.cpp
===
--- lldb/trunk/source/Core/ArchSpec.cpp
+++ lldb/trunk/source/Core/ArchSpec.cpp
@@ -188,6 +188,8 @@
{eByteOrderBig, 4, 4, 4, llvm::Triple::ppc, ArchSpec::eCore_ppc_ppc970,
"ppc970"},
+{eByteOrderLittle, 8, 4, 4, llvm::Triple::ppc64le,
+ ArchSpec::eCore_ppc64le_generic, "powerpc64le"},
{eByteOrderBig, 8, 4, 4, llvm::Triple::ppc64, ArchSpec::eCore_ppc64_generic,
"powerpc64"},
{eByteOrderBig, 8, 4, 4, llvm::Triple::ppc64,
@@ -414,6 +416,8 @@
0xu, 0xu}, // Intel MCU // FIXME: is this correct?
{ArchSpec::eCore_ppc_generic, llvm::ELF::EM_PPC, LLDB_INVALID_CPUTYPE,
0xu, 0xu}, // PowerPC
+{ArchSpec::eCore_ppc64le_generic, llvm::ELF::EM_PPC64, LLDB_INVALID_CPUTYPE,
+ 0xu, 0xu}, // PowerPC64le
{ArchSpec::eCore_ppc64_generic, llvm::ELF::EM_PPC64, LLDB_INVALID_CPUTYPE,
0xu, 0xu}, // PowerPC64
{ArchSpec::eCore_arm_generic, llvm::ELF::EM_ARM, LLDB_INVALID_CPUTYPE,
Index: lldb/trunk/include/lldb/Core/ArchSpec.h
===
--- lldb/trunk/include/lldb/Core/ArchSpec.h
+++ lldb/trunk/include/lldb/Core/ArchSpec.h
@@ -177,6 +177,7 @@
eCore_ppc_ppc7450,
eCore_ppc_ppc970,
+eCore_ppc64le_generic,
eCore_ppc64_generic,
eCore_ppc64_ppc970_64,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33213: Use socketpair on all Unix platforms
eugene accepted this revision. eugene added a comment. I did mark it Accepted. https://reviews.llvm.org/D33213 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33213: Use socketpair on all Unix platforms
This revision was automatically updated to reflect the committed changes.
Closed by commit rL314127: Use socketpair on all Unix platforms (authored by
eugene).
Repository:
rL LLVM
https://reviews.llvm.org/D33213
Files:
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
Index: lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
===
--- lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
+++ lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
@@ -106,6 +106,7 @@
// than llgs listening for a connection from address on port.
{"setsid", no_argument, NULL,
'S'}, // Call setsid() to make llgs run in its own session.
+{"fd", required_argument, NULL, 'F'},
{NULL, 0, NULL, 0}};
//--
@@ -132,13 +133,13 @@
"[--log-file log-file-name] "
"[--log-channels log-channel-list] "
"[--setsid] "
+ "[--fd file-descriptor]"
"[--named-pipe named-pipe-path] "
"[--native-regs] "
"[--attach pid] "
"[[HOST]:PORT] "
"[-- PROGRAM ARG1 ARG2 ...]\n",
progname, subcommand);
- exit(0);
}
void handle_attach_to_pid(GDBRemoteCommunicationServerLLGS &gdb_server,
@@ -232,10 +233,34 @@
GDBRemoteCommunicationServerLLGS &gdb_server,
bool reverse_connect, const char *const host_and_port,
const char *const progname, const char *const subcommand,
- const char *const named_pipe_path, int unnamed_pipe_fd) {
+ const char *const named_pipe_path, int unnamed_pipe_fd,
+ int connection_fd) {
Status error;
- if (host_and_port && host_and_port[0]) {
+ std::unique_ptr connection_up;
+ if (connection_fd != -1) {
+// Build the connection string.
+char connection_url[512];
+snprintf(connection_url, sizeof(connection_url), "fd://%d", connection_fd);
+
+// Create the connection.
+#if !defined LLDB_DISABLE_POSIX && !defined _WIN32
+::fcntl(connection_fd, F_SETFD, FD_CLOEXEC);
+#endif
+connection_up.reset(new ConnectionFileDescriptor);
+auto connection_result = connection_up->Connect(connection_url, &error);
+if (connection_result != eConnectionStatusSuccess) {
+ fprintf(stderr, "error: failed to connect to client at '%s' "
+ "(connection status: %d)",
+ connection_url, static_cast(connection_result));
+ exit(-1);
+}
+if (error.Fail()) {
+ fprintf(stderr, "error: failed to connect to client at '%s': %s",
+ connection_url, error.AsCString());
+ exit(-1);
+}
+ } else if (host_and_port && host_and_port[0]) {
// Parse out host and port.
std::string final_host_and_port;
std::string connection_host;
@@ -255,7 +280,6 @@
connection_portno = StringConvert::ToUInt32(connection_port.c_str(), 0);
}
-std::unique_ptr connection_up;
if (reverse_connect) {
// llgs will connect to the gdb-remote client.
@@ -328,14 +352,14 @@
}
connection_up.reset(conn);
}
-error = gdb_server.InitializeConnection(std::move(connection_up));
-if (error.Fail()) {
- fprintf(stderr, "Failed to initialize connection: %s\n",
- error.AsCString());
- exit(-1);
-}
-printf("Connection established.\n");
}
+ error = gdb_server.InitializeConnection(std::move(connection_up));
+ if (error.Fail()) {
+fprintf(stderr, "Failed to initialize connection: %s\n",
+error.AsCString());
+exit(-1);
+ }
+ printf("Connection established.\n");
}
//--
@@ -364,6 +388,7 @@
log_channels; // e.g. "lldb process threads:gdb-remote default:linux all"
int unnamed_pipe_fd = -1;
bool reverse_connect = false;
+ int connection_fd = -1;
// ProcessLaunchInfo launch_info;
ProcessAttachInfo attach_info;
@@ -413,6 +438,10 @@
reverse_connect = true;
break;
+case 'F':
+ connection_fd = StringConvert::ToUInt32(optarg, -1);
+ break;
+
#ifndef _WIN32
case 'S':
// Put llgs into a new session. Terminals group processes
@@ -472,7 +501,8 @@
argc -= optind;
argv += optind;
- if (argc == 0) {
+ if (argc == 0 && connection_fd == -1) {
+fputs("No arguments\n", stderr);
display_usage(progname, subcommand);
exit(255);
}
@@ -501,7 +531,7 @@
ConnectToRemote(mainloop, gdb_server, reverse_connect, host_and_port,
progname, subcommand, named_pipe_path.c_str(),
- unnamed_pipe_fd);
+ unnamed_pipe_fd, connection_fd);
if (!gdb_server.IsConnected()) {
fprintf(stderr, "no
[Lldb-commits] [PATCH] D38323: Enable breakpoints and read/write GPRs for ppc64le
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315008: Enable breakpoints and read/write GPRs for ppc64le
(authored by eugene).
Repository:
rL LLVM
https://reviews.llvm.org/D38323
Files:
lldb/trunk/source/Plugins/Process/Linux/CMakeLists.txt
lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.h
lldb/trunk/source/Plugins/Process/Utility/CMakeLists.txt
lldb/trunk/source/Plugins/Process/Utility/RegisterInfoPOSIX_ppc64le.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterInfoPOSIX_ppc64le.h
lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_ppc64le.h
lldb/trunk/source/Plugins/Process/Utility/lldb-ppc64le-register-enums.h
lldb/trunk/source/Target/Platform.cpp
lldb/trunk/source/Target/Thread.cpp
lldb/trunk/source/Utility/PPC64LE_DWARF_Registers.h
lldb/trunk/source/Utility/PPC64LE_ehframe_Registers.h
Index: lldb/trunk/source/Target/Platform.cpp
===
--- lldb/trunk/source/Target/Platform.cpp
+++ lldb/trunk/source/Target/Platform.cpp
@@ -1878,6 +1878,12 @@
trap_opcode_size = sizeof(g_ppc_opcode);
} break;
+ case llvm::Triple::ppc64le: {
+static const uint8_t g_ppc64le_opcode[] = {0x08, 0x00, 0xe0, 0x7f}; // trap
+trap_opcode = g_ppc64le_opcode;
+trap_opcode_size = sizeof(g_ppc64le_opcode);
+ } break;
+
case llvm::Triple::x86:
case llvm::Triple::x86_64: {
static const uint8_t g_i386_opcode[] = {0xCC};
Index: lldb/trunk/source/Target/Thread.cpp
===
--- lldb/trunk/source/Target/Thread.cpp
+++ lldb/trunk/source/Target/Thread.cpp
@@ -2075,6 +2075,7 @@
case llvm::Triple::mips64el:
case llvm::Triple::ppc:
case llvm::Triple::ppc64:
+case llvm::Triple::ppc64le:
case llvm::Triple::systemz:
case llvm::Triple::hexagon:
m_unwinder_ap.reset(new UnwindLLDB(*this));
Index: lldb/trunk/source/Utility/PPC64LE_DWARF_Registers.h
===
--- lldb/trunk/source/Utility/PPC64LE_DWARF_Registers.h
+++ lldb/trunk/source/Utility/PPC64LE_DWARF_Registers.h
@@ -0,0 +1,63 @@
+//===-- PPC64LE_DWARF_Registers.h ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef utility_PPC64LE_DWARF_Registers_h_
+#define utility_PPC64LE_DWARF_Registers_h_
+
+#include "lldb/lldb-private.h"
+
+namespace ppc64le_dwarf {
+
+enum {
+ dwarf_r0_ppc64le = 0,
+ dwarf_r1_ppc64le,
+ dwarf_r2_ppc64le,
+ dwarf_r3_ppc64le,
+ dwarf_r4_ppc64le,
+ dwarf_r5_ppc64le,
+ dwarf_r6_ppc64le,
+ dwarf_r7_ppc64le,
+ dwarf_r8_ppc64le,
+ dwarf_r9_ppc64le,
+ dwarf_r10_ppc64le,
+ dwarf_r11_ppc64le,
+ dwarf_r12_ppc64le,
+ dwarf_r13_ppc64le,
+ dwarf_r14_ppc64le,
+ dwarf_r15_ppc64le,
+ dwarf_r16_ppc64le,
+ dwarf_r17_ppc64le,
+ dwarf_r18_ppc64le,
+ dwarf_r19_ppc64le,
+ dwarf_r20_ppc64le,
+ dwarf_r21_ppc64le,
+ dwarf_r22_ppc64le,
+ dwarf_r23_ppc64le,
+ dwarf_r24_ppc64le,
+ dwarf_r25_ppc64le,
+ dwarf_r26_ppc64le,
+ dwarf_r27_ppc64le,
+ dwarf_r28_ppc64le,
+ dwarf_r29_ppc64le,
+ dwarf_r30_ppc64le,
+ dwarf_r31_ppc64le,
+ dwarf_lr_ppc64le = 65,
+ dwarf_ctr_ppc64le,
+ dwarf_cr_ppc64le = 68,
+ dwarf_xer_ppc64le = 76,
+ dwarf_pc_ppc64le,
+ dwarf_softe_ppc64le,
+ dwarf_trap_ppc64le,
+ dwarf_origr3_ppc64le,
+ dwarf_msr_ppc64le,
+};
+
+} // namespace ppc64le_dwarf
+
+#endif // utility_PPC64LE_DWARF_Registers_h_
Index: lldb/trunk/source/Utility/PPC64LE_ehframe_Registers.h
===
--- lldb/trunk/source/Utility/PPC64LE_ehframe_Registers.h
+++ lldb/trunk/source/Utility/PPC64LE_ehframe_Registers.h
@@ -0,0 +1,63 @@
+//===-- PPC64LE_ehframe_Registers.h -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef utility_PPC64LE_ehframe_Registers_h_
+#define utility_PPC64LE_ehframe_Registers_h_
+
+// The register numbers used in the eh_frame unwind information.
+// Should be the same as DWARF register numbers.
+
+namespace ppc64le_ehframe {
+
+enum {
+ r0 = 0,
+ r1,
+ r2,
+ r3,
+ r4,
+ r5,
+ r6,
+ r7,
+ r8,
+ r9,
+ r10,
+ r11,
+ r12,
+ r13,
+ r14,
+ r15,
+ r16,
+ r17,
+ r18,
+ r19,
+ r20,
+ r21,
+ r22,
+ r23,
+ r24,
+ r25,
+ r26,
+ r27,
+ r28,
+ r29,
+ r30,
+ r31,
[Lldb-commits] [PATCH] D38938: Logging: provide a way to safely disable logging in a forked process
eugene added a comment. DisableUnlocked methods makes me uneasy. Maybe we can take a log lock before forking and then properly release it in both parent and child processes? https://reviews.llvm.org/D38938 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D38938: Logging: provide a way to safely disable logging in a forked process
eugene accepted this revision.
eugene added inline comments.
This revision is now accepted and ready to land.
Comment at: source/Utility/Log.cpp:333
+
+void Log::DisableLoggingChild() {
+ for (auto &c: *g_channel_map)
A little comment here describing nature of a deadlock if we don't disable
logging would be nice.
https://reviews.llvm.org/D38938
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D30234: Reformat inferior's main.cpp in lldb-server test
eugene created this revision.
eugene added a project: LLDB.
main.cpp is complete mess of tabs and spaces. This change brings it to
compliance with LLVM coding style.
https://reviews.llvm.org/D30234
Files:
packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
Index: packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
===
--- packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
+++ packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
@@ -1,3 +1,12 @@
+//===-- main.cpp *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
#include
#include
#include
@@ -15,22 +24,22 @@
#if defined(__APPLE__)
__OSX_AVAILABLE_STARTING(__MAC_10_6, __IPHONE_3_2)
-int pthread_threadid_np(pthread_t,__uint64_t*);
+int pthread_threadid_np(pthread_t, __uint64_t *);
#elif defined(__linux__)
#include
#endif
-static const char *const RETVAL_PREFIX = "retval:";
-static const char *const SLEEP_PREFIX= "sleep:";
-static const char *const STDERR_PREFIX = "stderr:";
-static const char *const SET_MESSAGE_PREFIX = "set-message:";
-static const char *const PRINT_MESSAGE_COMMAND = "print-message:";
-static const char *const GET_DATA_ADDRESS_PREFIX = "get-data-address-hex:";
-static const char *const GET_STACK_ADDRESS_COMMAND = "get-stack-address-hex:";
-static const char *const GET_HEAP_ADDRESS_COMMAND= "get-heap-address-hex:";
+static const char *const RETVAL_PREFIX = "retval:";
+static const char *const SLEEP_PREFIX = "sleep:";
+static const char *const STDERR_PREFIX = "stderr:";
+static const char *const SET_MESSAGE_PREFIX = "set-message:";
+static const char *const PRINT_MESSAGE_COMMAND = "print-message:";
+static const char *const GET_DATA_ADDRESS_PREFIX = "get-data-address-hex:";
+static const char *const GET_STACK_ADDRESS_COMMAND = "get-stack-address-hex:";
+static const char *const GET_HEAP_ADDRESS_COMMAND = "get-heap-address-hex:";
-static const char *const GET_CODE_ADDRESS_PREFIX = "get-code-address-hex:";
-static const char *const CALL_FUNCTION_PREFIX= "call-function:";
+static const char *const GET_CODE_ADDRESS_PREFIX = "get-code-address-hex:";
+static const char *const CALL_FUNCTION_PREFIX = "call-function:";
static const char *const THREAD_PREFIX = "thread:";
static const char *const THREAD_COMMAND_NEW = "new";
@@ -50,342 +59,301 @@
static volatile char g_c1 = '0';
static volatile char g_c2 = '1';
-static void
-print_thread_id ()
-{
- // Put in the right magic here for your platform to spit out the thread id (tid) that debugserver/lldb-gdbserver would see as a TID.
- // Otherwise, let the else clause print out the unsupported text so that the unit test knows to skip verifying thread ids.
+static void print_thread_id() {
+// Put in the right magic here for your platform to spit out the thread id (tid)
+// that debugserver/lldb-gdbserver would see as a TID. Otherwise, let the else
+// clause print out the unsupported text so that the unit test knows to skip
+// verifying thread ids.
#if defined(__APPLE__)
- __uint64_t tid = 0;
- pthread_threadid_np(pthread_self(), &tid);
- printf ("%" PRIx64, tid);
-#elif defined (__linux__)
- // This is a call to gettid() via syscall.
- printf ("%" PRIx64, static_cast (syscall (__NR_gettid)));
+ __uint64_t tid = 0;
+ pthread_threadid_np(pthread_self(), &tid);
+ printf("%" PRIx64, tid);
+#elif defined(__linux__)
+ // This is a call to gettid() via syscall.
+ printf("%" PRIx64, static_cast(syscall(__NR_gettid)));
#else
- printf("{no-tid-support}");
+ printf("{no-tid-support}");
#endif
}
-static void
-signal_handler (int signo)
-{
- const char *signal_name = nullptr;
- switch (signo)
- {
- case SIGUSR1: signal_name = "SIGUSR1"; break;
- case SIGSEGV: signal_name = "SIGSEGV"; break;
- default: signal_name = nullptr;
- }
-
- // Print notice that we received the signal on a given thread.
- pthread_mutex_lock (&g_print_mutex);
- if (signal_name)
- printf ("received %s on thread id: ", signal_name);
- else
- printf ("received signo %d (%s) on thread id: ", signo, strsignal (signo));
- print_thread_id ();
- printf ("\n");
- pthread_mutex_unlock (&g_print_mutex);
-
- // Reset the signal handler if we're one of the expected signal handlers.
- switch (signo)
- {
-case SIGSEGV:
- if (g_is_segfaulting)
- {
- // Fix up the pointer we're writing to. This needs to happen if nothing intercepts the SIGSEGV
- // (i.e. if somebody runs this from the command line).
- longjmp(g_jump_buffer, 1);
- }
-break;
-case SIGUSR1:
-if (g_is_segfaulti
[Lldb-commits] [PATCH] D30234: Reformat inferior's main.cpp in lldb-server test
eugene added a comment. In https://reviews.llvm.org/D30234#682990, @jmajors wrote: > Do we have any plans to put something like cpplint into effect? First step would be to have warning free build. Then we can turn "warnings as errors" on. https://reviews.llvm.org/D30234 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D30234: Reformat inferior's main.cpp in lldb-server test
eugene updated this revision to Diff 89311.
https://reviews.llvm.org/D30234
Files:
packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
Index: packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
===
--- packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
+++ packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
@@ -1,36 +1,45 @@
-#include
-#include
+//===-- main.cpp *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
#include
#include
-#include
#include
#include
#include
#include
#include
#include
#include
#include
+#include
+#include
+#include
#include
#if defined(__APPLE__)
__OSX_AVAILABLE_STARTING(__MAC_10_6, __IPHONE_3_2)
-int pthread_threadid_np(pthread_t,__uint64_t*);
+int pthread_threadid_np(pthread_t, __uint64_t *);
#elif defined(__linux__)
#include
#endif
-static const char *const RETVAL_PREFIX = "retval:";
-static const char *const SLEEP_PREFIX= "sleep:";
-static const char *const STDERR_PREFIX = "stderr:";
-static const char *const SET_MESSAGE_PREFIX = "set-message:";
-static const char *const PRINT_MESSAGE_COMMAND = "print-message:";
-static const char *const GET_DATA_ADDRESS_PREFIX = "get-data-address-hex:";
-static const char *const GET_STACK_ADDRESS_COMMAND = "get-stack-address-hex:";
-static const char *const GET_HEAP_ADDRESS_COMMAND= "get-heap-address-hex:";
+static const char *const RETVAL_PREFIX = "retval:";
+static const char *const SLEEP_PREFIX = "sleep:";
+static const char *const STDERR_PREFIX = "stderr:";
+static const char *const SET_MESSAGE_PREFIX = "set-message:";
+static const char *const PRINT_MESSAGE_COMMAND = "print-message:";
+static const char *const GET_DATA_ADDRESS_PREFIX = "get-data-address-hex:";
+static const char *const GET_STACK_ADDRESS_COMMAND = "get-stack-address-hex:";
+static const char *const GET_HEAP_ADDRESS_COMMAND = "get-heap-address-hex:";
-static const char *const GET_CODE_ADDRESS_PREFIX = "get-code-address-hex:";
-static const char *const CALL_FUNCTION_PREFIX= "call-function:";
+static const char *const GET_CODE_ADDRESS_PREFIX = "get-code-address-hex:";
+static const char *const CALL_FUNCTION_PREFIX = "call-function:";
static const char *const THREAD_PREFIX = "thread:";
static const char *const THREAD_COMMAND_NEW = "new";
@@ -50,342 +59,299 @@
static volatile char g_c1 = '0';
static volatile char g_c2 = '1';
-static void
-print_thread_id ()
-{
- // Put in the right magic here for your platform to spit out the thread id (tid) that debugserver/lldb-gdbserver would see as a TID.
- // Otherwise, let the else clause print out the unsupported text so that the unit test knows to skip verifying thread ids.
+static void print_thread_id() {
+// Put in the right magic here for your platform to spit out the thread id (tid)
+// that debugserver/lldb-gdbserver would see as a TID. Otherwise, let the else
+// clause print out the unsupported text so that the unit test knows to skip
+// verifying thread ids.
#if defined(__APPLE__)
- __uint64_t tid = 0;
- pthread_threadid_np(pthread_self(), &tid);
- printf ("%" PRIx64, tid);
-#elif defined (__linux__)
- // This is a call to gettid() via syscall.
- printf ("%" PRIx64, static_cast (syscall (__NR_gettid)));
+ __uint64_t tid = 0;
+ pthread_threadid_np(pthread_self(), &tid);
+ printf("%" PRIx64, tid);
+#elif defined(__linux__)
+ // This is a call to gettid() via syscall.
+ printf("%" PRIx64, static_cast(syscall(__NR_gettid)));
#else
- printf("{no-tid-support}");
+ printf("{no-tid-support}");
#endif
}
-static void
-signal_handler (int signo)
-{
- const char *signal_name = nullptr;
- switch (signo)
- {
- case SIGUSR1: signal_name = "SIGUSR1"; break;
- case SIGSEGV: signal_name = "SIGSEGV"; break;
- default: signal_name = nullptr;
- }
-
- // Print notice that we received the signal on a given thread.
- pthread_mutex_lock (&g_print_mutex);
- if (signal_name)
- printf ("received %s on thread id: ", signal_name);
- else
- printf ("received signo %d (%s) on thread id: ", signo, strsignal (signo));
- print_thread_id ();
- printf ("\n");
- pthread_mutex_unlock (&g_print_mutex);
-
- // Reset the signal handler if we're one of the expected signal handlers.
- switch (signo)
- {
-case SIGSEGV:
- if (g_is_segfaulting)
- {
- // Fix up the pointer we're writing to. This needs to happen if nothing intercepts the SIGSEGV
- // (i.e. if somebody runs this from the command line).
- longjmp(g_jump_buffer, 1);
- }
-break;
-case SIGUSR1:
-if (g_is
[Lldb-commits] [PATCH] D30234: Reformat inferior's main.cpp in lldb-server test
eugene updated this revision to Diff 89312.
https://reviews.llvm.org/D30234
Files:
packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
Index: packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
===
--- packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
+++ packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
@@ -1,3 +1,12 @@
+//===-- main.cpp *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
#include
#include
#include
@@ -15,22 +24,22 @@
#if defined(__APPLE__)
__OSX_AVAILABLE_STARTING(__MAC_10_6, __IPHONE_3_2)
-int pthread_threadid_np(pthread_t,__uint64_t*);
+int pthread_threadid_np(pthread_t, __uint64_t *);
#elif defined(__linux__)
#include
#endif
-static const char *const RETVAL_PREFIX = "retval:";
-static const char *const SLEEP_PREFIX= "sleep:";
-static const char *const STDERR_PREFIX = "stderr:";
-static const char *const SET_MESSAGE_PREFIX = "set-message:";
-static const char *const PRINT_MESSAGE_COMMAND = "print-message:";
-static const char *const GET_DATA_ADDRESS_PREFIX = "get-data-address-hex:";
-static const char *const GET_STACK_ADDRESS_COMMAND = "get-stack-address-hex:";
-static const char *const GET_HEAP_ADDRESS_COMMAND= "get-heap-address-hex:";
+static const char *const RETVAL_PREFIX = "retval:";
+static const char *const SLEEP_PREFIX = "sleep:";
+static const char *const STDERR_PREFIX = "stderr:";
+static const char *const SET_MESSAGE_PREFIX = "set-message:";
+static const char *const PRINT_MESSAGE_COMMAND = "print-message:";
+static const char *const GET_DATA_ADDRESS_PREFIX = "get-data-address-hex:";
+static const char *const GET_STACK_ADDRESS_COMMAND = "get-stack-address-hex:";
+static const char *const GET_HEAP_ADDRESS_COMMAND = "get-heap-address-hex:";
-static const char *const GET_CODE_ADDRESS_PREFIX = "get-code-address-hex:";
-static const char *const CALL_FUNCTION_PREFIX= "call-function:";
+static const char *const GET_CODE_ADDRESS_PREFIX = "get-code-address-hex:";
+static const char *const CALL_FUNCTION_PREFIX = "call-function:";
static const char *const THREAD_PREFIX = "thread:";
static const char *const THREAD_COMMAND_NEW = "new";
@@ -50,342 +59,301 @@
static volatile char g_c1 = '0';
static volatile char g_c2 = '1';
-static void
-print_thread_id ()
-{
- // Put in the right magic here for your platform to spit out the thread id (tid) that debugserver/lldb-gdbserver would see as a TID.
- // Otherwise, let the else clause print out the unsupported text so that the unit test knows to skip verifying thread ids.
+static void print_thread_id() {
+// Put in the right magic here for your platform to spit out the thread id (tid)
+// that debugserver/lldb-gdbserver would see as a TID. Otherwise, let the else
+// clause print out the unsupported text so that the unit test knows to skip
+// verifying thread ids.
#if defined(__APPLE__)
- __uint64_t tid = 0;
- pthread_threadid_np(pthread_self(), &tid);
- printf ("%" PRIx64, tid);
-#elif defined (__linux__)
- // This is a call to gettid() via syscall.
- printf ("%" PRIx64, static_cast (syscall (__NR_gettid)));
+ __uint64_t tid = 0;
+ pthread_threadid_np(pthread_self(), &tid);
+ printf("%" PRIx64, tid);
+#elif defined(__linux__)
+ // This is a call to gettid() via syscall.
+ printf("%" PRIx64, static_cast(syscall(__NR_gettid)));
#else
- printf("{no-tid-support}");
+ printf("{no-tid-support}");
#endif
}
-static void
-signal_handler (int signo)
-{
- const char *signal_name = nullptr;
- switch (signo)
- {
- case SIGUSR1: signal_name = "SIGUSR1"; break;
- case SIGSEGV: signal_name = "SIGSEGV"; break;
- default: signal_name = nullptr;
- }
-
- // Print notice that we received the signal on a given thread.
- pthread_mutex_lock (&g_print_mutex);
- if (signal_name)
- printf ("received %s on thread id: ", signal_name);
- else
- printf ("received signo %d (%s) on thread id: ", signo, strsignal (signo));
- print_thread_id ();
- printf ("\n");
- pthread_mutex_unlock (&g_print_mutex);
-
- // Reset the signal handler if we're one of the expected signal handlers.
- switch (signo)
- {
-case SIGSEGV:
- if (g_is_segfaulting)
- {
- // Fix up the pointer we're writing to. This needs to happen if nothing intercepts the SIGSEGV
- // (i.e. if somebody runs this from the command line).
- longjmp(g_jump_buffer, 1);
- }
-break;
-case SIGUSR1:
-if (g_is_segfaulting)
-{
-// Fix up the pointer we're writing to. This is used to test gdb remote signal delivery.
[Lldb-commits] [PATCH] D30234: Reformat inferior's main.cpp in lldb-server test
eugene added a comment. In https://reviews.llvm.org/D30234#683018, @jingham wrote: > Anyway, it might be better just to do that to this file using the top level > .clang-format. Note that you are making several choices which were not the > choices made by clang-format using the .clang-format file that was used to > implement this reformatting. We probably shouldn't revise that decision > piecemeal. It wasn't my intention to revise any formatting decisions made earlier. I just ran 'clang-format -style=file -i main.cpp' assuming that it will actually make the file complaint with the lldb's coding style by using top level .clang-format. Is there any document other than http://llvm.org/docs/CodingStandards.html describing LLDB coding conventions? https://reviews.llvm.org/D30234 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D30234: Reformat inferior's main.cpp in lldb-server test
eugene marked an inline comment as done.
eugene added inline comments.
Comment at: packages/Python/lldbsuite/test/tools/lldb-server/main.cpp:32-42
+static const char *const RETVAL_PREFIX = "retval:";
+static const char *const SLEEP_PREFIX = "sleep:";
+static const char *const STDERR_PREFIX = "stderr:";
+static const char *const SET_MESSAGE_PREFIX = "set-message:";
+static const char *const PRINT_MESSAGE_COMMAND = "print-message:";
+static const char *const GET_DATA_ADDRESS_PREFIX = "get-data-address-hex:";
+static const char *const GET_STACK_ADDRESS_COMMAND = "get-stack-address-hex:";
jingham wrote:
> This change seems a shame, the original was much easier to read.
Agree. But I think consistency is better than beauty in case of code formatting.
Comment at: packages/Python/lldbsuite/test/tools/lldb-server/main.cpp:62
-static void
-print_thread_id ()
-{
- // Put in the right magic here for your platform to spit out the thread
id (tid) that debugserver/lldb-gdbserver would see as a TID.
- // Otherwise, let the else clause print out the unsupported text so
that the unit test knows to skip verifying thread ids.
+static void print_thread_id() {
+// Put in the right magic here for your platform to spit out the thread id
(tid)
jingham wrote:
> clang-format moved the initial { for functions into the function definition
> line universally when it was run over the lldb sources. If we want to revise
> that decision, and go back to the initial function curly starting a line,
> that would be fine by me, but I don't think we should do it piecemeal.
>
> Ditto for separating the return type & function name onto separate lines.
> That was the way we did it originally, but the clang-format style that was
> chosen for the reformatting undid that. I much prefer the way you changed it
> to here, but that's a decision we should make globally, not file by file.
Could you please elaborate.
I just ran 'clang-format -style=file -i main.cpp' assuming that it will
actually make the file complaint with the lldb's coding style by using top
level .clang-format.
https://reviews.llvm.org/D30234
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D30234: Reformat inferior's main.cpp in lldb-server test
eugene updated this revision to Diff 89404.
eugene added a comment.
I have added local .clang-format file. But it didn't change the way main.cpp
was formated.
https://reviews.llvm.org/D30234
Files:
packages/Python/lldbsuite/test/tools/lldb-server/.clang-format
packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
Index: packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
===
--- packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
+++ packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
@@ -1,3 +1,12 @@
+//===-- main.cpp *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
#include
#include
#include
@@ -15,22 +24,22 @@
#if defined(__APPLE__)
__OSX_AVAILABLE_STARTING(__MAC_10_6, __IPHONE_3_2)
-int pthread_threadid_np(pthread_t,__uint64_t*);
+int pthread_threadid_np(pthread_t, __uint64_t *);
#elif defined(__linux__)
#include
#endif
-static const char *const RETVAL_PREFIX = "retval:";
-static const char *const SLEEP_PREFIX= "sleep:";
-static const char *const STDERR_PREFIX = "stderr:";
-static const char *const SET_MESSAGE_PREFIX = "set-message:";
-static const char *const PRINT_MESSAGE_COMMAND = "print-message:";
-static const char *const GET_DATA_ADDRESS_PREFIX = "get-data-address-hex:";
-static const char *const GET_STACK_ADDRESS_COMMAND = "get-stack-address-hex:";
-static const char *const GET_HEAP_ADDRESS_COMMAND= "get-heap-address-hex:";
+static const char *const RETVAL_PREFIX = "retval:";
+static const char *const SLEEP_PREFIX = "sleep:";
+static const char *const STDERR_PREFIX = "stderr:";
+static const char *const SET_MESSAGE_PREFIX = "set-message:";
+static const char *const PRINT_MESSAGE_COMMAND = "print-message:";
+static const char *const GET_DATA_ADDRESS_PREFIX = "get-data-address-hex:";
+static const char *const GET_STACK_ADDRESS_COMMAND = "get-stack-address-hex:";
+static const char *const GET_HEAP_ADDRESS_COMMAND = "get-heap-address-hex:";
-static const char *const GET_CODE_ADDRESS_PREFIX = "get-code-address-hex:";
-static const char *const CALL_FUNCTION_PREFIX= "call-function:";
+static const char *const GET_CODE_ADDRESS_PREFIX = "get-code-address-hex:";
+static const char *const CALL_FUNCTION_PREFIX = "call-function:";
static const char *const THREAD_PREFIX = "thread:";
static const char *const THREAD_COMMAND_NEW = "new";
@@ -50,342 +59,301 @@
static volatile char g_c1 = '0';
static volatile char g_c2 = '1';
-static void
-print_thread_id ()
-{
- // Put in the right magic here for your platform to spit out the thread id (tid) that debugserver/lldb-gdbserver would see as a TID.
- // Otherwise, let the else clause print out the unsupported text so that the unit test knows to skip verifying thread ids.
+static void print_thread_id() {
+// Put in the right magic here for your platform to spit out the thread id (tid)
+// that debugserver/lldb-gdbserver would see as a TID. Otherwise, let the else
+// clause print out the unsupported text so that the unit test knows to skip
+// verifying thread ids.
#if defined(__APPLE__)
- __uint64_t tid = 0;
- pthread_threadid_np(pthread_self(), &tid);
- printf ("%" PRIx64, tid);
-#elif defined (__linux__)
- // This is a call to gettid() via syscall.
- printf ("%" PRIx64, static_cast (syscall (__NR_gettid)));
+ __uint64_t tid = 0;
+ pthread_threadid_np(pthread_self(), &tid);
+ printf("%" PRIx64, tid);
+#elif defined(__linux__)
+ // This is a call to gettid() via syscall.
+ printf("%" PRIx64, static_cast(syscall(__NR_gettid)));
#else
- printf("{no-tid-support}");
+ printf("{no-tid-support}");
#endif
}
-static void
-signal_handler (int signo)
-{
- const char *signal_name = nullptr;
- switch (signo)
- {
- case SIGUSR1: signal_name = "SIGUSR1"; break;
- case SIGSEGV: signal_name = "SIGSEGV"; break;
- default: signal_name = nullptr;
- }
-
- // Print notice that we received the signal on a given thread.
- pthread_mutex_lock (&g_print_mutex);
- if (signal_name)
- printf ("received %s on thread id: ", signal_name);
- else
- printf ("received signo %d (%s) on thread id: ", signo, strsignal (signo));
- print_thread_id ();
- printf ("\n");
- pthread_mutex_unlock (&g_print_mutex);
-
- // Reset the signal handler if we're one of the expected signal handlers.
- switch (signo)
- {
-case SIGSEGV:
- if (g_is_segfaulting)
- {
- // Fix up the pointer we're writing to. This needs to happen if nothing intercepts the SIGSEGV
- // (i.e. if somebody runs this from the command line).
- longjmp(g_jump_buffer, 1);
- }
-bre
[Lldb-commits] [PATCH] D30286: Implement QPassSignals GDB package in lldb-server
eugene created this revision.
eugene added a project: LLDB.
Herald added a subscriber: emaste.
https://reviews.llvm.org/D30286
Files:
include/lldb/Host/common/NativeProcessProtocol.h
packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
packages/Python/lldbsuite/test/tools/lldb-server/signal-filtering/Makefile
packages/Python/lldbsuite/test/tools/lldb-server/signal-filtering/TestGdbRemote_QPassSignals.py
packages/Python/lldbsuite/test/tools/lldb-server/signal-filtering/main.cpp
source/Host/common/NativeProcessProtocol.cpp
source/Plugins/Process/Linux/NativeProcessLinux.cpp
source/Plugins/Process/POSIX/CrashReason.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
source/Utility/StringExtractorGDBRemote.cpp
source/Utility/StringExtractorGDBRemote.h
Index: source/Utility/StringExtractorGDBRemote.h
===
--- source/Utility/StringExtractorGDBRemote.h
+++ source/Utility/StringExtractorGDBRemote.h
@@ -96,6 +96,7 @@
// debug server packages
eServerPacketType_QEnvironmentHexEncoded,
eServerPacketType_QListThreadsInStopReply,
+eServerPacketType_QPassSignals,
eServerPacketType_QRestoreRegisterState,
eServerPacketType_QSaveRegisterState,
eServerPacketType_QSetLogging,
Index: source/Utility/StringExtractorGDBRemote.cpp
===
--- source/Utility/StringExtractorGDBRemote.cpp
+++ source/Utility/StringExtractorGDBRemote.cpp
@@ -91,6 +91,10 @@
return eServerPacketType_QEnvironmentHexEncoded;
break;
+case 'P':
+ if (PACKET_STARTS_WITH("QPassSignals:"))
+return eServerPacketType_QPassSignals;
+
case 'S':
if (PACKET_MATCHES("QStartNoAckMode"))
return eServerPacketType_QStartNoAckMode;
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -203,6 +203,8 @@
PacketResult Handle_qFileLoadAddress(StringExtractorGDBRemote &packet);
+ PacketResult Handle_QPassSignals(StringExtractorGDBRemote &packet);
+
void SetCurrentThreadID(lldb::tid_t tid);
lldb::tid_t GetCurrentThreadID() const;
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -181,6 +181,9 @@
&GDBRemoteCommunicationServerLLGS::Handle_Z);
RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_z,
&GDBRemoteCommunicationServerLLGS::Handle_z);
+ RegisterMemberFunctionHandler(
+ StringExtractorGDBRemote::eServerPacketType_QPassSignals,
+ &GDBRemoteCommunicationServerLLGS::Handle_QPassSignals);
RegisterPacketHandler(StringExtractorGDBRemote::eServerPacketType_k,
[this](StringExtractorGDBRemote packet, Error &error,
@@ -3072,6 +3075,43 @@
return SendPacketNoLock(response.GetString());
}
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_QPassSignals(
+StringExtractorGDBRemote &packet) {
+ std::vector signals;
+ packet.SetFilePos(strlen("QPassSignals:"));
+
+ // Read sequence of hex signal numbers divided by a semicolon and
+ // optionally spaces.
+ while (packet.GetBytesLeft() > 0) {
+int signal = packet.GetS32(-1, 16);
+if (signal < 0) {
+ return SendIllFormedResponse(packet, "Failed to parse signal number.");
+}
+signals.push_back(signal);
+
+packet.SkipSpaces();
+char separator = packet.GetChar();
+if (separator == '\0') {
+ break; // End of string
+}
+if (separator != ';') {
+ return SendIllFormedResponse(packet, "Invalid separator,"
+" expected semicolon.");
+}
+ }
+
+ // Fail if we don't have a current process.
+ if (!m_debugged_process_sp)
+return SendErrorResponse(68);
+
+ Error error = m_debugged_process_sp->IgnoreSignals(signals);
+ if (error.Fail())
+return SendErrorResponse(69);
+
+ return SendOKResponse();
+}
+
void GDBRemoteCommunicationServerLLGS::MaybeCloseInferiorTerminalConnection() {
Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ so
[Lldb-commits] [PATCH] D30286: Implement QPassSignals GDB package in lldb-server
eugene updated this revision to Diff 89575.
eugene edited the summary of this revision.
eugene added a comment.
Herald added subscribers: srhines, danalbert.
Addressing code review comments.
https://reviews.llvm.org/D30286
Files:
include/lldb/Host/common/NativeProcessProtocol.h
packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
packages/Python/lldbsuite/test/tools/lldb-server/signal-filtering/Makefile
packages/Python/lldbsuite/test/tools/lldb-server/signal-filtering/TestGdbRemote_QPassSignals.py
packages/Python/lldbsuite/test/tools/lldb-server/signal-filtering/main.cpp
source/Host/common/NativeProcessProtocol.cpp
source/Plugins/Process/Linux/NativeProcessLinux.cpp
source/Plugins/Process/POSIX/CrashReason.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
source/Utility/StringExtractorGDBRemote.cpp
source/Utility/StringExtractorGDBRemote.h
Index: source/Utility/StringExtractorGDBRemote.h
===
--- source/Utility/StringExtractorGDBRemote.h
+++ source/Utility/StringExtractorGDBRemote.h
@@ -96,6 +96,7 @@
// debug server packages
eServerPacketType_QEnvironmentHexEncoded,
eServerPacketType_QListThreadsInStopReply,
+eServerPacketType_QPassSignals,
eServerPacketType_QRestoreRegisterState,
eServerPacketType_QSaveRegisterState,
eServerPacketType_QSetLogging,
Index: source/Utility/StringExtractorGDBRemote.cpp
===
--- source/Utility/StringExtractorGDBRemote.cpp
+++ source/Utility/StringExtractorGDBRemote.cpp
@@ -91,6 +91,10 @@
return eServerPacketType_QEnvironmentHexEncoded;
break;
+case 'P':
+ if (PACKET_STARTS_WITH("QPassSignals:"))
+return eServerPacketType_QPassSignals;
+
case 'S':
if (PACKET_MATCHES("QStartNoAckMode"))
return eServerPacketType_QStartNoAckMode;
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -203,6 +203,8 @@
PacketResult Handle_qFileLoadAddress(StringExtractorGDBRemote &packet);
+ PacketResult Handle_QPassSignals(StringExtractorGDBRemote &packet);
+
void SetCurrentThreadID(lldb::tid_t tid);
lldb::tid_t GetCurrentThreadID() const;
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -181,6 +181,9 @@
&GDBRemoteCommunicationServerLLGS::Handle_Z);
RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_z,
&GDBRemoteCommunicationServerLLGS::Handle_z);
+ RegisterMemberFunctionHandler(
+ StringExtractorGDBRemote::eServerPacketType_QPassSignals,
+ &GDBRemoteCommunicationServerLLGS::Handle_QPassSignals);
RegisterPacketHandler(StringExtractorGDBRemote::eServerPacketType_k,
[this](StringExtractorGDBRemote packet, Error &error,
@@ -3072,6 +3075,40 @@
return SendPacketNoLock(response.GetString());
}
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_QPassSignals(
+StringExtractorGDBRemote &packet) {
+ std::vector signals;
+ packet.SetFilePos(strlen("QPassSignals:"));
+
+ // Read sequence of hex signal numbers divided by a semicolon and
+ // optionally spaces.
+ while (packet.GetBytesLeft() > 0) {
+int signal = packet.GetS32(-1, 16);
+if (signal < 0)
+ return SendIllFormedResponse(packet, "Failed to parse signal number.");
+signals.push_back(signal);
+
+packet.SkipSpaces();
+char separator = packet.GetChar();
+if (separator == '\0')
+ break; // End of string
+if (separator != ';')
+ return SendIllFormedResponse(packet, "Invalid separator,"
+" expected semicolon.");
+ }
+
+ // Fail if we don't have a current process.
+ if (!m_debugged_process_sp)
+return SendErrorResponse(68);
+
+ Error error = m_debugged_process_sp->IgnoreSignals(signals);
+ if (error.Fail())
+return SendErrorResponse(69);
+
+ return SendOKResponse();
+}
+
void GDBRemoteCommunicationServerLLGS::MaybeCloseInferiorTerminalConnection() {
Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- source/P
[Lldb-commits] [PATCH] D30457: [LLDB][MIPS] Core Dump Support
eugene added inline comments.
Comment at: source/Plugins/Process/Utility/RegisterContextFreeBSD_mips64.cpp:31
+// Number of register sets provided by this context.
+enum { k_num_register_sets = 1 };
+
Here and below why not use
constexpr size_t k_num_register_sets = 1;
it seems much more straightforward than enum-for-consts trick.
Comment at: source/Plugins/Process/Utility/RegisterContextFreeBSD_mips64.cpp:104
+ return &g_reg_sets_mips64[set];
+ return NULL;
+}
nullptr?
Comment at: source/Plugins/Process/elf-core/ThreadElfCore.h:75
+ else if (!abi.compare("o32"))
+return 96;
+ // N32 ABI
96, 72, 128... where do these number come from?
May be they should be put them into constants and document them somehow.
https://reviews.llvm.org/D30457
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D30520: Make LLDB skip server-client roundtrip for signals that don't require any actions
eugene created this revision.
If QPassSignals packaet is supported by lldb-server, lldb-client will utilize
it and ask the server to ignore signals that don't require stops or
notifications.
Such signals will be immediately re-injected into inferior to continue normal
execution.
https://reviews.llvm.org/D30520
Files:
include/lldb/Target/UnixSignals.h
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
source/Target/UnixSignals.cpp
unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
Index: unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -211,10 +211,11 @@
std::async(std::launch::async,
[&] { return client.GetModulesInfo(file_specs, triple); });
HandlePacket(
- server, "jModulesInfo:["
- R"({"file":"/foo/bar.so","triple":"i386-pc-linux"},)"
- R"({"file":"/foo/baz.so","triple":"i386-pc-linux"},)"
- R"({"file":"/foo/baw.so","triple":"i386-pc-linux"}])",
+ server,
+ "jModulesInfo:["
+ R"({"file":"/foo/bar.so","triple":"i386-pc-linux"},)"
+ R"({"file":"/foo/baz.so","triple":"i386-pc-linux"},)"
+ R"({"file":"/foo/baw.so","triple":"i386-pc-linux"}])",
R"([{"uuid":"404142434445464748494a4b4c4d4e4f","triple":"i386-pc-linux",)"
R"("file_path":"/foo/bar.so","file_offset":0,"file_size":1234}]])");
@@ -314,3 +315,27 @@
<< ss.GetString();
ASSERT_EQ(10, num_packets);
}
+
+TEST_F(GDBRemoteCommunicationClientTest, SendSignalsToIgnore) {
+ TestClient client;
+ MockServer server;
+ Connect(client, server);
+ if (HasFailure())
+return;
+
+ const lldb::tid_t tid = 0x47;
+ const uint32_t reg_num = 4;
+ std::future result = std::async(std::launch::async, [&] {
+return client.SendSignalsToIgnore({2, 3, 5, 7, 0xB, 0xD, 0x11});
+ });
+
+ HandlePacket(server, "QPassSignals:2;3;5;7;B;D;11", "OK");
+ EXPECT_TRUE(result.get().Success());
+
+ result = std::async(std::launch::async, [&] {
+return client.SendSignalsToIgnore(std::vector());
+ });
+
+ HandlePacket(server, "QPassSignals:", "OK");
+ EXPECT_TRUE(result.get().Success());
+}
Index: source/Target/UnixSignals.cpp
===
--- source/Target/UnixSignals.cpp
+++ source/Target/UnixSignals.cpp
@@ -123,12 +123,14 @@
Signal new_signal(name, default_suppress, default_stop, default_notify,
description, alias);
m_signals.insert(std::make_pair(signo, new_signal));
+ ++m_version;
}
void UnixSignals::RemoveSignal(int signo) {
collection::iterator pos = m_signals.find(signo);
if (pos != m_signals.end())
m_signals.erase(pos);
+ ++m_version;
}
const char *UnixSignals::GetSignalAsCString(int signo) const {
@@ -217,6 +219,7 @@
collection::iterator pos = m_signals.find(signo);
if (pos != m_signals.end()) {
pos->second.m_suppress = value;
+++m_version;
return true;
}
return false;
@@ -240,6 +243,7 @@
collection::iterator pos = m_signals.find(signo);
if (pos != m_signals.end()) {
pos->second.m_stop = value;
+++m_version;
return true;
}
return false;
@@ -263,6 +267,7 @@
collection::iterator pos = m_signals.find(signo);
if (pos != m_signals.end()) {
pos->second.m_notify = value;
+++m_version;
return true;
}
return false;
@@ -284,3 +289,7 @@
std::advance(it, index);
return it->first;
}
+
+uint64_t UnixSignals::GetVersion() const {
+ return m_version;
+}
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -403,12 +403,15 @@
//--
std::string m_partial_profile_data;
std::map m_thread_id_to_used_usec_map;
+ uint64_t m_last_signals_version = 0;
static bool NewThreadNotifyBreakpointHit(void *baton,
StoppointCallbackContext *context,
lldb::user_id_t break_id,
lldb::user_id_t break_loc_id);
+ Error SendSignalsToIgnoreIfNeeded();
+
//--
// ContinueDelegate interface
//--
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
-
[Lldb-commits] [PATCH] D30520: Make LLDB skip server-client roundtrip for signals that don't require any actions
eugene updated this revision to Diff 90407.
eugene added a comment.
Addressing code review commends, and moving a signal filtering method to the
base Process class.
https://reviews.llvm.org/D30520
Files:
include/lldb/Target/Process.h
include/lldb/Target/UnixSignals.h
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
source/Target/Process.cpp
source/Target/UnixSignals.cpp
unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
Index: unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -314,3 +314,27 @@
<< ss.GetString();
ASSERT_EQ(10, num_packets);
}
+
+TEST_F(GDBRemoteCommunicationClientTest, SendSignalsToIgnore) {
+ TestClient client;
+ MockServer server;
+ Connect(client, server);
+ if (HasFailure())
+return;
+
+ const lldb::tid_t tid = 0x47;
+ const uint32_t reg_num = 4;
+ std::future result = std::async(std::launch::async, [&] {
+return client.SendSignalsToIgnore({2, 3, 5, 7, 0xB, 0xD, 0x11});
+ });
+
+ HandlePacket(server, "QPassSignals:02;03;05;07;0b;0d;11", "OK");
+ EXPECT_TRUE(result.get().Success());
+
+ result = std::async(std::launch::async, [&] {
+return client.SendSignalsToIgnore(std::vector());
+ });
+
+ HandlePacket(server, "QPassSignals:", "OK");
+ EXPECT_TRUE(result.get().Success());
+}
Index: source/Target/UnixSignals.cpp
===
--- source/Target/UnixSignals.cpp
+++ source/Target/UnixSignals.cpp
@@ -123,12 +123,14 @@
Signal new_signal(name, default_suppress, default_stop, default_notify,
description, alias);
m_signals.insert(std::make_pair(signo, new_signal));
+ ++m_version;
}
void UnixSignals::RemoveSignal(int signo) {
collection::iterator pos = m_signals.find(signo);
if (pos != m_signals.end())
m_signals.erase(pos);
+ ++m_version;
}
const char *UnixSignals::GetSignalAsCString(int signo) const {
@@ -217,6 +219,7 @@
collection::iterator pos = m_signals.find(signo);
if (pos != m_signals.end()) {
pos->second.m_suppress = value;
+++m_version;
return true;
}
return false;
@@ -240,6 +243,7 @@
collection::iterator pos = m_signals.find(signo);
if (pos != m_signals.end()) {
pos->second.m_stop = value;
+++m_version;
return true;
}
return false;
@@ -263,6 +267,7 @@
collection::iterator pos = m_signals.find(signo);
if (pos != m_signals.end()) {
pos->second.m_notify = value;
+++m_version;
return true;
}
return false;
@@ -284,3 +289,5 @@
std::advance(it, index);
return it->first;
}
+
+uint64_t UnixSignals::GetVersion() const { return m_version; }
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -1621,6 +1621,13 @@
return PrivateResume();
}
+Error Process::WillResume() {
+ UpdateAutomaticSignalFiltering();
+ return Error();
+}
+
+void Process::DidLaunch() { UpdateAutomaticSignalFiltering(); }
+
Error Process::ResumeSynchronous(Stream *stream) {
Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_STATE |
LIBLLDB_LOG_PROCESS));
@@ -6217,3 +6224,9 @@
find_it->second->HandleArrivalOfStructuredData(*this, type_name, object_sp);
return true;
}
+
+Error Process::UpdateAutomaticSignalFiltering() {
+ // Default implementation does nothign.
+ // No automatic signal filtering to speak of.
+ return Error();
+}
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -397,12 +397,15 @@
lldb::addr_t base_addr,
bool value_is_offset);
+ Error UpdateAutomaticSignalFiltering() override;
+
private:
//--
// For ProcessGDBRemote only
//--
std::string m_partial_profile_data;
std::map m_thread_id_to_used_usec_map;
+ uint64_t m_last_signals_version = 0;
static bool NewThreadNotifyBreakpointHit(void *baton,
StoppointCallbackContext *context,
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGD
[Lldb-commits] [PATCH] D30520: Make LLDB skip server-client roundtrip for signals that don't require any actions
eugene updated this revision to Diff 90424.
eugene marked an inline comment as done.
eugene added a comment.
Added comment to UnixSignals::m_version.
Still thinking about the best way to test it all.
https://reviews.llvm.org/D30520
Files:
include/lldb/Target/Process.h
include/lldb/Target/UnixSignals.h
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
source/Target/Process.cpp
source/Target/UnixSignals.cpp
unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
Index: unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -314,3 +314,27 @@
<< ss.GetString();
ASSERT_EQ(10, num_packets);
}
+
+TEST_F(GDBRemoteCommunicationClientTest, SendSignalsToIgnore) {
+ TestClient client;
+ MockServer server;
+ Connect(client, server);
+ if (HasFailure())
+return;
+
+ const lldb::tid_t tid = 0x47;
+ const uint32_t reg_num = 4;
+ std::future result = std::async(std::launch::async, [&] {
+return client.SendSignalsToIgnore({2, 3, 5, 7, 0xB, 0xD, 0x11});
+ });
+
+ HandlePacket(server, "QPassSignals:02;03;05;07;0b;0d;11", "OK");
+ EXPECT_TRUE(result.get().Success());
+
+ result = std::async(std::launch::async, [&] {
+return client.SendSignalsToIgnore(std::vector());
+ });
+
+ HandlePacket(server, "QPassSignals:", "OK");
+ EXPECT_TRUE(result.get().Success());
+}
Index: source/Target/UnixSignals.cpp
===
--- source/Target/UnixSignals.cpp
+++ source/Target/UnixSignals.cpp
@@ -123,12 +123,14 @@
Signal new_signal(name, default_suppress, default_stop, default_notify,
description, alias);
m_signals.insert(std::make_pair(signo, new_signal));
+ ++m_version;
}
void UnixSignals::RemoveSignal(int signo) {
collection::iterator pos = m_signals.find(signo);
if (pos != m_signals.end())
m_signals.erase(pos);
+ ++m_version;
}
const char *UnixSignals::GetSignalAsCString(int signo) const {
@@ -217,6 +219,7 @@
collection::iterator pos = m_signals.find(signo);
if (pos != m_signals.end()) {
pos->second.m_suppress = value;
+++m_version;
return true;
}
return false;
@@ -240,6 +243,7 @@
collection::iterator pos = m_signals.find(signo);
if (pos != m_signals.end()) {
pos->second.m_stop = value;
+++m_version;
return true;
}
return false;
@@ -263,6 +267,7 @@
collection::iterator pos = m_signals.find(signo);
if (pos != m_signals.end()) {
pos->second.m_notify = value;
+++m_version;
return true;
}
return false;
@@ -284,3 +289,5 @@
std::advance(it, index);
return it->first;
}
+
+uint64_t UnixSignals::GetVersion() const { return m_version; }
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -1621,6 +1621,15 @@
return PrivateResume();
}
+Error Process::WillResume() {
+ UpdateAutomaticSignalFiltering();
+ return Error();
+}
+
+void Process::DidLaunch() {
+ UpdateAutomaticSignalFiltering();
+}
+
Error Process::ResumeSynchronous(Stream *stream) {
Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_STATE |
LIBLLDB_LOG_PROCESS));
@@ -6217,3 +6226,9 @@
find_it->second->HandleArrivalOfStructuredData(*this, type_name, object_sp);
return true;
}
+
+Error Process::UpdateAutomaticSignalFiltering() {
+ // Default implementation does nothign.
+ // No automatic signal filtering to speak of.
+ return Error();
+}
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -397,12 +397,15 @@
lldb::addr_t base_addr,
bool value_is_offset);
+ Error UpdateAutomaticSignalFiltering() override;
+
private:
//--
// For ProcessGDBRemote only
//--
std::string m_partial_profile_data;
std::map m_thread_id_to_used_usec_map;
+ uint64_t m_last_signals_version = 0;
static bool NewThreadNotifyBreakpointHit(void *baton,
StoppointCallbackContext *context,
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- sour
[Lldb-commits] [PATCH] D30520: Make LLDB skip server-client roundtrip for signals that don't require any actions
eugene added inline comments.
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:3766-3776
+ llvm::SmallVector signals_to_ignore;
+ int32_t signo = m_unix_signals_sp->GetFirstSignalNumber();
+ while (signo != LLDB_INVALID_SIGNAL_NUMBER) {
+bool should_ignore = !m_unix_signals_sp->GetShouldStop(signo) &&
+ !m_unix_signals_sp->GetShouldNotify(signo) &&
+ !m_unix_signals_sp->GetShouldSuppress(signo);
+
jingham wrote:
> This code should go into the UnixSignals class. Any other Process plugin
> that wanted to implement expedited signal handling would also have to do this
> computation, and we shouldn't duplicate the code.
I think it would be a mistake to over-engineer it and try to anticipate needs
of any possible implementation at this point.
Maybe another implementation would require a list of signals that need to be
stopped instead of ignored.
Whenever we have an alternative implementation that needs to do such thins we
can always go back and generalize this code, as for now I think we need to keep
things simple.
I like the concise interface that UnixSignals provides and I don't want to
pollute it with things can be easily done in the code that uses it.
Comment at: source/Target/Process.cpp:1629-1630
+
+void Process::DidLaunch() { UpdateAutomaticSignalFiltering(); }
+
Error Process::ResumeSynchronous(Stream *stream) {
jingham wrote:
> For this to work, you are relying on the Constructor of the signals class to
> set up the object by calling AddSignal on all the signals. Otherwise
> initially m_version in the signals class and m_last_signal_version will start
> out at 0 and you won't prime the filtering here.
>
> You should probably add a comment to that effect somewhere in UnixSignals so
> we don't forget this dependency.
I added a comment to UnixSignals::m_version
https://reviews.llvm.org/D30520
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D30520: Make LLDB skip server-client roundtrip for signals that don't require any actions
eugene updated this revision to Diff 90563.
eugene added a comment.
Herald added a subscriber: mgorny.
Moved signal filtering to UnixSignals and added unit tests for UnixSignals
class.
https://reviews.llvm.org/D30520
Files:
include/lldb/Target/Process.h
include/lldb/Target/UnixSignals.h
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
source/Target/Process.cpp
source/Target/UnixSignals.cpp
unittests/CMakeLists.txt
unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
unittests/Signals/CMakeLists.txt
unittests/Signals/UnixSignalsTest.cpp
Index: unittests/Signals/UnixSignalsTest.cpp
===
--- /dev/null
+++ unittests/Signals/UnixSignalsTest.cpp
@@ -0,0 +1,140 @@
+//===-- UnixSignalsTest.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include
+
+#include "gtest/gtest.h"
+
+#include "lldb/Target/UnixSignals.h"
+#include "llvm/Support/FormatVariadic.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using llvm::None;
+
+class TestSignals : public UnixSignals {
+public:
+ TestSignals() {
+m_signals.clear();
+AddSignal(2, "SIG2", false, true, true, "DESC2");
+AddSignal(4, "SIG4", true, false, true, "DESC4");
+AddSignal(8, "SIG8", true, true, true, "DESC8");
+AddSignal(16, "SIG16", true, false, false, "DESC16");
+ }
+};
+
+void AssertEqArrays(llvm::ArrayRef expected,
+llvm::ArrayRef observed, const char *file,
+int line) {
+ std::string location = llvm::formatv("{0}:{1}", file, line);
+ ASSERT_EQ(expected.size(), observed.size()) << location;
+
+ for (size_t i = 0; i < observed.size(); ++i) {
+ASSERT_EQ(expected[i], observed[i])
+<< "array index: " << i << "location:" << location;
+ }
+}
+
+#define ASSERT_EQ_ARRAYS(expected, observed) \
+ AssertEqArrays((expected), (observed), __FILE__, __LINE__);
+
+TEST(UnixSignalsTest, Iteration) {
+ TestSignals signals;
+
+ EXPECT_EQ(signals.GetNumSignals(), 4);
+ EXPECT_EQ(signals.GetFirstSignalNumber(), 2);
+ EXPECT_EQ(signals.GetNextSignalNumber(2), 4);
+ EXPECT_EQ(signals.GetNextSignalNumber(4), 8);
+ EXPECT_EQ(signals.GetNextSignalNumber(8), 16);
+ EXPECT_EQ(signals.GetNextSignalNumber(16), LLDB_INVALID_SIGNAL_NUMBER);
+}
+
+TEST(UnixSignalsTest, GetInfo) {
+ TestSignals signals;
+
+ bool should_suppress = false, should_stop = false, should_notify = false;
+ int32_t signo = 4;
+ std::string name =
+ signals.GetSignalInfo(signo, should_suppress, should_stop, should_notify);
+ EXPECT_EQ(name, "SIG4");
+ EXPECT_EQ(should_suppress, true);
+ EXPECT_EQ(should_stop, false);
+ EXPECT_EQ(should_notify, true);
+
+ EXPECT_EQ(signals.GetShouldSuppress(signo), true);
+ EXPECT_EQ(signals.GetShouldStop(signo), false);
+ EXPECT_EQ(signals.GetShouldNotify(signo), true);
+ EXPECT_EQ(signals.GetSignalAsCString(signo), name);
+}
+
+TEST(UnixSignalsTest, VersionChange) {
+ TestSignals signals;
+
+ int32_t signo = 8;
+ uint64_t ver = signals.GetVersion();
+ EXPECT_GT(ver, 0ull);
+ EXPECT_EQ(signals.GetShouldSuppress(signo), true);
+ EXPECT_EQ(signals.GetShouldStop(signo), true);
+ EXPECT_EQ(signals.GetShouldNotify(signo), true);
+
+ EXPECT_EQ(ver, signals.GetVersion());
+
+ signals.SetShouldSuppress(signo, false);
+ EXPECT_LT(ver, signals.GetVersion());
+ ver = signals.GetVersion();
+
+ signals.SetShouldStop(signo, true);
+ EXPECT_LT(ver, signals.GetVersion());
+ ver = signals.GetVersion();
+
+ signals.SetShouldNotify(signo, false);
+ EXPECT_LT(ver, signals.GetVersion());
+ ver = signals.GetVersion();
+
+ EXPECT_EQ(signals.GetShouldSuppress(signo), false);
+ EXPECT_EQ(signals.GetShouldStop(signo), true);
+ EXPECT_EQ(signals.GetShouldNotify(signo), false);
+
+ EXPECT_EQ(ver, signals.GetVersion());
+}
+
+TEST(UnixSignalsTest, GetFilteredSignals) {
+ TestSignals signals;
+
+ auto all_signals = signals.GetFilteredSignals(None, None, None);
+ std::vector expected = {2, 4, 8, 16};
+ ASSERT_EQ_ARRAYS(expected, all_signals);
+
+ auto supressed = signals.GetFilteredSignals(true, None, None);
+ expected = {4, 8, 16};
+ ASSERT_EQ_ARRAYS(expected, supressed);
+
+ auto not_supressed = signals.GetFilteredSignals(false, None, None);
+ expected = {2};
+ ASSERT_EQ_ARRAYS(expected, not_supressed);
+
+ auto stopped = signals.GetFilteredSignals(None, true, None);
+ expected = {2, 8};
+ ASSERT_EQ_ARRAYS(expected, stopped);
+
+ auto not_stopped = signals.GetFilteredSignals(No
[Lldb-commits] [PATCH] D30520: Make LLDB skip server-client roundtrip for signals that don't require any actions
eugene updated this revision to Diff 90755.
eugene added a comment.
Addressing review comments on SignalTests and getting rid of dependency on
DidLaunch and WillResume
https://reviews.llvm.org/D30520
Files:
include/lldb/Target/Process.h
include/lldb/Target/UnixSignals.h
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
source/Target/Process.cpp
source/Target/UnixSignals.cpp
unittests/CMakeLists.txt
unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
unittests/Signals/CMakeLists.txt
unittests/Signals/UnixSignalsTest.cpp
Index: unittests/Signals/UnixSignalsTest.cpp
===
--- /dev/null
+++ unittests/Signals/UnixSignalsTest.cpp
@@ -0,0 +1,140 @@
+//===-- UnixSignalsTest.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include
+
+#include "gtest/gtest.h"
+
+#include "lldb/Target/UnixSignals.h"
+#include "llvm/Support/FormatVariadic.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using llvm::None;
+
+class TestSignals : public UnixSignals {
+public:
+ TestSignals() {
+m_signals.clear();
+AddSignal(2, "SIG2", false, true, true, "DESC2");
+AddSignal(4, "SIG4", true, false, true, "DESC4");
+AddSignal(8, "SIG8", true, true, true, "DESC8");
+AddSignal(16, "SIG16", true, false, false, "DESC16");
+ }
+};
+
+void AssertEqArrays(llvm::ArrayRef expected,
+llvm::ArrayRef observed, const char *file,
+int line) {
+ std::string location = llvm::formatv("{0}:{1}", file, line);
+ ASSERT_EQ(expected.size(), observed.size()) << location;
+
+ for (size_t i = 0; i < observed.size(); ++i) {
+ASSERT_EQ(expected[i], observed[i])
+<< "array index: " << i << "location:" << location;
+ }
+}
+
+#define ASSERT_EQ_ARRAYS(expected, observed) \
+ AssertEqArrays((expected), (observed), __FILE__, __LINE__);
+
+TEST(UnixSignalsTest, Iteration) {
+ TestSignals signals;
+
+ EXPECT_EQ(4, signals.GetNumSignals());
+ EXPECT_EQ(2, signals.GetFirstSignalNumber());
+ EXPECT_EQ(4, signals.GetNextSignalNumber(2));
+ EXPECT_EQ(8, signals.GetNextSignalNumber(4));
+ EXPECT_EQ(16, signals.GetNextSignalNumber(8));
+ EXPECT_EQ(LLDB_INVALID_SIGNAL_NUMBER, signals.GetNextSignalNumber(16));
+}
+
+TEST(UnixSignalsTest, GetInfo) {
+ TestSignals signals;
+
+ bool should_suppress = false, should_stop = false, should_notify = false;
+ int32_t signo = 4;
+ std::string name =
+ signals.GetSignalInfo(signo, should_suppress, should_stop, should_notify);
+ EXPECT_EQ("SIG4", name);
+ EXPECT_EQ(true, should_suppress);
+ EXPECT_EQ(false, should_stop);
+ EXPECT_EQ(true, should_notify);
+
+ EXPECT_EQ(true, signals.GetShouldSuppress(signo));
+ EXPECT_EQ(false, signals.GetShouldStop(signo));
+ EXPECT_EQ(true, signals.GetShouldNotify(signo));
+ EXPECT_EQ(name, signals.GetSignalAsCString(signo));
+}
+
+TEST(UnixSignalsTest, VersionChange) {
+ TestSignals signals;
+
+ int32_t signo = 8;
+ uint64_t ver = signals.GetVersion();
+ EXPECT_GT(ver, 0ull);
+ EXPECT_EQ(true, signals.GetShouldSuppress(signo));
+ EXPECT_EQ(true, signals.GetShouldStop(signo));
+ EXPECT_EQ(true, signals.GetShouldNotify(signo));
+
+ EXPECT_EQ(signals.GetVersion(), ver);
+
+ signals.SetShouldSuppress(signo, false);
+ EXPECT_LT(ver, signals.GetVersion());
+ ver = signals.GetVersion();
+
+ signals.SetShouldStop(signo, true);
+ EXPECT_LT(ver, signals.GetVersion());
+ ver = signals.GetVersion();
+
+ signals.SetShouldNotify(signo, false);
+ EXPECT_LT(ver, signals.GetVersion());
+ ver = signals.GetVersion();
+
+ EXPECT_EQ(false, signals.GetShouldSuppress(signo));
+ EXPECT_EQ(true, signals.GetShouldStop(signo));
+ EXPECT_EQ(false, signals.GetShouldNotify(signo));
+
+ EXPECT_EQ(ver, signals.GetVersion());
+}
+
+TEST(UnixSignalsTest, GetFilteredSignals) {
+ TestSignals signals;
+
+ auto all_signals = signals.GetFilteredSignals(None, None, None);
+ std::vector expected = {2, 4, 8, 16};
+ ASSERT_EQ_ARRAYS(expected, all_signals);
+
+ auto supressed = signals.GetFilteredSignals(true, None, None);
+ expected = {4, 8, 16};
+ ASSERT_EQ_ARRAYS(expected, supressed);
+
+ auto not_supressed = signals.GetFilteredSignals(false, None, None);
+ expected = {2};
+ ASSERT_EQ_ARRAYS(expected, not_supressed);
+
+ auto stopped = signals.GetFilteredSignals(None, true, None);
+ expected = {2, 8};
+ ASSERT_EQ_ARRAYS(expected, stopped);
+
+ auto not_stopped = signals.GetFilteredSignals(None, false, None);
[Lldb-commits] [PATCH] D30520: Make LLDB skip server-client roundtrip for signals that don't require any actions
eugene marked 3 inline comments as done. eugene added a comment. Addressing review comments. Comment at: unittests/Signals/UnixSignalsTest.cpp:43 + +#define ASSERT_EQ_ARRAYS(expected, observed) \ + AssertEqArrays((expected), (observed), __FILE__, __LINE__); labath wrote: > This (and the function) should probably have an EXPECT_.. prefix instead, as > it does not abort the evaluation of the function it is in (like other > ASSERT_*** macros). This function calls ASSERT_EQ so it does abort evaluation. https://reviews.llvm.org/D30520 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D30520: Make LLDB skip server-client roundtrip for signals that don't require any actions
eugene added inline comments. Comment at: include/lldb/Target/Process.h:3148 + virtual Error UpdateAutomaticSignalFiltering(); + clayborg wrote: > Can we remove this and only have it in ProcessGDBRemote? Then we just call it > when we need to in ProcessGDBRemote? This seems like something that > ProcessGDBRemote should take care of without having Process knowing about it. > Seems like a adding a call to > ProcessGDBRemote::UpdateAutomaticSignalFiltering() can be done in > ProcessGDBRemote::WillResume() and ProcessGDBRemote::DidLaunch() might do the > trick? It seems like each process plug-in will have different needs regarding > signals. I 100% agree with you here. UpdateAutomaticSignalFiltering() is very much implementation specific and it shouldn't be in the lldb_private::Process. And it was how I originally implemented it in older revision. But Jim felt differently and I trusted his judgement given that he has vastly more experience working with LLDB codebase. https://reviews.llvm.org/D30520 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D30520: Make LLDB skip server-client roundtrip for signals that don't require any actions
eugene added a comment. UnixSignals is a nice self contained class that already does 99% of the work (see UnixSignals::GetFilteredSignals). I don't think we should have it call anybody. Only process knows when it is the right time to send actual QPassSignals packet, there is not need to somehow push this knowledge into UnixSignals. Let's just decide if UpdateAutomaticSignalFiltering() is specific enough to live in GDBProcess or it's generic enough to live in the base process. I'm fine either way. https://reviews.llvm.org/D30520 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D30520: Make LLDB skip server-client roundtrip for signals that don't require any actions
eugene updated this revision to Diff 90921.
eugene added a comment.
Rename ASSERT_EQ_ARRAYS to EXPECT_EQ_ARRAYS
https://reviews.llvm.org/D30520
Files:
include/lldb/Target/Process.h
include/lldb/Target/UnixSignals.h
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
source/Target/Process.cpp
source/Target/UnixSignals.cpp
unittests/CMakeLists.txt
unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
unittests/Signals/CMakeLists.txt
unittests/Signals/UnixSignalsTest.cpp
Index: unittests/Signals/UnixSignalsTest.cpp
===
--- /dev/null
+++ unittests/Signals/UnixSignalsTest.cpp
@@ -0,0 +1,140 @@
+//===-- UnixSignalsTest.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include
+
+#include "gtest/gtest.h"
+
+#include "lldb/Target/UnixSignals.h"
+#include "llvm/Support/FormatVariadic.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using llvm::None;
+
+class TestSignals : public UnixSignals {
+public:
+ TestSignals() {
+m_signals.clear();
+AddSignal(2, "SIG2", false, true, true, "DESC2");
+AddSignal(4, "SIG4", true, false, true, "DESC4");
+AddSignal(8, "SIG8", true, true, true, "DESC8");
+AddSignal(16, "SIG16", true, false, false, "DESC16");
+ }
+};
+
+void ExpectEqArrays(llvm::ArrayRef expected,
+llvm::ArrayRef observed, const char *file,
+int line) {
+ std::string location = llvm::formatv("{0}:{1}", file, line);
+ ASSERT_EQ(expected.size(), observed.size()) << location;
+
+ for (size_t i = 0; i < observed.size(); ++i) {
+ASSERT_EQ(expected[i], observed[i])
+<< "array index: " << i << "location:" << location;
+ }
+}
+
+#define EXPECT_EQ_ARRAYS(expected, observed) \
+ ExpectEqArrays((expected), (observed), __FILE__, __LINE__);
+
+TEST(UnixSignalsTest, Iteration) {
+ TestSignals signals;
+
+ EXPECT_EQ(4, signals.GetNumSignals());
+ EXPECT_EQ(2, signals.GetFirstSignalNumber());
+ EXPECT_EQ(4, signals.GetNextSignalNumber(2));
+ EXPECT_EQ(8, signals.GetNextSignalNumber(4));
+ EXPECT_EQ(16, signals.GetNextSignalNumber(8));
+ EXPECT_EQ(LLDB_INVALID_SIGNAL_NUMBER, signals.GetNextSignalNumber(16));
+}
+
+TEST(UnixSignalsTest, GetInfo) {
+ TestSignals signals;
+
+ bool should_suppress = false, should_stop = false, should_notify = false;
+ int32_t signo = 4;
+ std::string name =
+ signals.GetSignalInfo(signo, should_suppress, should_stop, should_notify);
+ EXPECT_EQ("SIG4", name);
+ EXPECT_EQ(true, should_suppress);
+ EXPECT_EQ(false, should_stop);
+ EXPECT_EQ(true, should_notify);
+
+ EXPECT_EQ(true, signals.GetShouldSuppress(signo));
+ EXPECT_EQ(false, signals.GetShouldStop(signo));
+ EXPECT_EQ(true, signals.GetShouldNotify(signo));
+ EXPECT_EQ(name, signals.GetSignalAsCString(signo));
+}
+
+TEST(UnixSignalsTest, VersionChange) {
+ TestSignals signals;
+
+ int32_t signo = 8;
+ uint64_t ver = signals.GetVersion();
+ EXPECT_GT(ver, 0ull);
+ EXPECT_EQ(true, signals.GetShouldSuppress(signo));
+ EXPECT_EQ(true, signals.GetShouldStop(signo));
+ EXPECT_EQ(true, signals.GetShouldNotify(signo));
+
+ EXPECT_EQ(signals.GetVersion(), ver);
+
+ signals.SetShouldSuppress(signo, false);
+ EXPECT_LT(ver, signals.GetVersion());
+ ver = signals.GetVersion();
+
+ signals.SetShouldStop(signo, true);
+ EXPECT_LT(ver, signals.GetVersion());
+ ver = signals.GetVersion();
+
+ signals.SetShouldNotify(signo, false);
+ EXPECT_LT(ver, signals.GetVersion());
+ ver = signals.GetVersion();
+
+ EXPECT_EQ(false, signals.GetShouldSuppress(signo));
+ EXPECT_EQ(true, signals.GetShouldStop(signo));
+ EXPECT_EQ(false, signals.GetShouldNotify(signo));
+
+ EXPECT_EQ(ver, signals.GetVersion());
+}
+
+TEST(UnixSignalsTest, GetFilteredSignals) {
+ TestSignals signals;
+
+ auto all_signals = signals.GetFilteredSignals(None, None, None);
+ std::vector expected = {2, 4, 8, 16};
+ EXPECT_EQ_ARRAYS(expected, all_signals);
+
+ auto supressed = signals.GetFilteredSignals(true, None, None);
+ expected = {4, 8, 16};
+ EXPECT_EQ_ARRAYS(expected, supressed);
+
+ auto not_supressed = signals.GetFilteredSignals(false, None, None);
+ expected = {2};
+ EXPECT_EQ_ARRAYS(expected, not_supressed);
+
+ auto stopped = signals.GetFilteredSignals(None, true, None);
+ expected = {2, 8};
+ EXPECT_EQ_ARRAYS(expected, stopped);
+
+ auto not_stopped = signals.GetFilteredSignals(None, false, None);
+ expected = {4, 16};
+ EXPECT_EQ_ARRAYS(expected, not
[Lldb-commits] [PATCH] D30702: Fix remaining threading issues in Log.h
eugene added inline comments.
Comment at: include/lldb/Utility/Log.h:152
- Flags &GetOptions();
+ const Flags GetOptions() const;
Seems like const on return value is not really needed.
Comment at: include/lldb/Utility/Log.h:163
+ std::atomic m_options{0};
+ std::atomic m_mask{0};
+
Do we actually need atomics here?
It seems like the stream itself is protected by the mutex, and It doesn't seem
to affect performance.
Can we use the same (or a different) mutex to protect flags and options?
https://reviews.llvm.org/D30702
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D30520: Make LLDB skip server-client roundtrip for signals that don't require any actions
This revision was automatically updated to reflect the committed changes.
Closed by commit rL297231: Make LLDB skip server-client roundtrip for signals
that don't require any… (authored by eugene).
Changed prior to commit:
https://reviews.llvm.org/D30520?vs=90921&id=90930#toc
Repository:
rL LLVM
https://reviews.llvm.org/D30520
Files:
lldb/trunk/include/lldb/Target/Process.h
lldb/trunk/include/lldb/Target/UnixSignals.h
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/trunk/source/Target/Process.cpp
lldb/trunk/source/Target/UnixSignals.cpp
lldb/trunk/unittests/CMakeLists.txt
lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
lldb/trunk/unittests/Signals/CMakeLists.txt
lldb/trunk/unittests/Signals/UnixSignalsTest.cpp
Index: lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -307,3 +307,27 @@
<< ss.GetString();
ASSERT_EQ(10, num_packets);
}
+
+TEST_F(GDBRemoteCommunicationClientTest, SendSignalsToIgnore) {
+ TestClient client;
+ MockServer server;
+ Connect(client, server);
+ if (HasFailure())
+return;
+
+ const lldb::tid_t tid = 0x47;
+ const uint32_t reg_num = 4;
+ std::future result = std::async(std::launch::async, [&] {
+return client.SendSignalsToIgnore({2, 3, 5, 7, 0xB, 0xD, 0x11});
+ });
+
+ HandlePacket(server, "QPassSignals:02;03;05;07;0b;0d;11", "OK");
+ EXPECT_TRUE(result.get().Success());
+
+ result = std::async(std::launch::async, [&] {
+return client.SendSignalsToIgnore(std::vector());
+ });
+
+ HandlePacket(server, "QPassSignals:", "OK");
+ EXPECT_TRUE(result.get().Success());
+}
Index: lldb/trunk/unittests/CMakeLists.txt
===
--- lldb/trunk/unittests/CMakeLists.txt
+++ lldb/trunk/unittests/CMakeLists.txt
@@ -63,6 +63,7 @@
add_subdirectory(Platform)
add_subdirectory(Process)
add_subdirectory(ScriptInterpreter)
+add_subdirectory(Signals)
add_subdirectory(Symbol)
add_subdirectory(SymbolFile)
add_subdirectory(Target)
Index: lldb/trunk/unittests/Signals/CMakeLists.txt
===
--- lldb/trunk/unittests/Signals/CMakeLists.txt
+++ lldb/trunk/unittests/Signals/CMakeLists.txt
@@ -0,0 +1,6 @@
+add_lldb_unittest(SignalsTests
+ UnixSignalsTest.cpp
+
+ LINK_LIBS
+lldbTarget
+ )
Index: lldb/trunk/unittests/Signals/UnixSignalsTest.cpp
===
--- lldb/trunk/unittests/Signals/UnixSignalsTest.cpp
+++ lldb/trunk/unittests/Signals/UnixSignalsTest.cpp
@@ -0,0 +1,140 @@
+//===-- UnixSignalsTest.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include
+
+#include "gtest/gtest.h"
+
+#include "lldb/Target/UnixSignals.h"
+#include "llvm/Support/FormatVariadic.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using llvm::None;
+
+class TestSignals : public UnixSignals {
+public:
+ TestSignals() {
+m_signals.clear();
+AddSignal(2, "SIG2", false, true, true, "DESC2");
+AddSignal(4, "SIG4", true, false, true, "DESC4");
+AddSignal(8, "SIG8", true, true, true, "DESC8");
+AddSignal(16, "SIG16", true, false, false, "DESC16");
+ }
+};
+
+void ExpectEqArrays(llvm::ArrayRef expected,
+llvm::ArrayRef observed, const char *file,
+int line) {
+ std::string location = llvm::formatv("{0}:{1}", file, line);
+ ASSERT_EQ(expected.size(), observed.size()) << location;
+
+ for (size_t i = 0; i < observed.size(); ++i) {
+ASSERT_EQ(expected[i], observed[i])
+<< "array index: " << i << "location:" << location;
+ }
+}
+
+#define EXPECT_EQ_ARRAYS(expected, observed) \
+ ExpectEqArrays((expected), (observed), __FILE__, __LINE__);
+
+TEST(UnixSignalsTest, Iteration) {
+ TestSignals signals;
+
+ EXPECT_EQ(4, signals.GetNumSignals());
+ EXPECT_EQ(2, signals.GetFirstSignalNumber());
+ EXPECT_EQ(4, signals.GetNextSignalNumber(2));
+ EXPECT_EQ(8, signals.GetNextSignalNumber(4));
+ EXPECT_EQ(16, signals.GetNextSignalNumber(8));
+ EXPECT_EQ(LLDB_INVALID_SIGNAL_NUMBER, signals.GetNextSignalNumber(16));
+}
+
+TEST(UnixSignalsTest, GetInfo) {
+ TestSignals signals;
+
+ bool shoul
[Lldb-commits] [PATCH] D30942: Remove some ProcFileReader occurences
eugene accepted this revision.
eugene added inline comments.
Comment at: source/Host/linux/Host.cpp:167
+ (llvm::Twine("/proc/") + llvm::Twine(pid) + "/exe").toVector(ProcExe);
+ llvm::SmallString<0> ExePath;
+ ExePath.assign(PATH_MAX, '\0');
std::string? Do we have any benefits from using 0 size stack allocated string
here?
https://reviews.llvm.org/D30942
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D62168: [DynamicLoader] Make sure we always set the rendezvous breakpoint
eugene accepted this revision. eugene added a comment. This revision is now accepted and ready to land. Makes sense to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62168/new/ https://reviews.llvm.org/D62168 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
