[Lldb-commits] [PATCH] D157214: [lldb] Fix typo in comments and in test
Eymay created this revision.
Herald added a project: All.
Eymay requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D157214
Files:
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
Index: lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
===
--- lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
+++ lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
@@ -91,7 +91,7 @@
def test_uuid_modules_elf_build_id_16(self):
"""
Test multiple modules having a MINIDUMP_MODULE.CvRecord that is valid,
-and contains a ELF build ID whose value is valid and is 16 bytes long.
+and contains an ELF build ID whose value is valid and is 16 bytes long.
"""
modules =
self.get_minidump_modules("linux-arm-uuids-elf-build-id-16.yaml")
self.assertEqual(2, len(modules))
@@ -101,7 +101,7 @@
def test_uuid_modules_elf_build_id_20(self):
"""
Test multiple modules having a MINIDUMP_MODULE.CvRecord that is valid,
-and contains a ELF build ID whose value is valid and is 20 bytes long.
+and contains an ELF build ID whose value is valid and is 20 bytes long.
"""
modules =
self.get_minidump_modules("linux-arm-uuids-elf-build-id-20.yaml")
self.assertEqual(2, len(modules))
@@ -115,7 +115,7 @@
def test_uuid_modules_elf_build_id_zero(self):
"""
Test multiple modules having a MINIDUMP_MODULE.CvRecord that is valid,
-and contains a ELF build ID whose value is all zero.
+and contains an ELF build ID whose value is all zero.
"""
modules =
self.get_minidump_modules("linux-arm-uuids-elf-build-id-zero.yaml")
self.assertEqual(2, len(modules))
@@ -125,7 +125,7 @@
def test_uuid_modules_elf_build_id_same(self):
"""
Test multiple modules having a MINIDUMP_MODULE.CvRecord that is
-valid, and contains a ELF build ID whose value is the same. There
+valid, and contains an ELF build ID whose value is the same. There
is an assert in the PlaceholderObjectFile that was firing when we
encountered this which was crashing the process that was checking
if PlaceholderObjectFile.m_base was the same as the address this
Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -348,7 +348,7 @@
@skipIfXmlSupportMissing
@skipIfRemote
-def test_flags_requried_attributes(self):
+def test_flags_required_attributes(self):
# flags must have an id and size so the flags with "C" is the only
valid one
# here.
self.setup_register_test(
Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -53,7 +53,7 @@
bool can_connect) {
lldb::ProcessSP process_sp;
if (crash_file && !can_connect) {
-// Read enough data for a ELF32 header or ELF64 header Note: Here we care
+// Read enough data for an ELF32 header or ELF64 header Note: Here we care
// about e_type field only, so it is safe to ignore possible presence of
// the header extension.
const size_t header_size = sizeof(llvm::ELF::Elf64_Ehdr);
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -2755,8 +2755,8 @@
assert(vm_nlist_bytes_read == nlist_byte_size * nlistCount);
// We don't know the size of the string table. It's cheaper
- // to map the whol VM region than to determine the size by
- // parsing all teh nlist entries.
+ // to map the whole VM region than to determine the size by
+ // parsing all the nlist entries.
vm_address_t string_address = (vm_address_t)stringTable;
vm_size_t region_size;
mach_msg_type_number_t info_count =
VM_REGION_BASIC_INFO_COUNT_64;
Index: lldb/test/API/functionalities/postmortem/minidump-new/Tes
[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.
bolshakov-a updated this revision to Diff 547589.
bolshakov-a added a comment.
Rebased.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140996/new/
https://reviews.llvm.org/D140996
Files:
clang-tools-extra/clangd/DumpAST.cpp
clang-tools-extra/clangd/FindTarget.cpp
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/ODRHash.h
clang/include/clang/AST/PropertiesBase.td
clang/include/clang/AST/RecursiveASTVisitor.h
clang/include/clang/AST/TemplateArgumentVisitor.h
clang/include/clang/AST/TemplateBase.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/ASTImporter.cpp
clang/lib/AST/ASTStructuralEquivalence.cpp
clang/lib/AST/Decl.cpp
clang/lib/AST/ItaniumMangle.cpp
clang/lib/AST/MicrosoftMangle.cpp
clang/lib/AST/ODRHash.cpp
clang/lib/AST/StmtProfile.cpp
clang/lib/AST/TemplateBase.cpp
clang/lib/AST/TypeLoc.cpp
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/Index/USRGeneration.cpp
clang/lib/Sema/SemaLookup.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/lib/Sema/SemaTemplateDeduction.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/SemaTemplateVariadic.cpp
clang/lib/Sema/TreeTransform.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/CXX/drs/dr12xx.cpp
clang/test/CXX/temp/temp.arg/temp.arg.nontype/p1.cpp
clang/test/CodeGenCXX/mangle-ms-templates.cpp
clang/test/CodeGenCXX/mangle-template.cpp
clang/test/CodeGenCXX/template-arguments.cpp
clang/test/Index/USR/structural-value-tpl-arg.cpp
clang/test/Modules/odr_hash.cpp
clang/test/SemaCXX/warn-bool-conversion.cpp
clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
clang/tools/libclang/CIndex.cpp
clang/tools/libclang/CXCursor.cpp
clang/www/cxx_status.html
lldb/include/lldb/lldb-enumerations.h
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -7218,6 +7218,9 @@
case clang::TemplateArgument::Pack:
return eTemplateArgumentKindPack;
+
+ case clang::TemplateArgument::StructuralValue:
+return eTemplateArgumentKindStructuralValue;
}
llvm_unreachable("Unhandled clang::TemplateArgument::ArgKind");
}
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -856,6 +856,7 @@
eTemplateArgumentKindExpression,
eTemplateArgumentKindPack,
eTemplateArgumentKindNullPtr,
+ eTemplateArgumentKindStructuralValue,
};
/// Type of match to be performed when looking for a formatter for a data type.
Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -626,13 +626,21 @@
- Class types as non-type template parameters
+ Class types as non-type template parameters
https://wg21.link/p0732r2";>P0732R2
- Partial
+ Clang 12
+
+
+ Generalized non-type template parameters of scalar type
+ https://wg21.link/p1907r1";>P1907R1
+
+
+ Clang 17 (Partial)
+ Reference type template arguments referring to instantiation-dependent objects and subobjects
+ (i.e. declared inside a template but neither type- nor value-dependent) aren't fully supported.
+
+
-
-https://wg21.link/p1907r1";>P1907R1
-
Destroying operator delete
https://wg21.link/p0722r3";>P0722R3
Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -1463,6 +1463,9 @@
return CXTemplateArgumentKind_NullPtr;
case TemplateArgument::Integral:
return CXTemplateArgumentKind_Integral;
+ case TemplateArgument::StructuralValue:
+// FIXME: Expose these values.
+return CXTemplateArgumentKind_Invalid;
case TemplateArgument::Template:
return CXTemplateArgumentKind_Template;
case TemplateArgument::TemplateExpansion:
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1574,6 +1574,11 @@
return Visit(MakeCXCursor(E, StmtParent, TU, RegionOfInterest));
return false;
+ case TemplateArgument::StructuralValue:
+if (Expr *E = TAL.getSourceStructuralValueExpression())
+ return Visit(MakeCXCursor(E, StmtParent, TU, RegionOfInterest));
+return false;
+
case
[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.
bolshakov-a updated this revision to Diff 547603.
bolshakov-a added a comment.
Fixes after review.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140996/new/
https://reviews.llvm.org/D140996
Files:
clang-tools-extra/clangd/DumpAST.cpp
clang-tools-extra/clangd/FindTarget.cpp
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/ODRHash.h
clang/include/clang/AST/PropertiesBase.td
clang/include/clang/AST/RecursiveASTVisitor.h
clang/include/clang/AST/TemplateArgumentVisitor.h
clang/include/clang/AST/TemplateBase.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/ASTImporter.cpp
clang/lib/AST/ASTStructuralEquivalence.cpp
clang/lib/AST/Decl.cpp
clang/lib/AST/ItaniumMangle.cpp
clang/lib/AST/MicrosoftMangle.cpp
clang/lib/AST/ODRHash.cpp
clang/lib/AST/StmtProfile.cpp
clang/lib/AST/TemplateBase.cpp
clang/lib/AST/TypeLoc.cpp
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/Index/USRGeneration.cpp
clang/lib/Sema/SemaLookup.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/lib/Sema/SemaTemplateDeduction.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/SemaTemplateVariadic.cpp
clang/lib/Sema/TreeTransform.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/CXX/drs/dr12xx.cpp
clang/test/CXX/temp/temp.arg/temp.arg.nontype/p1.cpp
clang/test/CodeGenCXX/mangle-ms-templates.cpp
clang/test/CodeGenCXX/mangle-template.cpp
clang/test/CodeGenCXX/template-arguments.cpp
clang/test/Index/USR/structural-value-tpl-arg.cpp
clang/test/Modules/odr_hash.cpp
clang/test/SemaCXX/warn-bool-conversion.cpp
clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
clang/tools/libclang/CIndex.cpp
clang/tools/libclang/CXCursor.cpp
clang/www/cxx_status.html
lldb/include/lldb/lldb-enumerations.h
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -7218,6 +7218,9 @@
case clang::TemplateArgument::Pack:
return eTemplateArgumentKindPack;
+
+ case clang::TemplateArgument::StructuralValue:
+return eTemplateArgumentKindStructuralValue;
}
llvm_unreachable("Unhandled clang::TemplateArgument::ArgKind");
}
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -856,6 +856,7 @@
eTemplateArgumentKindExpression,
eTemplateArgumentKindPack,
eTemplateArgumentKindNullPtr,
+ eTemplateArgumentKindStructuralValue,
};
/// Type of match to be performed when looking for a formatter for a data type.
Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -626,13 +626,21 @@
- Class types as non-type template parameters
+ Class types as non-type template parameters
https://wg21.link/p0732r2";>P0732R2
- Partial
+ Clang 12
+
+
+ Generalized non-type template parameters of scalar type
+ https://wg21.link/p1907r1";>P1907R1
+
+
+ Clang 18 (Partial)
+ Reference type template arguments referring to instantiation-dependent objects and subobjects
+ (i.e. declared inside a template but neither type- nor value-dependent) aren't fully supported.
+
+
-
-https://wg21.link/p1907r1";>P1907R1
-
Destroying operator delete
https://wg21.link/p0722r3";>P0722R3
Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -1463,6 +1463,9 @@
return CXTemplateArgumentKind_NullPtr;
case TemplateArgument::Integral:
return CXTemplateArgumentKind_Integral;
+ case TemplateArgument::StructuralValue:
+// FIXME: Expose these values.
+return CXTemplateArgumentKind_Invalid;
case TemplateArgument::Template:
return CXTemplateArgumentKind_Template;
case TemplateArgument::TemplateExpansion:
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1574,6 +1574,11 @@
return Visit(MakeCXCursor(E, StmtParent, TU, RegionOfInterest));
return false;
+ case TemplateArgument::StructuralValue:
+if (Expr *E = TAL.getSourceStructuralValueExpression())
+ return Visit(MakeCXCursor(E, StmtParent, TU, RegionOfInterest));
+return false
[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.
bolshakov-a added a comment.
Btw, formatting of unrelated lines has leaked into `TemplateBase.h`. Sorry.
Comment at: clang/lib/AST/MicrosoftMangle.cpp:1624-1626
+ return const_cast(
+ V.getLValueBase().dyn_cast());
+}
aaron.ballman wrote:
> Does this work or is the `const_cast` actually required?
No, it doesn't compile, likewise the standard C++ `dynamic_cast` cannot remove
constness.
Comment at: clang/lib/AST/TemplateBase.cpp:408-409
case Integral:
-getAsIntegral().Profile(ID);
getIntegralType().Profile(ID);
+getAsIntegral().Profile(ID);
+break;
aaron.ballman wrote:
> Why did the order of these calls change?
I don't know, it is from 9e08e51a20d0d2. I've tried to invert the order along
with the order for `StructuralValue`, and all tests have been passed.
Comment at: clang/lib/Sema/SemaOverload.cpp:5983-5985
+ Expr *E = Result.get();
+ if (!isa(E))
+E = ConstantExpr::Create(S.Context, Result.get(), Value);
aaron.ballman wrote:
> I thought we could run into situations where we had a `ConstantExpr` but it
> did not yet have its result stored in it. Should we assert that isn't the
> case here?
If I understand correctly, the sole place where `ConstantExpr` is constructed
which may occur here is `BuildExpressionFromNonTypeTemplateArgumentValue`
function, and a value is set into it there. Should I add the assertion into
code?
Comment at: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp:56
+using CF = ComplexFloat<1.0f + 3.0fi>;
+using CF = ComplexFloat<3.0fi + 1.0f>;
aaron.ballman wrote:
> bolshakov-a wrote:
> > shafik wrote:
> > > Can we add an enum example e.g.:
> > >
> > > ```
> > > enum E{ E1, E2};
> > > template struct SE {};
> > > using IPE = SE;
> > > using IPE = SE;
> > >
> > > ```
> > What for? Enumerators as non-type template arguments are allowed since
> > C++98, AFAIK. And this test is about changes in C++20.
> Sometimes we're lacking coverage for existing features, so when updating code
> in the area, we'll sometimes ask for extra coverage just to be sure we're not
> regressing something we think might not have a lot of existing test coverage.
`temp_arg_nontype.cpp` test already has some `enum` cases. If a case with type
alias should be added, it shoud be added there, not in the
`temp_arg_nontype_cxx20.cpp`, I think.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140996/new/
https://reviews.llvm.org/D140996
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.
bolshakov-a added inline comments.
Comment at: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp:56
+using CF = ComplexFloat<1.0f + 3.0fi>;
+using CF = ComplexFloat<3.0fi + 1.0f>;
bolshakov-a wrote:
> aaron.ballman wrote:
> > bolshakov-a wrote:
> > > shafik wrote:
> > > > Can we add an enum example e.g.:
> > > >
> > > > ```
> > > > enum E{ E1, E2};
> > > > template struct SE {};
> > > > using IPE = SE;
> > > > using IPE = SE;
> > > >
> > > > ```
> > > What for? Enumerators as non-type template arguments are allowed since
> > > C++98, AFAIK. And this test is about changes in C++20.
> > Sometimes we're lacking coverage for existing features, so when updating
> > code in the area, we'll sometimes ask for extra coverage just to be sure
> > we're not regressing something we think might not have a lot of existing
> > test coverage.
> `temp_arg_nontype.cpp` test already has some `enum` cases. If a case with
> type alias should be added, it shoud be added there, not in the
> `temp_arg_nontype_cxx20.cpp`, I think.
I've just realized that C++98 didn't had type aliases. But `typedef`s should
probably go as well.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140996/new/
https://reviews.llvm.org/D140996
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin
RamNalamothu marked an inline comment as done. RamNalamothu added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156086/new/ https://reviews.llvm.org/D156086 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D157159: [lldb] Properly protect the Communication class with reader/writer lock
JDevlieghere updated this revision to Diff 547639.
JDevlieghere added a comment.
- Remove `Communication::Clear` method
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157159/new/
https://reviews.llvm.org/D157159
Files:
lldb/include/lldb/Core/Communication.h
lldb/include/lldb/Core/ThreadedCommunication.h
lldb/source/Core/Communication.cpp
lldb/source/Core/ThreadedCommunication.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
Index: lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
===
--- lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
+++ lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
@@ -557,7 +557,6 @@
}
}
StopAsyncThread();
- m_comm.Clear();
SetPrivateState(eStateDetached);
ResumePrivateStateThread();
Index: lldb/source/Core/ThreadedCommunication.cpp
===
--- lldb/source/Core/ThreadedCommunication.cpp
+++ lldb/source/Core/ThreadedCommunication.cpp
@@ -60,12 +60,6 @@
this, GetBroadcasterName());
}
-void ThreadedCommunication::Clear() {
- SetReadThreadBytesReceivedCallback(nullptr, nullptr);
- StopReadThread(nullptr);
- Communication::Clear();
-}
-
ConnectionStatus ThreadedCommunication::Disconnect(Status *error_ptr) {
assert((!m_read_thread_enabled || m_read_thread_did_exit) &&
"Disconnecting while the read thread is running is racy!");
Index: lldb/source/Core/Communication.cpp
===
--- lldb/source/Core/Communication.cpp
+++ lldb/source/Core/Communication.cpp
@@ -27,68 +27,57 @@
using namespace lldb_private;
Communication::Communication()
-: m_connection_sp(), m_write_mutex(), m_close_on_eof(true) {
-}
-
-Communication::~Communication() {
- Clear();
-}
+: m_connection_sp(), m_shared_mutex(), m_close_on_eof(true) {}
-void Communication::Clear() {
- Disconnect(nullptr);
-}
+Communication::~Communication() { Disconnect(nullptr); }
ConnectionStatus Communication::Connect(const char *url, Status *error_ptr) {
- Clear();
-
LLDB_LOG(GetLog(LLDBLog::Communication),
"{0} Communication::Connect (url = {1})", this, url);
- lldb::ConnectionSP connection_sp(m_connection_sp);
- if (connection_sp)
-return connection_sp->Connect(url, error_ptr);
+ std::unique_lock guard(m_shared_mutex);
+
+ DisconnectUnlocked();
+
+ if (m_connection_sp)
+return m_connection_sp->Connect(url, error_ptr);
if (error_ptr)
error_ptr->SetErrorString("Invalid connection.");
return eConnectionStatusNoConnection;
}
ConnectionStatus Communication::Disconnect(Status *error_ptr) {
+ std::unique_lock guard(m_shared_mutex);
+ return DisconnectUnlocked();
+}
+
+ConnectionStatus Communication::DisconnectUnlocked(Status *error_ptr) {
LLDB_LOG(GetLog(LLDBLog::Communication), "{0} Communication::Disconnect ()",
this);
- lldb::ConnectionSP connection_sp(m_connection_sp);
- if (connection_sp) {
-ConnectionStatus status = connection_sp->Disconnect(error_ptr);
-// We currently don't protect connection_sp with any mutex for multi-
-// threaded environments. So lets not nuke our connection class without
-// putting some multi-threaded protections in. We also probably don't want
-// to pay for the overhead it might cause if every time we access the
-// connection we have to take a lock.
-//
-// This unique pointer will cleanup after itself when this object goes
-// away, so there is no need to currently have it destroy itself
-// immediately upon disconnect.
-// connection_sp.reset();
+ if (m_connection_sp) {
+ConnectionStatus status = m_connection_sp->Disconnect(error_ptr);
return status;
}
return eConnectionStatusNoConnection;
}
bool Communication::IsConnected() const {
- lldb::ConnectionSP connection_sp(m_connection_sp);
- return (connection_sp ? connection_sp->IsConnected() : false);
+ std::shared_lock guard(m_shared_mutex);
+ return (m_connection_sp ? m_connection_sp->IsConnected() : false);
}
bool Communication::HasConnection() const {
+ std::shared_lock guard(m_shared_mutex);
return m_connection_sp.get() != nullptr;
}
size_t Communication::Read(void *dst, size_t dst_len,
const Timeout &timeout,
ConnectionStatus &status, Status *error_ptr) {
- Log *log = GetLog(LLDBLog::Communication);
+ std::shared_lock guard(m_shared_mutex);
LLDB_LOG(
- log,
+ GetLog(LLDBLog::Communication),
"this = {0}, dst = {1}, dst_len = {2}, timeout = {3}, connection = {4}",
this, dst, dst_len, timeout, m_connection_sp.get());
@@ -97,16 +86,20 @@
size_t Communication::Write(const void *src, size_t src_len,
ConnectionStatus &status, Status *error_ptr) {
- lldb::ConnectionSP connection_sp(m_connection_sp);
+
[Lldb-commits] [PATCH] D157159: [lldb] Properly protect the Communication class with reader/writer lock
JDevlieghere added inline comments.
Comment at: lldb/source/Core/ThreadedCommunication.cpp:63-67
-void ThreadedCommunication::Clear() {
- SetReadThreadBytesReceivedCallback(nullptr, nullptr);
- StopReadThread(nullptr);
- Communication::Clear();
-}
This wasn't used: nobody called `Clear` on an instance of
`ThreadedCommunication`.
Comment at: lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp:560
StopAsyncThread();
- m_comm.Clear();
According to the comment on line 541 this is wrong:
``` // We are running and we can't interrupt a running kernel, so we need to
// just close the connection to the kernel and hope for the best```
`m_comm` is an instance of `CommunicationKDP` which didn't override
`Communication::Clear` and hence just called `Disconnected`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157159/new/
https://reviews.llvm.org/D157159
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D157152: [lldb] Make TSan errors fatal when running the test suite
JDevlieghere updated this revision to Diff 547641.
JDevlieghere added a comment.
Include the unit tests
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157152/new/
https://reviews.llvm.org/D157152
Files:
lldb/test/API/lit.cfg.py
lldb/test/Shell/lit.cfg.py
lldb/test/Unit/lit.cfg.py
Index: lldb/test/Unit/lit.cfg.py
===
--- lldb/test/Unit/lit.cfg.py
+++ lldb/test/Unit/lit.cfg.py
@@ -34,5 +34,9 @@
)
llvm_config.with_environment("PATH", os.path.dirname(sys.executable),
append_path=True)
+# Enable sanitizer runtime flags.
+config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1"
+config.environment["TSAN_OPTIONS"] = "halt_on_error=1"
+
# testFormat: The test format to use to interpret tests.
config.test_format = lit.formats.GoogleTest(config.llvm_build_mode, "Tests")
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -48,6 +48,10 @@
]
)
+# Enable sanitizer runtime flags.
+config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1"
+config.environment["TSAN_OPTIONS"] = "halt_on_error=1"
+
# Support running the test suite under the lldb-repro wrapper. This makes it
# possible to capture a test suite run and then rerun all the test from the
# just captured reproducer.
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -123,6 +123,7 @@
)
if "Thread" in config.llvm_use_sanitizer:
+config.environment["TSAN_OPTIONS"] = "halt_on_error=1"
if "Darwin" in config.host_os:
config.environment["DYLD_INSERT_LIBRARIES"] =
find_sanitizer_runtime(
"libclang_rt.tsan_osx_dynamic.dylib"
Index: lldb/test/Unit/lit.cfg.py
===
--- lldb/test/Unit/lit.cfg.py
+++ lldb/test/Unit/lit.cfg.py
@@ -34,5 +34,9 @@
)
llvm_config.with_environment("PATH", os.path.dirname(sys.executable), append_path=True)
+# Enable sanitizer runtime flags.
+config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1"
+config.environment["TSAN_OPTIONS"] = "halt_on_error=1"
+
# testFormat: The test format to use to interpret tests.
config.test_format = lit.formats.GoogleTest(config.llvm_build_mode, "Tests")
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -48,6 +48,10 @@
]
)
+# Enable sanitizer runtime flags.
+config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1"
+config.environment["TSAN_OPTIONS"] = "halt_on_error=1"
+
# Support running the test suite under the lldb-repro wrapper. This makes it
# possible to capture a test suite run and then rerun all the test from the
# just captured reproducer.
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -123,6 +123,7 @@
)
if "Thread" in config.llvm_use_sanitizer:
+config.environment["TSAN_OPTIONS"] = "halt_on_error=1"
if "Darwin" in config.host_os:
config.environment["DYLD_INSERT_LIBRARIES"] = find_sanitizer_runtime(
"libclang_rt.tsan_osx_dynamic.dylib"
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
