[Lldb-commits] [PATCH] D157168: [lldb] [mach-o corefiles] If we have LC_NOTE metadata and can't find a binary, don't fall back to an exhaustive scan

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added reviewers: bulbazord, JDevlieghere.
jasonmolenda added a project: LLDB.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

We have some corefiles which are intended to debug a standalone binary, and 
have an LC_NOTE mach-o load command with the UUID and/or address of that 
binary.  If the search for that binary fails for any reason, we don't load the 
intended binary.  ProcessMachCore then falls back to doing an exhaustive scan 
of the corefile memory pages looking for a darwin kernel or a userland dyld 
binary.  And the case where this comes up most often, there is a darwin kernel. 
 lldb starts doing a darwin kernel debug session and that's not at all what the 
user is intending to do.

This patch changes ProcessMachCore::LoadBinariesViaMetadata to return if any 
metadata record was found, even if we didn't find the binary, and 
ProcessMachCore::LoadBinariesAndSetDYLD will only start an exhaustive search if 
we had no metadata.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157168

Files:
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.h

Index: lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
===
--- lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
+++ lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
@@ -86,7 +86,7 @@
 
 private:
   void CreateMemoryRegions();
-  void LoadBinariesViaMetadata();
+  bool LoadBinariesViaMetadata();
   void LoadBinariesViaExhaustiveSearch();
   void LoadBinariesAndSetDYLD();
   void CleanupMemoryRegionPermissions();
Index: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
===
--- lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -223,16 +223,20 @@
   }
 }
 
-void ProcessMachCore::LoadBinariesViaMetadata() {
+bool ProcessMachCore::LoadBinariesViaMetadata() {
   Log *log(GetLog(LLDBLog::DynamicLoader | LLDBLog::Process));
   ObjectFile *core_objfile = m_core_module_sp->GetObjectFile();
-  bool found_main_binary_definitively = false;
 
   addr_t objfile_binary_value;
   bool objfile_binary_value_is_offset;
   UUID objfile_binary_uuid;
   ObjectFile::BinaryType type;
 
+  // This will be set to true if we had a metadata hint
+  // specifying a UUID or address -- and we should not fall back
+  // to doing an exhaustive search.
+  bool found_binary_spec_in_metadata = false;
+
   if (core_objfile->GetCorefileMainBinaryInfo(objfile_binary_value,
   objfile_binary_value_is_offset,
   objfile_binary_uuid, type)) {
@@ -244,6 +248,7 @@
   objfile_binary_uuid.GetAsString().c_str(),
   objfile_binary_value, objfile_binary_value_is_offset, type);
 }
+found_binary_spec_in_metadata = true;
 
 // If this is the xnu kernel, don't load it now.  Note the correct
 // DynamicLoader plugin to use, and the address of the kernel, and
@@ -251,7 +256,6 @@
 if (type == ObjectFile::eBinaryTypeKernel) {
   m_mach_kernel_addr = objfile_binary_value;
   m_dyld_plugin_name = DynamicLoaderDarwinKernel::GetPluginNameStatic();
-  found_main_binary_definitively = true;
 } else if (type == ObjectFile::eBinaryTypeUser) {
   m_dyld_addr = objfile_binary_value;
   m_dyld_plugin_name = DynamicLoaderMacOSXDYLD::GetPluginNameStatic();
@@ -263,7 +267,6 @@
   this, llvm::StringRef(), objfile_binary_uuid,
   objfile_binary_value, objfile_binary_value_is_offset,
   force_symbol_search, notify, set_address_in_target)) {
-found_main_binary_definitively = true;
 m_dyld_plugin_name = DynamicLoaderStatic::GetPluginNameStatic();
   }
 }
@@ -274,7 +277,6 @@
   // load command is present, let's use the contents.
   UUID ident_uuid;
   addr_t ident_binary_addr = LLDB_INVALID_ADDRESS;
-  if (!found_main_binary_definitively) {
 std::string corefile_identifier = core_objfile->GetIdentifierString();
 
 // Search for UUID= and stext= strings in the identifier str.
@@ -285,6 +287,7 @@
   if (log)
 log->Printf("Got a UUID from LC_IDENT/kern ver str LC_NOTE: %s",
 ident_uuid.GetAsString().c_str());
+  found_binary_spec_in_metadata = true;
 }
 if (corefile_identifier.find("stext=") != std::string::npos) {
   size_t p = corefile_identifier.find("stext=") + strlen("stext=");
@@ -295,6 +298,7 @@
   log->Printf("Got a load address from LC_IDENT/kern ver str "
   "LC_NOTE: 0x%" PRIx64,
   ident_binary_addr);
+found_binary_spec_in_metadata = true;
   }
 }
 
@@ -307,7 +311,7 

[Lldb-commits] [PATCH] D157167: [lldb] Add flag to DynamicLoader::LoadBinaryWithUUIDAndAddress to control whether we fall back to reading the binary out of memory

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added reviewers: bulbazord, JDevlieghere.
jasonmolenda added a project: LLDB.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added subscribers: lldb-commits, wangpc.

DynamicLoader::LoadBinaryWithUUIDAndAddress() tries a few different ways of 
finding a binary, and if it can't find an actual binary (and symbol file) on 
the local computer, it will try to read a binary out of memory, with the 
thought that there may be a symbol table in memory so we'll get some symbol 
names.  It will create a placeholder name of `memory-image-0x...`.  It seemed 
like a good idea when I wrote this method, but in practice for binaries outside 
of userland binaries in a corefile, this hasn't been helpful.

This patch adds a flag to LoadBinaryWithUUIDAndAddress to control if we should 
create a memory image module, or not.  The only case where we still enable this 
behavior is in ObjectFileMachO::LoadCoreFileImages() when a binary has segment 
load addresses.  This is the case with the "all image infos" LC_NOTE that lldb 
creates for userland corefiles with binaries that typically have some symbols 
in their in-memory symbol tables.

The other type of userland corefiles (gcore-created) will not use this LC_NOTE 
but an entirely different codepath -- through the Darwin userland DynamicLoader 
plugin -- is taken in those cases, so we'll still read them out of memory.

This patch makes firmware/standalone binary loading not create these 
`memory-image-0x...` placeholder binaries with no symbols in them, the scenario 
where it is more annoying than useful.

The test case TestMultipleBinaryCorefile.py tests this standalone-binary 
corefile scenario, so it needed adjusting to the new setup where the binary 
image is not added to the target.  I also cleaned up the fake dsym-for-uuid.sh 
it creates to return a properly formatted DBGError message for 
https://reviews.llvm.org/D157160


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157167

Files:
  lldb/include/lldb/Target/DynamicLoader.h
  lldb/source/Core/DynamicLoader.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
  
lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py
  
lldb/test/API/macosx/lc-note/multiple-binary-corefile/create-multibin-corefile.cpp

Index: lldb/test/API/macosx/lc-note/multiple-binary-corefile/create-multibin-corefile.cpp
===
--- lldb/test/API/macosx/lc-note/multiple-binary-corefile/create-multibin-corefile.cpp
+++ lldb/test/API/macosx/lc-note/multiple-binary-corefile/create-multibin-corefile.cpp
@@ -16,7 +16,7 @@
 #include 
 
 // Given a list of binaries, and optional slides to be applied,
-// create a corefile whose memory is those binaries laid at at
+// create a corefile whose memory is those binaries laid down at
 // their slid addresses.
 //
 // Add a 'main bin spec' LC_NOTE for the first binary, and
Index: lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py
===
--- lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py
+++ lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py
@@ -1,4 +1,4 @@
-"""Test corefiles with "main bin spec"/"load binary" with only addrs work."""
+"""Test corefiles with "main bin spec"/"load binary" with only vmaddrs works."""
 
 
 import os
@@ -35,6 +35,8 @@
 self.libtwo_exe,
 self.libtwo_slide,
 )
+if self.TraceOn():
+self.runCmd("script print('Creating corefile with command %s')" % cmd)
 call(cmd, shell=True)
 
 def load_corefile_and_test(self):
@@ -48,28 +50,13 @@
 self.runCmd("script print('image list after loading corefile:')")
 self.runCmd("image list")
 
-self.assertEqual(target.GetNumModules(), 3)
+## We don't have libone.dylib in the global module cache or from
+## dsymForUUID, and lldb will not read the binary out of memory.
+self.assertEqual(target.GetNumModules(), 2)
 fspec = target.GetModuleAtIndex(0).GetFileSpec()
 self.assertEqual(fspec.GetFilename(), self.aout_exe_basename)
 
-# libone.dylib was never loaded into lldb, see that we added a memory module.
 fspec = target.GetModuleAtIndex(1).GetFileSpec()
-self.assertIn("memory-image", fspec.GetFilename())
-
-dwarfdump_uuid_regex = re.compile("UUID: ([-0-9a-fA-F]+) \(([^\(]+)\) .*")
-dwarfdump_cmd_output = subprocess.check_output(
-('/usr/bin/dwarfdump --uuid "%s"' % self.libone_exe), shell=True
-).decode("utf-8")
-libone_uuid = None
-for line in 

[Lldb-commits] [lldb] a330759 - [lldb] Fix -Wsign-compare in TestSectionSize.cpp (NFC)

[Jie Fu via lldb-commits]

Author: Jie Fu
Date: 2023-08-05T08:34:19+08:00
New Revision: a330759d6e4b7b241b70092e3dd1d0e237ad2a8a

URL: 
https://github.com/llvm/llvm-project/commit/a330759d6e4b7b241b70092e3dd1d0e237ad2a8a
DIFF: 
https://github.com/llvm/llvm-project/commit/a330759d6e4b7b241b70092e3dd1d0e237ad2a8a.diff

LOG: [lldb] Fix -Wsign-compare in TestSectionSize.cpp (NFC)

In file included from 
/data/workspace/llvm-project/lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp:10:
/data/workspace/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1620:28:
 error: comparison of integers of different signs: 'c
onst unsigned long' and 'const int' [-Werror,-Wsign-compare]
GTEST_IMPL_CMP_HELPER_(NE, !=);
~~~^~~

Added: 


Modified: 
lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp

Removed: 




diff  --git a/lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp 
b/lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp
index 3ce1e20265f4d9..1a95a0d0fc4e97 100644
--- a/lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp
+++ b/lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp
@@ -69,10 +69,10 @@ symbols: []
   DataExtractor section_data;
   ASSERT_NE(object_file->ReadSectionData(swiftast_section.get(),
  section_data),
-0);
+(size_t)0);
 
   // Check that the section data size is equal to VirtualSize (496)
   // without the zero padding, instead of SizeOfRawData (512).
-  EXPECT_EQ(section_data.GetByteSize(), 496);
+  EXPECT_EQ(section_data.GetByteSize(), (uint64_t)496);
 }
 



___
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

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/source/Core/Communication.cpp:43
 
+  std::unique_lock guard(m_shared_mutex);
   LLDB_LOG(GetLog(LLDBLog::Communication),

JDevlieghere wrote:
> augusto2112 wrote:
> > Do you think it's possible that between the call to `Clear` and this lock 
> > someone acquires the lock?
> Yes, you're both correct. We can't use the `ClearUnlocked` trick here because 
> it's `Disconnect` that locks and `Clear` is virtual. I'll need to play around 
> with this. Maybe we don't need a `Clear` and we can call `DisconnectUnlocked` 
> directly ? 
Yeah calling `Disconnect` in an unlocked fashion would solve that if you called 
it after locking in `Connect`. 


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] D157159: [lldb] Properly protect the Communication class with reader/writer lock

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Core/Communication.cpp:43
 
+  std::unique_lock guard(m_shared_mutex);
   LLDB_LOG(GetLog(LLDBLog::Communication),

augusto2112 wrote:
> Do you think it's possible that between the call to `Clear` and this lock 
> someone acquires the lock?
Yes, you're both correct. We can't use the `ClearUnlocked` trick here because 
it's `Disconnect` that locks and `Clear` is virtual. I'll need to play around 
with this. Maybe we don't need a `Clear` and we can call `DisconnectUnlocked` 
directly ? 


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] D157165: [lldb] [darwin kernel debug] When looking for a Darwin kernel symbol file, call GetSharedModules before DownloadObjectAndSymbolFile

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

ModuleList::GetSharedModule will call in to the DebugSymbols framework and may 
be able to find a file on the local system by Spotlight search, or by using the 
Platform's override of GetSharedModule, and may be faster than calling 
Symbols::DownloadObjectAndSymbolFile which calls out to an external program and 
is likely to do a network filesystem or download of the binary/symbol file.

A small perf improvement, NFC.  This is very much jason code, not really 
looking for a review of this one.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157165

Files:
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp


Index: 
lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
===
--- 
lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
+++ 
lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
@@ -763,20 +763,6 @@
   module_spec.GetUUID() = m_uuid;
   module_spec.GetArchitecture() = target.GetArchitecture();
 
-  // For the kernel, we really do need an on-disk file copy of the binary
-  // to do anything useful. This will force a call to dsymForUUID if it
-  // exists, instead of depending on the DebugSymbols preferences being
-  // set.
-  if (IsKernel()) {
-Status error;
-if (Symbols::DownloadObjectAndSymbolFile(module_spec, error, true)) {
-  if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
-m_module_sp = std::make_shared(module_spec.GetFileSpec(),
-   target.GetArchitecture());
-  }
-}
-  }
-
   // If the current platform is PlatformDarwinKernel, create a ModuleSpec
   // with the filename set to be the bundle ID for this kext, e.g.
   // "com.apple.filesystems.msdosfs", and ask the platform to find it.
@@ -806,6 +792,22 @@
 m_module_sp = target.GetOrCreateModule(module_spec, true /* notify */);
   }
 
+  // For the kernel, we really do need an on-disk file copy of the binary
+  // to do anything useful. This will force a call to dsymForUUID if it
+  // exists, instead of depending on the DebugSymbols preferences being
+  // set.
+  Status kernel_search_error;
+  if (IsKernel() &&
+  (!m_module_sp || !m_module_sp->GetSymbolFileFileSpec())) {
+if (Symbols::DownloadObjectAndSymbolFile(module_spec,
+ kernel_search_error, true)) {
+  if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+m_module_sp = std::make_shared(module_spec.GetFileSpec(),
+   target.GetArchitecture());
+  }
+}
+  }
+
   if (IsKernel() && !m_module_sp) {
 Stream  = target.GetDebugger().GetOutputStream();
 s.Printf("WARNING: Unable to locate kernel binary on the debugger "


Index: lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
===
--- lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
+++ lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
@@ -763,20 +763,6 @@
   module_spec.GetUUID() = m_uuid;
   module_spec.GetArchitecture() = target.GetArchitecture();
 
-  // For the kernel, we really do need an on-disk file copy of the binary
-  // to do anything useful. This will force a call to dsymForUUID if it
-  // exists, instead of depending on the DebugSymbols preferences being
-  // set.
-  if (IsKernel()) {
-Status error;
-if (Symbols::DownloadObjectAndSymbolFile(module_spec, error, true)) {
-  if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
-m_module_sp = std::make_shared(module_spec.GetFileSpec(),
-   target.GetArchitecture());
-  }
-}
-  }
-
   // If the current platform is PlatformDarwinKernel, create a ModuleSpec
   // with the filename set to be the bundle ID for this kext, e.g.
   // "com.apple.filesystems.msdosfs", and ask the platform to find it.
@@ -806,6 +792,22 @@
 m_module_sp = target.GetOrCreateModule(module_spec, true /* notify */);
   }
 
+  // For the kernel, we really do need an on-disk file copy of the binary
+  // to do anything useful. This will force a call to dsymForUUID if it
+  // exists, instead of depending on the DebugSymbols preferences being
+  // set.
+  Status kernel_search_error;
+  if 

[Lldb-commits] [PATCH] D157137: [lldb/crashlog] Skip images with empty path and 0 UUID from loading

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/examples/python/symbolication.py:413-418
 name = os.path.basename(self.path)
+if not name:
+if self.uuid == uuid.UUID(int=0):
+return None
+else:
+return "error: invalid image path with valid UUID (%s) 
" % uuid_str

It looks like `name` is only used below on line 435 to create `obj_file`, so 
you could move that computation down there and check the validity of 
`self.path` instead. Does that work? Or am I missing something?

Also, it seems like you might want to do this chunk of work at the beginning of 
the function to make sure you have a valid image before you try to process it? 
Is there an edge case where we'd still want to process a module with a UUID of 
0 and no path?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157137/new/

https://reviews.llvm.org/D157137

___
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

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/source/Core/Communication.cpp:41-43
   Clear();
 
+  std::unique_lock guard(m_shared_mutex);

I think this one //may// be slightly incorrect. Here's how I imagine it going 
wrong:

- Thread 1 calls Connect. It only runs `Clear()` (which disconnects) and then 
Thread 2 is scheduled.
- Thread 2 then calls Connect. It also only runs `Clear()` and then Thread 1 is 
scheduled.
- Thread 1 grabs the lock, connects, and exits.
- Thread 2 also grabs the lock, connects, and exits.

I'm not sure how gracefully `Connection::Connect` handles another connection if 
not properly disconnected from the first url beforehand. Reading through some 
of the `Connection` derived class implementations of `Connect` I'm not sure 
exactly what will happen though. The documentation isn't much clearer either...


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] D157159: [lldb] Properly protect the Communication class with reader/writer lock

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 accepted this revision.
augusto2112 added a comment.
This revision is now accepted and ready to land.

On the description you mention `unique_ptr` but I only see `shared_ptr` here. 
Was that a typo or do you mean that users of this class kept it behind a 
`unique_ptr`?

In any case I think this change is great.




Comment at: lldb/source/Core/Communication.cpp:43
 
+  std::unique_lock guard(m_shared_mutex);
   LLDB_LOG(GetLog(LLDBLog::Communication),

Do you think it's possible that between the call to `Clear` and this lock 
someone acquires the lock?


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] [lldb] 34fe58e - [lldb] Add a deduction guides for scoped_lock in OptionValue.cpp & ThreadList.cpp (NFC)

[Jie Fu via lldb-commits]

Author: Jie Fu
Date: 2023-08-05T07:56:38+08:00
New Revision: 34fe58e0bc76eff973c9dd7daeddf13c38d184d9

URL: 
https://github.com/llvm/llvm-project/commit/34fe58e0bc76eff973c9dd7daeddf13c38d184d9
DIFF: 
https://github.com/llvm/llvm-project/commit/34fe58e0bc76eff973c9dd7daeddf13c38d184d9.diff

LOG: [lldb] Add a deduction guides for scoped_lock in OptionValue.cpp & 
ThreadList.cpp (NFC)

/data/llvm-project/lldb/source/Interpreter/OptionValue.cpp:28:3: error: 
'scoped_lock' may not intend to support class template argument deduction 
[-Werror,-Wctad-maybe-unsupported]
  std::scoped_lock lock(m_mutex, other.m_mutex);
  ^
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/mutex:692:11:
 note: add a deduction guide to suppress this warning
class scoped_lock
  ^
1 error generated.

/data/llvm-project/lldb/source/Target/ThreadList.cpp:739:5: error: 
'scoped_lock' may not intend to support class template argument deduction 
[-Werror,-Wctad-maybe-unsupported]
std::scoped_lock guard(GetMutex(), rhs.GetMutex());
^
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/mutex:692:11:
 note: add a deduction guide to suppress this warning
class scoped_lock
  ^
1 error generated.

Added: 


Modified: 
lldb/source/Interpreter/OptionValue.cpp
lldb/source/Target/ThreadList.cpp

Removed: 




diff  --git a/lldb/source/Interpreter/OptionValue.cpp 
b/lldb/source/Interpreter/OptionValue.cpp
index 5ff7e3c3208980..fe1d8829c5adf6 100644
--- a/lldb/source/Interpreter/OptionValue.cpp
+++ b/lldb/source/Interpreter/OptionValue.cpp
@@ -25,7 +25,7 @@ OptionValue::OptionValue(const OptionValue ) {
 }
 
 OptionValue& OptionValue::operator=(const OptionValue ) {
-  std::scoped_lock lock(m_mutex, other.m_mutex);
+  std::scoped_lock lock(m_mutex, other.m_mutex);
 
   m_parent_wp = other.m_parent_wp;
   m_callback = other.m_callback;

diff  --git a/lldb/source/Target/ThreadList.cpp 
b/lldb/source/Target/ThreadList.cpp
index c5c3f667c90471..1ba0c435b993d3 100644
--- a/lldb/source/Target/ThreadList.cpp
+++ b/lldb/source/Target/ThreadList.cpp
@@ -736,7 +736,7 @@ void ThreadList::Update(ThreadList ) {
   if (this != ) {
 // Lock both mutexes to make sure neither side changes anyone on us while
 // the assignment occurs
-std::scoped_lock guard(GetMutex(), rhs.GetMutex());
+std::scoped_lock 
guard(GetMutex(), rhs.GetMutex());
 
 m_process = rhs.m_process;
 m_stop_id = rhs.m_stop_id;



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157160: Surface error messages from the DebugSymbols DBGShellCommands external agent; add logging for LC_NOTEs in Mach-O corefiles

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added a reviewer: bulbazord.
jasonmolenda added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

On Darwin systems, lldb will call into the DebugSymbols framework to find the 
debug info (and/or binary) for a given UUID.  The user may have set a 
DBGShellCommands preference in com.apple.DebugSymbols to find that binary by 
UUID.  lldb will also try a binary that is set in the env var 
LLDB_APPLE_DSYMFORUUID_EXECUTABLE (mostly used by the testsuite) and for 
certain "force a binary load" binaries (e.g. the Darwin kernel binary/symbols, 
without which kernel debugging is largely useless), we have a special case in 
source/Symbol/LocateSymbolFileMacOSX.cpp that knows about a binary called 
/usr/local/bin/dsymForUUID that may be installed.

These DBGShellCommands programs return a plist (xml) output giving lldb the 
location of the dSYM and/or binary.  They can also return an error message in 
that xml response.  Today lldb does not print that error, making it difficult 
for users to understand why a binary was not successfully loaded.

I also added some simple logging as lldb processes LC_NOTEs in a mach-o 
corefile binary that can be enabled, to understand what lldb is seeing as it 
loads one of these.

I have another patch that I'll be uploading soon where I fix the DBGError 
return from a test case TestMultipleBinaryCorefile.py, although the error 
message is sent to the Debugger's Stdout channel so I'm not capturing/testing 
it in the API test right now.  (the dsym-for-uuid.sh that this test case 
creates DOES emit a DBGError right now on an unknown uuid, but it doesn't 
format it correctly and lldb ignores it).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157160

Files:
  lldb/source/Core/DynamicLoader.cpp
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

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
@@ -5414,6 +5414,8 @@
 
 std::string ObjectFileMachO::GetIdentifierString() {
   std::string result;
+  Log *log(
+  GetLog(LLDBLog::Symbols | LLDBLog::Process | LLDBLog::DynamicLoader));
   ModuleSP module_sp(GetModule());
   if (module_sp) {
 std::lock_guard guard(module_sp->GetMutex());
@@ -5449,6 +5451,9 @@
 result = buf;
 if (buf)
   free(buf);
+if (log)
+  log->Printf("LC_NOTE 'kern ver str' found with text '%s'",
+  result.c_str());
 return result;
   }
 }
@@ -5472,6 +5477,8 @@
   buf) == ident_command.cmdsize) {
   buf[ident_command.cmdsize - 1] = '\0';
   result = buf;
+  if (log)
+log->Printf("LC_IDENT found with text '%s'", result.c_str());
 }
 if (buf)
   free(buf);
@@ -5484,6 +5491,7 @@
 
 addr_t ObjectFileMachO::GetAddressMask() {
   addr_t mask = 0;
+  Log *log(GetLog(LLDBLog::Process));
   ModuleSP module_sp(GetModule());
   if (module_sp) {
 std::lock_guard guard(module_sp->GetMutex());
@@ -5511,6 +5519,10 @@
   if (num_addr_bits != 0) {
 mask = ~((1ULL << num_addr_bits) - 1);
   }
+  if (log)
+log->Printf("LC_NOTE 'addrable bits' found, value %d bits, "
+"mask 0x%" PRIx64,
+num_addr_bits, mask);
   break;
 }
   }
@@ -5526,6 +5538,8 @@
 bool _is_offset,
 UUID ,
 ObjectFile::BinaryType ) {
+  Log *log(
+  GetLog(LLDBLog::Symbols | LLDBLog::Process | LLDBLog::DynamicLoader));
   value = LLDB_INVALID_ADDRESS;
   value_is_offset = false;
   uuid.Clear();
@@ -5605,20 +5619,32 @@
   uuid = UUID(raw_uuid, sizeof(uuid_t));
   // convert the "main bin spec" type into our
   // ObjectFile::BinaryType enum
+  const char *typestr = "unrecognized type";
   switch (binspec_type) {
   case 0:
 type = eBinaryTypeUnknown;
+typestr = "uknown";
 break;
   case 1:
 type = eBinaryTypeKernel;
+typestr = "xnu kernel";
 break;
   case 2:
 type = eBinaryTypeUser;
+typestr = "userland dyld";
 break;
   case 3:
   

[Lldb-commits] [PATCH] D157159: [lldb] Properly protect the Communication class with reader/writer lock

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: aprantl, augusto2112.
Herald added a project: All.
JDevlieghere requested review of this revision.

The Communication class was trying to (ab)use `unique_ptr's` atomic properties 
to protected it from concurrent access. Replace it with a regular reader/writer 
lock.


https://reviews.llvm.org/D157159

Files:
  lldb/include/lldb/Core/Communication.h
  lldb/source/Core/Communication.cpp

Index: lldb/source/Core/Communication.cpp
===
--- lldb/source/Core/Communication.cpp
+++ lldb/source/Core/Communication.cpp
@@ -27,8 +27,7 @@
 using namespace lldb_private;
 
 Communication::Communication()
-: m_connection_sp(), m_write_mutex(), m_close_on_eof(true) {
-}
+: m_connection_sp(), m_shared_mutex(), m_close_on_eof(true) {}
 
 Communication::~Communication() {
   Clear();
@@ -41,54 +40,46 @@
 ConnectionStatus Communication::Connect(const char *url, Status *error_ptr) {
   Clear();
 
+  std::unique_lock guard(m_shared_mutex);
   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);
+  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);
+
   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 ,
ConnectionStatus , 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 +88,20 @@
 
 size_t Communication::Write(const void *src, size_t src_len,
 ConnectionStatus , Status *error_ptr) {
-  lldb::ConnectionSP connection_sp(m_connection_sp);
+  std::unique_lock guard(m_shared_mutex);
+  return WriteUnlocked(src, src_len, status, error_ptr);
+}
 
-  std::lock_guard guard(m_write_mutex);
+size_t Communication::WriteUnlocked(const void *src, size_t src_len,
+ConnectionStatus ,
+Status *error_ptr) {
   LLDB_LOG(GetLog(LLDBLog::Communication),
"{0} Communication::Write (src = {1}, src_len = {2}"
") connection = {3}",
-   this, src, (uint64_t)src_len, connection_sp.get());
+   this, src, (uint64_t)src_len, m_connection_sp.get());
 
-  if (connection_sp)
-return connection_sp->Write(src, src_len, status, error_ptr);
+  if (m_connection_sp)
+return m_connection_sp->Write(src, src_len, status, error_ptr);
 
   if (error_ptr)
 error_ptr->SetErrorString("Invalid connection.");
@@ -116,10 +111,12 @@
 
 size_t Communication::WriteAll(const void *src, size_t src_len,
ConnectionStatus , Status *error_ptr) {
+  std::unique_lock guard(m_shared_mutex);
   size_t total_written = 0;
   do
-total_written += Write(static_cast(src) + total_written,
-   src_len - total_written, status, error_ptr);
+total_written +=
+WriteUnlocked(static_cast(src) + 

[Lldb-commits] [PATCH] D157153: [lldb] Fix ThreadList::Update not locking the rhs's mutex

[Augusto Noronha via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd75dc9a8a86c: [lldb] Fix ThreadList::Update not locking the 
rhss mutex (authored by augusto2112).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157153/new/

https://reviews.llvm.org/D157153

Files:
  lldb/source/Target/ThreadList.cpp


Index: lldb/source/Target/ThreadList.cpp
===
--- lldb/source/Target/ThreadList.cpp
+++ lldb/source/Target/ThreadList.cpp
@@ -736,7 +736,7 @@
   if (this != ) {
 // Lock both mutexes to make sure neither side changes anyone on us while
 // the assignment occurs
-std::lock_guard guard(GetMutex());
+std::scoped_lock guard(GetMutex(), rhs.GetMutex());
 
 m_process = rhs.m_process;
 m_stop_id = rhs.m_stop_id;


Index: lldb/source/Target/ThreadList.cpp
===
--- lldb/source/Target/ThreadList.cpp
+++ lldb/source/Target/ThreadList.cpp
@@ -736,7 +736,7 @@
   if (this != ) {
 // Lock both mutexes to make sure neither side changes anyone on us while
 // the assignment occurs
-std::lock_guard guard(GetMutex());
+std::scoped_lock guard(GetMutex(), rhs.GetMutex());
 
 m_process = rhs.m_process;
 m_stop_id = rhs.m_stop_id;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] d75dc9a - [lldb] Fix ThreadList::Update not locking the rhs's mutex

[Augusto Noronha via lldb-commits]

Author: Augusto Noronha
Date: 2023-08-04T16:01:20-07:00
New Revision: d75dc9a8a86c4f69408dcab3a21416729d14652e

URL: 
https://github.com/llvm/llvm-project/commit/d75dc9a8a86c4f69408dcab3a21416729d14652e
DIFF: 
https://github.com/llvm/llvm-project/commit/d75dc9a8a86c4f69408dcab3a21416729d14652e.diff

LOG: [lldb] Fix ThreadList::Update not locking the rhs's mutex

ThreadList::Update is being caught by thread sanitizer. There's even a
comment on the function that both mutexes should be lock (even though
only the "this" mutex was actually being locked). Fix this by locking
both mutexes.

Differential Revision: https://reviews.llvm.org/D157153

Added: 


Modified: 
lldb/source/Target/ThreadList.cpp

Removed: 




diff  --git a/lldb/source/Target/ThreadList.cpp 
b/lldb/source/Target/ThreadList.cpp
index 006c8648be524a..c5c3f667c90471 100644
--- a/lldb/source/Target/ThreadList.cpp
+++ b/lldb/source/Target/ThreadList.cpp
@@ -736,7 +736,7 @@ void ThreadList::Update(ThreadList ) {
   if (this != ) {
 // Lock both mutexes to make sure neither side changes anyone on us while
 // the assignment occurs
-std::lock_guard guard(GetMutex());
+std::scoped_lock guard(GetMutex(), rhs.GetMutex());
 
 m_process = rhs.m_process;
 m_stop_id = rhs.m_stop_id;



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157153: [lldb] Fix ThreadList::Update not locking the rhs's mutex

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157153/new/

https://reviews.llvm.org/D157153

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157153: [lldb] Fix ThreadList::Update not locking the rhs's mutex

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 created this revision.
augusto2112 added reviewers: jingham, JDevlieghere.
Herald added a project: All.
augusto2112 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

ThreadList::Update is being caught by thread sanitizer. There's even a
comment on the function that both mutexes should be lock (even though
only the "this" mutex was actually being locked). Fix this by locking
both mutexes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157153

Files:
  lldb/source/Target/ThreadList.cpp


Index: lldb/source/Target/ThreadList.cpp
===
--- lldb/source/Target/ThreadList.cpp
+++ lldb/source/Target/ThreadList.cpp
@@ -736,7 +736,7 @@
   if (this != ) {
 // Lock both mutexes to make sure neither side changes anyone on us while
 // the assignment occurs
-std::lock_guard guard(GetMutex());
+std::scoped_lock guard(GetMutex(), rhs.GetMutex());
 
 m_process = rhs.m_process;
 m_stop_id = rhs.m_stop_id;


Index: lldb/source/Target/ThreadList.cpp
===
--- lldb/source/Target/ThreadList.cpp
+++ lldb/source/Target/ThreadList.cpp
@@ -736,7 +736,7 @@
   if (this != ) {
 // Lock both mutexes to make sure neither side changes anyone on us while
 // the assignment occurs
-std::lock_guard guard(GetMutex());
+std::scoped_lock guard(GetMutex(), rhs.GetMutex());
 
 m_process = rhs.m_process;
 m_stop_id = rhs.m_stop_id;
___
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

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: aprantl, augusto2112.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
JDevlieghere requested review of this revision.

Set the `halt_on_error` runtime flag to make TSan errors fatal when running the 
test suite. For the API tests the environment variables are set conditionally 
on whether the TSan is enabled. The Shell tests don't have that logic and I 
don't think it's worth duplicating it as setting the environment variable is 
harmless. For consistency, I've also mirrored the ASAN option 
(`detect_stack_use_after_return=1`) for the Shell tests.


https://reviews.llvm.org/D157152

Files:
  lldb/test/API/lit.cfg.py
  lldb/test/Shell/lit.cfg.py


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/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

[Lldb-commits] [PATCH] D151567: [LLVM][Support] Report EISDIR when opening a directory on AIX

[Hubert Tong via Phabricator via lldb-commits]

hubert.reinterpretcast added a comment.

I believe the unit tests should be updated to ensure that we get `EISDIR` when 
opening directories for reading and for writing: 
`llvm/unittests/Support/Path.cpp`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151567/new/

https://reviews.llvm.org/D151567

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D151567: [LLVM][Support] Report EISDIR when opening a directory on AIX

[Hubert Tong via Phabricator via lldb-commits]

hubert.reinterpretcast added inline comments.



Comment at: llvm/lib/Support/Unix/Path.inc:1020
+struct stat Status;
+if (stat(P.begin(), ) == -1) 
+  return std::error_code(errno, std::generic_category());

Please try using `fstat` on the result of `open` to address the comments from 
https://reviews.llvm.org/D156798.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151567/new/

https://reviews.llvm.org/D151567

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157041: [lldb] Protect OptionValue accesses from data races

[Augusto Noronha via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG318f600722e3: [lldb] Protect OptionValue accesses from data 
races (authored by augusto2112).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157041/new/

https://reviews.llvm.org/D157041

Files:
  lldb/include/lldb/Interpreter/OptionValue.h
  lldb/source/Interpreter/OptionValue.cpp

Index: lldb/source/Interpreter/OptionValue.cpp
===
--- lldb/source/Interpreter/OptionValue.cpp
+++ lldb/source/Interpreter/OptionValue.cpp
@@ -15,6 +15,25 @@
 using namespace lldb;
 using namespace lldb_private;
 
+OptionValue::OptionValue(const OptionValue ) {
+  std::lock_guard lock(other.m_mutex);
+
+  m_parent_wp = other.m_parent_wp;
+  m_callback = other.m_callback;
+  m_value_was_set = other.m_value_was_set;
+
+}
+
+OptionValue& OptionValue::operator=(const OptionValue ) {
+  std::scoped_lock lock(m_mutex, other.m_mutex);
+
+  m_parent_wp = other.m_parent_wp;
+  m_callback = other.m_callback;
+  m_value_was_set = other.m_value_was_set;
+
+  return *this;
+}
+
 Status OptionValue::SetSubValue(const ExecutionContext *exe_ctx,
 VarSetOperationType op, llvm::StringRef name,
 llvm::StringRef value) {
@@ -252,12 +271,14 @@
 }
 
 std::optional OptionValue::GetBooleanValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueBoolean *option_value = GetAsBoolean())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetBooleanValue(bool new_value) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueBoolean *option_value = GetAsBoolean()) {
 option_value->SetCurrentValue(new_value);
 return true;
@@ -266,12 +287,14 @@
 }
 
 std::optional OptionValue::GetCharValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueChar *option_value = GetAsChar())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetCharValue(char new_value) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueChar *option_value = GetAsChar()) {
 option_value->SetCurrentValue(new_value);
 return true;
@@ -280,12 +303,14 @@
 }
 
 std::optional OptionValue::GetEnumerationValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueEnumeration *option_value = GetAsEnumeration())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetEnumerationValue(int64_t value) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueEnumeration *option_value = GetAsEnumeration()) {
 option_value->SetCurrentValue(value);
 return true;
@@ -294,12 +319,14 @@
 }
 
 std::optional OptionValue::GetFileSpecValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueFileSpec *option_value = GetAsFileSpec())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetFileSpecValue(FileSpec file_spec) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueFileSpec *option_value = GetAsFileSpec()) {
 option_value->SetCurrentValue(file_spec, false);
 return true;
@@ -308,6 +335,7 @@
 }
 
 bool OptionValue::AppendFileSpecValue(FileSpec file_spec) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueFileSpecList *option_value = GetAsFileSpecList()) {
 option_value->AppendCurrentValue(file_spec);
 return true;
@@ -316,18 +344,21 @@
 }
 
 std::optional OptionValue::GetFileSpecListValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueFileSpecList *option_value = GetAsFileSpecList())
 return option_value->GetCurrentValue();
   return {};
 }
 
 std::optional OptionValue::GetFormatValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueFormat *option_value = GetAsFormat())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetFormatValue(lldb::Format new_value) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueFormat *option_value = GetAsFormat()) {
 option_value->SetCurrentValue(new_value);
 return true;
@@ -336,12 +367,14 @@
 }
 
 std::optional OptionValue::GetLanguageValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueLanguage *option_value = GetAsLanguage())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetLanguageValue(lldb::LanguageType new_language) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueLanguage *option_value = GetAsLanguage()) {
 option_value->SetCurrentValue(new_language);
 return true;
@@ -350,24 +383,28 @@
 }
 
 const FormatEntity::Entry *OptionValue::GetFormatEntity() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueFormatEntity *option_value = GetAsFormatEntity())
 return _value->GetCurrentValue();
   return nullptr;
 }
 
 const RegularExpression *OptionValue::GetRegexValue() const {
+  std::lock_guard lock(m_mutex);
   if 

[Lldb-commits] [lldb] 318f600 - [lldb] Protect OptionValue accesses from data races

[Augusto Noronha via lldb-commits]

Author: Augusto Noronha
Date: 2023-08-04T15:33:46-07:00
New Revision: 318f600722e3cc6745317bb68309f82656c97b27

URL: 
https://github.com/llvm/llvm-project/commit/318f600722e3cc6745317bb68309f82656c97b27
DIFF: 
https://github.com/llvm/llvm-project/commit/318f600722e3cc6745317bb68309f82656c97b27.diff

LOG: [lldb] Protect OptionValue accesses from data races

Thread sanitizer is catching data races in OptionValue, protect accesses
to OptionValue with a mutex.

Differential Revision: https://reviews.llvm.org/D157041

Added: 


Modified: 
lldb/include/lldb/Interpreter/OptionValue.h
lldb/source/Interpreter/OptionValue.cpp

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/OptionValue.h 
b/lldb/include/lldb/Interpreter/OptionValue.h
index 4fa0b23042669d..bfbdbead72c66d 100644
--- a/lldb/include/lldb/Interpreter/OptionValue.h
+++ b/lldb/include/lldb/Interpreter/OptionValue.h
@@ -23,6 +23,7 @@
 #include "lldb/lldb-private-enumerations.h"
 #include "lldb/lldb-private-interfaces.h"
 #include "llvm/Support/JSON.h"
+#include 
 
 namespace lldb_private {
 
@@ -70,6 +71,10 @@ class OptionValue {
 
   virtual ~OptionValue() = default;
 
+  OptionValue(const OptionValue );
+  
+  OptionValue& operator=(const OptionValue );
+
   // Subclasses should override these functions
   virtual Type GetType() const = 0;
 
@@ -382,6 +387,8 @@ class OptionValue {
 
   const FormatEntity::Entry *GetFormatEntity() const;
   const RegularExpression *GetRegexValue() const;
+  
+  mutable std::mutex m_mutex;
 };
 
 } // namespace lldb_private

diff  --git a/lldb/source/Interpreter/OptionValue.cpp 
b/lldb/source/Interpreter/OptionValue.cpp
index 8d429b6bc9cf21..5ff7e3c3208980 100644
--- a/lldb/source/Interpreter/OptionValue.cpp
+++ b/lldb/source/Interpreter/OptionValue.cpp
@@ -15,6 +15,25 @@
 using namespace lldb;
 using namespace lldb_private;
 
+OptionValue::OptionValue(const OptionValue ) {
+  std::lock_guard lock(other.m_mutex);
+
+  m_parent_wp = other.m_parent_wp;
+  m_callback = other.m_callback;
+  m_value_was_set = other.m_value_was_set;
+
+}
+
+OptionValue& OptionValue::operator=(const OptionValue ) {
+  std::scoped_lock lock(m_mutex, other.m_mutex);
+
+  m_parent_wp = other.m_parent_wp;
+  m_callback = other.m_callback;
+  m_value_was_set = other.m_value_was_set;
+
+  return *this;
+}
+
 Status OptionValue::SetSubValue(const ExecutionContext *exe_ctx,
 VarSetOperationType op, llvm::StringRef name,
 llvm::StringRef value) {
@@ -252,12 +271,14 @@ const OptionValueUUID *OptionValue::GetAsUUID() const {
 }
 
 std::optional OptionValue::GetBooleanValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueBoolean *option_value = GetAsBoolean())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetBooleanValue(bool new_value) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueBoolean *option_value = GetAsBoolean()) {
 option_value->SetCurrentValue(new_value);
 return true;
@@ -266,12 +287,14 @@ bool OptionValue::SetBooleanValue(bool new_value) {
 }
 
 std::optional OptionValue::GetCharValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueChar *option_value = GetAsChar())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetCharValue(char new_value) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueChar *option_value = GetAsChar()) {
 option_value->SetCurrentValue(new_value);
 return true;
@@ -280,12 +303,14 @@ bool OptionValue::SetCharValue(char new_value) {
 }
 
 std::optional OptionValue::GetEnumerationValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueEnumeration *option_value = GetAsEnumeration())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetEnumerationValue(int64_t value) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueEnumeration *option_value = GetAsEnumeration()) {
 option_value->SetCurrentValue(value);
 return true;
@@ -294,12 +319,14 @@ bool OptionValue::SetEnumerationValue(int64_t value) {
 }
 
 std::optional OptionValue::GetFileSpecValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueFileSpec *option_value = GetAsFileSpec())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetFileSpecValue(FileSpec file_spec) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueFileSpec *option_value = GetAsFileSpec()) {
 option_value->SetCurrentValue(file_spec, false);
 return true;
@@ -308,6 +335,7 @@ bool OptionValue::SetFileSpecValue(FileSpec file_spec) {
 }
 
 bool OptionValue::AppendFileSpecValue(FileSpec file_spec) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueFileSpecList *option_value = GetAsFileSpecList()) {
 option_value->AppendCurrentValue(file_spec);
 return true;
@@ -316,18 +344,21 @@ 

[Lldb-commits] [PATCH] D157041: [lldb] Protect OptionValue accesses from data races

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D157041#4562033 , @aprantl wrote:

>> If it's straightforward, I think would be nice to have a unit test with two 
>> threads modifying the same OptionValue. That way a TSan run would catch this 
>> issue. If that's more work than expected then this is fine as is.
>
> We might just want to set up a bot that runs the entire test suite under 
> TSan, assuming that we can get it clean?

Definitely, but last I checked there was still quite some work to be done :-)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157041/new/

https://reviews.llvm.org/D157041

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157041: [lldb] Protect OptionValue accesses from data races

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

> If it's straightforward, I think would be nice to have a unit test with two 
> threads modifying the same OptionValue. That way a TSan run would catch this 
> issue. If that's more work than expected then this is fine as is.

We might just want to set up a bot that runs the entire test suite under TSan, 
assuming that we can get it clean?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157041/new/

https://reviews.llvm.org/D157041

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] ecbe78c - [lldb] Fix Python test formatting (NFC)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2023-08-04T14:36:13-07:00
New Revision: ecbe78c124a78a4ea6e06b1d52ce281dcc394332

URL: 
https://github.com/llvm/llvm-project/commit/ecbe78c124a78a4ea6e06b1d52ce281dcc394332
DIFF: 
https://github.com/llvm/llvm-project/commit/ecbe78c124a78a4ea6e06b1d52ce281dcc394332.diff

LOG: [lldb] Fix Python test formatting (NFC)

All Python files in the LLVM repository were reformatted with Black [1].
Files inside the LLDB subproject were reformatted in 2238dcc39358. This
patch updates a handful of tests that were added or modified since then
and weren't formatted with Black.

[1] 
https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style/68257

Added: 


Modified: 
lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py

lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py

lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py

lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
lldb/test/API/commands/register/register/register_command/TestRegisters.py
lldb/test/API/commands/settings/TestSettings.py
lldb/test/API/functionalities/archives/TestBSDArchives.py
lldb/test/API/functionalities/completion/TestCompletion.py
lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py

lldb/test/API/functionalities/postmortem/elf-core/thread_crash/TestLinuxCoreThreads.py

lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
lldb/test/API/lang/objc/objc-po-hint/TestObjcPoHint.py
lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
lldb/test/API/python_api/value/TestValueAPI.py
lldb/test/API/source-manager/TestSourceManager.py
lldb/test/API/tools/lldb-vscode/disassemble/TestVSCode_disassemble.py
lldb/test/API/tools/lldb-vscode/runInTerminal/TestVSCode_runInTerminal.py

Removed: 




diff  --git 
a/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py 
b/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py
index 3312cb04832ab7..3fb7d00c93d29d 100644
--- 
a/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py
+++ 
b/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py
@@ -22,10 +22,17 @@ def test_process_launch_no_args(self):
 self.runCmd("continue")
 
 with open(outfile) as f:
-   self.assertEqual(dedent("""\
+self.assertEqual(
+dedent(
+"""\
 Got 1 argument(s).
 [0]: {}
-""".format(exe)), f.read())
+""".format(
+exe
+)
+),
+f.read(),
+)
 
 def test_process_launch_command_args(self):
 exe, outfile = self.setup()
@@ -35,13 +42,20 @@ def test_process_launch_command_args(self):
 self.runCmd("continue")
 
 with open(outfile) as f:
-   self.assertEqual(dedent("""\
+self.assertEqual(
+dedent(
+"""\
 Got 4 argument(s).
 [0]: {}
 [1]: A
 [2]: B
 [3]: C
-""".format(exe)), f.read())
+""".format(
+exe
+)
+),
+f.read(),
+)
 
 def test_process_launch_target_args(self):
 exe, outfile = self.setup()
@@ -51,9 +65,16 @@ def test_process_launch_target_args(self):
 self.runCmd("continue")
 
 with open(outfile) as f:
-   self.assertEqual(dedent("""\
+self.assertEqual(
+dedent(
+"""\
 Got 3 argument(s).
 [0]: {}
 [1]: D
 [2]: E
-""".format(exe)), f.read())
\ No newline at end of file
+""".format(
+exe
+)
+),
+f.read(),
+)

diff  --git 
a/lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
 
b/lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
index a8807d9014d7f7..3ca20a1803b0fc 100644
--- 
a/lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
+++ 
b/lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -14,10 +14,12 @@
 from lldbsuite.test.lldbtest import *
 from 

[Lldb-commits] [PATCH] D156804: [lldb] Bump SWIG minimum version to 4

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe0053bc04e6b: [lldb] Bump SWIG minimum version to 4 
(authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D156804?vs=546093=547354#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156804/new/

https://reviews.llvm.org/D156804

Files:
  lldb/bindings/python/python.swig
  lldb/cmake/modules/FindLuaAndSwig.cmake
  lldb/cmake/modules/FindPythonAndSwig.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/docs/resources/build.rst
  
lldb/test/API/python_api/default-constructor/TestDefaultConstructorForAPIObjects.py
  lldb/test/API/python_api/lldbutil/TestSwigVersion.py
  lldb/test/API/python_api/module_section/TestModuleAndSection.py
  lldb/test/API/python_api/value/linked_list/TestValueAPILinkedList.py

Index: lldb/test/API/python_api/value/linked_list/TestValueAPILinkedList.py
===
--- lldb/test/API/python_api/value/linked_list/TestValueAPILinkedList.py
+++ lldb/test/API/python_api/value/linked_list/TestValueAPILinkedList.py
@@ -20,9 +20,6 @@
 # Find the line number to break at.
 self.line = line_number("main.cpp", "// Break at this line")
 
-# Py3 asserts due to a bug in SWIG.  A fix for this was upstreamed into
-# SWIG 3.0.8.
-@skipIf(py_version=[">=", (3, 0)], swig_version=["<", (3, 0, 8)])
 def test(self):
 """Exercise SBValue API linked_list_iter."""
 d = {"EXE": self.exe_name}
Index: lldb/test/API/python_api/module_section/TestModuleAndSection.py
===
--- lldb/test/API/python_api/module_section/TestModuleAndSection.py
+++ lldb/test/API/python_api/module_section/TestModuleAndSection.py
@@ -10,9 +10,6 @@
 
 
 class ModuleAndSectionAPIsTestCase(TestBase):
-# Py3 asserts due to a bug in SWIG.  A fix for this was upstreamed into
-# SWIG 3.0.8.
-@skipIf(py_version=[">=", (3, 0)], swig_version=["<", (3, 0, 8)])
 def test_module_and_section(self):
 """Test module and section APIs."""
 self.build()
Index: lldb/test/API/python_api/lldbutil/TestSwigVersion.py
===
--- lldb/test/API/python_api/lldbutil/TestSwigVersion.py
+++ lldb/test/API/python_api/lldbutil/TestSwigVersion.py
@@ -21,6 +21,6 @@
 self.assertTrue(getattr(lldb, "swig_version"))
 self.assertIsInstance(lldb.swig_version, tuple)
 self.assertEqual(len(lldb.swig_version), 3)
-self.assertGreaterEqual(lldb.swig_version[0], 1)
+self.assertGreaterEqual(lldb.swig_version[0], 4)
 for v in lldb.swig_version:
 self.assertGreaterEqual(v, 0)
Index: lldb/test/API/python_api/default-constructor/TestDefaultConstructorForAPIObjects.py
===
--- lldb/test/API/python_api/default-constructor/TestDefaultConstructorForAPIObjects.py
+++ lldb/test/API/python_api/default-constructor/TestDefaultConstructorForAPIObjects.py
@@ -117,8 +117,6 @@
 
 sb_debugger.fuzz_obj(obj)
 
-# darwin: This test passes with swig 3.0.2, fails w/3.0.5 other tests fail
-# with 2.0.12 http://llvm.org/pr23488
 def test_SBError(self):
 obj = lldb.SBError()
 if self.TraceOn():
@@ -225,9 +223,6 @@
 
 sb_listener.fuzz_obj(obj)
 
-# Py3 asserts due to a bug in SWIG.  Trying to upstream a patch to fix
-# this in 3.0.8
-@skipIf(py_version=[">=", (3, 0)], swig_version=["<", (3, 0, 8)])
 def test_SBModule(self):
 obj = lldb.SBModule()
 if self.TraceOn():
Index: lldb/docs/resources/build.rst
===
--- lldb/docs/resources/build.rst
+++ lldb/docs/resources/build.rst
@@ -34,7 +34,7 @@
 scripting support.
 
 * `Python `_
-* `SWIG `_ 3 or later.
+* `SWIG `_ 4 or later.
 
 Optional Dependencies
 *
Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -50,7 +50,7 @@
   message(STATUS "${description}: ${${variable}}")
 endmacro()
 
-add_optional_dependency(LLDB_ENABLE_SWIG "Enable SWIG to generate LLDB bindings" SWIG SWIG_FOUND VERSION 3)
+add_optional_dependency(LLDB_ENABLE_SWIG "Enable SWIG to generate LLDB bindings" SWIG SWIG_FOUND VERSION 4)
 add_optional_dependency(LLDB_ENABLE_LIBEDIT "Enable editline support in LLDB" LibEdit LibEdit_FOUND)
 add_optional_dependency(LLDB_ENABLE_CURSES "Enable curses support in LLDB" CursesAndPanel CURSESANDPANEL_FOUND)
 add_optional_dependency(LLDB_ENABLE_LZMA "Enable LZMA 

[Lldb-commits] [lldb] e0053bc - [lldb] Bump SWIG minimum version to 4

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2023-08-04T14:34:01-07:00
New Revision: e0053bc04e6b80ece8b334b268c2942e012009b9

URL: 
https://github.com/llvm/llvm-project/commit/e0053bc04e6b80ece8b334b268c2942e012009b9
DIFF: 
https://github.com/llvm/llvm-project/commit/e0053bc04e6b80ece8b334b268c2942e012009b9.diff

LOG: [lldb] Bump SWIG minimum version to 4

SWIG 4 was released in 2019 and has been the de-facto standard for a
while now. All bots are running SWIG 4.0 or later.

This was motivated by #64279 which discovered that 662548c broke the
LLDB build with SWIG 3 on Windows.

Differential revision: https://reviews.llvm.org/D156804

Added: 


Modified: 
lldb/bindings/python/python.swig
lldb/cmake/modules/FindLuaAndSwig.cmake
lldb/cmake/modules/FindPythonAndSwig.cmake
lldb/cmake/modules/LLDBConfig.cmake
lldb/docs/resources/build.rst

lldb/test/API/python_api/default-constructor/TestDefaultConstructorForAPIObjects.py
lldb/test/API/python_api/lldbutil/TestSwigVersion.py
lldb/test/API/python_api/module_section/TestModuleAndSection.py
lldb/test/API/python_api/value/linked_list/TestValueAPILinkedList.py

Removed: 




diff  --git a/lldb/bindings/python/python.swig 
b/lldb/bindings/python/python.swig
index b1f6c4b9301da0..278c0eed2bab27 100644
--- a/lldb/bindings/python/python.swig
+++ b/lldb/bindings/python/python.swig
@@ -52,10 +52,6 @@ except ImportError:
 # Relative import should work if we are being loaded by Python.
 from . import $module"
 %enddef
-// These versions will not generate working python modules, so error out early.
-#if SWIG_VERSION >= 0x030009 && SWIG_VERSION < 0x030011
-#error Swig versions 3.0.9 and 3.0.10 are incompatible with lldb.
-#endif
 
 // The name of the module to be created.
 %module(docstring=DOCSTRING, moduleimport=MODULEIMPORT) lldb

diff  --git a/lldb/cmake/modules/FindLuaAndSwig.cmake 
b/lldb/cmake/modules/FindLuaAndSwig.cmake
index 763bf0a7bb9938..11548b76f843f0 100644
--- a/lldb/cmake/modules/FindLuaAndSwig.cmake
+++ b/lldb/cmake/modules/FindLuaAndSwig.cmake
@@ -15,7 +15,7 @@ else()
 LUA_INCLUDE_DIR)
 endif()
   else()
-message(STATUS "SWIG 3 or later is required for Lua support in LLDB but 
could not be found")
+message(STATUS "SWIG 4 or later is required for Lua support in LLDB but 
could not be found")
   endif()
 
 

diff  --git a/lldb/cmake/modules/FindPythonAndSwig.cmake 
b/lldb/cmake/modules/FindPythonAndSwig.cmake
index d9305ab31f2258..d62cced0d095e9 100644
--- a/lldb/cmake/modules/FindPythonAndSwig.cmake
+++ b/lldb/cmake/modules/FindPythonAndSwig.cmake
@@ -40,7 +40,7 @@ else()
   if (LLDB_ENABLE_SWIG)
 FindPython3()
   else()
-message(STATUS "SWIG 3 or later is required for Python support in LLDB but 
could not be found")
+message(STATUS "SWIG 4 or later is required for Python support in LLDB but 
could not be found")
   endif()
 
   get_property(MULTI_CONFIG GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG)

diff  --git a/lldb/cmake/modules/LLDBConfig.cmake 
b/lldb/cmake/modules/LLDBConfig.cmake
index ca64ba798eda2d..19283b3cbb0194 100644
--- a/lldb/cmake/modules/LLDBConfig.cmake
+++ b/lldb/cmake/modules/LLDBConfig.cmake
@@ -50,7 +50,7 @@ macro(add_optional_dependency variable description package 
found)
   message(STATUS "${description}: ${${variable}}")
 endmacro()
 
-add_optional_dependency(LLDB_ENABLE_SWIG "Enable SWIG to generate LLDB 
bindings" SWIG SWIG_FOUND VERSION 3)
+add_optional_dependency(LLDB_ENABLE_SWIG "Enable SWIG to generate LLDB 
bindings" SWIG SWIG_FOUND VERSION 4)
 add_optional_dependency(LLDB_ENABLE_LIBEDIT "Enable editline support in LLDB" 
LibEdit LibEdit_FOUND)
 add_optional_dependency(LLDB_ENABLE_CURSES "Enable curses support in LLDB" 
CursesAndPanel CURSESANDPANEL_FOUND)
 add_optional_dependency(LLDB_ENABLE_LZMA "Enable LZMA compression support in 
LLDB" LibLZMA LIBLZMA_FOUND)

diff  --git a/lldb/docs/resources/build.rst b/lldb/docs/resources/build.rst
index fdbb4d8a005a16..d4f7a558f87953 100644
--- a/lldb/docs/resources/build.rst
+++ b/lldb/docs/resources/build.rst
@@ -34,7 +34,7 @@ If you want to run the test suite, you'll need to build LLDB 
with Python
 scripting support.
 
 * `Python `_
-* `SWIG `_ 3 or later.
+* `SWIG `_ 4 or later.
 
 Optional Dependencies
 *

diff  --git 
a/lldb/test/API/python_api/default-constructor/TestDefaultConstructorForAPIObjects.py
 
b/lldb/test/API/python_api/default-constructor/TestDefaultConstructorForAPIObjects.py
index d56c5b01d5f295..ec5b4da0f29b26 100644
--- 
a/lldb/test/API/python_api/default-constructor/TestDefaultConstructorForAPIObjects.py
+++ 
b/lldb/test/API/python_api/default-constructor/TestDefaultConstructorForAPIObjects.py
@@ -117,8 +117,6 @@ def test_SBDebugger(self):
 
 sb_debugger.fuzz_obj(obj)
 
-# darwin: This test passes with swig 3.0.2, fails 

[Lldb-commits] [PATCH] D156804: [lldb] Bump SWIG minimum version to 4

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

I didn't expect this to be controversial and I hear no objection so I'm going 
to go ahead and land this.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156804/new/

https://reviews.llvm.org/D156804

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157137: [lldb/crashlog] Skip images with empty path and 0 UUID from loading

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: JDevlieghere, bulbazord.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch skips images with an empty path or a 0 UUID from loading as a
SymbolFileJSON.

rdar://112107986

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157137

Files:
  lldb/examples/python/symbolication.py


Index: lldb/examples/python/symbolication.py
===
--- lldb/examples/python/symbolication.py
+++ lldb/examples/python/symbolication.py
@@ -411,6 +411,11 @@
 )
 if not self.module and self.section_infos:
 name = os.path.basename(self.path)
+if not name:
+if self.uuid == uuid.UUID(int=0):
+return None
+else:
+return "error: invalid image path with valid UUID (%s) 
" % uuid_str
 if obj_dir and os.path.isdir(obj_dir):
 data = {
 "triple": target.triple,


Index: lldb/examples/python/symbolication.py
===
--- lldb/examples/python/symbolication.py
+++ lldb/examples/python/symbolication.py
@@ -411,6 +411,11 @@
 )
 if not self.module and self.section_infos:
 name = os.path.basename(self.path)
+if not name:
+if self.uuid == uuid.UUID(int=0):
+return None
+else:
+return "error: invalid image path with valid UUID (%s) " % uuid_str
 if obj_dir and os.path.isdir(obj_dir):
 data = {
 "triple": target.triple,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157028: [llvm] Extract common `OptTable` bits into macros

[Jan Svoboda via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3f092f37b736: [llvm] Extract common `OptTable` bits into 
macros (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157028/new/

https://reviews.llvm.org/D157028

Files:
  clang/include/clang/Driver/Options.h
  clang/lib/Driver/DriverOptions.cpp
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp
  lld/COFF/Driver.h
  lld/COFF/DriverUtils.cpp
  lld/ELF/Driver.h
  lld/ELF/DriverUtils.cpp
  lld/MachO/Driver.h
  lld/MinGW/Driver.cpp
  lld/wasm/Driver.cpp
  lldb/tools/driver/Driver.cpp
  lldb/tools/lldb-server/lldb-gdbserver.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  llvm/include/llvm/Option/OptTable.h
  llvm/lib/ExecutionEngine/JITLink/COFFDirectiveParser.cpp
  llvm/lib/ExecutionEngine/JITLink/COFFDirectiveParser.h
  llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/dsymutil/dsymutil.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp
  llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp
  llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp
  llvm/tools/llvm-dwp/llvm-dwp.cpp
  llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp
  llvm/tools/llvm-ifs/llvm-ifs.cpp
  llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp
  llvm/tools/llvm-lipo/llvm-lipo.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-mt/llvm-mt.cpp
  llvm/tools/llvm-nm/llvm-nm.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/tools/llvm-objdump/ObjdumpOptID.h
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-rc/llvm-rc.cpp
  llvm/tools/llvm-readobj/llvm-readobj.cpp
  llvm/tools/llvm-size/llvm-size.cpp
  llvm/tools/llvm-strings/llvm-strings.cpp
  llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
  llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
  llvm/tools/sancov/sancov.cpp
  llvm/unittests/Option/OptionParsingTest.cpp

Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp
@@ -17,9 +17,7 @@
 
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR, VALUES)  \
-  OPT_##ID,
+#define OPTION(...) LLVM_MAKE_OPT_ID(__VA_ARGS__),
 #include "Opts.inc"
   LastOption
 #undef OPTION
@@ -47,10 +45,7 @@
 };
 
 static constexpr OptTable::Info InfoTable[] = {
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR, VALUES)  \
-  {PREFIX, NAME,  HELPTEXT,METAVAR, OPT_##ID,  Option::KIND##Class,\
-   PARAM,  FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS, VALUES},
+#define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__),
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/tools/sancov/sancov.cpp
===
--- llvm/tools/sancov/sancov.cpp
+++ llvm/tools/sancov/sancov.cpp
@@ -63,9 +63,7 @@
 using namespace llvm::opt;
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR, VALUES)  \
-  OPT_##ID,
+#define OPTION(...) LLVM_MAKE_OPT_ID(__VA_ARGS__),
 #include "Opts.inc"
 #undef OPTION
 };
@@ -78,13 +76,7 @@
 #undef PREFIX
 
 static constexpr opt::OptTable::Info InfoTable[] = {
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR, VALUES)  \
-  {\
-  PREFIX,  NAME,  HELPTEXT,\
-  METAVAR, OPT_##ID,  opt::Option::KIND##Class,\
-  PARAM,   FLAGS, OPT_##GROUP, \
-  OPT_##ALIAS, ALIASARGS, VALUES},
+#define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__),
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
===
--- llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
+++ llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
@@ -28,9 +28,7 @@
 namespace {
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR, VALUES)  \
-  OPT_##ID,
+#define OPTION(...) LLVM_MAKE_OPT_ID(__VA_ARGS__),
 #include "Opts.inc"
 #undef OPTION
 };
@@ -43,13 +41,7 @@
 #undef PREFIX
 
 static constexpr 

[Lldb-commits] [lldb] 3f092f3 - [llvm] Extract common `OptTable` bits into macros

[Jan Svoboda via lldb-commits]

Author: Jan Svoboda
Date: 2023-08-04T13:57:13-07:00
New Revision: 3f092f37b7362447cbb13f5502dae4bdd5762afd

URL: 
https://github.com/llvm/llvm-project/commit/3f092f37b7362447cbb13f5502dae4bdd5762afd
DIFF: 
https://github.com/llvm/llvm-project/commit/3f092f37b7362447cbb13f5502dae4bdd5762afd.diff

LOG: [llvm] Extract common `OptTable` bits into macros

All command-line tools using `llvm::opt` create an enum of option IDs and a 
table of `OptTable::Info` object. Most of the tools use the same ID 
(`OPT_##ID`), kind (`Option::KIND##Class`), group ID (`OPT_##GROUP`) and alias 
ID (`OPT_##ALIAS`). This patch extracts that common code into canonical macros. 
This results in fewer changes when tweaking the `OPTION` macros emitted by the 
TableGen backend.

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D157028

Added: 


Modified: 
clang/include/clang/Driver/Options.h
clang/lib/Driver/DriverOptions.cpp
clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp
lld/COFF/Driver.h
lld/COFF/DriverUtils.cpp
lld/ELF/Driver.h
lld/ELF/DriverUtils.cpp
lld/MachO/Driver.h
lld/MinGW/Driver.cpp
lld/wasm/Driver.cpp
lldb/tools/driver/Driver.cpp
lldb/tools/lldb-server/lldb-gdbserver.cpp
lldb/tools/lldb-vscode/lldb-vscode.cpp
llvm/include/llvm/Option/OptTable.h
llvm/lib/ExecutionEngine/JITLink/COFFDirectiveParser.cpp
llvm/lib/ExecutionEngine/JITLink/COFFDirectiveParser.h
llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
llvm/tools/dsymutil/dsymutil.cpp
llvm/tools/llvm-cvtres/llvm-cvtres.cpp
llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp
llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp
llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp
llvm/tools/llvm-dwp/llvm-dwp.cpp
llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp
llvm/tools/llvm-ifs/llvm-ifs.cpp
llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp
llvm/tools/llvm-lipo/llvm-lipo.cpp
llvm/tools/llvm-ml/llvm-ml.cpp
llvm/tools/llvm-mt/llvm-mt.cpp
llvm/tools/llvm-nm/llvm-nm.cpp
llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
llvm/tools/llvm-objdump/ObjdumpOptID.h
llvm/tools/llvm-objdump/llvm-objdump.cpp
llvm/tools/llvm-rc/llvm-rc.cpp
llvm/tools/llvm-readobj/llvm-readobj.cpp
llvm/tools/llvm-size/llvm-size.cpp
llvm/tools/llvm-strings/llvm-strings.cpp
llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
llvm/tools/sancov/sancov.cpp
llvm/unittests/Option/OptionParsingTest.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.h 
b/clang/include/clang/Driver/Options.h
index 54c6f5faa37c22..fb0e536d972cbf 100644
--- a/clang/include/clang/Driver/Options.h
+++ b/clang/include/clang/Driver/Options.h
@@ -9,11 +9,7 @@
 #ifndef LLVM_CLANG_DRIVER_OPTIONS_H
 #define LLVM_CLANG_DRIVER_OPTIONS_H
 
-namespace llvm {
-namespace opt {
-class OptTable;
-}
-}
+#include "llvm/Option/OptTable.h"
 
 namespace clang {
 namespace driver {
@@ -43,9 +39,7 @@ enum ClangFlags {
 
 enum ID {
 OPT_INVALID = 0, // This is not an option ID.
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  
\
-   HELPTEXT, METAVAR, VALUES)  
\
-  OPT_##ID,
+#define OPTION(...) LLVM_MAKE_OPT_ID(__VA_ARGS__),
 #include "clang/Driver/Options.inc"
 LastOption
 #undef OPTION

diff  --git a/clang/lib/Driver/DriverOptions.cpp 
b/clang/lib/Driver/DriverOptions.cpp
index 2a6868d179159d..b25801a8f3f494 100644
--- a/clang/lib/Driver/DriverOptions.cpp
+++ b/clang/lib/Driver/DriverOptions.cpp
@@ -36,10 +36,7 @@ static constexpr const llvm::ArrayRef
 PrefixTable(PrefixTable_init, std::size(PrefixTable_init) - 1);
 
 static constexpr OptTable::Info InfoTable[] = {
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  
\
-   HELPTEXT, METAVAR, VALUES)  
\
-  {PREFIX, NAME,  HELPTEXT,METAVAR, OPT_##ID,  Option::KIND##Class,
\
-   PARAM,  FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS, VALUES},
+#define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__),
 #include "clang/Driver/Options.inc"
 #undef OPTION
 };

diff  --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp 
b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
index c553cf86da8e32..aea6792f1f3b84 100644
--- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -117,9 +117,7 @@ enum WrapperFlags {
 
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  
\
-   HELPTEXT, METAVAR, VALUES)  
\
-  OPT_##ID,
+#define 

[Lldb-commits] [lldb] e9040e8 - [lldb][PECOFF] Exclude alignment padding when reading section data

[Hiroshi Yamauchi via lldb-commits]

Author: Hiroshi Yamauchi
Date: 2023-08-04T13:38:30-07:00
New Revision: e9040e875d9252f726c41579f70663154718c3c6

URL: 
https://github.com/llvm/llvm-project/commit/e9040e875d9252f726c41579f70663154718c3c6
DIFF: 
https://github.com/llvm/llvm-project/commit/e9040e875d9252f726c41579f70663154718c3c6.diff

LOG: [lldb][PECOFF] Exclude alignment padding when reading section data

There can be zero padding bytes at a section end for file alignment in
PECOFF. Exclude those padding bytes when reading the section data.

Differential Revision: https://reviews.llvm.org/D157059

Added: 
lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp

Modified: 
lldb/include/lldb/Symbol/ObjectFile.h
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
lldb/source/Symbol/ObjectFile.cpp
lldb/unittests/ObjectFile/PECOFF/CMakeLists.txt
llvm/lib/ObjectYAML/COFFYAML.cpp
llvm/test/tools/yaml2obj/COFF/invalid-raw-data.yaml
llvm/test/tools/yaml2obj/COFF/xrelocs.yaml

Removed: 




diff  --git a/lldb/include/lldb/Symbol/ObjectFile.h 
b/lldb/include/lldb/Symbol/ObjectFile.h
index 9b006dfe05e7e4..2fc1cc582d2ceb 100644
--- a/lldb/include/lldb/Symbol/ObjectFile.h
+++ b/lldb/include/lldb/Symbol/ObjectFile.h
@@ -654,6 +654,12 @@ class ObjectFile : public 
std::enable_shared_from_this,
   virtual size_t ReadSectionData(Section *section,
  DataExtractor _data);
 
+  // Returns the section data size. This is special-cased for PECOFF
+  // due to file alignment.
+  virtual size_t GetSectionDataSize(Section *section) {
+return section->GetFileSize();
+  }
+
   /// Returns true if the object file exists only in memory.
   bool IsInMemory() const { return m_memory_addr != LLDB_INVALID_ADDRESS; }
 

diff  --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp 
b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index 77506b40ff5c36..3941d6be2457b9 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -1027,6 +1027,16 @@ SectionType 
ObjectFilePECOFF::GetSectionType(llvm::StringRef sect_name,
   return eSectionTypeOther;
 }
 
+size_t ObjectFilePECOFF::GetSectionDataSize(Section *section) {
+  // For executables, SizeOfRawData (getFileSize()) is aligned by
+  // FileAlignment and the actual section size is in VirtualSize
+  // (getByteSize()). See the comment on
+  // llvm::object::COFFObjectFile::getSectionSize().
+  if (m_binary->getPE32Header() || m_binary->getPE32PlusHeader())
+return std::min(section->GetByteSize(), section->GetFileSize());
+  return section->GetFileSize();
+}
+
 void ObjectFilePECOFF::CreateSections(SectionList _section_list) {
   if (m_sections_up)
 return;

diff  --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h 
b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
index 5d0465b4ee44f6..c59701fa65ec3f 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -263,6 +263,7 @@ class ObjectFilePECOFF : public lldb_private::ObjectFile {
   llvm::StringRef GetSectionName(const section_header_t );
   static lldb::SectionType GetSectionType(llvm::StringRef sect_name,
   const section_header_t );
+  size_t GetSectionDataSize(lldb_private::Section *section) override;
 
   typedef std::vector SectionHeaderColl;
   typedef SectionHeaderColl::iterator SectionHeaderCollIter;

diff  --git a/lldb/source/Symbol/ObjectFile.cpp 
b/lldb/source/Symbol/ObjectFile.cpp
index bebc9589418cbb..364cb850a28a48 100644
--- a/lldb/source/Symbol/ObjectFile.cpp
+++ b/lldb/source/Symbol/ObjectFile.cpp
@@ -550,8 +550,8 @@ size_t ObjectFile::ReadSectionData(Section *section,
 
   // The object file now contains a full mmap'ed copy of the object file
   // data, so just use this
-  return GetData(section->GetFileOffset(), section->GetFileSize(),
-  section_data);
+  return GetData(section->GetFileOffset(), GetSectionDataSize(section),
+ section_data);
 }
 
 bool ObjectFile::SplitArchivePathWithObject(llvm::StringRef path_with_object,

diff  --git a/lldb/unittests/ObjectFile/PECOFF/CMakeLists.txt 
b/lldb/unittests/ObjectFile/PECOFF/CMakeLists.txt
index 3ce5a7b9739cc4..cdcbdc8a745380 100644
--- a/lldb/unittests/ObjectFile/PECOFF/CMakeLists.txt
+++ b/lldb/unittests/ObjectFile/PECOFF/CMakeLists.txt
@@ -1,5 +1,6 @@
 add_lldb_unittest(ObjectFilePECOFFTests
   TestPECallFrameInfo.cpp
+  TestSectionSize.cpp
 
   LINK_LIBS
 lldbUtilityHelpers

diff  --git a/lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp 
b/lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp
new file mode 100644
index 00..3ce1e20265f4d9
--- /dev/null
+++ 

[Lldb-commits] [PATCH] D157059: [lldb][PECOFF] Exclude alignment padding when reading section data

[Hiroshi Yamauchi via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
hjyamauchi marked an inline comment as done.
Closed by commit rGe9040e875d92: [lldb][PECOFF] Exclude alignment padding when 
reading section data (authored by hjyamauchi).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157059/new/

https://reviews.llvm.org/D157059

Files:
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  lldb/source/Symbol/ObjectFile.cpp
  lldb/unittests/ObjectFile/PECOFF/CMakeLists.txt
  lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp
  llvm/lib/ObjectYAML/COFFYAML.cpp
  llvm/test/tools/yaml2obj/COFF/invalid-raw-data.yaml
  llvm/test/tools/yaml2obj/COFF/xrelocs.yaml

Index: llvm/test/tools/yaml2obj/COFF/xrelocs.yaml
===
--- llvm/test/tools/yaml2obj/COFF/xrelocs.yaml
+++ llvm/test/tools/yaml2obj/COFF/xrelocs.yaml
@@ -30,6 +30,7 @@
 # CHECK-YAML-NEXT: Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_LNK_NRELOC_OVFL, IMAGE_SCN_MEM_READ ]
 # CHECK-YAML-NEXT: Alignment:   16
 # CHECK-YAML-NEXT: SectionData: ''
+# CHECK-YAML-NEXT: SizeOfRawData:   16
 # CHECK-YAML-NEXT: Relocations:
 # CHECK-YAML-NEXT:   - VirtualAddress:  0
 # CHECK-YAML-NEXT: SymbolName:  foo
Index: llvm/test/tools/yaml2obj/COFF/invalid-raw-data.yaml
===
--- llvm/test/tools/yaml2obj/COFF/invalid-raw-data.yaml
+++ llvm/test/tools/yaml2obj/COFF/invalid-raw-data.yaml
@@ -1,5 +1,5 @@
 # RUN: not yaml2obj %s -o %t 2>&1 | FileCheck %s
-# CHECK: YAML:18:5: error: unknown key 'SizeOfRawData'
+# CHECK: YAML:14:5: error: StructuredData and SizeOfRawData can't be used together
 
 --- !COFF
 OptionalHeader:
Index: llvm/lib/ObjectYAML/COFFYAML.cpp
===
--- llvm/lib/ObjectYAML/COFFYAML.cpp
+++ llvm/lib/ObjectYAML/COFFYAML.cpp
@@ -689,11 +689,12 @@
 return;
   }
 
-  // Uninitialized sections, such as .bss, typically have no data, but the size
-  // is carried in SizeOfRawData, even though PointerToRawData is zero.
-  if (Sec.SectionData.binary_size() == 0 && Sec.StructuredData.empty() &&
-  NC->Characteristics & COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA)
-IO.mapOptional("SizeOfRawData", Sec.Header.SizeOfRawData);
+  IO.mapOptional("SizeOfRawData", Sec.Header.SizeOfRawData, 0U);
+
+  if (!Sec.StructuredData.empty() && Sec.Header.SizeOfRawData) {
+IO.setError("StructuredData and SizeOfRawData can't be used together");
+return;
+  }
 
   IO.mapOptional("Relocations", Sec.Relocations);
 }
Index: lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp
===
--- /dev/null
+++ lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp
@@ -0,0 +1,78 @@
+//===-- TestSectionFileSize.cpp ---===//
+//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
+#include "Plugins/Process/Utility/lldb-x86-register-enums.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "TestingSupport/TestUtilities.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Symbol/CallFrameInfo.h"
+#include "lldb/Symbol/UnwindPlan.h"
+#include "llvm/Testing/Support/Error.h"
+
+using namespace lldb_private;
+using namespace lldb;
+
+class SectionSizeTest : public testing::Test {
+  SubsystemRAII subsystems;
+};
+
+TEST_F(SectionSizeTest, NoAlignmentPadding) {
+  llvm::Expected ExpectedFile = TestFile::fromYaml(
+  R"(
+--- !COFF
+OptionalHeader:
+  SectionAlignment: 4096
+  FileAlignment:   512
+header:
+  Machine: IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE ]
+sections:
+  - Name:swiftast
+VirtualSize: 496
+SizeOfRawData:   512
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+SectionData: 
+
+symbols: []
+...
+)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+
+  ModuleSP module_sp = std::make_shared(ExpectedFile->moduleSpec());
+  ObjectFile *object_file = module_sp->GetObjectFile();
+  ASSERT_NE(object_file, nullptr);
+
+  SectionList *section_list = object_file->GetSectionList();
+  

[Lldb-commits] [PATCH] D157041: [lldb] Protect OptionValue accesses from data races

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 updated this revision to Diff 547318.
augusto2112 added a comment.

Remove lock from DeepCopy


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157041/new/

https://reviews.llvm.org/D157041

Files:
  lldb/include/lldb/Interpreter/OptionValue.h
  lldb/source/Interpreter/OptionValue.cpp

Index: lldb/source/Interpreter/OptionValue.cpp
===
--- lldb/source/Interpreter/OptionValue.cpp
+++ lldb/source/Interpreter/OptionValue.cpp
@@ -15,6 +15,25 @@
 using namespace lldb;
 using namespace lldb_private;
 
+OptionValue::OptionValue(const OptionValue ) {
+  std::lock_guard lock(other.m_mutex);
+
+  m_parent_wp = other.m_parent_wp;
+  m_callback = other.m_callback;
+  m_value_was_set = other.m_value_was_set;
+
+}
+
+OptionValue& OptionValue::operator=(const OptionValue ) {
+  std::scoped_lock lock(m_mutex, other.m_mutex);
+
+  m_parent_wp = other.m_parent_wp;
+  m_callback = other.m_callback;
+  m_value_was_set = other.m_value_was_set;
+
+  return *this;
+}
+
 Status OptionValue::SetSubValue(const ExecutionContext *exe_ctx,
 VarSetOperationType op, llvm::StringRef name,
 llvm::StringRef value) {
@@ -252,12 +271,14 @@
 }
 
 std::optional OptionValue::GetBooleanValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueBoolean *option_value = GetAsBoolean())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetBooleanValue(bool new_value) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueBoolean *option_value = GetAsBoolean()) {
 option_value->SetCurrentValue(new_value);
 return true;
@@ -266,12 +287,14 @@
 }
 
 std::optional OptionValue::GetCharValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueChar *option_value = GetAsChar())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetCharValue(char new_value) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueChar *option_value = GetAsChar()) {
 option_value->SetCurrentValue(new_value);
 return true;
@@ -280,12 +303,14 @@
 }
 
 std::optional OptionValue::GetEnumerationValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueEnumeration *option_value = GetAsEnumeration())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetEnumerationValue(int64_t value) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueEnumeration *option_value = GetAsEnumeration()) {
 option_value->SetCurrentValue(value);
 return true;
@@ -294,12 +319,14 @@
 }
 
 std::optional OptionValue::GetFileSpecValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueFileSpec *option_value = GetAsFileSpec())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetFileSpecValue(FileSpec file_spec) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueFileSpec *option_value = GetAsFileSpec()) {
 option_value->SetCurrentValue(file_spec, false);
 return true;
@@ -308,6 +335,7 @@
 }
 
 bool OptionValue::AppendFileSpecValue(FileSpec file_spec) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueFileSpecList *option_value = GetAsFileSpecList()) {
 option_value->AppendCurrentValue(file_spec);
 return true;
@@ -316,18 +344,21 @@
 }
 
 std::optional OptionValue::GetFileSpecListValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueFileSpecList *option_value = GetAsFileSpecList())
 return option_value->GetCurrentValue();
   return {};
 }
 
 std::optional OptionValue::GetFormatValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueFormat *option_value = GetAsFormat())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetFormatValue(lldb::Format new_value) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueFormat *option_value = GetAsFormat()) {
 option_value->SetCurrentValue(new_value);
 return true;
@@ -336,12 +367,14 @@
 }
 
 std::optional OptionValue::GetLanguageValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueLanguage *option_value = GetAsLanguage())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetLanguageValue(lldb::LanguageType new_language) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueLanguage *option_value = GetAsLanguage()) {
 option_value->SetCurrentValue(new_language);
 return true;
@@ -350,24 +383,28 @@
 }
 
 const FormatEntity::Entry *OptionValue::GetFormatEntity() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueFormatEntity *option_value = GetAsFormatEntity())
 return _value->GetCurrentValue();
   return nullptr;
 }
 
 const RegularExpression *OptionValue::GetRegexValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueRegex *option_value = GetAsRegex())
 return 

[Lldb-commits] [PATCH] D157041: [lldb] Protect OptionValue accesses from data races

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM. If it's straightforward, I think would be nice to have a unit test with 
two threads modifying the same `OptionValue`. That way a TSan run would catch 
this issue. If that's more work than expected then this is fine as is.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157041/new/

https://reviews.llvm.org/D157041

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157122: [lldb] Remove support for SBHostOS threading functionality

[Alex Langford via Phabricator via lldb-commits]

bulbazord updated this revision to Diff 547290.
bulbazord added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Updating release notes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157122/new/

https://reviews.llvm.org/D157122

Files:
  lldb/source/API/SBHostOS.cpp
  llvm/docs/ReleaseNotes.rst


Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -147,6 +147,10 @@
 * AArch64 Linux targets now provide access to the Thread Local Storage
   register ``tpidr``.
 
+* Methods in SBHostOS related to threads have had their implementations
+  removed. These methods will return a value indicating failure.
+  
+
 Changes to Sanitizers
 -
 * HWASan now defaults to detecting use-after-scope bugs.
Index: lldb/source/API/SBHostOS.cpp
===
--- lldb/source/API/SBHostOS.cpp
+++ lldb/source/API/SBHostOS.cpp
@@ -101,61 +101,23 @@
   lldb::thread_func_t thread_function,
   void *thread_arg, SBError *error_ptr) {
   LLDB_INSTRUMENT_VA(name, thread_function, thread_arg, error_ptr);
-  llvm::Expected thread =
-  ThreadLauncher::LaunchThread(name, [thread_function, thread_arg] {
-return thread_function(thread_arg);
-  });
-  if (!thread) {
-if (error_ptr)
-  error_ptr->SetError(Status(thread.takeError()));
-else
-  llvm::consumeError(thread.takeError());
-return LLDB_INVALID_HOST_THREAD;
-  }
-
-  return thread->Release();
+  return LLDB_INVALID_HOST_THREAD;
 }
 
 void SBHostOS::ThreadCreated(const char *name) { LLDB_INSTRUMENT_VA(name); }
 
 bool SBHostOS::ThreadCancel(lldb::thread_t thread, SBError *error_ptr) {
   LLDB_INSTRUMENT_VA(thread, error_ptr);
-
-  Status error;
-  HostThread host_thread(thread);
-  error = host_thread.Cancel();
-  if (error_ptr)
-error_ptr->SetError(error);
-  host_thread.Release();
-  return error.Success();
+  return false;
 }
 
 bool SBHostOS::ThreadDetach(lldb::thread_t thread, SBError *error_ptr) {
   LLDB_INSTRUMENT_VA(thread, error_ptr);
-
-  Status error;
-#if defined(_WIN32)
-  if (error_ptr)
-error_ptr->SetErrorString("ThreadDetach is not supported on this 
platform");
-#else
-  HostThread host_thread(thread);
-  error = host_thread.GetNativeThread().Detach();
-  if (error_ptr)
-error_ptr->SetError(error);
-  host_thread.Release();
-#endif
-  return error.Success();
+  return false;
 }
 
 bool SBHostOS::ThreadJoin(lldb::thread_t thread, lldb::thread_result_t *result,
   SBError *error_ptr) {
   LLDB_INSTRUMENT_VA(thread, result, error_ptr);
-
-  Status error;
-  HostThread host_thread(thread);
-  error = host_thread.Join(result);
-  if (error_ptr)
-error_ptr->SetError(error);
-  host_thread.Release();
-  return error.Success();
+  return false;
 }


Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -147,6 +147,10 @@
 * AArch64 Linux targets now provide access to the Thread Local Storage
   register ``tpidr``.
 
+* Methods in SBHostOS related to threads have had their implementations
+  removed. These methods will return a value indicating failure.
+  
+
 Changes to Sanitizers
 -
 * HWASan now defaults to detecting use-after-scope bugs.
Index: lldb/source/API/SBHostOS.cpp
===
--- lldb/source/API/SBHostOS.cpp
+++ lldb/source/API/SBHostOS.cpp
@@ -101,61 +101,23 @@
   lldb::thread_func_t thread_function,
   void *thread_arg, SBError *error_ptr) {
   LLDB_INSTRUMENT_VA(name, thread_function, thread_arg, error_ptr);
-  llvm::Expected thread =
-  ThreadLauncher::LaunchThread(name, [thread_function, thread_arg] {
-return thread_function(thread_arg);
-  });
-  if (!thread) {
-if (error_ptr)
-  error_ptr->SetError(Status(thread.takeError()));
-else
-  llvm::consumeError(thread.takeError());
-return LLDB_INVALID_HOST_THREAD;
-  }
-
-  return thread->Release();
+  return LLDB_INVALID_HOST_THREAD;
 }
 
 void SBHostOS::ThreadCreated(const char *name) { LLDB_INSTRUMENT_VA(name); }
 
 bool SBHostOS::ThreadCancel(lldb::thread_t thread, SBError *error_ptr) {
   LLDB_INSTRUMENT_VA(thread, error_ptr);
-
-  Status error;
-  HostThread host_thread(thread);
-  error = host_thread.Cancel();
-  if (error_ptr)
-error_ptr->SetError(error);
-  host_thread.Release();
-  return error.Success();
+  return false;
 }
 
 bool SBHostOS::ThreadDetach(lldb::thread_t thread, SBError *error_ptr) {
   LLDB_INSTRUMENT_VA(thread, error_ptr);
-
-  Status error;
-#if defined(_WIN32)
-  if (error_ptr)
-   

[Lldb-commits] [PATCH] D157043: [lldb/crashlog] Make TextCrashLogParser more resilient to new lines

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.

LGTM with a test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157043/new/

https://reviews.llvm.org/D157043

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157041: [lldb] Protect OptionValue accesses from data races

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 updated this revision to Diff 547284.
augusto2112 added a comment.

Addressed comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157041/new/

https://reviews.llvm.org/D157041

Files:
  lldb/include/lldb/Interpreter/OptionValue.h
  lldb/source/Interpreter/OptionValue.cpp

Index: lldb/source/Interpreter/OptionValue.cpp
===
--- lldb/source/Interpreter/OptionValue.cpp
+++ lldb/source/Interpreter/OptionValue.cpp
@@ -15,6 +15,26 @@
 using namespace lldb;
 using namespace lldb_private;
 
+
+OptionValue::OptionValue(const OptionValue ) {
+  std::lock_guard lock(other.m_mutex);
+
+  m_parent_wp = other.m_parent_wp;
+  m_callback = other.m_callback;
+  m_value_was_set = other.m_value_was_set;
+
+}
+
+OptionValue& OptionValue::operator=(const OptionValue ) {
+  std::scoped_lock lock(m_mutex, other.m_mutex);
+
+  m_parent_wp = other.m_parent_wp;
+  m_callback = other.m_callback;
+  m_value_was_set = other.m_value_was_set;
+
+  return *this;
+}
+
 Status OptionValue::SetSubValue(const ExecutionContext *exe_ctx,
 VarSetOperationType op, llvm::StringRef name,
 llvm::StringRef value) {
@@ -216,6 +236,7 @@
 }
 
 OptionValueString *OptionValue::GetAsString() {
+  
   if (GetType() == OptionValue::eTypeString)
 return static_cast(this);
   return nullptr;
@@ -252,12 +273,14 @@
 }
 
 std::optional OptionValue::GetBooleanValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueBoolean *option_value = GetAsBoolean())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetBooleanValue(bool new_value) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueBoolean *option_value = GetAsBoolean()) {
 option_value->SetCurrentValue(new_value);
 return true;
@@ -266,12 +289,14 @@
 }
 
 std::optional OptionValue::GetCharValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueChar *option_value = GetAsChar())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetCharValue(char new_value) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueChar *option_value = GetAsChar()) {
 option_value->SetCurrentValue(new_value);
 return true;
@@ -280,12 +305,14 @@
 }
 
 std::optional OptionValue::GetEnumerationValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueEnumeration *option_value = GetAsEnumeration())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetEnumerationValue(int64_t value) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueEnumeration *option_value = GetAsEnumeration()) {
 option_value->SetCurrentValue(value);
 return true;
@@ -294,12 +321,14 @@
 }
 
 std::optional OptionValue::GetFileSpecValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueFileSpec *option_value = GetAsFileSpec())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetFileSpecValue(FileSpec file_spec) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueFileSpec *option_value = GetAsFileSpec()) {
 option_value->SetCurrentValue(file_spec, false);
 return true;
@@ -308,6 +337,7 @@
 }
 
 bool OptionValue::AppendFileSpecValue(FileSpec file_spec) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueFileSpecList *option_value = GetAsFileSpecList()) {
 option_value->AppendCurrentValue(file_spec);
 return true;
@@ -316,18 +346,21 @@
 }
 
 std::optional OptionValue::GetFileSpecListValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueFileSpecList *option_value = GetAsFileSpecList())
 return option_value->GetCurrentValue();
   return {};
 }
 
 std::optional OptionValue::GetFormatValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueFormat *option_value = GetAsFormat())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetFormatValue(lldb::Format new_value) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueFormat *option_value = GetAsFormat()) {
 option_value->SetCurrentValue(new_value);
 return true;
@@ -336,12 +369,14 @@
 }
 
 std::optional OptionValue::GetLanguageValue() const {
+  std::lock_guard lock(m_mutex);
   if (const OptionValueLanguage *option_value = GetAsLanguage())
 return option_value->GetCurrentValue();
   return {};
 }
 
 bool OptionValue::SetLanguageValue(lldb::LanguageType new_language) {
+  std::lock_guard lock(m_mutex);
   if (OptionValueLanguage *option_value = GetAsLanguage()) {
 option_value->SetCurrentValue(new_language);
 return true;
@@ -350,24 +385,30 @@
 }
 
 const FormatEntity::Entry *OptionValue::GetFormatEntity() const {
+  std::lock_guard lock(m_mutex);
+
   if (const OptionValueFormatEntity *option_value = GetAsFormatEntity())
 return _value->GetCurrentValue();
   return nullptr;
 }
 
 const 

[Lldb-commits] [PATCH] D157122: [lldb] Remove support for SBHostOS threading functionality

[Alex Langford via Phabricator via lldb-commits]

bulbazord created this revision.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

As stated on Discourse*, these methods have been deprecated. I am
removing their implementation. They will now do nothing and return a
value indicating failure (where appropriate).
Due to the LLDB project's commitment to ABI stability at the SB API
layer, we cannot remove these symbols completely.

Discourse link: 
https://discourse.llvm.org/t/do-you-use-the-threading-functionality-in-sbhostos/71973


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157122

Files:
  lldb/source/API/SBHostOS.cpp


Index: lldb/source/API/SBHostOS.cpp
===
--- lldb/source/API/SBHostOS.cpp
+++ lldb/source/API/SBHostOS.cpp
@@ -101,61 +101,23 @@
   lldb::thread_func_t thread_function,
   void *thread_arg, SBError *error_ptr) {
   LLDB_INSTRUMENT_VA(name, thread_function, thread_arg, error_ptr);
-  llvm::Expected thread =
-  ThreadLauncher::LaunchThread(name, [thread_function, thread_arg] {
-return thread_function(thread_arg);
-  });
-  if (!thread) {
-if (error_ptr)
-  error_ptr->SetError(Status(thread.takeError()));
-else
-  llvm::consumeError(thread.takeError());
-return LLDB_INVALID_HOST_THREAD;
-  }
-
-  return thread->Release();
+  return LLDB_INVALID_HOST_THREAD;
 }
 
 void SBHostOS::ThreadCreated(const char *name) { LLDB_INSTRUMENT_VA(name); }
 
 bool SBHostOS::ThreadCancel(lldb::thread_t thread, SBError *error_ptr) {
   LLDB_INSTRUMENT_VA(thread, error_ptr);
-
-  Status error;
-  HostThread host_thread(thread);
-  error = host_thread.Cancel();
-  if (error_ptr)
-error_ptr->SetError(error);
-  host_thread.Release();
-  return error.Success();
+  return false;
 }
 
 bool SBHostOS::ThreadDetach(lldb::thread_t thread, SBError *error_ptr) {
   LLDB_INSTRUMENT_VA(thread, error_ptr);
-
-  Status error;
-#if defined(_WIN32)
-  if (error_ptr)
-error_ptr->SetErrorString("ThreadDetach is not supported on this 
platform");
-#else
-  HostThread host_thread(thread);
-  error = host_thread.GetNativeThread().Detach();
-  if (error_ptr)
-error_ptr->SetError(error);
-  host_thread.Release();
-#endif
-  return error.Success();
+  return false;
 }
 
 bool SBHostOS::ThreadJoin(lldb::thread_t thread, lldb::thread_result_t *result,
   SBError *error_ptr) {
   LLDB_INSTRUMENT_VA(thread, result, error_ptr);
-
-  Status error;
-  HostThread host_thread(thread);
-  error = host_thread.Join(result);
-  if (error_ptr)
-error_ptr->SetError(error);
-  host_thread.Release();
-  return error.Success();
+  return false;
 }


Index: lldb/source/API/SBHostOS.cpp
===
--- lldb/source/API/SBHostOS.cpp
+++ lldb/source/API/SBHostOS.cpp
@@ -101,61 +101,23 @@
   lldb::thread_func_t thread_function,
   void *thread_arg, SBError *error_ptr) {
   LLDB_INSTRUMENT_VA(name, thread_function, thread_arg, error_ptr);
-  llvm::Expected thread =
-  ThreadLauncher::LaunchThread(name, [thread_function, thread_arg] {
-return thread_function(thread_arg);
-  });
-  if (!thread) {
-if (error_ptr)
-  error_ptr->SetError(Status(thread.takeError()));
-else
-  llvm::consumeError(thread.takeError());
-return LLDB_INVALID_HOST_THREAD;
-  }
-
-  return thread->Release();
+  return LLDB_INVALID_HOST_THREAD;
 }
 
 void SBHostOS::ThreadCreated(const char *name) { LLDB_INSTRUMENT_VA(name); }
 
 bool SBHostOS::ThreadCancel(lldb::thread_t thread, SBError *error_ptr) {
   LLDB_INSTRUMENT_VA(thread, error_ptr);
-
-  Status error;
-  HostThread host_thread(thread);
-  error = host_thread.Cancel();
-  if (error_ptr)
-error_ptr->SetError(error);
-  host_thread.Release();
-  return error.Success();
+  return false;
 }
 
 bool SBHostOS::ThreadDetach(lldb::thread_t thread, SBError *error_ptr) {
   LLDB_INSTRUMENT_VA(thread, error_ptr);
-
-  Status error;
-#if defined(_WIN32)
-  if (error_ptr)
-error_ptr->SetErrorString("ThreadDetach is not supported on this platform");
-#else
-  HostThread host_thread(thread);
-  error = host_thread.GetNativeThread().Detach();
-  if (error_ptr)
-error_ptr->SetError(error);
-  host_thread.Release();
-#endif
-  return error.Success();
+  return false;
 }
 
 bool SBHostOS::ThreadJoin(lldb::thread_t thread, lldb::thread_result_t *result,
   SBError *error_ptr) {
   LLDB_INSTRUMENT_VA(thread, result, error_ptr);
-
-  Status error;
-  HostThread host_thread(thread);
-  error = host_thread.Join(result);
-  if (error_ptr)
-error_ptr->SetError(error);
-  host_thread.Release();
-  return error.Success();
+  return false;
 }

[Lldb-commits] [PATCH] D157044: [lldb/crashlog] Fix sticky image parsing logic

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.

LGTM. The `images` list was added in D146765 
. Do you remember why we needed it there and 
why we don't anymore?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157044/new/

https://reviews.llvm.org/D157044

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D156732: Display PC instead of for stack trace in vscode

[Tom Yang via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG786bab433469: Display PC instead of unknown for 
stack trace in vscode (authored by zhyty).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156732/new/

https://reviews.llvm.org/D156732

Files:
  lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/Makefile
  
lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/TestVSCode_stackTraceMissingFunctionName.py
  lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/main.cpp
  lldb/tools/lldb-vscode/JSONUtils.cpp


Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -701,8 +701,12 @@
   const char *function_name = frame.GetDisplayFunctionName();
   std::string frame_name =
   function_name == nullptr ? std::string() : function_name;
-  if (frame_name.empty())
-frame_name = "";
+  if (frame_name.empty()) {
+// If the function name is unavailable, display the pc address as a 
16-digit
+// hex string, e.g. "0x00012345"
+llvm::raw_string_ostream os(frame_name);
+os << llvm::format_hex(frame.GetPC(), 18);
+  }
   bool is_optimized = frame.GetFunction().GetIsOptimized();
   if (is_optimized)
 frame_name += " [opt]";
Index: lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/main.cpp
@@ -0,0 +1,6 @@
+int main() {
+  typedef void (*FooCallback)();
+  FooCallback foo_callback = (FooCallback)0;
+  foo_callback(); // Crash at zero!
+  return 0;
+}
Index: 
lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/TestVSCode_stackTraceMissingFunctionName.py
===
--- /dev/null
+++ 
lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/TestVSCode_stackTraceMissingFunctionName.py
@@ -0,0 +1,26 @@
+"""
+Test lldb-vscode stack trace response
+"""
+
+
+import vscode
+from lldbsuite.test.decorators import *
+import os
+
+import lldbvscode_testcase
+from lldbsuite.test import lldbtest, lldbutil
+
+
+class 
TestVSCode_stackTraceMissingFunctionName(lldbvscode_testcase.VSCodeTestCaseBase):
+@skipIfWindows
+@skipIfRemote
+def test_missingFunctionName(self):
+"""
+Test that the stack frame without a function name is given its pc in 
the response.
+"""
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+
+self.continue_to_next_stop()
+frame_without_function_name = self.get_stackFrames()[0]
+self.assertEquals(frame_without_function_name["name"], 
"0x")
Index: lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/Makefile
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/Makefile
@@ -0,0 +1,2 @@
+CXX_SOURCES := main.cpp
+include Makefile.rules


Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -701,8 +701,12 @@
   const char *function_name = frame.GetDisplayFunctionName();
   std::string frame_name =
   function_name == nullptr ? std::string() : function_name;
-  if (frame_name.empty())
-frame_name = "";
+  if (frame_name.empty()) {
+// If the function name is unavailable, display the pc address as a 16-digit
+// hex string, e.g. "0x00012345"
+llvm::raw_string_ostream os(frame_name);
+os << llvm::format_hex(frame.GetPC(), 18);
+  }
   bool is_optimized = frame.GetFunction().GetIsOptimized();
   if (is_optimized)
 frame_name += " [opt]";
Index: lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/main.cpp
@@ -0,0 +1,6 @@
+int main() {
+  typedef void (*FooCallback)();
+  FooCallback foo_callback = (FooCallback)0;
+  foo_callback(); // Crash at zero!
+  return 0;
+}
Index: lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/TestVSCode_stackTraceMissingFunctionName.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/TestVSCode_stackTraceMissingFunctionName.py
@@ -0,0 +1,26 @@
+"""
+Test lldb-vscode stack trace response
+"""
+
+
+import vscode
+from lldbsuite.test.decorators import *
+import os
+
+import lldbvscode_testcase
+from lldbsuite.test import lldbtest, lldbutil
+
+
+class 

[Lldb-commits] [lldb] 786bab4 - Display PC instead of for stack trace in vscode

[Tom Yang via lldb-commits]

Author: Tom Yang
Date: 2023-08-04T11:07:27-07:00
New Revision: 786bab43346939d5662c2a90f8c0ff72fe421614

URL: 
https://github.com/llvm/llvm-project/commit/786bab43346939d5662c2a90f8c0ff72fe421614
DIFF: 
https://github.com/llvm/llvm-project/commit/786bab43346939d5662c2a90f8c0ff72fe421614.diff

LOG: Display PC instead of  for stack trace in vscode

It isn't useful for users to see "" as a stack trace when lldb fails 
to symbolicate a stack frame. I've replaced "" with the value of the 
program counter instead.

Test Plan:

To test this, I opened a target that lldb fails to symbolicate in
VSCode, and observed in the CALL STACK section that instead of being
shown as "", those stack frames are represented by their
program counters.

I added a new test case, `TestVSCode_stackTraceMissingFunctionName` that
exercises this feature.

I also ran `lldb-dotest -p TestVSCode` and saw that the tests passed.

Differential Revision: https://reviews.llvm.org/D156732

Added: 
lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/Makefile

lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/TestVSCode_stackTraceMissingFunctionName.py
lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/main.cpp

Modified: 
lldb/tools/lldb-vscode/JSONUtils.cpp

Removed: 




diff  --git 
a/lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/Makefile 
b/lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/Makefile
new file mode 100644
index 00..3d0b98f13f3d7b
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/Makefile
@@ -0,0 +1,2 @@
+CXX_SOURCES := main.cpp
+include Makefile.rules

diff  --git 
a/lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/TestVSCode_stackTraceMissingFunctionName.py
 
b/lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/TestVSCode_stackTraceMissingFunctionName.py
new file mode 100644
index 00..8e7da6386d8e50
--- /dev/null
+++ 
b/lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/TestVSCode_stackTraceMissingFunctionName.py
@@ -0,0 +1,26 @@
+"""
+Test lldb-vscode stack trace response
+"""
+
+
+import vscode
+from lldbsuite.test.decorators import *
+import os
+
+import lldbvscode_testcase
+from lldbsuite.test import lldbtest, lldbutil
+
+
+class 
TestVSCode_stackTraceMissingFunctionName(lldbvscode_testcase.VSCodeTestCaseBase):
+@skipIfWindows
+@skipIfRemote
+def test_missingFunctionName(self):
+"""
+Test that the stack frame without a function name is given its pc in 
the response.
+"""
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+
+self.continue_to_next_stop()
+frame_without_function_name = self.get_stackFrames()[0]
+self.assertEquals(frame_without_function_name["name"], 
"0x")

diff  --git 
a/lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/main.cpp 
b/lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/main.cpp
new file mode 100644
index 00..a17a9390e75af1
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/stackTraceMissingFunctionName/main.cpp
@@ -0,0 +1,6 @@
+int main() {
+  typedef void (*FooCallback)();
+  FooCallback foo_callback = (FooCallback)0;
+  foo_callback(); // Crash at zero!
+  return 0;
+}

diff  --git a/lldb/tools/lldb-vscode/JSONUtils.cpp 
b/lldb/tools/lldb-vscode/JSONUtils.cpp
index 6acb07296da3be..03826f8936ae32 100644
--- a/lldb/tools/lldb-vscode/JSONUtils.cpp
+++ b/lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -701,8 +701,12 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame ) {
   const char *function_name = frame.GetDisplayFunctionName();
   std::string frame_name =
   function_name == nullptr ? std::string() : function_name;
-  if (frame_name.empty())
-frame_name = "";
+  if (frame_name.empty()) {
+// If the function name is unavailable, display the pc address as a 
16-digit
+// hex string, e.g. "0x00012345"
+llvm::raw_string_ostream os(frame_name);
+os << llvm::format_hex(frame.GetPC(), 18);
+  }
   bool is_optimized = frame.GetFunction().GetIsOptimized();
   if (is_optimized)
 frame_name += " [opt]";



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157041: [lldb] Protect OptionValue accesses from data races

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Despite their prevalence in LLDB, I still consider `recursive_mutex` as a 
potential code smell. So +1 on avoiding the need for a recursive mutex.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157041/new/

https://reviews.llvm.org/D157041

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D156949: [lldb] Update LLDB Code Ownership

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 547274.
JDevlieghere marked 7 inline comments as done.
JDevlieghere added a comment.

- Add Pavel's nominations


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156949/new/

https://reviews.llvm.org/D156949

Files:
  lldb/CODE_OWNERS.txt
  lldb/CodeOwners.rst

Index: lldb/CodeOwners.rst
===
--- /dev/null
+++ lldb/CodeOwners.rst
@@ -0,0 +1,252 @@
+
+LLDB Code Owners
+
+
+This file is a list of the `code owners `_ for LLDB.
+
+.. contents::
+   :depth: 2
+   :local:
+
+Current Code Owners
+===
+The following people are the active code owners for the project. Please reach
+out to them for code reviews, questions about their area of expertise, or other
+assistance.
+
+All parts of LLDB not covered by someone else
+--
+| Jonas Devlieghere
+| jonas\@devlieghere.com (email), jdevlieghere (Phabricator), jdevlieghere (GitHub), jdevlieghere (Discourse), jdevlieghere (Discord)
+
+Components
+--
+These code owners are responsible for particular high-level components within
+LLDB.
+
+ABI
+~~~
+| Jason Molenda
+| jmolenda\@apple.com (email), jasonmolenda (Phabricator), jasonmolenda (GitHub), jasonmolenda (Discourse), jasonmolenda (Discord)
+
+| David Spickett
+| david.spickett\@linaro.org (email), DavidSpickett (Phabricator), DavidSpickett (GitHub), DavidSpickett (Discourse), davidspickett (Discord)
+
+
+Breakpoint
+~~
+| Jim Ingham
+| jingham\@apple.com (email), jingham (Phabricator), jimingham (GitHub), jingham (Discourse)
+
+CMake & Build System
+
+| Jonas Devlieghere
+| jonas\@devlieghere.com (email), jdevlieghere (Phabricator), jdevlieghere (GitHub), jdevlieghere (Discourse), jdevlieghere (Discord)
+
+| Alex Langford
+| alangford\@apple.com (email), bulbazord (Phabricator), bulbazord (GitHub), bulbazord (Discourse), bulba_zord (Discord)
+
+Commands
+
+| Jim Ingham
+| jingham\@apple.com (email), jingham (Phabricator), jimingham (GitHub), jingham (Discourse)
+
+Expression Parser
+~
+| Michael Buch
+| michaelbuch12\@gmail.com (email), Michael137 (Phabricator), Michael137 (GitHub), Michael137 (Discourse)
+
+| Jim Ingham
+| jingham\@apple.com (email), jingham (Phabricator), jimingham (GitHub), jingham (Discourse)
+
+Interpreter
+~~~
+| Jim Ingham
+| jingham\@apple.com (email), jingham (Phabricator), jimingham (GitHub), jingham (Discourse)
+
+| Greg Clayton
+| gclayton\@fb.com (email), clayborg (Phabricator), clayborg (GitHub), clayborg (Discourse)
+
+
+Lua
+~~~
+| Jonas Delvieghere
+| jonas\@devlieghere.com (email), jdevlieghere (Phabricator), jdevlieghere (GitHub), jdevlieghere (Discourse), jdevlieghere (Discord)
+
+Python
+~~
+| Med Ismail Bennani
+| ismail\@bennani.ma (email), mib (Phabricator), medismailben (GitHub), mib (Discourse), mib#8727 (Discord)
+
+Target/Process Control
+~~
+| Med Ismail Bennani
+| ismail\@bennani.ma (email), mib (Phabricator), medismailben (GitHub), mib (Discourse), mib#8727 (Discord)
+
+| Jim Ingham
+| jingham\@apple.com (email), jingham (Phabricator), jimingham (GitHub), jingham (Discourse)
+
+Test Suite
+~~
+| Jonas Devlieghere
+| jonas\@devlieghere.com (email), jdevlieghere (Phabricator), jdevlieghere (GitHub), jdevlieghere (Discourse), jdevlieghere (Discord)
+
+| Pavel Labath
+| pavel\@labath.sk (email), labath (Phabricator), labath (GitHub), labath (Discourse)
+
+Trace
+~
+| Walter Erquinigo
+| a20012251\@gmail.com (email), wallace (Phabricator), walter-erquinigo (GitHub), wallace (Discourse), werquinigo (Discord)
+
+Unwinding
+~
+| Jason Molenda
+| jmolenda\@apple.com (email), jasonmolenda (Phabricator), jasonmolenda (GitHub), jasonmolenda (Discourse), jasonmolenda (Discord)
+
+Utility
+~~~
+| Jonas Devlieghere
+| jonas\@devlieghere.com (email), jdevlieghere (Phabricator), jdevlieghere (GitHub), jdevlieghere (Discourse), jdevlieghere (Discord)
+
+| Pavel Labath
+| pavel\@labath.sk (email), labath (Phabricator), labath (GitHub), labath (Discourse)
+
+ValueObject
+~~~
+| Jim Ingham
+| jingham\@apple.com (email), jingham (Phabricator), jimingham (GitHub), jingham (Discourse)
+
+Watchpoints
+~~~
+| Jason Molenda
+| jmolenda\@apple.com (email), jasonmolenda (Phabricator), jasonmolenda (GitHub), jasonmolenda (Discourse), jasonmolenda (Discord)
+
+File Formats
+
+The following people are responsible for decisions involving file and debug
+info formats.
+
+(PE)COFF
+
+| Saleem Abdulrasool
+| compnerd\@compnerd.org (email), compnerd (Phabricator), compnerd (GitHub), compnerd (Discourse), compnerd (Discord)
+
+Breakpad
+
+| Zequan Wu
+| zequanwu\@google.com (email), zequanwu (Phabricator), ZequanWu (GitHub), ZequanWu (Discourse)
+
+| Pavel Labath
+| pavel\@labath.sk (email), 

[Lldb-commits] [PATCH] D157059: [lldb][PECOFF] Exclude alignment padding when reading section data

[Hiroshi Yamauchi via Phabricator via lldb-commits]

hjyamauchi marked an inline comment as done.
hjyamauchi added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:1035
+  // llvm::object::COFFObjectFile::getSectionSize().
+  if (m_binary->getDOSHeader())
+return std::min(section->GetByteSize(), section->GetFileSize());

compnerd wrote:
> Can we use `m_binary->getPEHeader() || m_binary->getPE32Header()` here 
> instead?  We are technically ensuring that this is a linked (PE) binary 
> rather than an object file.  While a DOS header is present, it is not an 
> absolute requirement (theoretically, it is practically never going to change).
Done.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157059/new/

https://reviews.llvm.org/D157059

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157059: [lldb][PECOFF] Exclude alignment padding when reading section data

[Hiroshi Yamauchi via Phabricator via lldb-commits]

hjyamauchi updated this revision to Diff 547272.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157059/new/

https://reviews.llvm.org/D157059

Files:
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  lldb/source/Symbol/ObjectFile.cpp
  lldb/unittests/ObjectFile/PECOFF/CMakeLists.txt
  lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp
  llvm/lib/ObjectYAML/COFFYAML.cpp
  llvm/test/tools/yaml2obj/COFF/invalid-raw-data.yaml
  llvm/test/tools/yaml2obj/COFF/xrelocs.yaml

Index: llvm/test/tools/yaml2obj/COFF/xrelocs.yaml
===
--- llvm/test/tools/yaml2obj/COFF/xrelocs.yaml
+++ llvm/test/tools/yaml2obj/COFF/xrelocs.yaml
@@ -30,6 +30,7 @@
 # CHECK-YAML-NEXT: Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_LNK_NRELOC_OVFL, IMAGE_SCN_MEM_READ ]
 # CHECK-YAML-NEXT: Alignment:   16
 # CHECK-YAML-NEXT: SectionData: ''
+# CHECK-YAML-NEXT: SizeOfRawData:   16
 # CHECK-YAML-NEXT: Relocations:
 # CHECK-YAML-NEXT:   - VirtualAddress:  0
 # CHECK-YAML-NEXT: SymbolName:  foo
Index: llvm/test/tools/yaml2obj/COFF/invalid-raw-data.yaml
===
--- llvm/test/tools/yaml2obj/COFF/invalid-raw-data.yaml
+++ llvm/test/tools/yaml2obj/COFF/invalid-raw-data.yaml
@@ -1,5 +1,5 @@
 # RUN: not yaml2obj %s -o %t 2>&1 | FileCheck %s
-# CHECK: YAML:18:5: error: unknown key 'SizeOfRawData'
+# CHECK: YAML:14:5: error: StructuredData and SizeOfRawData can't be used together
 
 --- !COFF
 OptionalHeader:
Index: llvm/lib/ObjectYAML/COFFYAML.cpp
===
--- llvm/lib/ObjectYAML/COFFYAML.cpp
+++ llvm/lib/ObjectYAML/COFFYAML.cpp
@@ -689,11 +689,12 @@
 return;
   }
 
-  // Uninitialized sections, such as .bss, typically have no data, but the size
-  // is carried in SizeOfRawData, even though PointerToRawData is zero.
-  if (Sec.SectionData.binary_size() == 0 && Sec.StructuredData.empty() &&
-  NC->Characteristics & COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA)
-IO.mapOptional("SizeOfRawData", Sec.Header.SizeOfRawData);
+  IO.mapOptional("SizeOfRawData", Sec.Header.SizeOfRawData, 0U);
+
+  if (!Sec.StructuredData.empty() && Sec.Header.SizeOfRawData) {
+IO.setError("StructuredData and SizeOfRawData can't be used together");
+return;
+  }
 
   IO.mapOptional("Relocations", Sec.Relocations);
 }
Index: lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp
===
--- /dev/null
+++ lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp
@@ -0,0 +1,78 @@
+//===-- TestSectionFileSize.cpp ---===//
+//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
+#include "Plugins/Process/Utility/lldb-x86-register-enums.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "TestingSupport/TestUtilities.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Symbol/CallFrameInfo.h"
+#include "lldb/Symbol/UnwindPlan.h"
+#include "llvm/Testing/Support/Error.h"
+
+using namespace lldb_private;
+using namespace lldb;
+
+class SectionSizeTest : public testing::Test {
+  SubsystemRAII subsystems;
+};
+
+TEST_F(SectionSizeTest, NoAlignmentPadding) {
+  llvm::Expected ExpectedFile = TestFile::fromYaml(
+  R"(
+--- !COFF
+OptionalHeader:
+  SectionAlignment: 4096
+  FileAlignment:   512
+header:
+  Machine: IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE ]
+sections:
+  - Name:swiftast
+VirtualSize: 496
+SizeOfRawData:   512
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+SectionData: 
+
+symbols: []
+...
+)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+
+  ModuleSP module_sp = std::make_shared(ExpectedFile->moduleSpec());
+  ObjectFile *object_file = module_sp->GetObjectFile();
+  ASSERT_NE(object_file, nullptr);
+
+  SectionList *section_list = object_file->GetSectionList();
+  ASSERT_NE(section_list, nullptr);
+
+  SectionSP swiftast_section;
+  size_t section_count = section_list->GetNumSections(0);
+  for (size_t i = 0; i < section_count; ++i) {
+SectionSP section_sp = section_list->GetSectionAtIndex(i);
+if (section_sp->GetName() == "swiftast") {
+  swiftast_section = 

[Lldb-commits] [lldb] 4185656 - Fix the NSIndexSet data formatter for changes in macOS Sonoma.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2023-08-04T09:55:59-07:00
New Revision: 4185656625a249832ff67f628bd00e5278721c96

URL: 
https://github.com/llvm/llvm-project/commit/4185656625a249832ff67f628bd00e5278721c96
DIFF: 
https://github.com/llvm/llvm-project/commit/4185656625a249832ff67f628bd00e5278721c96.diff

LOG: Fix the NSIndexSet data formatter for changes in macOS Sonoma.

Added: 


Modified: 
lldb/source/Plugins/Language/ObjC/Cocoa.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Language/ObjC/Cocoa.cpp 
b/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
index 374ac763ab183d..f1a7e04bc9d1bf 100644
--- a/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
+++ b/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
@@ -237,7 +237,8 @@ bool lldb_private::formatters::NSIndexSetSummaryProvider(
   if (!process_sp)
 return false;
 
-  ObjCLanguageRuntime *runtime = ObjCLanguageRuntime::Get(*process_sp);
+  AppleObjCRuntime *runtime = llvm::dyn_cast_or_null(
+  ObjCLanguageRuntime::Get(*process_sp));
 
   if (!runtime)
 return false;
@@ -264,20 +265,56 @@ bool lldb_private::formatters::NSIndexSetSummaryProvider(
 
   do {
 if (class_name == "NSIndexSet" || class_name == "NSMutableIndexSet") {
+  // Foundation version 2000 added a bitmask if the index set fit in 64 
bits
+  // and a Tagged Pointer version if the bitmask is small enough to fit in
+  // the tagged pointer payload.  
+  // It also changed the layout (but not the size) of the set descriptor.
+
+  // First check whether this is a tagged pointer.  The bitmask will be in
+  // the payload of the tagged pointer.
+  uint64_t payload;
+  if (runtime->GetFoundationVersion() >= 2000  
+  && descriptor->GetTaggedPointerInfo(nullptr, nullptr, )) {
+count = llvm::popcount(payload);
+break;
+  }
+  // The first 32 bits describe the index set in all cases:
   Status error;
   uint32_t mode = process_sp->ReadUnsignedIntegerFromMemory(
-  valobj_addr + ptr_size, 4, 0, error);
+valobj_addr + ptr_size, 4, 0, error);
   if (error.Fail())
 return false;
-  // this means the set is empty - count = 0
-  if ((mode & 1) == 1) {
-count = 0;
-break;
+  // Now check if the index is held in a bitmask in the object:
+  if (runtime->GetFoundationVersion() >= 2000) {
+// The first two bits are "isSingleRange" and "isBitfield".  If this is
+// a bitfield we handle it here, otherwise set mode appropriately and
+// the rest of the treatment is in common.
+if ((mode & 2) == 2) {
+  // The bitfield is a 64 bit uint at the beginning of the data var.
+  uint64_t bitfield = process_sp->ReadUnsignedIntegerFromMemory(
+valobj_addr + 2 * ptr_size, 8, 0, error);
+  if (error.Fail())
+return false;
+  count = llvm::popcount(bitfield);
+  break;
+}
+// It wasn't a bitfield, so read the isSingleRange from its new loc:
+if ((mode & 1) == 1)
+  mode = 1; // this means the set only has one range
+else
+  mode = 2; // this means the set has multiple ranges
+  } else {
+// this means the set is empty - count = 0
+if ((mode & 1) == 1) {
+  count = 0;
+  break;
+}
+  
+if ((mode & 2) == 2)
+  mode = 1; // this means the set only has one range
+else
+  mode = 2; // this means the set has multiple ranges
   }
-  if ((mode & 2) == 2)
-mode = 1; // this means the set only has one range
-  else
-mode = 2; // this means the set has multiple ranges
   if (mode == 1) {
 count = process_sp->ReadUnsignedIntegerFromMemory(
 valobj_addr + 3 * ptr_size, ptr_size, 0, error);



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D149213: [lldb] Add basic support for Rust enums in TypeSystemClang

[Vladimir Makaev via Phabricator via lldb-commits]

VladimirMakaev added a comment.

I don't have commit access to the repo. Since this was accepted can somebody 
commit this to the repo? CC @clayborg


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149213/new/

https://reviews.llvm.org/D149213

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157041: [lldb] Protect OptionValue accesses from data races

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/source/Interpreter/OptionValue.cpp:55
 const OptionValueBoolean *OptionValue::GetAsBoolean() const {
+  std::lock_guard lock(m_mutex);
   if (GetType() == OptionValue::eTypeBoolean)

If you are following @kastiglione 's suggestion from above (I'd be fine with 
choosing symmetry over performance) then this method (and the ones below) also 
doesn't need any locks, since it just calls other thread-safe methods.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157041/new/

https://reviews.llvm.org/D157041

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D156949: [lldb] Update LLDB Code Ownership

[Zequan Wu via Phabricator via lldb-commits]

zequanwu added inline comments.



Comment at: lldb/CodeOwners.rst:132
+
+Breakpad
+

labath wrote:
> @zequanwu 
I accept this.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156949/new/

https://reviews.llvm.org/D156949

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D156270: [lldb][NFCI] Change logic to find clang resource dir in standalone builds

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

Yeah, that's why I CC-ed the two people who committed that API, as I don't 
really understand how it's supposed to work (and don't really have the capacity 
to figure it out).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156270/new/

https://reviews.llvm.org/D156270

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157059: [lldb][PECOFF] Exclude alignment padding when reading section data

[Tristan Labelle via Phabricator via lldb-commits]

MrTrillian accepted this revision.
MrTrillian added a comment.

Looking good!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157059/new/

https://reviews.llvm.org/D157059

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D156949: [lldb] Update LLDB Code Ownership

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: lldb/CodeOwners.rst:150
+
+ELF
+~~~

labath wrote:
> @DavidSpickett 
Ok with me.



Comment at: lldb/CodeOwners.rst:220
+
+lldb-server
+~~~

labath wrote:
> @DavidSpickett 
Ok with me.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156949/new/

https://reviews.llvm.org/D156949

___
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.

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman added a comment.
Herald added a subscriber: wangpc.

I've not spotted any major concerns with this patch, but I did have some minor 
nits to look into. I'd love to hear from @rsmith and @erichkeane before signing 
off on this, as the changes are pretty involved and they've both done some 
in-depth looks at the patch already.




Comment at: clang/include/clang/AST/TemplateBase.h:168-173
+  void initFromType(QualType, bool isNullPtr, bool IsDefaulted);
+  void initFromDeclaration(ValueDecl *, QualType, bool IsDefaulted);
+  void initFromIntegral(const ASTContext &, const llvm::APSInt &, QualType,
+bool IsDefaulted);
+  void initFromStructural(const ASTContext &, QualType, const APValue &,
+  bool IsDefaulted);

Can you add parameter names for the unnamed ones (we typically only leave a 
parameter unnamed when it is unused).



Comment at: clang/include/clang/AST/TemplateBase.h:382-385
+  /// Get the value of an StructuralValue.
+  const APValue () const { return *Value.Value; }
+
+  /// Get the type of an StructuralValue.





Comment at: clang/lib/AST/MicrosoftMangle.cpp:1624-1626
+  return const_cast(
+  V.getLValueBase().dyn_cast());
+}

Does this work or is the `const_cast` actually required?



Comment at: clang/lib/AST/ODRHash.cpp:1324-1327
+if (auto *AT = TypeSoFar->getAsArrayTypeUnsafe()) {
+  if (auto *CAT = dyn_cast(AT))
+OnePastTheEnd |= CAT->getSize() == E.getAsArrayIndex();
+  TypeSoFar = AT->getElementType();





Comment at: clang/lib/AST/ODRHash.cpp:1330
+  const Decl *D = E.getAsBaseOrMember().getPointer();
+  if (auto *FD = dyn_cast(D)) {
+if (FD->getParent()->isUnion())





Comment at: clang/lib/AST/ODRHash.cpp:1341-1342
+}
+ID.AddInteger((Value.isNullPointer() ? 1 : 0) | (OnePastTheEnd ? 2 : 0) |
+  (Value.hasLValuePath() ? 4 : 0));
+break;

I think this might be a bit clearer, but I don't insist on the change.



Comment at: clang/lib/AST/TemplateBase.cpp:163
 
-TemplateArgument::TemplateArgument(ASTContext , const llvm::APSInt ,
-   QualType Type, bool IsDefaulted) {
+void TemplateArgument::initFromType(QualType T, bool isNullPtr,
+bool IsDefaulted) {





Comment at: clang/lib/AST/TemplateBase.cpp:408-409
   case Integral:
-getAsIntegral().Profile(ID);
 getIntegralType().Profile(ID);
+getAsIntegral().Profile(ID);
+break;

Why did the order of these calls change?



Comment at: clang/lib/AST/TemplateBase.cpp:619
+  case TemplateArgument::UncommonValue: {
+// FIXME: We're guessing at LangOptions!
+SmallString<32> Str;

bolshakov-a wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > It's probably okay enough, but this is now the third instance of adding 
> > > the same bug to this helper method -- maybe we should fix that instead?
> > Agreed, seems to me we should do the work NOW to just wire in the lang-opts 
> > to this function.
> The problem here is because this function is called from a stream insertion 
> operator, and there isn't obviously any way to pass 3rd parameter into it 
> without switching it into an ordinary function.
Okay, that's reasonable enough to hold off on changing. Thanks!



Comment at: clang/lib/Sema/SemaOverload.cpp:5983-5985
+  Expr *E = Result.get();
+  if (!isa(E))
+E = ConstantExpr::Create(S.Context, Result.get(), Value);

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?



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:
> 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.



Comment at: clang/www/cxx_status.html:1061
+
+  Clang 17 (Partial)
+  Reference type template arguments referring to 
instantiation-dependent objects and subobjects




CHANGES SINCE LAST ACTION
  

[Lldb-commits] [PATCH] D156949: [lldb] Update LLDB Code Ownership

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

I've suggested some additional/alternative/backup (choose your interpretation) 
owners for the components I'm listed as the only owner (I only wish I could 
find someone to take over android). If they accept, then take this as my 
endorsement to add to or replace me.




Comment at: lldb/CodeOwners.rst:107
+
+Utility
+~~~

@JDevlieghere



Comment at: lldb/CodeOwners.rst:132
+
+Breakpad
+

@zequanwu 



Comment at: lldb/CodeOwners.rst:150
+
+ELF
+~~~

@DavidSpickett 



Comment at: lldb/CodeOwners.rst:220
+
+lldb-server
+~~~

@DavidSpickett 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156949/new/

https://reviews.llvm.org/D156949

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D156949: [lldb] Update LLDB Code Ownership

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman added inline comments.



Comment at: lldb/CodeOwners.rst:7-8
+particular part of LLDB are reviewed, either by themself or by someone else.
+They are also the gatekeepers for their part of LLDB, with the final word on
+what goes in or not.
+

tonic wrote:
> JDevlieghere wrote:
> > DavidSpickett wrote:
> > > JDevlieghere wrote:
> > > > DavidSpickett wrote:
> > > > > This could be taken to mean every review must have approval from a 
> > > > > code owner, on top of whatever other review has been done. Is that 
> > > > > the intent? Someone coming from a project with strong maintainer 
> > > > > rules (e.g. GDB, so I gather) may take it that way.
> > > > I copied this from the Clang `CodeOwners.rst` with the aim of being 
> > > > consistent, but I'm happy to tweak it. We  could qualify the last 
> > > > sentence with something like "when consensus cannot be reached" or if 
> > > > we think "gatekeeper" is too strong of a work maybe we can use 
> > > > "tie-breaker", though I like that the former implies a sense of duty. 
> > > > Happy to take suggestions!
> > > My understanding was that llvm in general didn't have this hard 
> > > requirement for an owner to acknowledge every review.
> > > 
> > > So yeah:
> > > "They are also the gatekeepers for their part of LLDB, with the final 
> > > word on what goes in or not when consensus cannot be reached."
> > > 
> > > Sounds good to me.
> > > My understanding was that llvm in general didn't have this hard 
> > > requirement for an owner to acknowledge every review.
> > 
> > Yup, that's my understanding as well!
> > 
> I would not put policy regarding code owners in this document. The policy is 
> already in the DeveloperPolicy for the project. You could reference back to 
> that document if you want. 
Good suggestion! I'm going to make similar changes to the Clang code owners 
file.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156949/new/

https://reviews.llvm.org/D156949

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D156997: [LLDB][CMake][NFC] Remove unused LLDB_LINKER_SUPPORTS_GROUPS variable

[J. Ryan Stinnett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe383776ad0fb: [LLDB][CMake][NFC] Remove unused 
LLDB_LINKER_SUPPORTS_GROUPS variable (authored by jryans).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156997/new/

https://reviews.llvm.org/D156997

Files:
  lldb/cmake/modules/LLDBConfig.cmake


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -17,12 +17,6 @@
 "`CMakeFiles'. Please delete them.")
 endif()
 
-set(LLDB_LINKER_SUPPORTS_GROUPS OFF)
-if (LLVM_COMPILER_IS_GCC_COMPATIBLE AND NOT "${CMAKE_SYSTEM_NAME}" MATCHES 
"Darwin")
-  # The Darwin linker doesn't understand --start-group/--end-group.
-  set(LLDB_LINKER_SUPPORTS_GROUPS ON)
-endif()
-
 macro(add_optional_dependency variable description package found)
   cmake_parse_arguments(ARG
 "QUIET"


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -17,12 +17,6 @@
 "`CMakeFiles'. Please delete them.")
 endif()
 
-set(LLDB_LINKER_SUPPORTS_GROUPS OFF)
-if (LLVM_COMPILER_IS_GCC_COMPATIBLE AND NOT "${CMAKE_SYSTEM_NAME}" MATCHES "Darwin")
-  # The Darwin linker doesn't understand --start-group/--end-group.
-  set(LLDB_LINKER_SUPPORTS_GROUPS ON)
-endif()
-
 macro(add_optional_dependency variable description package found)
   cmake_parse_arguments(ARG
 "QUIET"
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] e383776 - [LLDB][CMake][NFC] Remove unused LLDB_LINKER_SUPPORTS_GROUPS variable

[J. Ryan Stinnett via lldb-commits]

Author: J. Ryan Stinnett
Date: 2023-08-04T11:24:36+01:00
New Revision: e383776ad0fbbe3d3704558fc8852c826680ce8f

URL: 
https://github.com/llvm/llvm-project/commit/e383776ad0fbbe3d3704558fc8852c826680ce8f
DIFF: 
https://github.com/llvm/llvm-project/commit/e383776ad0fbbe3d3704558fc8852c826680ce8f.diff

LOG: [LLDB][CMake][NFC] Remove unused LLDB_LINKER_SUPPORTS_GROUPS variable

The `LLDB_LINKER_SUPPORTS_GROUPS` CMake variable was added back in 2019 as part
of https://reviews.llvm.org/D71306, but it appears to have been unused even
then. This removes the unused variable.

Differential Revision: https://reviews.llvm.org/D156997

Added: 


Modified: 
lldb/cmake/modules/LLDBConfig.cmake

Removed: 




diff  --git a/lldb/cmake/modules/LLDBConfig.cmake 
b/lldb/cmake/modules/LLDBConfig.cmake
index ce90ecabc6a5a5..ca64ba798eda2d 100644
--- a/lldb/cmake/modules/LLDBConfig.cmake
+++ b/lldb/cmake/modules/LLDBConfig.cmake
@@ -17,12 +17,6 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR)
 "`CMakeFiles'. Please delete them.")
 endif()
 
-set(LLDB_LINKER_SUPPORTS_GROUPS OFF)
-if (LLVM_COMPILER_IS_GCC_COMPATIBLE AND NOT "${CMAKE_SYSTEM_NAME}" MATCHES 
"Darwin")
-  # The Darwin linker doesn't understand --start-group/--end-group.
-  set(LLDB_LINKER_SUPPORTS_GROUPS ON)
-endif()
-
 macro(add_optional_dependency variable description package found)
   cmake_parse_arguments(ARG
 "QUIET"



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D156493: [lldb-vsocde] Adding support for the "disassemble" request.

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> Do you have any additional information about the failure? The link looks like 
> a timeout, so I'm not sure where things are timing out.

Sorry for the false alarm, 
https://github.com/llvm/llvm-project/commit/9a3f0cd717f68ccf9e348bce2d76a2372482f4f2
 fixed a few vscode tests including this one. I've enabled it again.

> Also it would be very useful if the logs could be printed in a pretty form 
> like:

And I've done this in 
https://github.com/llvm/llvm-project/commit/165f45a877742a74988d63f36aee635c8e0a47da.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156493/new/

https://reviews.llvm.org/D156493

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 27a0a74 - Revert "[lldb][lldb-vscode] Skip disassembler test on Arm"

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2023-08-04T09:26:40Z
New Revision: 27a0a743cff3734f56358ee6c1f5b0dab3342dc4

URL: 
https://github.com/llvm/llvm-project/commit/27a0a743cff3734f56358ee6c1f5b0dab3342dc4
DIFF: 
https://github.com/llvm/llvm-project/commit/27a0a743cff3734f56358ee6c1f5b0dab3342dc4.diff

LOG: Revert "[lldb][lldb-vscode] Skip disassembler test on Arm"

This reverts commit 54458c525aa47219a3ef2bee2be33d6096b1585c.

The timeouts we were seeing have been fixed by 
9a3f0cd717f68ccf9e348bce2d76a2372482f4f2
and I think this was experiencing the same issue.

Added: 


Modified: 
lldb/test/API/tools/lldb-vscode/disassemble/TestVSCode_disassemble.py

Removed: 




diff  --git 
a/lldb/test/API/tools/lldb-vscode/disassemble/TestVSCode_disassemble.py 
b/lldb/test/API/tools/lldb-vscode/disassemble/TestVSCode_disassemble.py
index 7f133afb6a22b4..6d3ebdf296e2b1 100644
--- a/lldb/test/API/tools/lldb-vscode/disassemble/TestVSCode_disassemble.py
+++ b/lldb/test/API/tools/lldb-vscode/disassemble/TestVSCode_disassemble.py
@@ -14,7 +14,6 @@
 class TestVSCode_disassemble(lldbvscode_testcase.VSCodeTestCaseBase):
 @skipIfWindows
 @skipIfRemote
-@skipIf(archs=["arm"])
 def test_disassemble(self):
 """
 Tests the 'disassemble' request.



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits