[Lldb-commits] [PATCH] D153636: [LLDB] Fix the use of "platform process launch" with no extra arguments

[Pavel Labath via Phabricator via lldb-commits]

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

Thanks for fixing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153636

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

[Lldb-commits] [PATCH] D153827: [lldb][NFCI] Remove use of ConstString from ProcessElfCore

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Looks reasonable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153827

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

[Lldb-commits] [PATCH] D153825: [lldb][NFCI] Change return type of GetProcessPluginName

[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/D153825/new/

https://reviews.llvm.org/D153825

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

[Lldb-commits] [PATCH] D153818: [lldb][NFCI] Remove use of ConstString from PluginManager

[Jonas Devlieghere via Phabricator via lldb-commits]

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

🚂


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153818

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

[Lldb-commits] [PATCH] D153834: [lldb] Add two-level caching in the source manager

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: bulbazord, clayborg, jingham, jasonmolenda.
Herald added a project: All.
JDevlieghere requested review of this revision.

We recently saw an uptick in internal reports complaining that LLDB is slow 
when source on NFS are inaccessible. I looked at the SourceManger and its cache 
and I think there’s some room for improvement in terms of reducing file system 
accesses:

1. We always resolve the path.
2. We always check the timestamp.
3. We always recheck the file system for negative cache hits.

D153726  fixes (1) but (2) and (3) are 
necessary because of the cache’s current design. Source files are cached at the 
debugger level which means that the source file cache can span multiple targets 
and processes. It wouldn't be correct to not reload a modified or new file from 
disk.

We can however significantly reduce the number of file system accesses by using 
a two level cache design: one cache at the debugger level and one at the 
process level.

- The cache at the debugger level works the way it does today. There is no 
negative cache: if we can't find the file on disk, we'll try again next time 
the cache is queried. If a cached file's timestamp changes or if its path 
remapping changes, the cached file is evicted and we reload it from disk.
- The cache at the process level is design to avoid accessing the file system. 
It doesn't check the file's modification time. It caches negative results, so 
if a file didn't exist, it doesn't try to reread it from disk. Checking if the 
path remapping changed is cheap (doesn't involve checking the file system) so 
this is the only way for a file to get evicted from the process cache.

The result of this patch is that LLDB will not show you new content if a file 
is modified or created while a process is running. I would argue that this is 
what most people would expect, but it is a change from how LLDB behaves today.

For an average stop, we query the cache four times. With the current 
implementation, that's 4 stats to get the modification time, If the file 
doesn't exist on disk, that's another 4 stats. Before D153726 
, if the path starts with a `~` there are 
another additional 4 calls to realpath. When debugging sources on a slow 
(network) filesystem, this quickly adds up.

In addition to the two level caching, this patch also adds a `source` logging 
channel and synchronization to the source file cache. The logging was helpful 
during development and hopefully will help us triage issues in the future. The 
synchronization isn't a new requirement: as the cache is shared across targets, 
there is no guarantees that it can't be accessed concurrently.


https://reviews.llvm.org/D153834

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Utility/LLDBLog.h
  lldb/source/Commands/CommandObjectSource.cpp
  lldb/source/Core/SourceManager.cpp
  lldb/source/Utility/LLDBLog.cpp
  lldb/test/API/source-manager/TestSourceManager.py
  lldb/unittests/Core/SourceManagerTest.cpp

Index: lldb/unittests/Core/SourceManagerTest.cpp
===
--- lldb/unittests/Core/SourceManagerTest.cpp
+++ lldb/unittests/Core/SourceManagerTest.cpp
@@ -30,7 +30,7 @@
   // Insert: foo
   FileSpec foo_file_spec("foo");
   auto foo_file_sp =
-  std::make_shared(foo_file_spec, nullptr);
+  std::make_shared(foo_file_spec, lldb::DebuggerSP());
   cache.AddSourceFile(foo_file_spec, foo_file_sp);
 
   // Query: foo, expect found.
@@ -44,7 +44,7 @@
   // Insert: foo
   FileSpec foo_file_spec("foo");
   auto foo_file_sp =
-  std::make_shared(foo_file_spec, nullptr);
+  std::make_shared(foo_file_spec, lldb::DebuggerSP());
   cache.AddSourceFile(foo_file_spec, foo_file_sp);
 
   // Query: bar, expect not found.
@@ -62,8 +62,8 @@
   FileSystem::Instance().Resolve(resolved_foo_file_spec);
 
   // Create the file with the resolved file spec.
-  auto foo_file_sp =
-  std::make_shared(resolved_foo_file_spec, nullptr);
+  auto foo_file_sp = std::make_shared(
+  resolved_foo_file_spec, lldb::DebuggerSP());
 
   // Cache the result with the unresolved file spec.
   cache.AddSourceFile(foo_file_spec, foo_file_sp);
Index: lldb/test/API/source-manager/TestSourceManager.py
===
--- lldb/test/API/source-manager/TestSourceManager.py
+++ lldb/test/API/source-manager/TestSourceManager.py
@@ -10,6 +10,7 @@
 """
 
 import os
+import io
 import stat
 
 import lldb
@@ -37,6 +38,33 @@
 self.file = self.getBuildArtifact("main-copy.c")
 self.line = line_number("main.c", "// Set break point at this line.")
 
+def modify_content(self):
+
+# Read the main.c file content.
+with io.open(self.file, "r", newline="\n") as f:
+original_content = f.read()
+

[Lldb-commits] [PATCH] D153824: [lldb][NFCI] Apply LLDB_DEPRECATED to things that have deprecated comments

[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/D153824/new/

https://reviews.llvm.org/D153824

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

[Lldb-commits] [PATCH] D153827: [lldb][NFCI] Remove use of ConstString from ProcessElfCore

[Alex Langford via Phabricator via lldb-commits]

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

I'm not convinced that it makes sense for the paths to be ConstStrings. We're
going to be putting them into FileSpecs (which are backed by
ConstStrings, for now) but otherwise there's no need to store them as
ConstStrings upfront.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153827

Files:
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.h


Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
@@ -20,7 +20,6 @@
 #include 
 
 #include "lldb/Target/PostMortemProcess.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Status.h"
 
 #include "Plugins/ObjectFile/ELF/ELFHeader.h"
@@ -117,7 +116,7 @@
 lldb::addr_t start;
 lldb::addr_t end;
 lldb::addr_t file_ofs;
-lldb_private::ConstString path;
+std::string path;
   };
 
   // For ProcessElfCore only
Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -258,8 +258,8 @@
 if (!m_nt_file_entries.empty()) {
   ModuleSpec exe_module_spec;
   exe_module_spec.GetArchitecture() = arch;
-  exe_module_spec.GetFileSpec().SetFile(
-  m_nt_file_entries[0].path.GetCString(), FileSpec::Style::native);
+  exe_module_spec.GetFileSpec().SetFile(m_nt_file_entries[0].path,
+FileSpec::Style::native);
   if (exe_module_spec.GetFileSpec()) {
 exe_module_sp = GetTarget().GetOrCreateModule(exe_module_spec, 
   true /* notify */);
@@ -938,7 +938,7 @@
   for (uint64_t i = 0; i < count; ++i) {
 const char *path = note.data.GetCStr(&offset);
 if (path && path[0])
-  m_nt_file_entries[i].path.SetCString(path);
+  m_nt_file_entries[i].path.assign(path);
   }
   break;
 }


Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
@@ -20,7 +20,6 @@
 #include 
 
 #include "lldb/Target/PostMortemProcess.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Status.h"
 
 #include "Plugins/ObjectFile/ELF/ELFHeader.h"
@@ -117,7 +116,7 @@
 lldb::addr_t start;
 lldb::addr_t end;
 lldb::addr_t file_ofs;
-lldb_private::ConstString path;
+std::string path;
   };
 
   // For ProcessElfCore only
Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -258,8 +258,8 @@
 if (!m_nt_file_entries.empty()) {
   ModuleSpec exe_module_spec;
   exe_module_spec.GetArchitecture() = arch;
-  exe_module_spec.GetFileSpec().SetFile(
-  m_nt_file_entries[0].path.GetCString(), FileSpec::Style::native);
+  exe_module_spec.GetFileSpec().SetFile(m_nt_file_entries[0].path,
+FileSpec::Style::native);
   if (exe_module_spec.GetFileSpec()) {
 exe_module_sp = GetTarget().GetOrCreateModule(exe_module_spec, 
   true /* notify */);
@@ -938,7 +938,7 @@
   for (uint64_t i = 0; i < count; ++i) {
 const char *path = note.data.GetCStr(&offset);
 if (path && path[0])
-  m_nt_file_entries[i].path.SetCString(path);
+  m_nt_file_entries[i].path.assign(path);
   }
   break;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D153735: [lldb][TargetGetModuleCallback] Implement Python interface

[Kazuki Sakamoto via Phabricator via lldb-commits]

splhack updated this revision to Diff 534804.
splhack added a comment.

Move SetTargetGetModuleCallback from SBDebugger to SBPlatform.
https://discourse.llvm.org/t/rfc-python-callback-for-target-get-module/71580/4?u=splhack


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153735

Files:
  lldb/bindings/python/python-typemaps.swig
  lldb/bindings/python/python-wrapper.swig
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBPlatform.h
  lldb/source/API/SBPlatform.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/test/API/python_api/target/TestGetModuleCallback.py
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -321,3 +321,11 @@
 lldb_private::python::SWIGBridge::ToSWIGWrapper(lldb::DataExtractorSP) {
   return python::PythonObject();
 }
+
+Status
+lldb_private::python::SWIGBridge::LLDBSWIGPythonCallTargetGetModuleCallback(
+void *callback_baton, lldb::DebuggerSP debugger_sp,
+const ModuleSpec &module_spec, FileSpec &module_file_spec,
+FileSpec &symbol_file_spec) {
+  return Status();
+}
Index: lldb/test/API/python_api/target/TestGetModuleCallback.py
===
--- /dev/null
+++ lldb/test/API/python_api/target/TestGetModuleCallback.py
@@ -0,0 +1,303 @@
+"""
+Test Target get module callback functionality
+"""
+
+import ctypes
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from pathlib import Path
+
+import lldb
+
+UNITTESTS_TARGET_INPUTS_PATH = "../../../../unittests/Target/Inputs"
+MODULE_PLATFORM_PATH = "/system/lib64/AndroidModule.so"
+MODULE_TRIPLE = "aarch64-none-linux"
+MODULE_RESOLVED_TRIPLE = "aarch64--linux-android"
+MODULE_UUID = "80008338-82A0-51E5-5922-C905D23890DA-BDDEFECC"
+MODULE_FUNCTION = "boom"
+MODULE_HIDDEN_FUNCTION = "boom_hidden"
+MODULE_FILE = "AndroidModule.so"
+MODULE_NON_EXISTENT_FILE = "non-existent-file"
+SYMBOL_FILE = "AndroidModule.unstripped.so"
+BREAKPAD_SYMBOL_FILE = "AndroidModule.so.sym"
+SYMBOL_STRIPPED = "stripped"
+SYMBOL_UNSTRIPPED = "unstripped"
+
+
+class GetModuleCallbackTestCase(TestBase):
+def setUp(self):
+TestBase.setUp(self)
+self.platform = self.dbg.GetSelectedPlatform()
+self.target = self.dbg.CreateTarget("")
+self.assertTrue(self.target)
+
+self.input_dir = (
+Path(self.getSourceDir()) / UNITTESTS_TARGET_INPUTS_PATH
+).resolve()
+self.assertTrue(self.input_dir.is_dir())
+
+def check_module_spec(self, module_spec: lldb.SBModuleSpec):
+self.assertEqual(
+MODULE_UUID.replace("-", ""),
+ctypes.string_at(
+int(module_spec.GetUUIDBytes()),
+module_spec.GetUUIDLength(),
+)
+.hex()
+.upper(),
+)
+
+self.assertEqual(MODULE_TRIPLE, module_spec.GetTriple())
+
+self.assertEqual(MODULE_PLATFORM_PATH, module_spec.GetFileSpec().fullpath)
+
+def check_module(self, module: lldb.SBModule, symbol_file: str, symbol_kind: str):
+self.assertTrue(module.IsValid())
+
+self.assertEqual(
+MODULE_UUID,
+module.GetUUIDString(),
+)
+
+self.assertEqual(MODULE_RESOLVED_TRIPLE, module.GetTriple())
+
+self.assertEqual(MODULE_PLATFORM_PATH, module.GetPlatformFileSpec().fullpath)
+
+self.assertEqual(
+str(self.input_dir / MODULE_FILE),
+module.GetFileSpec().fullpath,
+)
+
+self.assertEqual(
+str(self.input_dir / symbol_file),
+module.GetSymbolFileSpec().fullpath,
+)
+
+sc_list = module.FindFunctions(MODULE_FUNCTION, lldb.eSymbolTypeCode)
+self.assertEqual(1, sc_list.GetSize())
+sc_list = module.FindFunctions(MODULE_HIDDEN_FUNCTION, lldb.eSymbolTypeCode)
+self.assertEqual(0 if symbol_kind == SYMBOL_STRIPPED else 1, sc_list.GetSize())
+
+def test_set_non_callable(self):
+# The callback should be callable.
+non_callable = "a"
+
+with self.assertRaises(TypeError, msg="Need a callable object or None!"):
+self.platform.SetTargetGetModuleCallback(non_callable)
+
+def test_set_wrong_args(self):
+# The callback should accept 4 argument.
+def test_args3(a, b, c):
+pass
+
+with self.assertRaises(TypeError, msg="Expected 4 argument callable object"):
+self.platform.SetTargetGetModuleCallback(test_

[Lldb-commits] [PATCH] D153734: [lldb][TargetGetModuleCallback] Call get module callback

[Kazuki Sakamoto via Phabricator via lldb-commits]

splhack updated this revision to Diff 534803.
splhack added a comment.

Move SetTargetGetModuleCallback from Debugger to Platform.
https://discourse.llvm.org/t/rfc-python-callback-for-target-get-module/71580/4?u=splhack


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153734

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Target/Platform.h
  lldb/include/lldb/Target/Target.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Target.cpp
  lldb/unittests/Target/CMakeLists.txt
  lldb/unittests/Target/GetModuleCallbackTest.cpp
  lldb/unittests/Target/Inputs/AndroidModule.c
  lldb/unittests/Target/Inputs/AndroidModule.so
  lldb/unittests/Target/Inputs/AndroidModule.so.sym
  lldb/unittests/Target/Inputs/AndroidModule.unstripped.so

Index: lldb/unittests/Target/Inputs/AndroidModule.so.sym
===
--- /dev/null
+++ lldb/unittests/Target/Inputs/AndroidModule.so.sym
@@ -0,0 +1,21 @@
+MODULE Linux arm64 38830080A082E5515922C905D23890DA0 AndroidModule.so
+INFO CODE_ID 8000833882A051E55922C905D23890DABDDEFECC
+FILE 0 /private/tmp/test/AndroidModule.c
+FUNC 162c 8 0 boom
+162c 8 8 0
+FUNC 1634 8 0 boom_hidden
+1634 8 12 0
+PUBLIC 15cc 0 __on_dlclose
+PUBLIC 15dc 0 __emutls_unregister_key
+PUBLIC 15e4 0 __on_dlclose_late
+PUBLIC 15ec 0 __atexit_handler_wrapper
+PUBLIC 1600 0 atexit
+PUBLIC 161c 0 pthread_atfork
+STACK CFI INIT 15cc 10 .cfa: sp 0 + .ra: x30
+STACK CFI INIT 15dc 8 .cfa: sp 0 + .ra: x30
+STACK CFI INIT 15e4 8 .cfa: sp 0 + .ra: x30
+STACK CFI INIT 15ec 14 .cfa: sp 0 + .ra: x30
+STACK CFI INIT 1600 1c .cfa: sp 0 + .ra: x30
+STACK CFI INIT 161c 10 .cfa: sp 0 + .ra: x30
+STACK CFI INIT 162c 8 .cfa: sp 0 + .ra: x30
+STACK CFI INIT 1634 8 .cfa: sp 0 + .ra: x30
Index: lldb/unittests/Target/Inputs/AndroidModule.c
===
--- /dev/null
+++ lldb/unittests/Target/Inputs/AndroidModule.c
@@ -0,0 +1,13 @@
+// aarch64-linux-android29-clang -shared -Os -glldb -g3 -Wl,--build-id=sha1 \
+// AndroidModule.c -o AndroidModule.so
+// dump_syms AndroidModule.so > AndroidModule.so.sym
+// cp AndroidModule.so AndroidModule.unstripped.so
+// llvm-strip --strip-unneeded AndroidModule.so
+
+int boom(void) {
+  return 47;
+}
+
+__attribute__((visibility("hidden"))) int boom_hidden(void) {
+  return 48;
+}
Index: lldb/unittests/Target/GetModuleCallbackTest.cpp
===
--- /dev/null
+++ lldb/unittests/Target/GetModuleCallbackTest.cpp
@@ -0,0 +1,675 @@
+//===-- GetModuleCallbackTest.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 "Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h"
+#include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
+#include "Plugins/Platform/Android/PlatformAndroid.h"
+#include "Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h"
+#include "Plugins/SymbolFile/Symtab/SymbolFileSymtab.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "TestingSupport/TestUtilities.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/Target.h"
+#include "gmock/gmock.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::platform_android;
+using namespace lldb_private::breakpad;
+using namespace testing;
+
+namespace {
+
+constexpr llvm::StringLiteral k_process_plugin("mock-process-plugin");
+constexpr llvm::StringLiteral k_platform_dir("remote-android");
+constexpr llvm::StringLiteral k_cache_dir(".cache");
+constexpr llvm::StringLiteral k_module_file("AndroidModule.so");
+constexpr llvm::StringLiteral k_symbol_file("AndroidModule.unstripped.so");
+constexpr llvm::StringLiteral k_breakpad_symbol_file("AndroidModule.so.sym");
+constexpr llvm::StringLiteral k_arch("aarch64-none-linux");
+constexpr llvm::StringLiteral
+k_module_uuid("80008338-82A0-51E5-5922-C905D23890DA-BDDEFECC");
+constexpr llvm::StringLiteral k_function_symbol("boom");
+constexpr llvm::StringLiteral k_hidden_function_symbol("boom_hidden");
+const size_t k_module_size = 3784;
+
+class MockDebugger : public Debugger {
+public:
+  MockDebugger() : Debugger(nullptr, nullptr) {}
+
+  MOCK_METHOD4(CallTargetGetModuleCallback,
+   Status(void *, const ModuleSpec &, FileSpec &, FileSpec &));
+};
+
+ModuleSpec GetTestModuleSpec();
+
+class MockProcess : public Process {
+public:
+  MockProcess(TargetSP target_sp, ListenerSP listener_sp)
+  : Process(target_sp, listener_sp) {}
+
+  ll

[Lldb-commits] [PATCH] D153825: [lldb][NFCI] Change return type of GetProcessPluginName

[Alex Langford via Phabricator via lldb-commits]

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

Instead of just returning a raw `const char *`, I think llvm::StringRef
would make more sense. Most of the time that we use the return value of
`GetProcessPluginName` we're passing it to `CreateProcess` which takes a
StringRef anyway.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153825

Files:
  lldb/include/lldb/Host/ProcessLaunchInfo.h
  lldb/include/lldb/Target/Process.h
  lldb/source/Host/common/ProcessLaunchInfo.cpp
  lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
  lldb/source/Target/Target.cpp


Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3209,8 +3209,8 @@
   assert(m_process_sp);
 } else {
   // Use a Process plugin to construct the process.
-  const char *plugin_name = launch_info.GetProcessPluginName();
-  CreateProcess(launch_info.GetListener(), plugin_name, nullptr, false);
+  CreateProcess(launch_info.GetListener(),
+launch_info.GetProcessPluginName(), nullptr, false);
 }
 
 // Since we didn't have a platform launch the process, launch it here.
@@ -3371,14 +3371,14 @@
   } else {
 if (state != eStateConnected) {
   SaveScriptedLaunchInfo(attach_info);
-  const char *plugin_name = attach_info.GetProcessPluginName();
+  llvm::StringRef plugin_name = attach_info.GetProcessPluginName();
   process_sp =
   CreateProcess(attach_info.GetListenerForProcess(GetDebugger()),
 plugin_name, nullptr, false);
-  if (process_sp == nullptr) {
-error.SetErrorStringWithFormat(
-"failed to create process using plugin %s",
-(plugin_name) ? plugin_name : "null");
+  if (!process_sp) {
+error.SetErrorStringWithFormatv(
+"failed to create process using plugin '{0}'",
+plugin_name.empty() ? "" : plugin_name);
 return error;
   }
 }
Index: lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
===
--- lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
+++ lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
@@ -533,9 +533,9 @@
   if (!target || error.Fail())
 return process_sp;
 
-  const char *plugin_name = attach_info.GetProcessPluginName();
-  process_sp = target->CreateProcess(
-  attach_info.GetListenerForProcess(debugger), plugin_name, nullptr, 
false);
+  process_sp =
+  target->CreateProcess(attach_info.GetListenerForProcess(debugger),
+attach_info.GetProcessPluginName(), nullptr, 
false);
 
   process_sp->HijackProcessEvents(attach_info.GetHijackListener());
   if (process_sp)
Index: lldb/source/Host/common/ProcessLaunchInfo.cpp
===
--- lldb/source/Host/common/ProcessLaunchInfo.cpp
+++ lldb/source/Host/common/ProcessLaunchInfo.cpp
@@ -126,8 +126,8 @@
   m_working_dir = working_dir;
 }
 
-const char *ProcessLaunchInfo::GetProcessPluginName() const {
-  return (m_plugin_name.empty() ? nullptr : m_plugin_name.c_str());
+llvm::StringRef ProcessLaunchInfo::GetProcessPluginName() const {
+  return llvm::StringRef(m_plugin_name);
 }
 
 void ProcessLaunchInfo::SetProcessPluginName(llvm::StringRef plugin) {
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -147,8 +147,8 @@
 
   void SetResumeCount(uint32_t c) { m_resume_count = c; }
 
-  const char *GetProcessPluginName() const {
-return (m_plugin_name.empty() ? nullptr : m_plugin_name.c_str());
+  llvm::StringRef GetProcessPluginName() const {
+return llvm::StringRef(m_plugin_name);
   }
 
   void SetProcessPluginName(llvm::StringRef plugin) {
Index: lldb/include/lldb/Host/ProcessLaunchInfo.h
===
--- lldb/include/lldb/Host/ProcessLaunchInfo.h
+++ lldb/include/lldb/Host/ProcessLaunchInfo.h
@@ -68,7 +68,7 @@
 
   void SetWorkingDirectory(const FileSpec &working_dir);
 
-  const char *GetProcessPluginName() const;
+  llvm::StringRef GetProcessPluginName() const;
 
   void SetProcessPluginName(llvm::StringRef plugin);
 


Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3209,8 +3209,8 @@
   assert(m_process_sp);
 } else {
   // Use a Process plugin to construct the process.
-  const char *plugin_name = launch_info.GetProcessPluginName();
-  CreateProcess(launch_

[Lldb-commits] [PATCH] D153822: FileSystem::EnumerateDirectory should keep iterating if one entry has an invalid Status

[Jason Molenda via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2c1108f44342: FileSystem::EnumerateDirectory should skip 
entries w/o Status, not halt (authored by jasonmolenda).

Changed prior to commit:
  https://reviews.llvm.org/D153822?vs=534792&id=534800#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153822

Files:
  lldb/source/Host/common/FileSystem.cpp
  lldb/unittests/Host/FileSystemTest.cpp


Index: lldb/unittests/Host/FileSystemTest.cpp
===
--- lldb/unittests/Host/FileSystemTest.cpp
+++ lldb/unittests/Host/FileSystemTest.cpp
@@ -51,6 +51,11 @@
 FilesAndDirs.find(Path.str());
 if (I == FilesAndDirs.end())
   return make_error_code(llvm::errc::no_such_file_or_directory);
+// Simulate a broken symlink, where it points to a file/dir that
+// does not exist.
+if (I->second.isSymlink() &&
+I->second.getPermissions() == sys::fs::perms::no_perms)
+  return std::error_code(ENOENT, std::generic_category());
 return I->second;
   }
   ErrorOr>
@@ -152,6 +157,13 @@
   sys::fs::file_type::symlink_file, sys::fs::all_all);
 addEntry(Path, S);
   }
+
+  void addBrokenSymlink(StringRef Path) {
+vfs::Status S(Path, UniqueID(FSID, FileID++),
+  std::chrono::system_clock::now(), 0, 0, 0,
+  sys::fs::file_type::symlink_file, sys::fs::no_perms);
+addEntry(Path, S);
+  }
 };
 } // namespace
 
@@ -178,6 +190,7 @@
   D->addRegularFile("/foo");
   D->addDirectory("/bar");
   D->addSymlink("/baz");
+  D->addBrokenSymlink("/lux");
   D->addRegularFile("/qux", ~sys::fs::perms::all_read);
   D->setCurrentWorkingDirectory("/");
   return D;
Index: lldb/source/Host/common/FileSystem.cpp
===
--- lldb/source/Host/common/FileSystem.cpp
+++ lldb/source/Host/common/FileSystem.cpp
@@ -192,7 +192,7 @@
 const auto &Item = *Iter;
 ErrorOr Status = m_fs->status(Item.path());
 if (!Status)
-  break;
+  continue;
 if (!find_files && Status->isRegularFile())
   continue;
 if (!find_directories && Status->isDirectory())


Index: lldb/unittests/Host/FileSystemTest.cpp
===
--- lldb/unittests/Host/FileSystemTest.cpp
+++ lldb/unittests/Host/FileSystemTest.cpp
@@ -51,6 +51,11 @@
 FilesAndDirs.find(Path.str());
 if (I == FilesAndDirs.end())
   return make_error_code(llvm::errc::no_such_file_or_directory);
+// Simulate a broken symlink, where it points to a file/dir that
+// does not exist.
+if (I->second.isSymlink() &&
+I->second.getPermissions() == sys::fs::perms::no_perms)
+  return std::error_code(ENOENT, std::generic_category());
 return I->second;
   }
   ErrorOr>
@@ -152,6 +157,13 @@
   sys::fs::file_type::symlink_file, sys::fs::all_all);
 addEntry(Path, S);
   }
+
+  void addBrokenSymlink(StringRef Path) {
+vfs::Status S(Path, UniqueID(FSID, FileID++),
+  std::chrono::system_clock::now(), 0, 0, 0,
+  sys::fs::file_type::symlink_file, sys::fs::no_perms);
+addEntry(Path, S);
+  }
 };
 } // namespace
 
@@ -178,6 +190,7 @@
   D->addRegularFile("/foo");
   D->addDirectory("/bar");
   D->addSymlink("/baz");
+  D->addBrokenSymlink("/lux");
   D->addRegularFile("/qux", ~sys::fs::perms::all_read);
   D->setCurrentWorkingDirectory("/");
   return D;
Index: lldb/source/Host/common/FileSystem.cpp
===
--- lldb/source/Host/common/FileSystem.cpp
+++ lldb/source/Host/common/FileSystem.cpp
@@ -192,7 +192,7 @@
 const auto &Item = *Iter;
 ErrorOr Status = m_fs->status(Item.path());
 if (!Status)
-  break;
+  continue;
 if (!find_files && Status->isRegularFile())
   continue;
 if (!find_directories && Status->isDirectory())
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 2c1108f - FileSystem::EnumerateDirectory should skip entries w/o Status, not halt

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2023-06-26T17:46:53-07:00
New Revision: 2c1108f44342019a67ce55bb98e2a05b57a70b9a

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

LOG: FileSystem::EnumerateDirectory should skip entries w/o Status, not halt

EnumerateDirectory gets the vfs::Status of each directory entry to
decide how to process it.  If it is unable to get the Status for
a directory entry, it will currently halt the directory iteration
entirely.  It should only skip this entry.

Differential Revision: https://reviews.llvm.org/D153822
rdar://110861210

Added: 


Modified: 
lldb/source/Host/common/FileSystem.cpp
lldb/unittests/Host/FileSystemTest.cpp

Removed: 




diff  --git a/lldb/source/Host/common/FileSystem.cpp 
b/lldb/source/Host/common/FileSystem.cpp
index 96ff1bf13bd7d..1fe4e8fdd500e 100644
--- a/lldb/source/Host/common/FileSystem.cpp
+++ b/lldb/source/Host/common/FileSystem.cpp
@@ -192,7 +192,7 @@ void FileSystem::EnumerateDirectory(Twine path, bool 
find_directories,
 const auto &Item = *Iter;
 ErrorOr Status = m_fs->status(Item.path());
 if (!Status)
-  break;
+  continue;
 if (!find_files && Status->isRegularFile())
   continue;
 if (!find_directories && Status->isDirectory())

diff  --git a/lldb/unittests/Host/FileSystemTest.cpp 
b/lldb/unittests/Host/FileSystemTest.cpp
index facb7ae5f1768..4ed9beff4d303 100644
--- a/lldb/unittests/Host/FileSystemTest.cpp
+++ b/lldb/unittests/Host/FileSystemTest.cpp
@@ -51,6 +51,11 @@ class DummyFileSystem : public vfs::FileSystem {
 FilesAndDirs.find(Path.str());
 if (I == FilesAndDirs.end())
   return make_error_code(llvm::errc::no_such_file_or_directory);
+// Simulate a broken symlink, where it points to a file/dir that
+// does not exist.
+if (I->second.isSymlink() &&
+I->second.getPermissions() == sys::fs::perms::no_perms)
+  return std::error_code(ENOENT, std::generic_category());
 return I->second;
   }
   ErrorOr>
@@ -152,6 +157,13 @@ class DummyFileSystem : public vfs::FileSystem {
   sys::fs::file_type::symlink_file, sys::fs::all_all);
 addEntry(Path, S);
   }
+
+  void addBrokenSymlink(StringRef Path) {
+vfs::Status S(Path, UniqueID(FSID, FileID++),
+  std::chrono::system_clock::now(), 0, 0, 0,
+  sys::fs::file_type::symlink_file, sys::fs::no_perms);
+addEntry(Path, S);
+  }
 };
 } // namespace
 
@@ -178,6 +190,7 @@ static IntrusiveRefCntPtr 
GetSimpleDummyFS() {
   D->addRegularFile("/foo");
   D->addDirectory("/bar");
   D->addSymlink("/baz");
+  D->addBrokenSymlink("/lux");
   D->addRegularFile("/qux", ~sys::fs::perms::all_read);
   D->setCurrentWorkingDirectory("/");
   return D;



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

[Lldb-commits] [PATCH] D153824: [lldb][NFCI] Apply LLDB_DEPRECATED to things that have deprecated comments

[Alex Langford via Phabricator via lldb-commits]

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

Now that we have a proper way to deprecate things in the SB API, we
should apply it where we have manually done so with comments.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153824

Files:
  lldb/include/lldb/API/SBCommandReturnObject.h
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/API/SBFileSpec.h
  lldb/include/lldb/API/SBProcess.h
  lldb/include/lldb/API/SBValue.h

Index: lldb/include/lldb/API/SBValue.h
===
--- lldb/include/lldb/API/SBValue.h
+++ lldb/include/lldb/API/SBValue.h
@@ -105,7 +105,8 @@
 
   const char *GetLocation();
 
-  // Deprecated - use the one that takes SBError&
+  LLDB_DEPRECATED("Use the variant that takes an SBError &",
+  "SetValueFromCString(const char *, SBError &)")
   bool SetValueFromCString(const char *value_str);
 
   bool SetValueFromCString(const char *value_str, lldb::SBError &error);
@@ -123,7 +124,7 @@
   lldb::SBValue CreateChildAtOffset(const char *name, uint32_t offset,
 lldb::SBType type);
 
-  // Deprecated - use the expression evaluator to perform type casting
+  LLDB_DEPRECATED("Use the expression evaluator to perform type casting", "")
   lldb::SBValue Cast(lldb::SBType type);
 
   lldb::SBValue CreateValueFromExpression(const char *name,
@@ -294,7 +295,7 @@
 
   lldb::SBValue Dereference();
 
-  // Deprecated - please use GetType().IsPointerType() instead.
+  LLDB_DEPRECATED("Use GetType().IsPointerType() instead", "")
   bool TypeIsPointerType();
 
   lldb::SBType GetType();
Index: lldb/include/lldb/API/SBProcess.h
===
--- lldb/include/lldb/API/SBProcess.h
+++ lldb/include/lldb/API/SBProcess.h
@@ -48,7 +48,7 @@
 
   const char *GetPluginName();
 
-  // DEPRECATED: use GetPluginName()
+  LLDB_DEPRECATED("Use GetPluginName()", "GetPluginName()")
   const char *GetShortPluginName();
 
   void Clear();
Index: lldb/include/lldb/API/SBFileSpec.h
===
--- lldb/include/lldb/API/SBFileSpec.h
+++ lldb/include/lldb/API/SBFileSpec.h
@@ -19,8 +19,10 @@
 
   SBFileSpec(const lldb::SBFileSpec &rhs);
 
-  SBFileSpec(const char *path); // Deprecated, use SBFileSpec (const char *path,
-// bool resolve)
+  LLDB_DEPRECATED("Use the other constructor to determine if this the file "
+  "spec should be resolved",
+  "SBFileSpec(const char *, bool)")
+  SBFileSpec(const char *path);
 
   SBFileSpec(const char *path, bool resolve);
 
Index: lldb/include/lldb/API/SBDebugger.h
===
--- lldb/include/lldb/API/SBDebugger.h
+++ lldb/include/lldb/API/SBDebugger.h
@@ -115,7 +115,7 @@
 
   static void Terminate();
 
-  // Deprecated, use the one that takes a source_init_files bool.
+  LLDB_DEPRECATED("Use one of the other Create variants", "Create(bool)")
   static lldb::SBDebugger Create();
 
   static lldb::SBDebugger Create(bool source_init_files);
@@ -208,9 +208,14 @@
   lldb::SBListener GetListener();
 
 #ifndef SWIG
+  LLDB_DEPRECATED(
+  "Use HandleProcessEvent(const SBProcess &, const SBEvent &, SBFile, "
+  "SBFile) or HandleProcessEvent(const SBProcess &, const SBEvent &, "
+  "FileSP, FileSP)",
+  "HandleProcessEvent(const SBProcess &, const SBEvent &, SBFile, SBFile)")
   void HandleProcessEvent(const lldb::SBProcess &process,
   const lldb::SBEvent &event, FILE *out,
-  FILE *err); // DEPRECATED
+  FILE *err);
 #endif
 
   void HandleProcessEvent(const lldb::SBProcess &process,
@@ -326,8 +331,9 @@
   void SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
   void *baton);
 
-  // DEPRECATED
 #ifndef SWIG
+  LLDB_DEPRECATED("Use DispatchInput(const void *, size_t)",
+  "DispatchInput(const void *, size_t)")
   void DispatchInput(void *baton, const void *data, size_t data_len);
 #endif
 
Index: lldb/include/lldb/API/SBCommandReturnObject.h
===
--- lldb/include/lldb/API/SBCommandReturnObject.h
+++ lldb/include/lldb/API/SBCommandReturnObject.h
@@ -47,7 +47,9 @@
   const char *GetError();
 
 #ifndef SWIG
-  size_t PutOutput(FILE *fh); // DEPRECATED
+  LLDB_DEPRECATED("Use PutOutput(SBFile) or PutOutput(FileSP)",
+  "PutOutput(SBFile)")
+  size_t PutOutput(FILE *fh);
 #endif
 
   size_t PutOutput(SBFile file);
@@ -59,7 +61,9 @@
   size_t GetErrorSize();
 
 #ifndef SWIG
-  size_t PutError(FILE *fh); // DEPRECATED
+  LLDB_DEPR

[Lldb-commits] [PATCH] D153822: FileSystem::EnumerateDirectory should keep iterating if one entry has an invalid Status

[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 modulo capitalization.




Comment at: lldb/unittests/Host/FileSystemTest.cpp:54
   return make_error_code(llvm::errc::no_such_file_or_directory);
+// simulate a broken symlink, where it points to a file/dir that
+// does not exist.

Comments should start with a capital. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153822

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

[Lldb-commits] [PATCH] D153822: FileSystem::EnumerateDirectory should keep iterating if one entry has an invalid Status

[Jason Molenda via Phabricator via lldb-commits]

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

EnumerateDirectory gets the vfs::Status of each file entry, to check if we are 
asked to report on this file type, and if we can't get the Status, it stops the 
directory search.  Instead, if it can't get the Status of a directory entry, it 
should skip that entry and continue searching.

This happens with a broken symlink - the link points to another file, but that 
file doesn't exist, and the Status returned is of the destination file.  When 
we can't get the destination file Status, an error code is returned.

Add a test case to FileSystemTests unittest.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153822

Files:
  lldb/source/Host/common/FileSystem.cpp
  lldb/unittests/Host/FileSystemTest.cpp


Index: lldb/unittests/Host/FileSystemTest.cpp
===
--- lldb/unittests/Host/FileSystemTest.cpp
+++ lldb/unittests/Host/FileSystemTest.cpp
@@ -51,6 +51,11 @@
 FilesAndDirs.find(Path.str());
 if (I == FilesAndDirs.end())
   return make_error_code(llvm::errc::no_such_file_or_directory);
+// simulate a broken symlink, where it points to a file/dir that
+// does not exist.
+if (I->second.isSymlink() &&
+I->second.getPermissions() == sys::fs::perms::no_perms)
+  return std::error_code(ENOENT, std::generic_category());
 return I->second;
   }
   ErrorOr>
@@ -152,6 +157,13 @@
   sys::fs::file_type::symlink_file, sys::fs::all_all);
 addEntry(Path, S);
   }
+
+  void addBrokenSymlink(StringRef Path) {
+vfs::Status S(Path, UniqueID(FSID, FileID++),
+  std::chrono::system_clock::now(), 0, 0, 0,
+  sys::fs::file_type::symlink_file, sys::fs::no_perms);
+addEntry(Path, S);
+  }
 };
 } // namespace
 
@@ -178,6 +190,7 @@
   D->addRegularFile("/foo");
   D->addDirectory("/bar");
   D->addSymlink("/baz");
+  D->addBrokenSymlink("/lux");
   D->addRegularFile("/qux", ~sys::fs::perms::all_read);
   D->setCurrentWorkingDirectory("/");
   return D;
Index: lldb/source/Host/common/FileSystem.cpp
===
--- lldb/source/Host/common/FileSystem.cpp
+++ lldb/source/Host/common/FileSystem.cpp
@@ -192,7 +192,7 @@
 const auto &Item = *Iter;
 ErrorOr Status = m_fs->status(Item.path());
 if (!Status)
-  break;
+  continue;
 if (!find_files && Status->isRegularFile())
   continue;
 if (!find_directories && Status->isDirectory())


Index: lldb/unittests/Host/FileSystemTest.cpp
===
--- lldb/unittests/Host/FileSystemTest.cpp
+++ lldb/unittests/Host/FileSystemTest.cpp
@@ -51,6 +51,11 @@
 FilesAndDirs.find(Path.str());
 if (I == FilesAndDirs.end())
   return make_error_code(llvm::errc::no_such_file_or_directory);
+// simulate a broken symlink, where it points to a file/dir that
+// does not exist.
+if (I->second.isSymlink() &&
+I->second.getPermissions() == sys::fs::perms::no_perms)
+  return std::error_code(ENOENT, std::generic_category());
 return I->second;
   }
   ErrorOr>
@@ -152,6 +157,13 @@
   sys::fs::file_type::symlink_file, sys::fs::all_all);
 addEntry(Path, S);
   }
+
+  void addBrokenSymlink(StringRef Path) {
+vfs::Status S(Path, UniqueID(FSID, FileID++),
+  std::chrono::system_clock::now(), 0, 0, 0,
+  sys::fs::file_type::symlink_file, sys::fs::no_perms);
+addEntry(Path, S);
+  }
 };
 } // namespace
 
@@ -178,6 +190,7 @@
   D->addRegularFile("/foo");
   D->addDirectory("/bar");
   D->addSymlink("/baz");
+  D->addBrokenSymlink("/lux");
   D->addRegularFile("/qux", ~sys::fs::perms::all_read);
   D->setCurrentWorkingDirectory("/");
   return D;
Index: lldb/source/Host/common/FileSystem.cpp
===
--- lldb/source/Host/common/FileSystem.cpp
+++ lldb/source/Host/common/FileSystem.cpp
@@ -192,7 +192,7 @@
 const auto &Item = *Iter;
 ErrorOr Status = m_fs->status(Item.path());
 if (!Status)
-  break;
+  continue;
 if (!find_files && Status->isRegularFile())
   continue;
 if (!find_directories && Status->isDirectory())
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D153657: Don't allow ValueObject::Cast from a smaller type to a larger

[Jim Ingham via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf05e2fb013f0: Don't allow SBValue::Cast to cast from a 
smaller type to a larger, (authored by jingham).

Changed prior to commit:
  https://reviews.llvm.org/D153657?vs=534036&id=534775#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153657

Files:
  lldb/include/lldb/Core/ValueObject.h
  lldb/include/lldb/Core/ValueObjectConstResult.h
  lldb/include/lldb/Core/ValueObjectConstResultCast.h
  lldb/include/lldb/Core/ValueObjectConstResultChild.h
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Core/ValueObject.cpp
  lldb/source/Core/ValueObjectConstResult.cpp
  lldb/source/Core/ValueObjectConstResultCast.cpp
  lldb/source/Core/ValueObjectConstResultChild.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
  lldb/test/API/python_api/value/TestValueAPI.py
  lldb/test/API/python_api/value/main.c

Index: lldb/test/API/python_api/value/main.c
===
--- lldb/test/API/python_api/value/main.c
+++ lldb/test/API/python_api/value/main.c
@@ -29,6 +29,13 @@
   int b;
 };
 
+struct MyBiggerStruct
+{
+  int a;
+  int b;
+  int c;
+};
+
 int main (int argc, char const *argv[])
 {
 uint32_t uinthex = 0xE0A35F10;
@@ -37,6 +44,7 @@
 int i;
 MyInt a = 12345;
 struct MyStruct s = { 11, 22 };
+struct MyBiggerStruct f = { 33, 44, 55 }; 
 int *my_int_ptr = &g_my_int;
 printf("my_int_ptr points to location %p\n", my_int_ptr);
 const char **str_ptr = days_of_week;
Index: lldb/test/API/python_api/value/TestValueAPI.py
===
--- lldb/test/API/python_api/value/TestValueAPI.py
+++ lldb/test/API/python_api/value/TestValueAPI.py
@@ -146,6 +146,19 @@
 self.assertTrue(val_s.GetChildMemberWithName("a").AddressOf(), VALID_VARIABLE)
 self.assertTrue(val_a.Cast(val_i.GetType()).AddressOf(), VALID_VARIABLE)
 
+# Test some other cases of the Cast API.  We allow casts from one struct type
+# to another, which is a little weird, but we don't support casting from a
+# smaller type to a larger as we often wouldn't know how to get the extra data:
+val_f = target.EvaluateExpression("f")
+bad_cast = val_s.Cast(val_f.GetType())
+self.assertFailure(bad_cast.GetError(),
+   "Can only cast to a type that is equal to or smaller than the orignal type.")
+weird_cast = val_f.Cast(val_s.GetType())
+self.assertSuccess(weird_cast.GetError(),
+"Can cast from a larger to a smaller")
+self.assertEqual(weird_cast.GetChildMemberWithName("a").GetValueAsSigned(0), 33,
+ "Got the right value")
+
 # Check that lldb.value implements truth testing.
 self.assertFalse(lldb.value(frame0.FindVariable("bogus")))
 self.assertTrue(lldb.value(frame0.FindVariable("uinthex")))
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
@@ -157,7 +157,11 @@
   }
   if (!m_node_type)
 return nullptr;
-  node_sp = node_sp->Cast(m_node_type);
+  node_sp = m_next_element->Cast(m_node_type.GetPointerType())
+  ->Dereference(error);
+  if (!node_sp || error.Fail())
+  return nullptr;
+
   value_sp = node_sp->GetChildMemberWithName("__value_");
   hash_sp = node_sp->GetChildMemberWithName("__hash_");
   if (!value_sp || !hash_sp)
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -607,11 +607,13 @@
   if (idx == 1) {
 if (auto ptr_sp = valobj_sp->GetChildMemberWithName("__ptr_")) {
   Status status;
-  auto value_sp = ptr_sp->Dereference(status);
+  auto value_type_sp =
+valobj_sp->GetCompilerType()
+  .GetTypeTemplateArgument(0).GetPointerType();
+  ValueObjectSP cast_ptr_sp = ptr_sp->Cast(value_type_sp);
+  ValueObjectSP value_sp = cast_ptr_sp->Dereference(status);
   if (status.Success()) {
-auto value_type_sp =
-valobj_sp->GetCompilerType().GetTypeTemplateArgument(0);
-return value_sp->Cast(value_type_sp);
+return value_sp;
   }
 }
   }
Index: lldb/source/Core/ValueObjectConstResultChild.cpp
===
--- lldb/source/Core/ValueObjectConstResultChild.cpp
+++ lldb/source/Core/ValueObjectConstResultChild

[Lldb-commits] [lldb] f05e2fb - Don't allow SBValue::Cast to cast from a smaller type to a larger,

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2023-06-26T16:02:01-07:00
New Revision: f05e2fb013f0e2504471a9899dba7d70cc58a63d

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

LOG: Don't allow SBValue::Cast to cast from a smaller type to a larger,
as we don't in general know where the extra data should come from.

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

Added: 


Modified: 
lldb/include/lldb/Core/ValueObject.h
lldb/include/lldb/Core/ValueObjectConstResult.h
lldb/include/lldb/Core/ValueObjectConstResultCast.h
lldb/include/lldb/Core/ValueObjectConstResultChild.h
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/source/Core/ValueObject.cpp
lldb/source/Core/ValueObjectConstResult.cpp
lldb/source/Core/ValueObjectConstResultCast.cpp
lldb/source/Core/ValueObjectConstResultChild.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
lldb/test/API/python_api/value/TestValueAPI.py
lldb/test/API/python_api/value/main.c

Removed: 




diff  --git a/lldb/include/lldb/Core/ValueObject.h 
b/lldb/include/lldb/Core/ValueObject.h
index 2f2b212e238ca..2d8036ab07871 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -614,7 +614,9 @@ class ValueObject {
   virtual void SetLiveAddress(lldb::addr_t addr = LLDB_INVALID_ADDRESS,
   AddressType address_type = eAddressTypeLoad) {}
 
-  virtual lldb::ValueObjectSP Cast(const CompilerType &compiler_type);
+  lldb::ValueObjectSP Cast(const CompilerType &compiler_type);
+
+  virtual lldb::ValueObjectSP DoCast(const CompilerType &compiler_type);
 
   virtual lldb::ValueObjectSP CastPointerType(const char *name,
   CompilerType &ast_type);

diff  --git a/lldb/include/lldb/Core/ValueObjectConstResult.h 
b/lldb/include/lldb/Core/ValueObjectConstResult.h
index 4edd495216061..d61df859bebce 100644
--- a/lldb/include/lldb/Core/ValueObjectConstResult.h
+++ b/lldb/include/lldb/Core/ValueObjectConstResult.h
@@ -106,7 +106,7 @@ class ValueObjectConstResult : public ValueObject {
 
   lldb::LanguageType GetPreferredDisplayLanguage() override;
 
-  lldb::ValueObjectSP Cast(const CompilerType &compiler_type) override;
+  lldb::ValueObjectSP DoCast(const CompilerType &compiler_type) override;
 
 protected:
   bool UpdateValue() override;

diff  --git a/lldb/include/lldb/Core/ValueObjectConstResultCast.h 
b/lldb/include/lldb/Core/ValueObjectConstResultCast.h
index 5467ce3db403f..efcbe0dc6a0bd 100644
--- a/lldb/include/lldb/Core/ValueObjectConstResultCast.h
+++ b/lldb/include/lldb/Core/ValueObjectConstResultCast.h
@@ -51,7 +51,7 @@ class ValueObjectConstResultCast : public ValueObjectCast {
   size_t GetPointeeData(DataExtractor &data, uint32_t item_idx = 0,
 uint32_t item_count = 1) override;
 
-  lldb::ValueObjectSP Cast(const CompilerType &compiler_type) override;
+  lldb::ValueObjectSP DoCast(const CompilerType &compiler_type) override;
 
 protected:
   ValueObjectConstResultImpl m_impl;

diff  --git a/lldb/include/lldb/Core/ValueObjectConstResultChild.h 
b/lldb/include/lldb/Core/ValueObjectConstResultChild.h
index 26bd9f337a595..7e9da14e8e97f 100644
--- a/lldb/include/lldb/Core/ValueObjectConstResultChild.h
+++ b/lldb/include/lldb/Core/ValueObjectConstResultChild.h
@@ -60,7 +60,7 @@ class ValueObjectConstResultChild : public ValueObjectChild {
   size_t GetPointeeData(DataExtractor &data, uint32_t item_idx = 0,
 uint32_t item_count = 1) override;
 
-  lldb::ValueObjectSP Cast(const CompilerType &compiler_type) override;
+  lldb::ValueObjectSP DoCast(const CompilerType &compiler_type) override;
 
 protected:
   ValueObjectConstResultImpl m_impl;

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index a712f6b2b2a56..2bff1a7067cb3 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2604,6 +2604,17 @@ def assertSuccess(self, obj, msg=None):
 if not obj.Success():
 error = obj.GetCString()
 self.fail(self._formatMessage(msg, "'{}' is not 
success".format(error)))
+"""Assert that an lldb.SBError is in the "failure" state."""
+
+def assertFailure(self, obj, error_str = None, msg=None):
+if obj.Success():
+self.fail(self._formatMessage(msg, "Error not in a fail state"))
+
+if error_str == None:
+return
+  
+error = obj.GetCString()
+self.assertEqual(error, error_str, msg)
 
 """Assert that a command return object is successful"""
 

diff  --git a/lldb/source/Core/

[Lldb-commits] [PATCH] D153817: [lldb] Increase the maximum number of classes we can read from shared cache

[Alex Langford via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7ed3c2edf017: [lldb] Increase the maximum number of classes 
we can read from shared cache (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153817

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp


Index: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -2308,7 +2308,10 @@
 
   // The number of entries to pre-allocate room for.
   // Each entry is (addrsize + 4) bytes
-  const uint32_t max_num_classes = 163840;
+  // FIXME: It is not sustainable to continue incrementing this value every 
time
+  // the shared cache grows. This is because it requires allocating memory in
+  // the inferior process and some inferior processes have small memory limits.
+  const uint32_t max_num_classes = 212992;
 
   UtilityFunction *get_class_info_code = GetClassInfoUtilityFunction(exe_ctx);
   if (!get_class_info_code) {


Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -2308,7 +2308,10 @@
 
   // The number of entries to pre-allocate room for.
   // Each entry is (addrsize + 4) bytes
-  const uint32_t max_num_classes = 163840;
+  // FIXME: It is not sustainable to continue incrementing this value every time
+  // the shared cache grows. This is because it requires allocating memory in
+  // the inferior process and some inferior processes have small memory limits.
+  const uint32_t max_num_classes = 212992;
 
   UtilityFunction *get_class_info_code = GetClassInfoUtilityFunction(exe_ctx);
   if (!get_class_info_code) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 7ed3c2e - [lldb] Increase the maximum number of classes we can read from shared cache

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-06-26T14:59:55-07:00
New Revision: 7ed3c2edf017840e0c7c358aa48d2d67d4b55dcb

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

LOG: [lldb] Increase the maximum number of classes we can read from shared cache

The shared cache has grown past the previously hardcoded limit. As a
temporary measure, I am increasing the hardcoded number of classes we
can expect to read from the shared cache. This is not a good long-term
solution.

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

Added: 


Modified: 

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index fc9b4d27ccb66..ef54c2b5044b0 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -2308,7 +2308,10 @@ 
AppleObjCRuntimeV2::SharedCacheClassInfoExtractor::UpdateISAToDescriptorMap() {
 
   // The number of entries to pre-allocate room for.
   // Each entry is (addrsize + 4) bytes
-  const uint32_t max_num_classes = 163840;
+  // FIXME: It is not sustainable to continue incrementing this value every 
time
+  // the shared cache grows. This is because it requires allocating memory in
+  // the inferior process and some inferior processes have small memory limits.
+  const uint32_t max_num_classes = 212992;
 
   UtilityFunction *get_class_info_code = GetClassInfoUtilityFunction(exe_ctx);
   if (!get_class_info_code) {



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

[Lldb-commits] [PATCH] D153818: [lldb][NFCI] Remove use of ConstString from PluginManager

[Alex Langford via Phabricator via lldb-commits]

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

The underlying structures no longer use ConstString so we can remove it
wholesale from PluginManager now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153818

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

Index: lldb/source/Core/PluginManager.cpp
===
--- lldb/source/Core/PluginManager.cpp
+++ lldb/source/Core/PluginManager.cpp
@@ -13,7 +13,6 @@
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Interpreter/OptionValueProperties.h"
 #include "lldb/Target/Process.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/StringList.h"
@@ -1432,7 +1431,7 @@
 // This will put a plugin's settings under e.g.
 // "plugin...SETTINGNAME".
 static lldb::OptionValuePropertiesSP
-GetDebuggerPropertyForPlugins(Debugger &debugger, ConstString plugin_type_name,
+GetDebuggerPropertyForPlugins(Debugger &debugger, llvm::StringRef plugin_type_name,
   llvm::StringRef plugin_type_desc,
   bool can_create) {
   lldb::OptionValuePropertiesSP parent_properties_sp(
@@ -1469,7 +1468,7 @@
 // ".plugin..SETTINGNAME" and Platform
 // generic settings would be under "platform.SETTINGNAME".
 static lldb::OptionValuePropertiesSP GetDebuggerPropertyForPluginsOldStyle(
-Debugger &debugger, ConstString plugin_type_name,
+Debugger &debugger, llvm::StringRef plugin_type_name,
 llvm::StringRef plugin_type_desc, bool can_create) {
   static constexpr llvm::StringLiteral g_property_name("plugin");
   lldb::OptionValuePropertiesSP parent_properties_sp(
@@ -1503,13 +1502,13 @@
 namespace {
 
 typedef lldb::OptionValuePropertiesSP
-GetDebuggerPropertyForPluginsPtr(Debugger &, ConstString, llvm::StringRef,
+GetDebuggerPropertyForPluginsPtr(Debugger &, llvm::StringRef, llvm::StringRef,
  bool can_create);
 }
 
 static lldb::OptionValuePropertiesSP
-GetSettingForPlugin(Debugger &debugger, ConstString setting_name,
-ConstString plugin_type_name,
+GetSettingForPlugin(Debugger &debugger, llvm::StringRef setting_name,
+llvm::StringRef plugin_type_name,
 GetDebuggerPropertyForPluginsPtr get_debugger_property =
 GetDebuggerPropertyForPlugins) {
   lldb::OptionValuePropertiesSP properties_sp;
@@ -1524,7 +1523,7 @@
 }
 
 static bool
-CreateSettingForPlugin(Debugger &debugger, ConstString plugin_type_name,
+CreateSettingForPlugin(Debugger &debugger, llvm::StringRef plugin_type_name,
llvm::StringRef plugin_type_desc,
const lldb::OptionValuePropertiesSP &properties_sp,
llvm::StringRef description, bool is_global_property,
@@ -1544,42 +1543,41 @@
   return false;
 }
 
-static const char *kDynamicLoaderPluginName("dynamic-loader");
-static const char *kPlatformPluginName("platform");
-static const char *kProcessPluginName("process");
-static const char *kTracePluginName("trace");
-static const char *kObjectFilePluginName("object-file");
-static const char *kSymbolFilePluginName("symbol-file");
-static const char *kJITLoaderPluginName("jit-loader");
-static const char *kStructuredDataPluginName("structured-data");
+static constexpr llvm::StringLiteral kDynamicLoaderPluginName("dynamic-loader");
+static constexpr llvm::StringLiteral kPlatformPluginName("platform");
+static constexpr llvm::StringLiteral kProcessPluginName("process");
+static constexpr llvm::StringLiteral kTracePluginName("trace");
+static constexpr llvm::StringLiteral kObjectFilePluginName("object-file");
+static constexpr llvm::StringLiteral kSymbolFilePluginName("symbol-file");
+static constexpr llvm::StringLiteral kJITLoaderPluginName("jit-loader");
+static constexpr llvm::StringLiteral
+kStructuredDataPluginName("structured-data");
 
 lldb::OptionValuePropertiesSP
 PluginManager::GetSettingForDynamicLoaderPlugin(Debugger &debugger,
-ConstString setting_name) {
-  return GetSettingForPlugin(debugger, setting_name,
- ConstString(kDynamicLoaderPluginName));
+llvm::StringRef setting_name) {
+  return GetSettingForPlugin(debugger, setting_name, kDynamicLoaderPluginName);
 }
 
 bool PluginManager::CreateSettingForDynamicLoaderPlugin(
 Debugger &debugger, const lldb::OptionValuePropertiesSP &properties_sp,
 llvm::StringRef description, bool is_global_property) {
-  return CreateSettingForPlugin(debugger, ConstString(kDynamicLoaderPluginName),
+  return CreateSettingForPlugin(debugger, kDynam

[Lldb-commits] [PATCH] D153817: [lldb] Increase the maximum number of classes we can read from shared cache

[Alex Langford via Phabricator via lldb-commits]

bulbazord updated this revision to Diff 534757.
bulbazord added a comment.

Updating number


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153817

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp


Index: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -2308,7 +2308,10 @@
 
   // The number of entries to pre-allocate room for.
   // Each entry is (addrsize + 4) bytes
-  const uint32_t max_num_classes = 163840;
+  // FIXME: It is not sustainable to continue incrementing this value every 
time
+  // the shared cache grows. This is because it requires allocating memory in
+  // the inferior process and some inferior processes have small memory limits.
+  const uint32_t max_num_classes = 212992;
 
   UtilityFunction *get_class_info_code = GetClassInfoUtilityFunction(exe_ctx);
   if (!get_class_info_code) {


Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -2308,7 +2308,10 @@
 
   // The number of entries to pre-allocate room for.
   // Each entry is (addrsize + 4) bytes
-  const uint32_t max_num_classes = 163840;
+  // FIXME: It is not sustainable to continue incrementing this value every time
+  // the shared cache grows. This is because it requires allocating memory in
+  // the inferior process and some inferior processes have small memory limits.
+  const uint32_t max_num_classes = 212992;
 
   UtilityFunction *get_class_info_code = GetClassInfoUtilityFunction(exe_ctx);
   if (!get_class_info_code) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D153817: [lldb] Increase the maximum number of classes we can read from shared cache

[Alex Langford via Phabricator via lldb-commits]

bulbazord updated this revision to Diff 534753.
bulbazord added a comment.

Update FIXME with more info


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153817

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp


Index: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -2308,7 +2308,10 @@
 
   // The number of entries to pre-allocate room for.
   // Each entry is (addrsize + 4) bytes
-  const uint32_t max_num_classes = 163840;
+  // FIXME: It is not sustainable to continue incrementing this value every 
time
+  // the shared cache grows. This is because it requires allocating memory in
+  // the inferior process and some inferior processes have small memory limits.
+  const uint32_t max_num_classes = 20;
 
   UtilityFunction *get_class_info_code = GetClassInfoUtilityFunction(exe_ctx);
   if (!get_class_info_code) {


Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -2308,7 +2308,10 @@
 
   // The number of entries to pre-allocate room for.
   // Each entry is (addrsize + 4) bytes
-  const uint32_t max_num_classes = 163840;
+  // FIXME: It is not sustainable to continue incrementing this value every time
+  // the shared cache grows. This is because it requires allocating memory in
+  // the inferior process and some inferior processes have small memory limits.
+  const uint32_t max_num_classes = 20;
 
   UtilityFunction *get_class_info_code = GetClassInfoUtilityFunction(exe_ctx);
   if (!get_class_info_code) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D153817: [lldb] Increase the maximum number of classes we can read from shared cache

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

In D153817#4450534 , @jasonmolenda 
wrote:

> LGTM.  You might want to mention why it isn't sustainable -- because we must 
> allocate memory in the inferior process, and some inferior processes have 
> quite small memory limits.

Sounds good, will update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153817

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

[Lldb-commits] [PATCH] D153817: [lldb] Increase the maximum number of classes we can read from shared cache

[Jason Molenda via Phabricator via lldb-commits]

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

LGTM.  You might want to mention why it isn't sustainable -- because we must 
allocate memory in the inferior process, and some inferior processes have quite 
small memory limits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153817

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

[Lldb-commits] [PATCH] D153817: [lldb] Increase the maximum number of classes we can read from shared cache

[Alex Langford via Phabricator via lldb-commits]

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

The shared cache has grown past the previously hardcoded limit. As a
temporary measure, I am increasing the hardcoded number of classes we
can expect to read from the shared cache. This is not a good long-term
solution.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153817

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp


Index: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -2308,7 +2308,9 @@
 
   // The number of entries to pre-allocate room for.
   // Each entry is (addrsize + 4) bytes
-  const uint32_t max_num_classes = 163840;
+  // FIXME: It is not sustainable to continue incrementing this value every 
time
+  // the shared cache grows.
+  const uint32_t max_num_classes = 20;
 
   UtilityFunction *get_class_info_code = GetClassInfoUtilityFunction(exe_ctx);
   if (!get_class_info_code) {


Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -2308,7 +2308,9 @@
 
   // The number of entries to pre-allocate room for.
   // Each entry is (addrsize + 4) bytes
-  const uint32_t max_num_classes = 163840;
+  // FIXME: It is not sustainable to continue incrementing this value every time
+  // the shared cache grows.
+  const uint32_t max_num_classes = 20;
 
   UtilityFunction *get_class_info_code = GetClassInfoUtilityFunction(exe_ctx);
   if (!get_class_info_code) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D153810: [lldb][NFCI] TypeSystemClang::CreateStructForIdentifier should take a StringRef

[Alex Langford via Phabricator via lldb-commits]

bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, Michael137, augusto2112, kastiglione, 
fdeazeve.
Herald added a subscriber: emaste.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This doesn't really use fast comparison or string uniqueness. In fact,
all of the current callers pass an empty string for type_name. The only
reason I don't remove it is because it looks like it is used downstream
for swift.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153810

Files:
  lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -280,13 +280,13 @@
   }
 
   CompilerType CreateStructForIdentifier(
-  ConstString type_name,
+  llvm::StringRef type_name,
   const std::initializer_list>
   &type_fields,
   bool packed = false);
 
   CompilerType GetOrCreateStructForIdentifier(
-  ConstString type_name,
+  llvm::StringRef type_name,
   const std::initializer_list>
   &type_fields,
   bool packed = false);
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2308,12 +2308,12 @@
 }
 
 CompilerType TypeSystemClang::CreateStructForIdentifier(
-ConstString type_name,
+llvm::StringRef type_name,
 const std::initializer_list>
 &type_fields,
 bool packed) {
   CompilerType type;
-  if (!type_name.IsEmpty() &&
+  if (!type_name.empty() &&
   (type = GetTypeForIdentifier(type_name))
   .IsValid()) {
 lldbassert(0 && "Trying to create a type for an existing name");
@@ -2321,8 +2321,7 @@
   }
 
   type = CreateRecordType(nullptr, OptionalClangModuleID(), lldb::eAccessPublic,
-  type_name.GetCString(), clang::TTK_Struct,
-  lldb::eLanguageTypeC);
+  type_name, clang::TTK_Struct, lldb::eLanguageTypeC);
   StartTagDeclarationDefinition(type);
   for (const auto &field : type_fields)
 AddFieldToRecordType(type, field.first, field.second, lldb::eAccessPublic,
@@ -2334,7 +2333,7 @@
 }
 
 CompilerType TypeSystemClang::GetOrCreateStructForIdentifier(
-ConstString type_name,
+llvm::StringRef type_name,
 const std::initializer_list>
 &type_fields,
 bool packed) {
Index: lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
===
--- lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
+++ lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
@@ -277,7 +277,7 @@
 
   ast->AddFieldToRecordType(
   union_type, "_rt",
-  ast->CreateStructForIdentifier(ConstString(),
+  ast->CreateStructForIdentifier(llvm::StringRef(),
  {
  {"_pid", pid_type},
  {"_uid", uid_type},
@@ -287,7 +287,7 @@
 
   ast->AddFieldToRecordType(
   union_type, "_child",
-  ast->CreateStructForIdentifier(ConstString(),
+  ast->CreateStructForIdentifier(llvm::StringRef(),
  {
  {"_pid", pid_type},
  {"_uid", uid_type},
@@ -299,7 +299,7 @@
 
   ast->AddFieldToRecordType(
   union_type, "_fault",
-  ast->CreateStructForIdentifier(ConstString(),
+  ast->CreateStructForIdentifier(llvm::StringRef(),
  {
  {"_addr", voidp_type},
  {"_trap", int_type},
@@ -310,7 +310,7 @@
 
   ast->AddFieldToRecordType(
   union_type, "_poll",
-  ast->CreateStructForIdentifier(ConstString(),
+  ast->CreateStructForIdentifier(llvm::StringRef(),
  {
  {"_band", long_type},
  {"_fd", int_type},
@@ -319,7 +319,7 @@
 
   ast->AddFieldToRecordType(union_type, "_syscall",
 ast->CreateStructForIdentifier(
-ConstString(),
+

[Lldb-commits] [PATCH] D153802: [lldb] Delete RewriteObjCClassReferences (NFC)

[Dave Lee via Phabricator via lldb-commits]

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

The `RewriteObjCClassReferences` pass was applicable only to the code generated 
for the
fragile ObjC ABI (v1). That ABI is no longer active (last used for i386 macOS), 
which
means this pass has no effect.

Sources: `OBJC_CLASS_REFERENCES_` is emitted only by `CGObjCMac`, and not by
`CGObjCNonFragileABIMac`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153802

Files:
  lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
  lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h

Index: lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
===
--- lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
+++ lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
@@ -265,15 +265,6 @@
   /// True on success; false otherwise
   bool RewriteObjCClassReference(llvm::Instruction *class_load);
 
-  /// The top-level pass implementation
-  ///
-  /// \param[in] basic_block
-  /// The basic block currently being processed.
-  ///
-  /// \return
-  /// True on success; false otherwise
-  bool RewriteObjCClassReferences(llvm::BasicBlock &basic_block);
-
   /// A basic block-level pass to find all newly-declared persistent
   /// variables and register them with the ClangExprDeclMap.  This allows them
   /// to be materialized and dematerialized like normal external variables.
@@ -425,9 +416,6 @@
   /// The address of the function sel_registerName, cast to the appropriate
   /// function pointer type.
   llvm::FunctionCallee m_sel_registerName;
-  /// The address of the function objc_getClass, cast to the appropriate
-  /// function pointer type.
-  llvm::FunctionCallee m_objc_getClass;
   /// The type of an integer large enough to hold a pointer.
   llvm::IntegerType *m_intptr_ty = nullptr;
   /// The stream on which errors should be printed.
Index: lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -859,132 +859,6 @@
   return true;
 }
 
-static bool IsObjCClassReference(Value *value) {
-  GlobalVariable *global_variable = dyn_cast(value);
-
-  return !(!global_variable || !global_variable->hasName() ||
-   !global_variable->getName().startswith("OBJC_CLASS_REFERENCES_"));
-}
-
-// This function does not report errors; its callers are responsible.
-bool IRForTarget::RewriteObjCClassReference(Instruction *class_load) {
-  lldb_private::Log *log(GetLog(LLDBLog::Expressions));
-
-  LoadInst *load = dyn_cast(class_load);
-
-  if (!load)
-return false;
-
-  // Unpack the class name from the reference.  In LLVM IR, a reference to an
-  // Objective-C class gets represented as
-  //
-  // %tmp = load ptr, ptr @OBJC_CLASS_REFERENCES_, align 4
-  //
-  // @OBJC_CLASS_REFERENCES_ is a reference to a character array called
-  // @OBJC_CLASS_NAME_. @OBJC_CLASS_NAME contains the string.
-
-  // Find the pointer's initializer and get the string from its target
-
-  GlobalVariable *_objc_class_references_ =
-  dyn_cast(load->getPointerOperand());
-
-  if (!_objc_class_references_ ||
-  !_objc_class_references_->hasInitializer())
-return false;
-
-  // Find the string's initializer (a ConstantArray) and get the string from it
-
-  GlobalVariable *_objc_class_name_ =
-  dyn_cast(_objc_class_references_->getInitializer());
-
-  if (!_objc_class_name_ || !_objc_class_name_->hasInitializer())
-return false;
-
-  Constant *ocn_initializer = _objc_class_name_->getInitializer();
-
-  ConstantDataArray *ocn_initializer_array =
-  dyn_cast(ocn_initializer);
-
-  if (!ocn_initializer_array->isString())
-return false;
-
-  std::string ocn_initializer_string =
-  std::string(ocn_initializer_array->getAsString());
-
-  LLDB_LOG(log, "Found Objective-C class reference \"{0}\"",
-   ocn_initializer_string);
-
-  // Construct a call to objc_getClass
-
-  if (!m_objc_getClass) {
-lldb::addr_t objc_getClass_addr;
-
-bool missing_weak = false;
-static lldb_private::ConstString g_objc_getClass_str("objc_getClass");
-objc_getClass_addr = m_execution_unit.FindSymbol(g_objc_getClass_str,
- missing_weak);
-if (objc_getClass_addr == LLDB_INVALID_ADDRESS || missing_weak)
-  return false;
-
-LLDB_LOG(log, "Found objc_getClass at {0}", objc_getClass_addr);
-
-// Build the function type: %struct._objc_class *objc_getClass(i8*)
-
-Type *class_type = load->getType();
-Type *type_array[1];
-type_array[0] = llvm::Type::getInt8PtrTy(m_module->getContext());
-
-ArrayRef ogC

[Lldb-commits] [PATCH] D153726: [lldb] Avoid FileSystem::Resolve for cached files in the SourceManager

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 534667.
JDevlieghere marked an inline comment as done.
JDevlieghere added a comment.

Remove unused `Resolver`.


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

https://reviews.llvm.org/D153726

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/include/lldb/Host/FileSystem.h
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/common/FileSystem.cpp
  lldb/unittests/Core/SourceManagerTest.cpp

Index: lldb/unittests/Core/SourceManagerTest.cpp
===
--- lldb/unittests/Core/SourceManagerTest.cpp
+++ lldb/unittests/Core/SourceManagerTest.cpp
@@ -10,12 +10,17 @@
 #include "lldb/Host/FileSystem.h"
 #include "gtest/gtest.h"
 
+#include "TestingSupport/MockTildeExpressionResolver.h"
+
 using namespace lldb;
 using namespace lldb_private;
 
 class SourceFileCache : public ::testing::Test {
 public:
-  void SetUp() override { FileSystem::Initialize(); }
+  void SetUp() override {
+FileSystem::Initialize(std::unique_ptr(
+new MockTildeExpressionResolver("Jonas", "/jonas")));
+  }
   void TearDown() override { FileSystem::Terminate(); }
 };
 
@@ -26,7 +31,7 @@
   FileSpec foo_file_spec("foo");
   auto foo_file_sp =
   std::make_shared(foo_file_spec, nullptr);
-  cache.AddSourceFile(foo_file_sp);
+  cache.AddSourceFile(foo_file_spec, foo_file_sp);
 
   // Query: foo, expect found.
   FileSpec another_foo_file_spec("foo");
@@ -40,9 +45,32 @@
   FileSpec foo_file_spec("foo");
   auto foo_file_sp =
   std::make_shared(foo_file_spec, nullptr);
-  cache.AddSourceFile(foo_file_sp);
+  cache.AddSourceFile(foo_file_spec, foo_file_sp);
 
   // Query: bar, expect not found.
   FileSpec bar_file_spec("bar");
   ASSERT_EQ(cache.FindSourceFile(bar_file_spec), nullptr);
 }
+
+TEST_F(SourceFileCache, FindSourceFileByUnresolvedPath) {
+  SourceManager::SourceFileCache cache;
+
+  FileSpec foo_file_spec("~/foo");
+
+  // Mimic the resolution in SourceManager::GetFile.
+  FileSpec resolved_foo_file_spec = foo_file_spec;
+  FileSystem::Instance().Resolve(resolved_foo_file_spec);
+
+  // Create the file with the resolved file spec.
+  auto foo_file_sp =
+  std::make_shared(resolved_foo_file_spec, nullptr);
+
+  // Cache the result with the unresolved file spec.
+  cache.AddSourceFile(foo_file_spec, foo_file_sp);
+
+  // Query the unresolved path.
+  EXPECT_EQ(cache.FindSourceFile(FileSpec("~/foo")), foo_file_sp);
+
+  // Query the resolved path.
+  EXPECT_EQ(cache.FindSourceFile(FileSpec("/jonas/foo")), foo_file_sp);
+}
Index: lldb/source/Host/common/FileSystem.cpp
===
--- lldb/source/Host/common/FileSystem.cpp
+++ lldb/source/Host/common/FileSystem.cpp
@@ -9,8 +9,6 @@
 #include "lldb/Host/FileSystem.h"
 
 #include "lldb/Utility/DataBufferLLVM.h"
-#include "lldb/Utility/LLDBAssert.h"
-#include "lldb/Utility/TildeExpressionResolver.h"
 
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Errno.h"
@@ -46,16 +44,6 @@
 
 FileSystem &FileSystem::Instance() { return *InstanceImpl(); }
 
-void FileSystem::Initialize() {
-  lldbassert(!InstanceImpl() && "Already initialized.");
-  InstanceImpl().emplace();
-}
-
-void FileSystem::Initialize(IntrusiveRefCntPtr fs) {
-  lldbassert(!InstanceImpl() && "Already initialized.");
-  InstanceImpl().emplace(fs);
-}
-
 void FileSystem::Terminate() {
   lldbassert(InstanceImpl() && "Already terminated.");
   InstanceImpl().reset();
@@ -239,9 +227,9 @@
 
   // Resolve tilde in path.
   SmallString<128> resolved(path.begin(), path.end());
-  StandardTildeExpressionResolver Resolver;
-  Resolver.ResolveFullPath(llvm::StringRef(path.begin(), path.size()),
-   resolved);
+  assert(m_tilde_resolver && "must initialize tilde resolver in constructor");
+  m_tilde_resolver->ResolveFullPath(llvm::StringRef(path.begin(), path.size()),
+resolved);
 
   // Try making the path absolute if it exists.
   SmallString<128> absolute(resolved.begin(), resolved.end());
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -75,13 +75,10 @@
   if (!file_spec)
 return nullptr;
 
-  FileSpec resolved_fspec = file_spec;
-  resolve_tilde(resolved_fspec);
-
   DebuggerSP debugger_sp(m_debugger_wp.lock());
   FileSP file_sp;
   if (debugger_sp && debugger_sp->GetUseSourceCache())
-file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(resolved_fspec);
+file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(file_spec);
 
   TargetSP target_sp(m_target_wp.lock());
 
@@ -99,12 +96,12 @@
   // If file_sp is no good or it points to a non-existent file, reset it.
   if (!file_sp || !FileSystem::Instance().Exists(file_sp->GetFileSpec())) {
 if (target_sp)
-  file_sp = std::make_shared(resolved_fspec, tar

[Lldb-commits] [PATCH] D153726: [lldb] Avoid FileSystem::Resolve for cached files in the SourceManager

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/source/Host/common/FileSystem.cpp:230
   SmallString<128> resolved(path.begin(), path.end());
   StandardTildeExpressionResolver Resolver;
+  m_tilde_resolver->ResolveFullPath(llvm::StringRef(path.begin(), path.size()),

nit: Unused `Resolver`.


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

https://reviews.llvm.org/D153726

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

[Lldb-commits] [lldb] fec7c31 - [LLDB] Fix 582582fb474b8cd4103e65c3e5a705b3aff61794

[walter erquinigo via lldb-commits]

Author: walter erquinigo
Date: 2023-06-26T13:16:57-05:00
New Revision: fec7c313ab97d67f1335a4b0a8d12e63d517d0a4

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

LOG: [LLDB] Fix 582582fb474b8cd4103e65c3e5a705b3aff61794

This issue has been seen in

- https://lab.llvm.org/buildbot/#/builders/17/builds/39525
- https://lab.llvm.org/buildbot/#/builders/68/builds/55140

The reason is that a new language tag has been added for Mojo, but other recent 
languages need to be added to the language array so that a name lookup array 
doesn't have gaps.

`ninja check-lldb-shell-process` now passes.

Added: 


Modified: 
lldb/include/lldb/lldb-enumerations.h
lldb/source/Target/Language.cpp

Removed: 




diff  --git a/lldb/include/lldb/lldb-enumerations.h 
b/lldb/include/lldb/lldb-enumerations.h
index 3981eb0acfccf..c78608b9ff0af 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -490,6 +490,9 @@ enum LanguageType {
   eLanguageTypeFortran18 = 0x002d,
   eLanguageTypeAda2005 = 0x002e,
   eLanguageTypeAda2012 = 0x002f,
+  eLanguageTypeHIP = 0x0030,
+  eLanguageTypeAssembly = 0x0031,
+  eLanguageTypeC_sharp = 0x0032,
   eLanguageTypeMojo = 0x0033,
 
   // Vendor Extensions

diff  --git a/lldb/source/Target/Language.cpp b/lldb/source/Target/Language.cpp
index 77ba946012b00..78785352676da 100644
--- a/lldb/source/Target/Language.cpp
+++ b/lldb/source/Target/Language.cpp
@@ -209,6 +209,9 @@ struct language_name_pair language_names[] = {
 {"fortran18", eLanguageTypeFortran18},
 {"ada2005", eLanguageTypeAda2005},
 {"ada2012", eLanguageTypeAda2012},
+{"HIP", eLanguageTypeHIP},
+{"assembly", eLanguageTypeAssembly},
+{"c-sharp", eLanguageTypeC_sharp},
 {"mojo", eLanguageTypeMojo},
 // Vendor Extensions
 {"assembler", eLanguageTypeMipsAssembler},



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

[Lldb-commits] [PATCH] D153726: [lldb] Avoid FileSystem::Resolve for cached files in the SourceManager

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 534663.
Herald added a reviewer: jdoerfert.
Herald added subscribers: jplehr, sstefan1.

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

https://reviews.llvm.org/D153726

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/include/lldb/Host/FileSystem.h
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/common/FileSystem.cpp
  lldb/unittests/Core/SourceManagerTest.cpp

Index: lldb/unittests/Core/SourceManagerTest.cpp
===
--- lldb/unittests/Core/SourceManagerTest.cpp
+++ lldb/unittests/Core/SourceManagerTest.cpp
@@ -10,12 +10,17 @@
 #include "lldb/Host/FileSystem.h"
 #include "gtest/gtest.h"
 
+#include "TestingSupport/MockTildeExpressionResolver.h"
+
 using namespace lldb;
 using namespace lldb_private;
 
 class SourceFileCache : public ::testing::Test {
 public:
-  void SetUp() override { FileSystem::Initialize(); }
+  void SetUp() override {
+FileSystem::Initialize(std::unique_ptr(
+new MockTildeExpressionResolver("Jonas", "/jonas")));
+  }
   void TearDown() override { FileSystem::Terminate(); }
 };
 
@@ -26,7 +31,7 @@
   FileSpec foo_file_spec("foo");
   auto foo_file_sp =
   std::make_shared(foo_file_spec, nullptr);
-  cache.AddSourceFile(foo_file_sp);
+  cache.AddSourceFile(foo_file_spec, foo_file_sp);
 
   // Query: foo, expect found.
   FileSpec another_foo_file_spec("foo");
@@ -40,9 +45,32 @@
   FileSpec foo_file_spec("foo");
   auto foo_file_sp =
   std::make_shared(foo_file_spec, nullptr);
-  cache.AddSourceFile(foo_file_sp);
+  cache.AddSourceFile(foo_file_spec, foo_file_sp);
 
   // Query: bar, expect not found.
   FileSpec bar_file_spec("bar");
   ASSERT_EQ(cache.FindSourceFile(bar_file_spec), nullptr);
 }
+
+TEST_F(SourceFileCache, FindSourceFileByUnresolvedPath) {
+  SourceManager::SourceFileCache cache;
+
+  FileSpec foo_file_spec("~/foo");
+
+  // Mimic the resolution in SourceManager::GetFile.
+  FileSpec resolved_foo_file_spec = foo_file_spec;
+  FileSystem::Instance().Resolve(resolved_foo_file_spec);
+
+  // Create the file with the resolved file spec.
+  auto foo_file_sp =
+  std::make_shared(resolved_foo_file_spec, nullptr);
+
+  // Cache the result with the unresolved file spec.
+  cache.AddSourceFile(foo_file_spec, foo_file_sp);
+
+  // Query the unresolved path.
+  EXPECT_EQ(cache.FindSourceFile(FileSpec("~/foo")), foo_file_sp);
+
+  // Query the resolved path.
+  EXPECT_EQ(cache.FindSourceFile(FileSpec("/jonas/foo")), foo_file_sp);
+}
Index: lldb/source/Host/common/FileSystem.cpp
===
--- lldb/source/Host/common/FileSystem.cpp
+++ lldb/source/Host/common/FileSystem.cpp
@@ -9,8 +9,6 @@
 #include "lldb/Host/FileSystem.h"
 
 #include "lldb/Utility/DataBufferLLVM.h"
-#include "lldb/Utility/LLDBAssert.h"
-#include "lldb/Utility/TildeExpressionResolver.h"
 
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Errno.h"
@@ -46,16 +44,6 @@
 
 FileSystem &FileSystem::Instance() { return *InstanceImpl(); }
 
-void FileSystem::Initialize() {
-  lldbassert(!InstanceImpl() && "Already initialized.");
-  InstanceImpl().emplace();
-}
-
-void FileSystem::Initialize(IntrusiveRefCntPtr fs) {
-  lldbassert(!InstanceImpl() && "Already initialized.");
-  InstanceImpl().emplace(fs);
-}
-
 void FileSystem::Terminate() {
   lldbassert(InstanceImpl() && "Already terminated.");
   InstanceImpl().reset();
@@ -240,8 +228,8 @@
   // Resolve tilde in path.
   SmallString<128> resolved(path.begin(), path.end());
   StandardTildeExpressionResolver Resolver;
-  Resolver.ResolveFullPath(llvm::StringRef(path.begin(), path.size()),
-   resolved);
+  m_tilde_resolver->ResolveFullPath(llvm::StringRef(path.begin(), path.size()),
+resolved);
 
   // Try making the path absolute if it exists.
   SmallString<128> absolute(resolved.begin(), resolved.end());
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -75,13 +75,10 @@
   if (!file_spec)
 return nullptr;
 
-  FileSpec resolved_fspec = file_spec;
-  resolve_tilde(resolved_fspec);
-
   DebuggerSP debugger_sp(m_debugger_wp.lock());
   FileSP file_sp;
   if (debugger_sp && debugger_sp->GetUseSourceCache())
-file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(resolved_fspec);
+file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(file_spec);
 
   TargetSP target_sp(m_target_wp.lock());
 
@@ -99,12 +96,12 @@
   // If file_sp is no good or it points to a non-existent file, reset it.
   if (!file_sp || !FileSystem::Instance().Exists(file_sp->GetFileSpec())) {
 if (target_sp)
-  file_sp = std::make_shared(resolved_fspec, target_sp.get());
+  file_sp = std::make_shared(file_spec, target_sp.get());
 else
-  file_sp = st

[Lldb-commits] [lldb] 9442e81 - [lldb][NFCI] Remove ConstString from Process::ConfigureStructuredData

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-06-26T11:09:09-07:00
New Revision: 9442e81f02a7e83132432269c9ada7c03eab7a38

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

LOG: [lldb][NFCI] Remove ConstString from Process::ConfigureStructuredData

This is a follow-up to b4827a3c0a7ef121ca376713e115b04eff0f5194.

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

Added: 


Modified: 
lldb/include/lldb/Target/Process.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
lldb/source/Target/Process.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index 0f9b0c07df9bd..cf0ddeed67e83 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -2537,7 +2537,7 @@ void PruneThreadPlans();
   /// \return
   /// Returns the result of attempting to configure the feature.
   virtual Status
-  ConfigureStructuredData(ConstString type_name,
+  ConfigureStructuredData(llvm::StringRef type_name,
   const StructuredData::ObjectSP &config_sp);
 
   /// Broadcasts the given structured data object from the given plugin.

diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 23b9cfdcce163..84d2010acefe4 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -3922,7 +3922,7 @@ StructuredData::ObjectSP 
ProcessGDBRemote::GetSharedCacheInfo() {
 }
 
 Status ProcessGDBRemote::ConfigureStructuredData(
-ConstString type_name, const StructuredData::ObjectSP &config_sp) {
+llvm::StringRef type_name, const StructuredData::ObjectSP &config_sp) {
   return m_gdb_comm.ConfigureRemoteStructuredData(type_name, config_sp);
 }
 

diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
index b04ebf20a0a39..aec3a8dfb9867 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -211,7 +211,7 @@ class ProcessGDBRemote : public Process,
  lldb::addr_t image_count) override;
 
   Status
-  ConfigureStructuredData(ConstString type_name,
+  ConfigureStructuredData(llvm::StringRef type_name,
   const StructuredData::ObjectSP &config_sp) override;
 
   StructuredData::ObjectSP GetLoadedDynamicLibrariesInfos() override;

diff  --git 
a/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp 
b/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
index 60e94b89e37e9..a5818915773c4 100644
--- a/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
+++ b/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
@@ -822,8 +822,8 @@ class EnableCommand : public CommandObjectParsed {
 // Send configuration to the feature by way of the process. Construct the
 // options we will use.
 auto config_sp = m_options_sp->BuildConfigurationData(m_enable);
-const Status error = process_sp->ConfigureStructuredData(
-ConstString(GetDarwinLogTypeName()), config_sp);
+const Status error =
+process_sp->ConfigureStructuredData(GetDarwinLogTypeName(), config_sp);
 
 // Report results.
 if (!error.Success()) {
@@ -1831,8 +1831,8 @@ void StructuredDataDarwinLog::EnableNow() {
 
   // We can run it directly.
   // Send configuration to the feature by way of the process.
-  const Status error = process_sp->ConfigureStructuredData(
-  ConstString(GetDarwinLogTypeName()), config_sp);
+  const Status error =
+  process_sp->ConfigureStructuredData(GetDarwinLogTypeName(), config_sp);
 
   // Report results.
   if (!error.Success()) {

diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 05e8a0ad9c477..4059660e94266 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -6007,7 +6007,7 @@ Status 
Process::GetMemoryRegions(lldb_private::MemoryRegionInfos ®ion_list) {
 }
 
 Status
-Process::ConfigureStructuredData(ConstString type_name,
+Process::ConfigureStructuredData(llvm::StringRef type_name,
  const StructuredData::ObjectSP &config_sp) {
   // If you get this, the Process-derived class needs to implement a method to
   // enable an already-reported asynchronous structured data feature. See



___
lldb-commits mailing list
lldb-commits@lists.llv

[Lldb-commits] [PATCH] D153675: [lldb][NFCI] Remove ConstString from Process::ConfigureStructuredData

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9442e81f02a7: [lldb][NFCI] Remove ConstString from 
Process::ConfigureStructuredData (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153675

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
  lldb/source/Target/Process.cpp


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -6007,7 +6007,7 @@
 }
 
 Status
-Process::ConfigureStructuredData(ConstString type_name,
+Process::ConfigureStructuredData(llvm::StringRef type_name,
  const StructuredData::ObjectSP &config_sp) {
   // If you get this, the Process-derived class needs to implement a method to
   // enable an already-reported asynchronous structured data feature. See
Index: lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
===
--- lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
+++ lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
@@ -822,8 +822,8 @@
 // Send configuration to the feature by way of the process. Construct the
 // options we will use.
 auto config_sp = m_options_sp->BuildConfigurationData(m_enable);
-const Status error = process_sp->ConfigureStructuredData(
-ConstString(GetDarwinLogTypeName()), config_sp);
+const Status error =
+process_sp->ConfigureStructuredData(GetDarwinLogTypeName(), config_sp);
 
 // Report results.
 if (!error.Success()) {
@@ -1831,8 +1831,8 @@
 
   // We can run it directly.
   // Send configuration to the feature by way of the process.
-  const Status error = process_sp->ConfigureStructuredData(
-  ConstString(GetDarwinLogTypeName()), config_sp);
+  const Status error =
+  process_sp->ConfigureStructuredData(GetDarwinLogTypeName(), config_sp);
 
   // Report results.
   if (!error.Success()) {
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -211,7 +211,7 @@
  lldb::addr_t image_count) override;
 
   Status
-  ConfigureStructuredData(ConstString type_name,
+  ConfigureStructuredData(llvm::StringRef type_name,
   const StructuredData::ObjectSP &config_sp) override;
 
   StructuredData::ObjectSP GetLoadedDynamicLibrariesInfos() override;
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -3922,7 +3922,7 @@
 }
 
 Status ProcessGDBRemote::ConfigureStructuredData(
-ConstString type_name, const StructuredData::ObjectSP &config_sp) {
+llvm::StringRef type_name, const StructuredData::ObjectSP &config_sp) {
   return m_gdb_comm.ConfigureRemoteStructuredData(type_name, config_sp);
 }
 
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -2537,7 +2537,7 @@
   /// \return
   /// Returns the result of attempting to configure the feature.
   virtual Status
-  ConfigureStructuredData(ConstString type_name,
+  ConfigureStructuredData(llvm::StringRef type_name,
   const StructuredData::ObjectSP &config_sp);
 
   /// Broadcasts the given structured data object from the given plugin.


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -6007,7 +6007,7 @@
 }
 
 Status
-Process::ConfigureStructuredData(ConstString type_name,
+Process::ConfigureStructuredData(llvm::StringRef type_name,
  const StructuredData::ObjectSP &config_sp) {
   // If you get this, the Process-derived class needs to implement a method to
   // enable an already-reported asynchronous structured data feature. See
Index: lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
===
--- lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
+++ lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
@@ -822,8 +822,8 @@
 // 

[Lldb-commits] [PATCH] D153657: Don't allow ValueObject::Cast from a smaller type to a larger

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

In D153657#4449608 , @jingham wrote:

> In D153657#4445292 , @bulbazord 
> wrote:
>
>> I think this patch is probably okay to do, but it does break a supported use 
>> case that I'm aware of: One way you can do "object oriented programming" in 
>> C is to do exactly what you're trying to prevent. Using the test, you could 
>> say that "MyStruct" is the base class and "MyBiggerStruct" would be a 
>> derived class. You might have a pointer to a "MyStruct" object, even though 
>> you called `malloc(sizeof(MyBiggerStruct))`, and maybe you can perform that 
>> cast if you're sure that you actually have a `MyBiggerStruct` object.
>>
>> I know there's not necessarily a good way to support that without also 
>> supporting the bug you're trying to fix though. :(
>
> You can't actually cast structures to structures in C.  You can only cast 
> pointers.  So in your underlying code you always have to have pointers around 
> if you are playing these games, and you can cast the pointer to the derived 
> type and then dereference it, which is what you would have had to do in the 
> source language anyway.

Right, this makes sense, that was my disconnect. Thanks for clarifying and 
helping me understand!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153657

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

[Lldb-commits] [PATCH] D153673: [lldb][NFCI] Remove unneeded ConstString constructions for OptionValueProperties::AppendProperty

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2014572d9a68: [lldb][NFCI] Remove unneeded ConstString 
constructions for… (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153673

Files:
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/PluginManager.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp
  lldb/unittests/Interpreter/TestOptionValue.cpp

Index: lldb/unittests/Interpreter/TestOptionValue.cpp
===
--- lldb/unittests/Interpreter/TestOptionValue.cpp
+++ lldb/unittests/Interpreter/TestOptionValue.cpp
@@ -85,11 +85,10 @@
 const bool is_global = false;
 
 auto dict_sp = std::make_shared(1 << eTypeUInt64);
-props_sp->AppendProperty(ConstString("dict"), "", is_global, dict_sp);
+props_sp->AppendProperty("dict", "", is_global, dict_sp);
 
 auto file_list_sp = std::make_shared();
-props_sp->AppendProperty(ConstString("file-list"), "", is_global,
- file_list_sp);
+props_sp->AppendProperty("file-list", "", is_global, file_list_sp);
 return props_sp;
   }
 
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -4112,7 +4112,7 @@
 m_experimental_properties_up =
 std::make_unique();
 m_collection_sp->AppendProperty(
-ConstString(Properties::GetExperimentalSettingsName()),
+Properties::GetExperimentalSettingsName(),
 "Experimental settings - setting these won't produce "
 "errors if the setting is not present.",
 true, m_experimental_properties_up->GetValueProperties());
@@ -4123,12 +4123,12 @@
 m_experimental_properties_up =
 std::make_unique();
 m_collection_sp->AppendProperty(
-ConstString(Properties::GetExperimentalSettingsName()),
+Properties::GetExperimentalSettingsName(),
 "Experimental settings - setting these won't produce "
 "errors if the setting is not present.",
 true, m_experimental_properties_up->GetValueProperties());
 m_collection_sp->AppendProperty(
-ConstString("process"), "Settings specific to processes.", true,
+"process", "Settings specific to processes.", true,
 Process::GetGlobalProperties().GetValueProperties());
 m_collection_sp->SetValueChangedCallback(
 ePropertySaveObjectsDir, [this] { CheckJITObjectsDir(); });
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -167,7 +167,7 @@
 std::make_shared(ConstString("process"));
 m_collection_sp->Initialize(g_process_properties);
 m_collection_sp->AppendProperty(
-ConstString("thread"), "Settings specific to threads.", true,
+"thread", "Settings specific to threads.", true,
 Thread::GetGlobalProperties().GetValueProperties());
   } else {
 m_collection_sp =
@@ -180,7 +180,7 @@
   m_experimental_properties_up =
   std::make_unique();
   m_collection_sp->AppendProperty(
-  ConstString(Properties::GetExperimentalSettingsName()),
+  Properties::GetExperimentalSettingsName(),
   "Experimental settings - setting these won't produce "
   "errors if the setting is not present.",
   true, m_experimental_properties_up->GetValueProperties());
Index: lldb/source/Core/PluginManager.cpp
===
--- lldb/source/Core/PluginManager.cpp
+++ lldb/source/Core/PluginManager.cpp
@@ -1438,7 +1438,7 @@
   lldb::OptionValuePropertiesSP parent_properties_sp(
   debugger.GetValueProperties());
   if (parent_properties_sp) {
-static ConstString g_property_name("plugin");
+static constexpr llvm::StringLiteral g_property_name("plugin");
 
 OptionValuePropertiesSP plugin_properties_sp =
 parent_properties_sp->GetSubProperty(nullptr, g_property_name);
@@ -1471,7 +1471,7 @@
 static lldb::OptionValuePropertiesSP GetDebuggerPropertyForPluginsOldStyle(
 Debugger &debugger, ConstString plugin_type_name,
 llvm::StringRef plugin_type_desc, bool can_create) {
-  static ConstString g_property_name("plugin");
+  static constexpr llvm::StringLiteral g_property_name("plugin");
   lldb::OptionValuePropertiesSP parent_properties_sp(
   debugger.GetValueProperties());
   if (parent_properties_sp) {
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -815,17 +815,17 @@
   // LLDB will start querying them during construction.
   m_collection_sp->Initialize(g_debugger_properties);
   m_collection_sp->AppendProperty(
-  

[Lldb-commits] [lldb] 2014572 - [lldb][NFCI] Remove unneeded ConstString constructions for OptionValueProperties::AppendProperty

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-06-26T11:06:29-07:00
New Revision: 2014572d9a6839745a920ec19ebfa73814548061

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

LOG: [lldb][NFCI] Remove unneeded ConstString constructions for 
OptionValueProperties::AppendProperty

I removed ConstString from OptionValueProperties in 643ba926c1f6, but
there are a few call sites that still create a ConstString as an
argument. I did not catch these initially because ConstString has an
implicit conversion method to StringRef.

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

Added: 


Modified: 
lldb/source/Core/Debugger.cpp
lldb/source/Core/PluginManager.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/Target.cpp
lldb/unittests/Interpreter/TestOptionValue.cpp

Removed: 




diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 535b276cd8c75..bf672fe013847 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -815,17 +815,17 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback, 
void *baton)
   // LLDB will start querying them during construction.
   m_collection_sp->Initialize(g_debugger_properties);
   m_collection_sp->AppendProperty(
-  ConstString("target"), "Settings specify to debugging targets.", true,
+  "target", "Settings specify to debugging targets.", true,
   Target::GetGlobalProperties().GetValueProperties());
   m_collection_sp->AppendProperty(
-  ConstString("platform"), "Platform settings.", true,
+  "platform", "Platform settings.", true,
   Platform::GetGlobalPlatformProperties().GetValueProperties());
   m_collection_sp->AppendProperty(
-  ConstString("symbols"), "Symbol lookup and cache settings.", true,
+  "symbols", "Symbol lookup and cache settings.", true,
   ModuleList::GetGlobalModuleListProperties().GetValueProperties());
   if (m_command_interpreter_up) {
 m_collection_sp->AppendProperty(
-ConstString("interpreter"),
+"interpreter",
 "Settings specify to the debugger's command interpreter.", true,
 m_command_interpreter_up->GetValueProperties());
   }

diff  --git a/lldb/source/Core/PluginManager.cpp 
b/lldb/source/Core/PluginManager.cpp
index 057342d24340b..bf23cefe3a7f6 100644
--- a/lldb/source/Core/PluginManager.cpp
+++ b/lldb/source/Core/PluginManager.cpp
@@ -1438,7 +1438,7 @@ GetDebuggerPropertyForPlugins(Debugger &debugger, 
ConstString plugin_type_name,
   lldb::OptionValuePropertiesSP parent_properties_sp(
   debugger.GetValueProperties());
   if (parent_properties_sp) {
-static ConstString g_property_name("plugin");
+static constexpr llvm::StringLiteral g_property_name("plugin");
 
 OptionValuePropertiesSP plugin_properties_sp =
 parent_properties_sp->GetSubProperty(nullptr, g_property_name);
@@ -1471,7 +1471,7 @@ GetDebuggerPropertyForPlugins(Debugger &debugger, 
ConstString plugin_type_name,
 static lldb::OptionValuePropertiesSP GetDebuggerPropertyForPluginsOldStyle(
 Debugger &debugger, ConstString plugin_type_name,
 llvm::StringRef plugin_type_desc, bool can_create) {
-  static ConstString g_property_name("plugin");
+  static constexpr llvm::StringLiteral g_property_name("plugin");
   lldb::OptionValuePropertiesSP parent_properties_sp(
   debugger.GetValueProperties());
   if (parent_properties_sp) {

diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 246279de086ae..05e8a0ad9c477 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -167,7 +167,7 @@ ProcessProperties::ProcessProperties(lldb_private::Process 
*process)
 std::make_shared(ConstString("process"));
 m_collection_sp->Initialize(g_process_properties);
 m_collection_sp->AppendProperty(
-ConstString("thread"), "Settings specific to threads.", true,
+"thread", "Settings specific to threads.", true,
 Thread::GetGlobalProperties().GetValueProperties());
   } else {
 m_collection_sp =
@@ -180,7 +180,7 @@ ProcessProperties::ProcessProperties(lldb_private::Process 
*process)
   m_experimental_properties_up =
   std::make_unique();
   m_collection_sp->AppendProperty(
-  ConstString(Properties::GetExperimentalSettingsName()),
+  Properties::GetExperimentalSettingsName(),
   "Experimental settings - setting these won't produce "
   "errors if the setting is not present.",
   true, m_experimental_properties_up->GetValueProperties());

diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 94317f3c0535c..1444bb6d1217e 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -4112,7 +4112,7 @@ TargetProperties::TargetProperties(Target *tar

[Lldb-commits] [PATCH] D153489: [lldb] Print hint if object description is requested but not implemented

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 added a comment.

ping :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153489

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

[Lldb-commits] [PATCH] D153685: [lldb] Add `source cache dump` and `source cache clear` subcommand

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
JDevlieghere marked 2 inline comments as done.
Closed by commit rGd49caf4afc1b: [lldb] Add `source cache dump` and `source 
cache clear` subcommand (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D153685?vs=534159&id=534641#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153685

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Commands/CommandObjectSource.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/SourceManager.cpp
  lldb/test/API/source-manager/TestSourceManager.py

Index: lldb/test/API/source-manager/TestSourceManager.py
===
--- lldb/test/API/source-manager/TestSourceManager.py
+++ lldb/test/API/source-manager/TestSourceManager.py
@@ -317,3 +317,24 @@
 "that has no source code associated " "with it.",
 ],
 )
+
+def test_source_cache_dump_and_clear(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+lldbutil.run_break_set_by_file_and_line(
+self, self.file, self.line, num_expected_locations=1, loc_exact=True
+)
+self.runCmd("run", RUN_SUCCEEDED)
+
+# Make sure the main source file is in the source cache.
+self.expect(
+"source cache dump",
+substrs=["Modification time", "Lines", "Path", " 7", self.file],
+)
+
+# Clear the cache.
+self.expect("source cache clear")
+
+# Make sure the main source file is no longer in the source cache.
+self.expect("source cache dump", matching=False, substrs=[self.file])
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -727,3 +727,15 @@
 file_sp = pos->second;
   return file_sp;
 }
+
+void SourceManager::SourceFileCache::Dump(Stream &stream) const {
+  stream << "Modification time   LinesPath\n";
+  stream << "---  \n";
+  for (auto &entry : m_file_cache) {
+if (!entry.second)
+  continue;
+FileSP file = entry.second;
+stream.Format("{0:%Y-%m-%d %H:%M:%S} {1,8:d} {2}\n", file->GetTimestamp(),
+  file->GetNumLines(), entry.first.GetPath());
+  }
+}
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -5105,8 +5105,8 @@
   CommandObjectTargetDumpTypesystem(CommandInterpreter &interpreter)
   : CommandObjectParsed(
 interpreter, "target dump typesystem",
-"Dump the state of the target's internal type system.\n"
-"Intended to be used for debugging LLDB itself.",
+"Dump the state of the target's internal type system. Intended to "
+"be used for debugging LLDB itself.",
 nullptr, eCommandRequiresTarget) {}
 
   ~CommandObjectTargetDumpTypesystem() override = default;
Index: lldb/source/Commands/CommandObjectSource.cpp
===
--- lldb/source/Commands/CommandObjectSource.cpp
+++ lldb/source/Commands/CommandObjectSource.cpp
@@ -1201,6 +1201,62 @@
   std::string m_reverse_name;
 };
 
+class CommandObjectSourceCacheDump : public CommandObjectParsed {
+public:
+  CommandObjectSourceCacheDump(CommandInterpreter &interpreter)
+  : CommandObjectParsed(interpreter, "source cache dump",
+"Dump the state of the source code cache. Intended "
+"to be used for debugging LLDB itself.",
+nullptr) {}
+
+  ~CommandObjectSourceCacheDump() override = default;
+
+protected:
+  bool DoExecute(Args &command, CommandReturnObject &result) override {
+SourceManager::SourceFileCache &cache = GetDebugger().GetSourceFileCache();
+cache.Dump(result.GetOutputStream());
+result.SetStatus(eReturnStatusSuccessFinishResult);
+return result.Succeeded();
+  }
+};
+
+class CommandObjectSourceCacheClear : public CommandObjectParsed {
+public:
+  CommandObjectSourceCacheClear(CommandInterpreter &interpreter)
+  : CommandObjectParsed(interpreter, "source cache clear",
+"Clear the source code cache.\n", nullptr) {}
+
+  ~CommandObjectSourceCacheClear() override = default;
+
+protected:
+  bool DoExecute(Args &command, CommandReturnObject &result) override {
+SourceManager::SourceFileCache &cache = GetDebugger().GetSourceFileCache();
+cache.Clear();
+r

[Lldb-commits] [lldb] d49caf4 - [lldb] Add `source cache dump` and `source cache clear` subcommand

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2023-06-26T10:40:08-07:00
New Revision: d49caf4afc1bc460600b316f9736e01ceeebded5

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

LOG: [lldb] Add `source cache dump` and `source cache clear` subcommand

Add two new source subcommands: source cache dump and source cache
clear. As the name implies the first one dumps the source cache while
the later clears the cache.

This patch was motivated by a handful of (internal) bug reports related
to sources not being available. Right now those issues can be hard to
diagnose. The new commands give users, as well as us as developers, more
insight into and control over the source cache.

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

Added: 


Modified: 
lldb/include/lldb/Core/Debugger.h
lldb/include/lldb/Core/SourceManager.h
lldb/source/Commands/CommandObjectSource.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Core/SourceManager.cpp
lldb/test/API/source-manager/TestSourceManager.py

Removed: 




diff  --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index 4e3c177d33e59..3996c4c58e2ba 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -76,8 +76,6 @@ class DataRecorder;
 class Debugger : public std::enable_shared_from_this,
  public UserID,
  public Properties {
-  friend class SourceManager; // For GetSourceFileCache.
-
 public:
   /// Broadcaster event bits definitions.
   enum {
@@ -515,6 +513,10 @@ class Debugger : public 
std::enable_shared_from_this,
   void FlushProcessOutput(Process &process, bool flush_stdout,
   bool flush_stderr);
 
+  SourceManager::SourceFileCache &GetSourceFileCache() {
+return m_source_file_cache;
+  }
+
 protected:
   friend class CommandInterpreter;
   friend class REPL;
@@ -599,10 +601,6 @@ class Debugger : public 
std::enable_shared_from_this,
   // Ensures two threads don't attempt to flush process output in parallel.
   std::mutex m_output_flush_mutex;
 
-  SourceManager::SourceFileCache &GetSourceFileCache() {
-return m_source_file_cache;
-  }
-
   void InstanceInitialize();
 
   // these should never be NULL

diff  --git a/lldb/include/lldb/Core/SourceManager.h 
b/lldb/include/lldb/Core/SourceManager.h
index c272f7f431129..2ad6294e0f6cc 100644
--- a/lldb/include/lldb/Core/SourceManager.h
+++ b/lldb/include/lldb/Core/SourceManager.h
@@ -65,6 +65,8 @@ class SourceManager {
 
 uint32_t GetNumLines();
 
+llvm::sys::TimePoint<> GetTimestamp() const { return m_mod_time; }
+
   protected:
 bool CalculateLineOffsets(uint32_t line = UINT32_MAX);
 
@@ -105,6 +107,8 @@ class SourceManager {
 // Removes all elements from the cache.
 void Clear() { m_file_cache.clear(); }
 
+void Dump(Stream &stream) const;
+
   protected:
 typedef std::map FileCache;
 FileCache m_file_cache;

diff  --git a/lldb/source/Commands/CommandObjectSource.cpp 
b/lldb/source/Commands/CommandObjectSource.cpp
index bbc3142c51be6..5fdf15767def4 100644
--- a/lldb/source/Commands/CommandObjectSource.cpp
+++ b/lldb/source/Commands/CommandObjectSource.cpp
@@ -1201,6 +1201,62 @@ class CommandObjectSourceList : public 
CommandObjectParsed {
   std::string m_reverse_name;
 };
 
+class CommandObjectSourceCacheDump : public CommandObjectParsed {
+public:
+  CommandObjectSourceCacheDump(CommandInterpreter &interpreter)
+  : CommandObjectParsed(interpreter, "source cache dump",
+"Dump the state of the source code cache. Intended 
"
+"to be used for debugging LLDB itself.",
+nullptr) {}
+
+  ~CommandObjectSourceCacheDump() override = default;
+
+protected:
+  bool DoExecute(Args &command, CommandReturnObject &result) override {
+SourceManager::SourceFileCache &cache = GetDebugger().GetSourceFileCache();
+cache.Dump(result.GetOutputStream());
+result.SetStatus(eReturnStatusSuccessFinishResult);
+return result.Succeeded();
+  }
+};
+
+class CommandObjectSourceCacheClear : public CommandObjectParsed {
+public:
+  CommandObjectSourceCacheClear(CommandInterpreter &interpreter)
+  : CommandObjectParsed(interpreter, "source cache clear",
+"Clear the source code cache.\n", nullptr) {}
+
+  ~CommandObjectSourceCacheClear() override = default;
+
+protected:
+  bool DoExecute(Args &command, CommandReturnObject &result) override {
+SourceManager::SourceFileCache &cache = GetDebugger().GetSourceFileCache();
+cache.Clear();
+result.SetStatus(eReturnStatusSuccessFinishNoResult);
+return result.Succeeded();
+  }
+};
+
+class CommandObjectSourceCache : public CommandObjectMultiword {

[Lldb-commits] [PATCH] D153711: [lldb][NFCI] Timer::DumpCategoryTimes should take a reference instead of a pointer

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5f69e6682bf0: [lldb][NFCI] Timer::DumpCategoryTimes should 
take a reference instead of a… (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153711

Files:
  lldb/include/lldb/Utility/Timer.h
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Utility/Timer.cpp
  lldb/unittests/Utility/TimerTest.cpp

Index: lldb/unittests/Utility/TimerTest.cpp
===
--- lldb/unittests/Utility/TimerTest.cpp
+++ lldb/unittests/Utility/TimerTest.cpp
@@ -21,7 +21,7 @@
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;
-  Timer::DumpCategoryTimes(&ss);
+  Timer::DumpCategoryTimes(ss);
   double seconds;
   ASSERT_EQ(1, sscanf(ss.GetData(), "%lf sec for CAT1", &seconds));
   EXPECT_LT(0.001, seconds);
@@ -39,7 +39,7 @@
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;
-  Timer::DumpCategoryTimes(&ss);
+  Timer::DumpCategoryTimes(ss);
   double seconds;
   // It should only appear once.
   ASSERT_EQ(ss.GetString().count("CAT1"), 1U);
@@ -59,7 +59,7 @@
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;
-  Timer::DumpCategoryTimes(&ss);
+  Timer::DumpCategoryTimes(ss);
   double seconds1, seconds2;
   ASSERT_EQ(2, sscanf(ss.GetData(),
   "%lf sec (total: %*fs; child: %*fs; count: %*d) for "
@@ -92,7 +92,7 @@
   // 0.105202764 sec (total: 0.132s; child: 0.027s; count: 1) for CAT1
   // 0.026772798 sec (total: 0.027s; child: 0.000s; count: 2) for CAT2
   StreamString ss;
-  Timer::DumpCategoryTimes(&ss);
+  Timer::DumpCategoryTimes(ss);
   double seconds1, total1, child1, seconds2;
   int count1, count2;
   ASSERT_EQ(
Index: lldb/source/Utility/Timer.cpp
===
--- lldb/source/Utility/Timer.cpp
+++ lldb/source/Utility/Timer.cpp
@@ -135,7 +135,7 @@
   }
 }
 
-void Timer::DumpCategoryTimes(Stream *s) {
+void Timer::DumpCategoryTimes(Stream &s) {
   std::vector sorted;
   for (Category *i = g_categories; i; i = i->m_next) {
 uint64_t nanos = i->m_nanos.load(std::memory_order_acquire);
@@ -153,9 +153,9 @@
   llvm::sort(sorted, CategoryMapIteratorSortCriterion);
 
   for (const auto &stats : sorted)
-s->Printf("%.9f sec (total: %.3fs; child: %.3fs; count: %" PRIu64
-  ") for %s\n",
-  stats.nanos / 10., stats.nanos_total / 10.,
-  (stats.nanos_total - stats.nanos) / 10., stats.count,
-  stats.name);
+s.Printf("%.9f sec (total: %.3fs; child: %.3fs; count: %" PRIu64
+ ") for %s\n",
+ stats.nanos / 10., stats.nanos_total / 10.,
+ (stats.nanos_total - stats.nanos) / 10., stats.count,
+ stats.name);
 }
Index: lldb/source/Commands/CommandObjectLog.cpp
===
--- lldb/source/Commands/CommandObjectLog.cpp
+++ lldb/source/Commands/CommandObjectLog.cpp
@@ -504,7 +504,7 @@
 
 protected:
   bool DoExecute(Args &args, CommandReturnObject &result) override {
-Timer::DumpCategoryTimes(&result.GetOutputStream());
+Timer::DumpCategoryTimes(result.GetOutputStream());
 Timer::SetDisplayDepth(0);
 result.SetStatus(eReturnStatusSuccessFinishResult);
 
@@ -527,7 +527,7 @@
 
 protected:
   bool DoExecute(Args &args, CommandReturnObject &result) override {
-Timer::DumpCategoryTimes(&result.GetOutputStream());
+Timer::DumpCategoryTimes(result.GetOutputStream());
 result.SetStatus(eReturnStatusSuccessFinishResult);
 
 if (!result.Succeeded()) {
Index: lldb/include/lldb/Utility/Timer.h
===
--- lldb/include/lldb/Utility/Timer.h
+++ lldb/include/lldb/Utility/Timer.h
@@ -56,7 +56,7 @@
 
   static void SetQuiet(bool value);
 
-  static void DumpCategoryTimes(Stream *s);
+  static void DumpCategoryTimes(Stream &s);
 
   static void ResetCategoryTimes();
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 5f69e66 - [lldb][NFCI] Timer::DumpCategoryTimes should take a reference instead of a pointer

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-06-26T10:36:41-07:00
New Revision: 5f69e6682bf0806c13098f79cb5f1ee30f0f8c06

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

LOG: [lldb][NFCI] Timer::DumpCategoryTimes should take a reference instead of a 
pointer

We are assuming that the pointer is always valid, might as well take a
reference instead.

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

Added: 


Modified: 
lldb/include/lldb/Utility/Timer.h
lldb/source/Commands/CommandObjectLog.cpp
lldb/source/Utility/Timer.cpp
lldb/unittests/Utility/TimerTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Timer.h 
b/lldb/include/lldb/Utility/Timer.h
index 201378bbeb2cc..3ce93c25c66f3 100644
--- a/lldb/include/lldb/Utility/Timer.h
+++ b/lldb/include/lldb/Utility/Timer.h
@@ -56,7 +56,7 @@ class Timer {
 
   static void SetQuiet(bool value);
 
-  static void DumpCategoryTimes(Stream *s);
+  static void DumpCategoryTimes(Stream &s);
 
   static void ResetCategoryTimes();
 

diff  --git a/lldb/source/Commands/CommandObjectLog.cpp 
b/lldb/source/Commands/CommandObjectLog.cpp
index 8549d22e70f98..5dd6f89898372 100644
--- a/lldb/source/Commands/CommandObjectLog.cpp
+++ b/lldb/source/Commands/CommandObjectLog.cpp
@@ -504,7 +504,7 @@ class CommandObjectLogTimerDisable : public 
CommandObjectParsed {
 
 protected:
   bool DoExecute(Args &args, CommandReturnObject &result) override {
-Timer::DumpCategoryTimes(&result.GetOutputStream());
+Timer::DumpCategoryTimes(result.GetOutputStream());
 Timer::SetDisplayDepth(0);
 result.SetStatus(eReturnStatusSuccessFinishResult);
 
@@ -527,7 +527,7 @@ class CommandObjectLogTimerDump : public 
CommandObjectParsed {
 
 protected:
   bool DoExecute(Args &args, CommandReturnObject &result) override {
-Timer::DumpCategoryTimes(&result.GetOutputStream());
+Timer::DumpCategoryTimes(result.GetOutputStream());
 result.SetStatus(eReturnStatusSuccessFinishResult);
 
 if (!result.Succeeded()) {

diff  --git a/lldb/source/Utility/Timer.cpp b/lldb/source/Utility/Timer.cpp
index 477541d7bb3d1..a059f877e0236 100644
--- a/lldb/source/Utility/Timer.cpp
+++ b/lldb/source/Utility/Timer.cpp
@@ -135,7 +135,7 @@ void Timer::ResetCategoryTimes() {
   }
 }
 
-void Timer::DumpCategoryTimes(Stream *s) {
+void Timer::DumpCategoryTimes(Stream &s) {
   std::vector sorted;
   for (Category *i = g_categories; i; i = i->m_next) {
 uint64_t nanos = i->m_nanos.load(std::memory_order_acquire);
@@ -153,9 +153,9 @@ void Timer::DumpCategoryTimes(Stream *s) {
   llvm::sort(sorted, CategoryMapIteratorSortCriterion);
 
   for (const auto &stats : sorted)
-s->Printf("%.9f sec (total: %.3fs; child: %.3fs; count: %" PRIu64
-  ") for %s\n",
-  stats.nanos / 10., stats.nanos_total / 10.,
-  (stats.nanos_total - stats.nanos) / 10., stats.count,
-  stats.name);
+s.Printf("%.9f sec (total: %.3fs; child: %.3fs; count: %" PRIu64
+ ") for %s\n",
+ stats.nanos / 10., stats.nanos_total / 10.,
+ (stats.nanos_total - stats.nanos) / 10., stats.count,
+ stats.name);
 }

diff  --git a/lldb/unittests/Utility/TimerTest.cpp 
b/lldb/unittests/Utility/TimerTest.cpp
index c6d1facdebbbc..b371ebff4fe77 100644
--- a/lldb/unittests/Utility/TimerTest.cpp
+++ b/lldb/unittests/Utility/TimerTest.cpp
@@ -21,7 +21,7 @@ TEST(TimerTest, CategoryTimes) {
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;
-  Timer::DumpCategoryTimes(&ss);
+  Timer::DumpCategoryTimes(ss);
   double seconds;
   ASSERT_EQ(1, sscanf(ss.GetData(), "%lf sec for CAT1", &seconds));
   EXPECT_LT(0.001, seconds);
@@ -39,7 +39,7 @@ TEST(TimerTest, CategoryTimesNested) {
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;
-  Timer::DumpCategoryTimes(&ss);
+  Timer::DumpCategoryTimes(ss);
   double seconds;
   // It should only appear once.
   ASSERT_EQ(ss.GetString().count("CAT1"), 1U);
@@ -59,7 +59,7 @@ TEST(TimerTest, CategoryTimes2) {
 std::this_thread::sleep_for(std::chrono::milliseconds(10));
   }
   StreamString ss;
-  Timer::DumpCategoryTimes(&ss);
+  Timer::DumpCategoryTimes(ss);
   double seconds1, seconds2;
   ASSERT_EQ(2, sscanf(ss.GetData(),
   "%lf sec (total: %*fs; child: %*fs; count: %*d) for "
@@ -92,7 +92,7 @@ TEST(TimerTest, CategoryTimesStats) {
   // 0.105202764 sec (total: 0.132s; child: 0.027s; count: 1) for CAT1
   // 0.026772798 sec (total: 0.027s; child: 0.000s; count: 2) for CAT2
   StreamString ss;
-  Timer::DumpCategoryTimes(&ss);
+  Timer::DumpCategoryTimes(ss);
   double seconds1, total1, child1, seconds2;
   int count1, count2;
 

[Lldb-commits] [PATCH] D153657: Don't allow ValueObject::Cast from a smaller type to a larger

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D153657#4445292 , @bulbazord wrote:

> I think this patch is probably okay to do, but it does break a supported use 
> case that I'm aware of: One way you can do "object oriented programming" in C 
> is to do exactly what you're trying to prevent. Using the test, you could say 
> that "MyStruct" is the base class and "MyBiggerStruct" would be a derived 
> class. You might have a pointer to a "MyStruct" object, even though you 
> called `malloc(sizeof(MyBiggerStruct))`, and maybe you can perform that cast 
> if you're sure that you actually have a `MyBiggerStruct` object.
>
> I know there's not necessarily a good way to support that without also 
> supporting the bug you're trying to fix though. :(

You can't actually cast structures to structures in C.  You can only cast 
pointers.  So in your underlying code you always have to have pointers around 
if you are playing these games, and you can cast the pointer to the derived 
type and then dereference it, which is what you would have had to do in the 
source language anyway.




Comment at: lldb/include/lldb/Core/ValueObject.h:617-619
+  lldb::ValueObjectSP Cast(const CompilerType &compiler_type);
+
+  virtual lldb::ValueObjectSP DoCast(const CompilerType &compiler_type);

bulbazord wrote:
> I'm a little concerned about the API surface here. Maybe this is just my 
> misunderstandings about ValueObject, but...
> 
> Given a `ValueObjectSP` (which may contain a `ValueObject`, 
> `ValueObjectConstResult`, etc), which one of these are you supposed to call? 
> If you call `Cast`, you'll get what you want. If you call `DoCast`, you might 
> circumvent the safety checks that `Cast` is providing... but if you have 
> something like a `ValueObjectConstResult`, this actually calls 
> `ValueObject::Cast` which does the right thing. Am I understanding this 
> correctly?
> 
> I also have a little confusion about which one I would call based on the 
> names... Maybe it would make more sense to call `DoCast` something like 
> `CastImpl`? 
We use this naming convention pretty consistently in lldb when there's an 
overridden method that we need to do some generic preparation for.  We use 
`Action` for the public call, and `DoAction` as the overridden one.  I could 
put in some words in the header, but this is a pretty well-established 
convention: in lldb, you should never call the DoAction version unless you are 
the Action wrapper.



Comment at: lldb/source/Core/ValueObject.cpp:2792
+  CompilerType my_type = GetCompilerType();
+  bool unused;
+

bulbazord wrote:
> What is the purpose of this `bool unused`?
That's leftover from a previous overly-complex solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153657

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

[Lldb-commits] [PATCH] D153710: [lldb][NFCI] UUID::Dump should take a reference instead of a pointer

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG718e0cd6e724: [lldb][NFCI] UUID::Dump should take a 
reference instead of a pointer (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153710

Files:
  lldb/include/lldb/Core/ModuleSpec.h
  lldb/include/lldb/Utility/UUID.h
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Interpreter/OptionValueUUID.cpp
  lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Utility/UUID.cpp

Index: lldb/source/Utility/UUID.cpp
===
--- lldb/source/Utility/UUID.cpp
+++ lldb/source/Utility/UUID.cpp
@@ -61,7 +61,7 @@
   return result;
 }
 
-void UUID::Dump(Stream *s) const { s->PutCString(GetAsString()); }
+void UUID::Dump(Stream &s) const { s.PutCString(GetAsString()); }
 
 static inline int xdigit_to_int(char ch) {
   ch = tolower(ch);
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2271,7 +2271,7 @@
   message << " (uuid ";
 
   if (dump_uuid.IsValid())
-dump_uuid.Dump(&message);
+dump_uuid.Dump(message);
   else
 message << "not specified";
 
Index: lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
===
--- lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
+++ lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
@@ -54,11 +54,11 @@
 if (feedback_strm) {
   feedback_strm->PutCString(
   "warning: UUID mismatch detected between modules:\n");
-  module->GetUUID().Dump(feedback_strm);
+  module->GetUUID().Dump(*feedback_strm);
   feedback_strm->PutChar(' ');
   module->GetFileSpec().Dump(feedback_strm->AsRawOstream());
   feedback_strm->PutCString("\n");
-  dsym_uuid.Dump(feedback_strm);
+  dsym_uuid.Dump(*feedback_strm);
   feedback_strm->PutChar(' ');
   ofile->GetFileSpec().Dump(feedback_strm->AsRawOstream());
   feedback_strm->EOL();
Index: lldb/source/Interpreter/OptionValueUUID.cpp
===
--- lldb/source/Interpreter/OptionValueUUID.cpp
+++ lldb/source/Interpreter/OptionValueUUID.cpp
@@ -23,7 +23,7 @@
   if (dump_mask & eDumpOptionValue) {
 if (dump_mask & eDumpOptionType)
   strm.PutCString(" = ");
-m_uuid.Dump(&strm);
+m_uuid.Dump(strm);
   }
 }
 
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -1318,7 +1318,7 @@
 
 static void DumpModuleUUID(Stream &strm, Module *module) {
   if (module && module->GetUUID().IsValid())
-module->GetUUID().Dump(&strm);
+module->GetUUID().Dump(strm);
   else
 strm.PutCString("");
 }
@@ -2560,7 +2560,7 @@
 return true;
   } else {
 StreamString strm;
-module_spec.GetUUID().Dump(&strm);
+module_spec.GetUUID().Dump(strm);
 if (module_spec.GetFileSpec()) {
   if (module_spec.GetSymbolFileSpec()) {
 result.AppendErrorWithFormat(
@@ -2584,7 +2584,7 @@
   }
 } else {
   StreamString strm;
-  module_spec.GetUUID().Dump(&strm);
+  module_spec.GetUUID().Dump(strm);
   result.AppendErrorWithFormat(
   "Unable to locate the executable or symbol file with UUID %s",
   strm.GetData());
@@ -4240,7 +4240,7 @@
 StreamString ss_symfile_uuid;
 if (module_spec.GetUUID().IsValid()) {
   ss_symfile_uuid << " (";
-  module_spec.GetUUID().Dump(&ss_symfile_uuid);
+  module_spec.GetUUID().Dump(ss_symfile_uuid);
   ss_symfile_uuid << ')';
 }
 result.AppendErrorWithFormat(
@@ -4275,7 +4275,7 @@
 if (!DownloadObjectAndSymbolFile(module_spec, result, flush)) {
   StreamString error_strm;
   error_strm.PutCString("unable to find debug symbols for UUID ");
-  module_spec.GetUUID().Dump(&error_strm);
+  module_spec.GetUUID().Dump(error_strm);
   result.AppendError(error_strm.GetString());
   return false;
 }
Index: lldb/include/lldb/Utility/UUID.h
===
--- lldb/include/lldb/Utility/UUID.h
+++ lldb/include/lldb/Utility/UUID.h
@@ -61,7 +61,7 @@
 
   void Clear() { m_bytes.clear(); }
 
-  void Dump(Stream *s) const;
+  void Dump(Stream &s) const;
 
   llvm::ArrayRef GetBytes() const { return m_bytes; }
 
Index: lldb/include/lldb/Core/ModuleSpec.h
===

[Lldb-commits] [lldb] 718e0cd - [lldb][NFCI] UUID::Dump should take a reference instead of a pointer

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-06-26T10:33:18-07:00
New Revision: 718e0cd6e7240a1233991eec472aa904800dce00

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

LOG: [lldb][NFCI] UUID::Dump should take a reference instead of a pointer

We always assume the Stream pointer is valid, might as well be taking a
reference instead.

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

Added: 


Modified: 
lldb/include/lldb/Core/ModuleSpec.h
lldb/include/lldb/Utility/UUID.h
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Interpreter/OptionValueUUID.cpp
lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
lldb/source/Target/Target.cpp
lldb/source/Utility/UUID.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/ModuleSpec.h 
b/lldb/include/lldb/Core/ModuleSpec.h
index 451e347c9c7ba..4cbbbfa8a26e1 100644
--- a/lldb/include/lldb/Core/ModuleSpec.h
+++ b/lldb/include/lldb/Core/ModuleSpec.h
@@ -194,7 +194,7 @@ class ModuleSpec {
   if (dumped_something)
 strm.PutCString(", ");
   strm.PutCString("uuid = ");
-  m_uuid.Dump(&strm);
+  m_uuid.Dump(strm);
   dumped_something = true;
 }
 if (m_object_name) {

diff  --git a/lldb/include/lldb/Utility/UUID.h 
b/lldb/include/lldb/Utility/UUID.h
index 56aa3fa8450b0..66058d482f012 100644
--- a/lldb/include/lldb/Utility/UUID.h
+++ b/lldb/include/lldb/Utility/UUID.h
@@ -61,7 +61,7 @@ class UUID {
 
   void Clear() { m_bytes.clear(); }
 
-  void Dump(Stream *s) const;
+  void Dump(Stream &s) const;
 
   llvm::ArrayRef GetBytes() const { return m_bytes; }
 

diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index dd4fbe84e5737..300053bcf5ed7 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -1318,7 +1318,7 @@ static void DumpModuleArchitecture(Stream &strm, Module 
*module,
 
 static void DumpModuleUUID(Stream &strm, Module *module) {
   if (module && module->GetUUID().IsValid())
-module->GetUUID().Dump(&strm);
+module->GetUUID().Dump(strm);
   else
 strm.PutCString("");
 }
@@ -2560,7 +2560,7 @@ class CommandObjectTargetModulesAdd : public 
CommandObjectParsed {
 return true;
   } else {
 StreamString strm;
-module_spec.GetUUID().Dump(&strm);
+module_spec.GetUUID().Dump(strm);
 if (module_spec.GetFileSpec()) {
   if (module_spec.GetSymbolFileSpec()) {
 result.AppendErrorWithFormat(
@@ -2584,7 +2584,7 @@ class CommandObjectTargetModulesAdd : public 
CommandObjectParsed {
   }
 } else {
   StreamString strm;
-  module_spec.GetUUID().Dump(&strm);
+  module_spec.GetUUID().Dump(strm);
   result.AppendErrorWithFormat(
   "Unable to locate the executable or symbol file with UUID %s",
   strm.GetData());
@@ -4240,7 +4240,7 @@ class CommandObjectTargetSymbolsAdd : public 
CommandObjectParsed {
 StreamString ss_symfile_uuid;
 if (module_spec.GetUUID().IsValid()) {
   ss_symfile_uuid << " (";
-  module_spec.GetUUID().Dump(&ss_symfile_uuid);
+  module_spec.GetUUID().Dump(ss_symfile_uuid);
   ss_symfile_uuid << ')';
 }
 result.AppendErrorWithFormat(
@@ -4275,7 +4275,7 @@ class CommandObjectTargetSymbolsAdd : public 
CommandObjectParsed {
 if (!DownloadObjectAndSymbolFile(module_spec, result, flush)) {
   StreamString error_strm;
   error_strm.PutCString("unable to find debug symbols for UUID ");
-  module_spec.GetUUID().Dump(&error_strm);
+  module_spec.GetUUID().Dump(error_strm);
   result.AppendError(error_strm.GetString());
   return false;
 }

diff  --git a/lldb/source/Interpreter/OptionValueUUID.cpp 
b/lldb/source/Interpreter/OptionValueUUID.cpp
index 283f9c1b67b35..ff35870a76e89 100644
--- a/lldb/source/Interpreter/OptionValueUUID.cpp
+++ b/lldb/source/Interpreter/OptionValueUUID.cpp
@@ -23,7 +23,7 @@ void OptionValueUUID::DumpValue(const ExecutionContext 
*exe_ctx, Stream &strm,
   if (dump_mask & eDumpOptionValue) {
 if (dump_mask & eDumpOptionType)
   strm.PutCString(" = ");
-m_uuid.Dump(&strm);
+m_uuid.Dump(strm);
   }
 }
 

diff  --git a/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp 
b/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
index 0adf204323b39..7a1568f0b26c1 100644
--- a/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
+++ b/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
@@ -54,11 +54,11 @@ static bool UUIDsMatch(Module *module, ObjectFile *ofile,
 if (feedback_strm) {
   feedback_strm->PutC

[Lldb-commits] [PATCH] D153073: [LLDB] Add DWARF definitions for the new Mojo language

[walter erquinigo via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG582582fb474b: [LLDB] Add DWARF definitions for the new Mojo 
language (authored by wallace).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153073

Files:
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Target/Language.cpp
  llvm/include/llvm-c/DebugInfo.h
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/BinaryFormat/Dwarf.h
  llvm/lib/IR/DIBuilder.cpp

Index: llvm/lib/IR/DIBuilder.cpp
===
--- llvm/lib/IR/DIBuilder.cpp
+++ llvm/lib/IR/DIBuilder.cpp
@@ -140,7 +140,7 @@
 DICompileUnit::DebugNameTableKind NameTableKind, bool RangesBaseAddress,
 StringRef SysRoot, StringRef SDK) {
 
-  assert(((Lang <= dwarf::DW_LANG_Ada2012 && Lang >= dwarf::DW_LANG_C89) ||
+  assert(((Lang <= dwarf::DW_LANG_Mojo && Lang >= dwarf::DW_LANG_C89) ||
   (Lang <= dwarf::DW_LANG_hi_user && Lang >= dwarf::DW_LANG_lo_user)) &&
  "Invalid Language tag");
 
Index: llvm/include/llvm/BinaryFormat/Dwarf.h
===
--- llvm/include/llvm/BinaryFormat/Dwarf.h
+++ llvm/include/llvm/BinaryFormat/Dwarf.h
@@ -268,6 +268,7 @@
   case DW_LANG_Fortran18:
   case DW_LANG_Ada2005:
   case DW_LANG_Ada2012:
+  case DW_LANG_Mojo:
 result = false;
 break;
   }
@@ -334,6 +335,7 @@
   case DW_LANG_C17:
   case DW_LANG_Ada2005:
   case DW_LANG_Ada2012:
+  case DW_LANG_Mojo:
 result = false;
 break;
   }
@@ -398,6 +400,7 @@
   case DW_LANG_Fortran18:
   case DW_LANG_Ada2005:
   case DW_LANG_Ada2012:
+  case DW_LANG_Mojo:
 return false;
   }
   llvm_unreachable("Unknown language kind.");
Index: llvm/include/llvm/BinaryFormat/Dwarf.def
===
--- llvm/include/llvm/BinaryFormat/Dwarf.def
+++ llvm/include/llvm/BinaryFormat/Dwarf.def
@@ -948,6 +948,7 @@
 HANDLE_DW_LANG(0x002d, Fortran18, 0, 0, DWARF)
 HANDLE_DW_LANG(0x002e, Ada2005, 0, 0, DWARF)
 HANDLE_DW_LANG(0x002f, Ada2012, 0, 0, DWARF)
+HANDLE_DW_LANG(0x0033, Mojo, 0, 0, DWARF)
 // Vendor extensions:
 HANDLE_DW_LANG(0x8001, Mips_Assembler, std::nullopt, 0, MIPS)
 HANDLE_DW_LANG(0x8e57, GOOGLE_RenderScript, 0, 0, GOOGLE)
Index: llvm/include/llvm-c/DebugInfo.h
===
--- llvm/include/llvm-c/DebugInfo.h
+++ llvm/include/llvm-c/DebugInfo.h
@@ -125,6 +125,7 @@
   LLVMDWARFSourceLanguageFortran18,
   LLVMDWARFSourceLanguageAda2005,
   LLVMDWARFSourceLanguageAda2012,
+  LLVMDWARFSourceLanguageMojo,
   // Vendor extensions:
   LLVMDWARFSourceLanguageMips_Assembler,
   LLVMDWARFSourceLanguageGOOGLE_RenderScript,
Index: lldb/source/Target/Language.cpp
===
--- lldb/source/Target/Language.cpp
+++ lldb/source/Target/Language.cpp
@@ -209,9 +209,9 @@
 {"fortran18", eLanguageTypeFortran18},
 {"ada2005", eLanguageTypeAda2005},
 {"ada2012", eLanguageTypeAda2012},
+{"mojo", eLanguageTypeMojo},
 // Vendor Extensions
 {"assembler", eLanguageTypeMipsAssembler},
-{"mojo", eLanguageTypeMojo},
 // Now synonyms, in arbitrary order
 {"objc", eLanguageTypeObjC},
 {"objc++", eLanguageTypeObjC_plus_plus},
@@ -457,12 +457,12 @@
   return std::pair();
 }
 
-bool Language::DemangledNameContainsPath(llvm::StringRef path, 
+bool Language::DemangledNameContainsPath(llvm::StringRef path,
  ConstString demangled) const {
   // The base implementation does a simple contains comparision:
   if (path.empty())
 return false;
-  return demangled.GetStringRef().contains(path); 
+  return demangled.GetStringRef().contains(path);
 }
 
 DumpValueObjectOptions::DeclPrintingHelper Language::GetDeclPrintingHelper() {
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -490,16 +490,16 @@
   eLanguageTypeFortran18 = 0x002d,
   eLanguageTypeAda2005 = 0x002e,
   eLanguageTypeAda2012 = 0x002f,
+  eLanguageTypeMojo = 0x0033,
 
   // Vendor Extensions
   // Note: Language::GetNameForLanguageType
   // assumes these can be used as indexes into array language_names, and
   // Language::SetLanguageFromCString and Language::AsCString assume these can
   // be used as indexes into array g_languages.
-  eLanguageTypeMipsAssembler,   ///< Mips_Assembler.
+  eLanguageTypeMipsAssembler, ///< Mips_Assembler.
   // Mojo will move to the common list of languages once the DWARF committee
   // creates a language code for it.
-  eLanguageTypeMojo,
   eNumLanguageTypes
 };
 
_

[Lldb-commits] [lldb] 582582f - [LLDB] Add DWARF definitions for the new Mojo language

[walter erquinigo via lldb-commits]

Author: walter erquinigo
Date: 2023-06-26T11:31:48-05:00
New Revision: 582582fb474b8cd4103e65c3e5a705b3aff61794

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

LOG: [LLDB] Add DWARF definitions for the new Mojo language

The new language Mojo recently received a proper DWARF code, which can be seen 
in https://dwarfstd.org/languages.html, and this patch adds the basic 
definitions for using this language in DWARF.

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

Added: 


Modified: 
lldb/include/lldb/lldb-enumerations.h
lldb/source/Target/Language.cpp
llvm/include/llvm-c/DebugInfo.h
llvm/include/llvm/BinaryFormat/Dwarf.def
llvm/include/llvm/BinaryFormat/Dwarf.h
llvm/lib/IR/DIBuilder.cpp

Removed: 




diff  --git a/lldb/include/lldb/lldb-enumerations.h 
b/lldb/include/lldb/lldb-enumerations.h
index da5b88c1fb164..3981eb0acfccf 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -490,16 +490,16 @@ enum LanguageType {
   eLanguageTypeFortran18 = 0x002d,
   eLanguageTypeAda2005 = 0x002e,
   eLanguageTypeAda2012 = 0x002f,
+  eLanguageTypeMojo = 0x0033,
 
   // Vendor Extensions
   // Note: Language::GetNameForLanguageType
   // assumes these can be used as indexes into array language_names, and
   // Language::SetLanguageFromCString and Language::AsCString assume these can
   // be used as indexes into array g_languages.
-  eLanguageTypeMipsAssembler,   ///< Mips_Assembler.
+  eLanguageTypeMipsAssembler, ///< Mips_Assembler.
   // Mojo will move to the common list of languages once the DWARF committee
   // creates a language code for it.
-  eLanguageTypeMojo,
   eNumLanguageTypes
 };
 

diff  --git a/lldb/source/Target/Language.cpp b/lldb/source/Target/Language.cpp
index a307cb345c005..77ba946012b00 100644
--- a/lldb/source/Target/Language.cpp
+++ b/lldb/source/Target/Language.cpp
@@ -209,9 +209,9 @@ struct language_name_pair language_names[] = {
 {"fortran18", eLanguageTypeFortran18},
 {"ada2005", eLanguageTypeAda2005},
 {"ada2012", eLanguageTypeAda2012},
+{"mojo", eLanguageTypeMojo},
 // Vendor Extensions
 {"assembler", eLanguageTypeMipsAssembler},
-{"mojo", eLanguageTypeMojo},
 // Now synonyms, in arbitrary order
 {"objc", eLanguageTypeObjC},
 {"objc++", eLanguageTypeObjC_plus_plus},
@@ -457,12 +457,12 @@ Language::GetFormatterPrefixSuffix(llvm::StringRef 
type_hint) {
   return std::pair();
 }
 
-bool Language::DemangledNameContainsPath(llvm::StringRef path, 
+bool Language::DemangledNameContainsPath(llvm::StringRef path,
  ConstString demangled) const {
   // The base implementation does a simple contains comparision:
   if (path.empty())
 return false;
-  return demangled.GetStringRef().contains(path);  
   
+  return demangled.GetStringRef().contains(path);
 }
 
 DumpValueObjectOptions::DeclPrintingHelper Language::GetDeclPrintingHelper() {

diff  --git a/llvm/include/llvm-c/DebugInfo.h b/llvm/include/llvm-c/DebugInfo.h
index fe3a64ed3dfda..5924294708cc3 100644
--- a/llvm/include/llvm-c/DebugInfo.h
+++ b/llvm/include/llvm-c/DebugInfo.h
@@ -125,6 +125,7 @@ typedef enum {
   LLVMDWARFSourceLanguageFortran18,
   LLVMDWARFSourceLanguageAda2005,
   LLVMDWARFSourceLanguageAda2012,
+  LLVMDWARFSourceLanguageMojo,
   // Vendor extensions:
   LLVMDWARFSourceLanguageMips_Assembler,
   LLVMDWARFSourceLanguageGOOGLE_RenderScript,

diff  --git a/llvm/include/llvm/BinaryFormat/Dwarf.def 
b/llvm/include/llvm/BinaryFormat/Dwarf.def
index d0357bec0bbf5..40d958c867de9 100644
--- a/llvm/include/llvm/BinaryFormat/Dwarf.def
+++ b/llvm/include/llvm/BinaryFormat/Dwarf.def
@@ -948,6 +948,7 @@ HANDLE_DW_LANG(0x002c, C17, 0, 0, DWARF)
 HANDLE_DW_LANG(0x002d, Fortran18, 0, 0, DWARF)
 HANDLE_DW_LANG(0x002e, Ada2005, 0, 0, DWARF)
 HANDLE_DW_LANG(0x002f, Ada2012, 0, 0, DWARF)
+HANDLE_DW_LANG(0x0033, Mojo, 0, 0, DWARF)
 // Vendor extensions:
 HANDLE_DW_LANG(0x8001, Mips_Assembler, std::nullopt, 0, MIPS)
 HANDLE_DW_LANG(0x8e57, GOOGLE_RenderScript, 0, 0, GOOGLE)

diff  --git a/llvm/include/llvm/BinaryFormat/Dwarf.h 
b/llvm/include/llvm/BinaryFormat/Dwarf.h
index c4d0232ac6c75..869352b35e323 100644
--- a/llvm/include/llvm/BinaryFormat/Dwarf.h
+++ b/llvm/include/llvm/BinaryFormat/Dwarf.h
@@ -268,6 +268,7 @@ inline bool isCPlusPlus(SourceLanguage S) {
   case DW_LANG_Fortran18:
   case DW_LANG_Ada2005:
   case DW_LANG_Ada2012:
+  case DW_LANG_Mojo:
 result = false;
 break;
   }
@@ -334,6 +335,7 @@ inline bool isFortran(SourceLanguage S) {
   case DW_LANG_C17:
   case DW_LANG_Ada2005:
   case DW_LANG_Ada2012:
+  case DW_LANG_Mojo:
 result = false;
 break;
   }
@@ -398,6 +400,7 @@ inline bool 

[Lldb-commits] [PATCH] D153626: [lldb] Use SmallVector for handling register data

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 534596.
DavidSpickett added a comment.

Drop {} in one place now that the resize is not conditional. The logic
there also seems faulty, but in other ways I will stay out of for now.

I did attempt another sanitizers build and realised I should have been
setting leak checker options. That got me to missing symbols, and
no amount of preloading seemed to help there. Not sure what went wrong.

I looked at asserting or adding some checker methods. Given that you're
going to resize to X then pass X as the size to the subsequent call,
there's not much value in it. It would prevent some typos maybe.

On the same theme I looked at adding overloads that took smallvector
directly, but with only 3/4 calls to each one it just didn't seem worth it.

The existing uses are compact enough to check manually and/or in a
sanitized build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153626

Files:
  lldb/include/lldb/Utility/RegisterValue.h
  lldb/source/Host/common/NativeRegisterContext.cpp
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Target/RegisterContext.cpp
  lldb/source/Utility/RegisterValue.cpp

Index: lldb/source/Utility/RegisterValue.cpp
===
--- lldb/source/Utility/RegisterValue.cpp
+++ lldb/source/Utility/RegisterValue.cpp
@@ -48,11 +48,6 @@
 return 0;
   }
 
-  if (dst_len > kMaxRegisterByteSize) {
-error.SetErrorString("destination is too big");
-return 0;
-  }
-
   const uint32_t src_len = reg_info.byte_size;
 
   // Extract the register data into a data extractor
@@ -96,12 +91,6 @@
   //   |AABB| Address contents
   //   |AABB| Register contents [on little-endian hardware]
   //   |AABB| Register contents [on big-endian hardware]
-  if (src_len > kMaxRegisterByteSize) {
-error.SetErrorStringWithFormat(
-"register buffer is too small to receive %u bytes of data.", src_len);
-return 0;
-  }
-
   const uint32_t dst_len = reg_info.byte_size;
 
   if (src_len > dst_len) {
@@ -128,9 +117,10 @@
   case eTypeInvalid:
 break;
   case eTypeBytes: {
-DataExtractor data(buffer.bytes, buffer.length, buffer.byte_order, 1);
-if (scalar.SetValueFromData(data, lldb::eEncodingUint,
-	  buffer.length).Success())
+DataExtractor data(buffer.bytes.data(), buffer.bytes.size(),
+   buffer.byte_order, 1);
+if (scalar.SetValueFromData(data, lldb::eEncodingUint, buffer.bytes.size())
+.Success())
   return true;
   } break;
   case eTypeUInt8:
@@ -190,9 +180,6 @@
   if (src_len > reg_info.byte_size)
 src_len = reg_info.byte_size;
 
-  // Zero out the value in case we get partial data...
-  memset(buffer.bytes, 0, sizeof(buffer.bytes));
-
   type128 int128;
 
   m_type = eTypeInvalid;
@@ -232,16 +219,14 @@
 break;
   case eEncodingVector: {
 m_type = eTypeBytes;
-buffer.length = reg_info.byte_size;
+assert(reg_info.byte_size <= kMaxRegisterByteSize);
+buffer.bytes.resize(reg_info.byte_size);
 buffer.byte_order = src.GetByteOrder();
-assert(buffer.length <= kMaxRegisterByteSize);
-if (buffer.length > kMaxRegisterByteSize)
-  buffer.length = kMaxRegisterByteSize;
 if (src.CopyByteOrderedData(
-src_offset,// offset within "src" to start extracting data
-src_len,   // src length
-buffer.bytes,  // dst buffer
-buffer.length, // dst length
+src_offset,  // offset within "src" to start extracting data
+src_len, // src length
+buffer.bytes.data(), // dst buffer
+buffer.bytes.size(), // dst length
 buffer.byte_order) == 0) // dst byte order
 {
   error.SetErrorStringWithFormat(
@@ -488,9 +473,7 @@
 m_scalar = rhs.m_scalar;
 break;
   case eTypeBytes:
-assert(rhs.buffer.length <= kMaxRegisterByteSize);
-::memcpy(buffer.bytes, rhs.buffer.bytes, kMaxRegisterByteSize);
-buffer.length = rhs.buffer.length;
+buffer.bytes = rhs.buffer.bytes;
 buffer.byte_order = rhs.buffer.byte_order;
 break;
   }
@@ -509,12 +492,12 @@
   case eTypeUInt16:
 return m_scalar.UShort(fail_value);
   case eTypeBytes: {
-switch (buffer.length) {
+switch (buffer.bytes.size()) {
 default:
   break;
 case 1:
 case 2:
-  return *reinterpret_cast(buffer.bytes);
+  

[Lldb-commits] [PATCH] D153685: [lldb] Add `source cache dump` and `source cache clear` subcommand

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked 4 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/source/Commands/CommandObjectSource.cpp:1266
 : CommandObjectMultiword(interpreter, "source",
  "Commands for examining "
  "source code described by "

jasonmolenda wrote:
> Maybe we can make this less accurate and easier to understand. like "Commands 
> for examining source files in the current process".  
But that wouldn't just be less accurate, it would be wrong. The current cache 
is shared across targets and processes belonging to the same debugger.



Comment at: lldb/source/Core/SourceManager.cpp:738
+FileSP file = entry.second;
+stream.Format("{0:%Y-%m-%d %H:%M:%S} {1,8:d} {2}\n", file->GetTimestamp(),
+  file->GetNumLines(), entry.first.GetPath());

jasonmolenda wrote:
> m/d/Y would be dope tho
🇺🇸


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

https://reviews.llvm.org/D153685

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

[Lldb-commits] [PATCH] D153626: [lldb] Use SmallVector for handling register data

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 534580.
DavidSpickett added a comment.

Make a resize in EmulateInstructionARM64::EmulateLDPSTP uncondintional.

Though in this case the register for an ldp/stp could only be an x register
which is 8 bytes, so there would have been enough allocated already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153626

Files:
  lldb/include/lldb/Utility/RegisterValue.h
  lldb/source/Host/common/NativeRegisterContext.cpp
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Target/RegisterContext.cpp
  lldb/source/Utility/RegisterValue.cpp

Index: lldb/source/Utility/RegisterValue.cpp
===
--- lldb/source/Utility/RegisterValue.cpp
+++ lldb/source/Utility/RegisterValue.cpp
@@ -48,11 +48,6 @@
 return 0;
   }
 
-  if (dst_len > kMaxRegisterByteSize) {
-error.SetErrorString("destination is too big");
-return 0;
-  }
-
   const uint32_t src_len = reg_info.byte_size;
 
   // Extract the register data into a data extractor
@@ -96,12 +91,6 @@
   //   |AABB| Address contents
   //   |AABB| Register contents [on little-endian hardware]
   //   |AABB| Register contents [on big-endian hardware]
-  if (src_len > kMaxRegisterByteSize) {
-error.SetErrorStringWithFormat(
-"register buffer is too small to receive %u bytes of data.", src_len);
-return 0;
-  }
-
   const uint32_t dst_len = reg_info.byte_size;
 
   if (src_len > dst_len) {
@@ -128,9 +117,10 @@
   case eTypeInvalid:
 break;
   case eTypeBytes: {
-DataExtractor data(buffer.bytes, buffer.length, buffer.byte_order, 1);
-if (scalar.SetValueFromData(data, lldb::eEncodingUint,
-	  buffer.length).Success())
+DataExtractor data(buffer.bytes.data(), buffer.bytes.size(),
+   buffer.byte_order, 1);
+if (scalar.SetValueFromData(data, lldb::eEncodingUint, buffer.bytes.size())
+.Success())
   return true;
   } break;
   case eTypeUInt8:
@@ -190,9 +180,6 @@
   if (src_len > reg_info.byte_size)
 src_len = reg_info.byte_size;
 
-  // Zero out the value in case we get partial data...
-  memset(buffer.bytes, 0, sizeof(buffer.bytes));
-
   type128 int128;
 
   m_type = eTypeInvalid;
@@ -232,16 +219,14 @@
 break;
   case eEncodingVector: {
 m_type = eTypeBytes;
-buffer.length = reg_info.byte_size;
+assert(reg_info.byte_size <= kMaxRegisterByteSize);
+buffer.bytes.resize(reg_info.byte_size);
 buffer.byte_order = src.GetByteOrder();
-assert(buffer.length <= kMaxRegisterByteSize);
-if (buffer.length > kMaxRegisterByteSize)
-  buffer.length = kMaxRegisterByteSize;
 if (src.CopyByteOrderedData(
-src_offset,// offset within "src" to start extracting data
-src_len,   // src length
-buffer.bytes,  // dst buffer
-buffer.length, // dst length
+src_offset,  // offset within "src" to start extracting data
+src_len, // src length
+buffer.bytes.data(), // dst buffer
+buffer.bytes.size(), // dst length
 buffer.byte_order) == 0) // dst byte order
 {
   error.SetErrorStringWithFormat(
@@ -488,9 +473,7 @@
 m_scalar = rhs.m_scalar;
 break;
   case eTypeBytes:
-assert(rhs.buffer.length <= kMaxRegisterByteSize);
-::memcpy(buffer.bytes, rhs.buffer.bytes, kMaxRegisterByteSize);
-buffer.length = rhs.buffer.length;
+buffer.bytes = rhs.buffer.bytes;
 buffer.byte_order = rhs.buffer.byte_order;
 break;
   }
@@ -509,12 +492,12 @@
   case eTypeUInt16:
 return m_scalar.UShort(fail_value);
   case eTypeBytes: {
-switch (buffer.length) {
+switch (buffer.bytes.size()) {
 default:
   break;
 case 1:
 case 2:
-  return *reinterpret_cast(buffer.bytes);
+  return *reinterpret_cast(buffer.bytes.data());
 }
   } break;
   }
@@ -538,13 +521,13 @@
   case eTypeLongDouble:
 return m_scalar.UInt(fail_value);
   case eTypeBytes: {
-switch (buffer.length) {
+switch (buffer.bytes.size()) {
 default:
   break;
 case 1:
 case 2:
 case 4:
-  return *reinterpret_cast(buffer.bytes);
+  return *reinterpret_cast(buffer.bytes.data());
 }
   } break;
   }
@@ -569,17 +552,17 @@
   case eTypeLongDouble:
 return m_scalar.ULongLong(fail_value);
   case eTypeBytes: {
-switch (buffe

[Lldb-commits] [PATCH] D153740: [llvm][Support] Deprecate llvm::writeFileAtomically API

[Haojian Wu via Phabricator via lldb-commits]

hokein added a comment.

In D153740#4448408 , @avl wrote:

> added @jkorous who originally added llvm::writeFileAtomically.
>
>> Let me know what you think about it -- I considered keeping the 
>> llvm::writeFileAtomically and migrating its underlying implementation to 
>> llvm::writeToOutput, but it doesn't seem to worth, there are only 4 in-tree 
>> usages of this API, I think it is probably better just remove it.
>
> I think it is OK to leave only one API (f.e. llvm::writeToOutput) and remove 
> another. It would probably be better to split this patch to  lldb, thinlto, 
> clang and removal parts.

Sounds good. I will split it once https://reviews.llvm.org/D153652 is landed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153740

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

[Lldb-commits] [PATCH] D153740: [llvm][Support] Deprecate llvm::writeFileAtomically API

[Alexey Lapshin via Phabricator via lldb-commits]

avl added a reviewer: jkorous.
avl added a subscriber: jkorous.
avl added a comment.

added @jkorous who originally added llvm::writeFileAtomically.

> Let me know what you think about it -- I considered keeping the 
> llvm::writeFileAtomically and migrating its underlying implementation to 
> llvm::writeToOutput, but it doesn't seem to worth, there are only 4 in-tree 
> usages of this API, I think it is probably better just remove it.

I think it is OK to leave only one API (f.e. llvm::writeToOutput) and remove 
another. It would probably be better to split this patch to  lldb, thinlto, 
clang and removal parts.




Comment at: lldb/tools/lldb-server/lldb-platform.cpp:107
   Status status;
-  if (auto Err =
-  handleErrors(llvm::writeFileAtomically(
-   temp_file_path, file_spec.GetPath(), socket_id),
-   [&status, &file_spec](const AtomicFileWriteError &E) {
- std::string ErrorMsgBuffer;
- llvm::raw_string_ostream S(ErrorMsgBuffer);
- E.log(S);
-
- switch (E.Error) {
- case atomic_write_error::failed_to_create_uniq_file:
-   status = Status("Failed to create temp file: %s",
-   ErrorMsgBuffer.c_str());
-   break;
- case atomic_write_error::output_stream_error:
-   status = Status("Failed to write to port file.");
-   break;
- case atomic_write_error::failed_to_rename_temp_file:
-   status = Status("Failed to rename file %s to %s: 
%s",
-   ErrorMsgBuffer.c_str(),
-   file_spec.GetPath().c_str(),
-   ErrorMsgBuffer.c_str());
-   break;
- }
-   })) {
+  if (auto Err = handleErrors(file_spec.GetPath(),
+  [&socket_id](llvm::raw_ostream &OS) {

the call to llvm::writeToOutput is probably missed here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153740

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

[Lldb-commits] [PATCH] D153626: [lldb] Use SmallVector for handling register data

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> The sanitized bot on GreenDragon runs on Intel and I assume the "risky' 
> changes only apply to arm64 as that's the only architecture that needs to 
> scale beyond the default 256? Anyway I haven't seen the leaks issue you've 
> mentioned locally so I'm happy to run a sanitized build on arm64.

The risk for existing targets is that `.size()` won't reflect the size we're 
about to write to it. With it defaulting to 256 bytes of stack space and us 
using it like an array, we'd probably skip a lot of the SmallVector asserts 
around that.

Potentially I could "proxy" `.resize` and assert that it had been called at 
some point but it's still possible to forget to resize it again if it was 
needed. It would make more sense to look at the methods we pass the 
SmallVector's storage to instead. If they could take the container directly, 
then they could do some checks.

I will look into that, and I will also try the sanitized build again because 
now I wonder if lit's env cleaning was removing the options I needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153626

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

[Lldb-commits] [PATCH] D153740: [llvm][Support] Deprecate llvm::writeFileAtomically API

[Haojian Wu via Phabricator via lldb-commits]

hokein added a comment.
Herald added a subscriber: JDevlieghere.

Let me know what you think about it -- I considered keeping the 
`llvm::writeFileAtomically` and migrating its underlying implementation to 
`llvm::writeToOutput`, but it doesn't seem to worth, there are only 4 in-tree 
usages of this API, I think it is probably better just remove it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153740

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

[Lldb-commits] [PATCH] D153740: [llvm][Support] Deprecate llvm::writeFileAtomically API

[Haojian Wu via Phabricator via lldb-commits]

hokein created this revision.
hokein added a reviewer: avl.
Herald added subscribers: ormris, kadircet, arphaman, steven_wu, hiraditya.
Herald added a project: All.
hokein requested review of this revision.
Herald added projects: clang, LLDB, LLVM, clang-tools-extra.
Herald added subscribers: llvm-commits, lldb-commits.

The new llvm::writeToOutput API does the same thing, and we're in favor
of it.

This patch migrates 4 exiting usages and removes the llvm::writeFileAtomically 
API.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153740

Files:
  clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  lldb/tools/lldb-server/lldb-platform.cpp
  llvm/include/llvm/Support/FileUtilities.h
  llvm/lib/LTO/ThinLTOCodeGenerator.cpp
  llvm/lib/Support/FileUtilities.cpp
  llvm/unittests/Support/CMakeLists.txt
  llvm/unittests/Support/FileUtilitiesTest.cpp

Index: llvm/unittests/Support/FileUtilitiesTest.cpp
===
--- llvm/unittests/Support/FileUtilitiesTest.cpp
+++ /dev/null
@@ -1,54 +0,0 @@
-//===- llvm/unittest/Support/FileUtilitiesTest.cpp - unit tests ---===//
-//
-// 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 "llvm/Support/FileUtilities.h"
-#include "llvm/Support/Errc.h"
-#include "llvm/Support/ErrorHandling.h"
-#include "llvm/Support/FileSystem.h"
-#include "llvm/Support/MemoryBuffer.h"
-#include "llvm/Support/Path.h"
-#include "llvm/Testing/Support/SupportHelpers.h"
-#include "gtest/gtest.h"
-#include 
-
-using namespace llvm;
-using namespace llvm::sys;
-
-using llvm::unittest::TempDir;
-
-#define ASSERT_NO_ERROR(x) \
-  if (std::error_code ASSERT_NO_ERROR_ec = x) {\
-SmallString<128> MessageStorage;   \
-raw_svector_ostream Message(MessageStorage);   \
-Message << #x ": did not return errc::success.\n"  \
-<< "error number: " << ASSERT_NO_ERROR_ec.value() << "\n"  \
-<< "error message: " << ASSERT_NO_ERROR_ec.message() << "\n";  \
-GTEST_FATAL_FAILURE_(MessageStorage.c_str());  \
-  } else { \
-  }
-
-namespace {
-TEST(writeFileAtomicallyTest, Test) {
-  // Create unique temporary directory for these tests
-  TempDir RootTestDirectory("writeFileAtomicallyTest", /*Unique*/ true);
-
-  SmallString<128> FinalTestfilePath(RootTestDirectory.path());
-  sys::path::append(FinalTestfilePath, "foo.txt");
-  const std::string TempUniqTestFileModel =
-  std::string(FinalTestfilePath) + "-";
-  const std::string TestfileContent = "fooFOOfoo";
-
-  llvm::Error Err = llvm::writeFileAtomically(TempUniqTestFileModel, FinalTestfilePath, TestfileContent);
-  ASSERT_FALSE(static_cast(Err));
-
-  std::ifstream FinalFileStream(std::string(FinalTestfilePath.str()));
-  std::string FinalFileContent;
-  FinalFileStream >> FinalFileContent;
-  ASSERT_EQ(FinalFileContent, TestfileContent);
-}
-} // anonymous namespace
Index: llvm/unittests/Support/CMakeLists.txt
===
--- llvm/unittests/Support/CMakeLists.txt
+++ llvm/unittests/Support/CMakeLists.txt
@@ -41,7 +41,6 @@
   ExtensibleRTTITest.cpp
   FileCollectorTest.cpp
   FileOutputBufferTest.cpp
-  FileUtilitiesTest.cpp
   FormatVariadicTest.cpp
   FSUniqueIDTest.cpp
   GlobPatternTest.cpp
Index: llvm/lib/Support/FileUtilities.cpp
===
--- llvm/lib/Support/FileUtilities.cpp
+++ llvm/lib/Support/FileUtilities.cpp
@@ -267,64 +267,6 @@
   return CompareFailed;
 }
 
-void llvm::AtomicFileWriteError::log(raw_ostream &OS) const {
-  OS << "atomic_write_error: ";
-  switch (Error) {
-  case atomic_write_error::failed_to_create_uniq_file:
-OS << "failed_to_create_uniq_file";
-return;
-  case atomic_write_error::output_stream_error:
-OS << "output_stream_error";
-return;
-  case atomic_write_error::failed_to_rename_temp_file:
-OS << "failed_to_rename_temp_file";
-return;
-  }
-  llvm_unreachable("unknown atomic_write_error value in "
-   "failed_to_rename_temp_file::log()");
-}
-
-llvm::Error llvm::writeFileAtomically(StringRef TempPathModel,
-  StringRef FinalPath, StringRef Buffer) {
-  return writeFileAtomically(TempPathModel, FinalPath,
- [&Buffer](llvm::raw_ostream &OS) {
-   OS.write