[Lldb-commits] [lldb] [lldb] Fix TestDiagnoseDereferenceFunctionReturn on linux (PR #128512)

[Pavel Labath via lldb-commits]

labath wrote:

Thanks.

As we've learned in #123217, all of this relies on parsing the stop reason 
strings, and I think the windows strings just don't have the expected format.

https://github.com/llvm/llvm-project/pull/128512
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 6c17380 - [lldb] Fix TestDiagnoseDereferenceFunctionReturn on linux (#128512)

[via lldb-commits]

Author: Pavel Labath
Date: 2025-02-25T09:03:03+01:00
New Revision: 6c17380ea896e9966645958ad3d76441cc25430c

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

LOG: [lldb] Fix TestDiagnoseDereferenceFunctionReturn on linux (#128512)

The test was failing because it was looking up the immediate value from
the call instruction as a load address, whereas in fact it was a file
address. This worked on darwin because (with ASLR disabled) the two
addresses are generally the same. On linux, this depends on the build
mode, but with the default (PIE) build type, the two are never the same.
The test also fails on a mac with ASLR enabled.

This path fixes the code to look up the value as a file address.

Added: 


Modified: 
lldb/source/Target/StackFrame.cpp

lldb/test/API/commands/frame/diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py

Removed: 




diff  --git a/lldb/source/Target/StackFrame.cpp 
b/lldb/source/Target/StackFrame.cpp
index 4d068638f42b6..f8061ffad1466 100644
--- a/lldb/source/Target/StackFrame.cpp
+++ b/lldb/source/Target/StackFrame.cpp
@@ -1670,13 +1670,14 @@ lldb::ValueObjectSP DoGuessValueAt(StackFrame &frame, 
ConstString reg,
 break;
   case Instruction::Operand::Type::Immediate: {
 SymbolContext sc;
-Address load_address;
-if (!frame.CalculateTarget()->ResolveLoadAddress(
-operands[0].m_immediate, load_address)) {
+if (!pc.GetModule())
+  break;
+Address address(operands[0].m_immediate,
+pc.GetModule()->GetSectionList());
+if (!address.IsValid())
   break;
-}
 frame.CalculateTarget()->GetImages().ResolveSymbolContextForAddress(
-load_address, eSymbolContextFunction, sc);
+address, eSymbolContextFunction, sc);
 if (!sc.function) {
   break;
 }

diff  --git 
a/lldb/test/API/commands/frame/diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py
 
b/lldb/test/API/commands/frame/diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py
index d0f6ebefa334a..f7b596606134b 100644
--- 
a/lldb/test/API/commands/frame/diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py
+++ 
b/lldb/test/API/commands/frame/diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py
@@ -10,7 +10,7 @@
 
 
 class TestDiagnoseDereferenceFunctionReturn(TestBase):
-@expectedFailureAll(oslist=no_match(lldbplatformutil.getDarwinOSTriples()))
+@expectedFailureAll(oslist=["windows"])
 @skipIf(
 archs=no_match(["x86_64"])
 )  #  frame diagnose doesn't work for armv7 or 
arm64
@@ -19,9 +19,6 @@ def test_diagnose_dereference_function_return(self):
 TestBase.setUp(self)
 self.build()
 exe = self.getBuildArtifact("a.out")
-# FIXME: This default changed in lldbtest.py and this test
-# seems to rely on having it turned off.
-self.runCmd("settings set target.disable-aslr true")
 self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
 self.runCmd("run", RUN_SUCCEEDED)
 self.expect("thread list", "Thread should be stopped", 
substrs=["stopped"])



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

[Lldb-commits] [lldb] [lldb] Fix TestDiagnoseDereferenceFunctionReturn on linux (PR #128512)

[Pavel Labath via lldb-commits]

https://github.com/labath closed 
https://github.com/llvm/llvm-project/pull/128512
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 5088e1b - [lldb] Avoid Function::GetAddressRange in ThreadPlanStepRange::InSymbol (#128515)

[via lldb-commits]

Author: Pavel Labath
Date: 2025-02-25T09:47:30+01:00
New Revision: 5088e1b435fd06de2bfccd3894dcc2f2c326630f

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

LOG: [lldb] Avoid Function::GetAddressRange in ThreadPlanStepRange::InSymbol 
(#128515)

The existing implementation would probably produce false positives for
discontinuous functions. I haven't tried reproducing it because setting
up discontinuous functions (and executing them, in particular) is pretty
complex and there's nothing particularly interesting happening here.

Added: 


Modified: 
lldb/source/Target/ThreadPlanStepRange.cpp

Removed: 




diff  --git a/lldb/source/Target/ThreadPlanStepRange.cpp 
b/lldb/source/Target/ThreadPlanStepRange.cpp
index de4cd5995f695..78e1270edcdab 100644
--- a/lldb/source/Target/ThreadPlanStepRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepRange.cpp
@@ -197,9 +197,11 @@ bool ThreadPlanStepRange::InRange() {
 bool ThreadPlanStepRange::InSymbol() {
   lldb::addr_t cur_pc = GetThread().GetRegisterContext()->GetPC();
   if (m_addr_context.function != nullptr) {
-return m_addr_context.function->GetAddressRange().ContainsLoadAddress(
-cur_pc, &GetTarget());
-  } else if (m_addr_context.symbol && m_addr_context.symbol->ValueIsAddress()) 
{
+AddressRange unused_range;
+return m_addr_context.function->GetRangeContainingLoadAddress(
+cur_pc, GetTarget(), unused_range);
+  }
+  if (m_addr_context.symbol && m_addr_context.symbol->ValueIsAddress()) {
 AddressRange range(m_addr_context.symbol->GetAddressRef(),
m_addr_context.symbol->GetByteSize());
 return range.ContainsLoadAddress(cur_pc, &GetTarget());



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

[Lldb-commits] [lldb] [lldb] Avoid Function::GetAddressRange in ThreadPlanStepRange::InSymbol (PR #128515)

[Pavel Labath via lldb-commits]

https://github.com/labath closed 
https://github.com/llvm/llvm-project/pull/128515
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Pavel Labath via lldb-commits]

@@ -202,6 +202,10 @@
 #cmakedefine LLVM_HAS_LOGF128
 
 /* Define if building LLVM with LLVM_BUILD_TELEMETRY */
+/* DEPRECATED - use LLVM_ENABLE_TELEMETRY */
 #cmakedefine LLVM_BUILD_TELEMETRY ${LLVM_BUILD_TELEMETRY}
 
+/* Define if building LLVM with LLVM_ENABLE_TELEMETRY */
+#cmakedefine LLVM_ENABLE_TELEMETRY ${LLVM_ENABLE_TELEMETRY}

labath wrote:

if you make this `#cmakedefine01` ..

https://github.com/llvm/llvm-project/pull/128534
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Avoid Function::GetAddressRange in ThreadPlanStepRange::InSymbol (PR #128515)

[LLVM Continuous Integration via lldb-commits]

llvm-ci wrote:

LLVM Buildbot has detected a new failure on builder `lldb-arm-ubuntu` running 
on `linaro-lldb-arm-ubuntu` while building `lldb` at step 6 "test".

Full details are available at: 
https://lab.llvm.org/buildbot/#/builders/18/builds/11957


Here is the relevant piece of the build log for the reference

```
Step 6 (test) failure: build (failure)
...
UNSUPPORTED: lldb-api :: 
tools/lldb-dap/exception/objc/TestDAP_exception_objc.py (1153 of 2914)
PASS: lldb-api :: tools/lldb-dap/disconnect/TestDAP_disconnect.py (1154 of 2914)
PASS: lldb-api :: tools/lldb-dap/exception/cpp/TestDAP_exception_cpp.py (1155 
of 2914)
UNSUPPORTED: lldb-api :: 
tools/lldb-dap/extendedStackTrace/TestDAP_extendedStackTrace.py (1156 of 2914)
PASS: lldb-api :: 
tools/lldb-dap/instruction-breakpoint/TestDAP_instruction_breakpoint.py (1157 
of 2914)
PASS: lldb-api :: tools/lldb-dap/locations/TestDAP_locations.py (1158 of 2914)
PASS: lldb-api :: tools/lldb-dap/memory/TestDAP_memory.py (1159 of 2914)
PASS: lldb-api :: tools/lldb-dap/evaluate/TestDAP_evaluate.py (1160 of 2914)
PASS: lldb-api :: tools/lldb-dap/optimized/TestDAP_optimized.py (1161 of 2914)
PASS: lldb-api :: tools/lldb-dap/module/TestDAP_module.py (1162 of 2914)
FAIL: lldb-api :: tools/lldb-dap/output/TestDAP_output.py (1163 of 2914)
 TEST 'lldb-api :: tools/lldb-dap/output/TestDAP_output.py' 
FAILED 
Script:
--
/usr/bin/python3.10 
/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/dotest.py 
-u CXXFLAGS -u CFLAGS --env 
LLVM_LIBS_DIR=/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./lib --env 
LLVM_INCLUDE_DIR=/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/include --env 
LLVM_TOOLS_DIR=/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin --arch 
armv8l --build-dir 
/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex 
--lldb-module-cache-dir 
/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api
 --clang-module-cache-dir 
/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api
 --executable /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin/lldb 
--compiler /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin/clang 
--dsymutil /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin/dsymutil 
--make /usr/bin/gmake --llvm-tools-dir 
/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin --lldb-obj-root 
/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/tools/lldb --lldb-libs-dir 
/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./lib 
/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/tools/lldb-dap/output
 -p TestDAP_output.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 21.0.0git (https://github.com/llvm/llvm-project.git revision 
5088e1b435fd06de2bfccd3894dcc2f2c326630f)
  clang revision 5088e1b435fd06de2bfccd3894dcc2f2c326630f
  llvm revision 5088e1b435fd06de2bfccd3894dcc2f2c326630f
Skipping the following test categories: ['libc++', 'dsym', 'gmodules', 
'debugserver', 'objc']
= DEBUG ADAPTER PROTOCOL LOGS =
1740474211.981302261 stdin/stdout --> 
Content-Length: 344

{
  "arguments": {
"adapterID": "lldb-native",
"clientID": "vscode",
"columnsStartAt1": true,
"linesStartAt1": true,
"locale": "en-us",
"pathFormat": "path",
"sourceInitFile": false,
"supportsRunInTerminalRequest": true,
"supportsStartDebuggingRequest": true,
"supportsVariablePaging": true,
"supportsVariableType": true
  },
  "command": "initialize",
  "seq": 1,
  "type": "request"
}
1740474211.984862804 stdin/stdout <-- 
Content-Length: 1631


```



https://github.com/llvm/llvm-project/pull/128515
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Pavel Labath via lldb-commits]

@@ -64,11 +64,19 @@ class Serializer {
 /// This struct can be extended as needed to add additional configuration
 /// points specific to a vendor's implementation.
 struct Config {
-  virtual ~Config() = default;
+#ifdef LLVM_ENABLE_TELEMETRY
+  static const bool BuildTimeEnableTelemetry = true;
+#else
+  static const bool BuildTimeEnableTelemetry = false;
+#endif
+  virtual ~Config() : EnableTelemetry(BuildTimeEnableTelemetry) {}
 
   // If true, telemetry will be enabled.
   const bool EnableTelemetry;
-  Config(bool E) : EnableTelemetry(E) {}
+
+  // Telemetry can only be enabled if both the runtime and buildtime flag
+  // are set.
+  Config(bool E) : EnableTelemetry(E && BuildTimeEnableTelemetry) {}

labath wrote:

Just a drive-by

```suggestion
  explicit Config(bool E) : EnableTelemetry(E && BuildTimeEnableTelemetry) {}
```

https://github.com/llvm/llvm-project/pull/128534
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [WIP] [lldb][TypeSystemClang] Create clang::SourceLocation from DWARF and attach to AST (PR #127829)

[Michael Buch via lldb-commits]

Michael137 wrote:

> Do we only pay the cost when a diagnostic is generated, or every time a 
> DW_AT_file is parsed?

Currently it's on creation of the decl

https://github.com/llvm/llvm-project/pull/127829
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Telemetry]Define DebuggerTelemetryInfo and related methods (PR #127696)

[Pavel Labath via lldb-commits]

@@ -56,16 +54,107 @@ void LLDBBaseTelemetryInfo::serialize(Serializer 
&serializer) const {
   return UUID(random_bytes).GetAsString();
 }
 
+void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const {
+  serializer.write("entry_kind", getKind());
+  serializer.write("session_id", SessionId);
+  serializer.write("start_time", ToNanosec(start_time));
+  if (end_time.has_value())
+serializer.write("end_time", ToNanosec(end_time.value()));
+}
+
+void DebuggerTelemetryInfo::serialize(Serializer &serializer) const {
+  LLDBBaseTelemetryInfo::serialize(serializer);
+
+  serializer.write("username", username);
+  serializer.write("lldb_git_sha", lldb_git_sha);
+  serializer.write("lldb_path", lldb_path);
+  serializer.write("cwd", cwd);
+  if (exit_desc.has_value()) {
+serializer.write("exit_code", exit_desc->exit_code);
+serializer.write("exit_desc", exit_desc->description);
+  }
+}
+
+void MiscTelemetryInfo::serialize(Serializer &serializer) const {
+  LLDBBaseTelemetryInfo::serialize(serializer);
+  serializer.write("target_uuid", target_uuid);
+  serializer.beginObject("meta_data");
+  for (const auto &kv : meta_data)
+serializer.write(kv.first, kv.second);
+  serializer.endObject();
+}
+
 TelemetryManager::TelemetryManager(std::unique_ptr config)
 : m_config(std::move(config)) {}
 
 llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
-  // Do nothing for now.
-  // In up-coming patch, this would be where the manager
-  // attach the session_uuid to the entry.
+  LLDBBaseTelemetryInfo *lldb_entry =
+  llvm::dyn_cast(entry);
+  std::string session_id = "";
+  if (Debugger *debugger = lldb_entry->debugger) {
+auto session_id_pos = session_ids.find(debugger);
+if (session_id_pos != session_ids.end())
+  session_id = session_id_pos->second;
+else
+  session_id_pos->second = session_id = MakeUUID(debugger);
+  }
+  lldb_entry->SessionId = session_id;
+
   return llvm::Error::success();
 }
 
+const Config *getConfig() { return m_config.get(); }
+
+void TelemetryManager::atDebuggerStartup(DebuggerTelemetryInfo *entry) {
+  UserIDResolver &resolver = lldb_private::HostInfo::GetUserIDResolver();
+  std::optional opt_username =
+  resolver.GetUserName(lldb_private::HostInfo::GetUserID());
+  if (opt_username)
+entry->username = *opt_username;
+
+  entry->lldb_git_sha =
+  lldb_private::GetVersion(); // TODO: find the real git sha?
+
+  entry->lldb_path = HostInfo::GetProgramFileSpec().GetPath();
+
+  llvm::SmallString<64> cwd;
+  if (!llvm::sys::fs::current_path(cwd)) {
+entry->cwd = cwd.c_str();
+  } else {
+MiscTelemetryInfo misc_info;
+misc_info.meta_data["internal_errors"] = "Cannot determine CWD";
+if (auto er = dispatch(&misc_info)) {
+  LLDB_LOG(GetLog(LLDBLog::Object),
+   "Failed to dispatch misc-info at startup");
+}
+  }
+
+  if (auto er = dispatch(entry)) {
+LLDB_LOG(GetLog(LLDBLog::Object), "Failed to dispatch entry at startup");
+  }
+}
+
+void TelemetryManager::atDebuggerExit(DebuggerTelemetryInfo *entry) {
+  // There must be a reference to the debugger at this point.
+  assert(entry->debugger != nullptr);
+
+  if (auto *selected_target =
+  entry->debugger->GetSelectedExecutionContext().GetTargetPtr()) {
+if (!selected_target->IsDummyTarget()) {
+  const lldb::ProcessSP proc = selected_target->GetProcessSP();
+  if (proc == nullptr) {
+// no process has been launched yet.
+entry->exit_desc = {-1, "no process launched."};
+  } else {
+entry->exit_desc = {proc->GetExitStatus(), ""};
+if (const char *description = proc->GetExitDescription())
+  entry->exit_desc->description = std::string(description);
+  }
+}
+  }

labath wrote:

> > There's no guarantee that Process object will  exist at the time the 
> > debugger terminates, or that it will have ran exactly once, etc.
> 
> Yes, that's why there' s a check - if there was no process when the debugger 
> terminates (ie `proc == nullptr`), then we reports taht no process has been 
> launched.

And if the process was launched twice this will get the last exit code -- is 
that what you want? What about the situation where the process is launched and 
then its target is deleted (`run; target delete; exit`)? Or when the process is 
launched, but then the user switches to debugging a different target (`run; 
target create/select; exit`)?

> 
> > If you're interesting in collecting the exit status, why not hook into a 
> > place that deals with that directly
> 
> This code is more interested in the exit of the _debugger_ (ie., did the 
> debugger exit normally or because some process crashed? or waht ...), which 
> is why the telemetry-callback is done from Debugger's cleanup code.

I don't see how one is relevant for the other. LLDB doesn't exit because the 
debugged process crashed. Any debuggee exit code is WAI as far as LLDB is 
concerne

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Pavel Labath via lldb-commits]

@@ -21,6 +21,8 @@ void TelemetryInfo::serialize(Serializer &serializer) const {
 }
 
 Error Manager::dispatch(TelemetryInfo *Entry) {
+  assert(Config::BuildTimeEnableTelemetry &&
+ "Telemetry should have been enabled");

labath wrote:

Won't this be a problem for unit tests? I see you're enabling them, but I 
suspect they won't actually succeed with this setting turned off..

https://github.com/llvm/llvm-project/pull/128534
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Pavel Labath via lldb-commits]

@@ -64,11 +64,19 @@ class Serializer {
 /// This struct can be extended as needed to add additional configuration
 /// points specific to a vendor's implementation.
 struct Config {
-  virtual ~Config() = default;
+#ifdef LLVM_ENABLE_TELEMETRY
+  static const bool BuildTimeEnableTelemetry = true;
+#else
+  static const bool BuildTimeEnableTelemetry = false;
+#endif

labath wrote:

.. then this can be 
```suggestion
  static const bool BuildTimeEnableTelemetry = LLVM_ENABLE_TELEMETRY;
```

https://github.com/llvm/llvm-project/pull/128534
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Pavel Labath via lldb-commits]

https://github.com/labath edited 
https://github.com/llvm/llvm-project/pull/128534
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Pavel Labath via lldb-commits]

@@ -202,6 +202,10 @@
 #cmakedefine LLVM_HAS_LOGF128
 
 /* Define if building LLVM with LLVM_BUILD_TELEMETRY */
+/* DEPRECATED - use LLVM_ENABLE_TELEMETRY */
 #cmakedefine LLVM_BUILD_TELEMETRY ${LLVM_BUILD_TELEMETRY}
 
+/* Define if building LLVM with LLVM_ENABLE_TELEMETRY */
+#cmakedefine LLVM_ENABLE_TELEMETRY ${LLVM_ENABLE_TELEMETRY}

labath wrote:

If you make this..
```suggestion
#cmakedefine01 LLVM_ENABLE_TELEMETRY ${LLVM_ENABLE_TELEMETRY}
```

https://github.com/llvm/llvm-project/pull/128534
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Pavel Labath via lldb-commits]

https://github.com/labath deleted 
https://github.com/llvm/llvm-project/pull/128534
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Pavel Labath via lldb-commits]

@@ -64,11 +64,19 @@ class Serializer {
 /// This struct can be extended as needed to add additional configuration
 /// points specific to a vendor's implementation.
 struct Config {
-  virtual ~Config() = default;
+#ifdef LLVM_ENABLE_TELEMETRY
+  static const bool BuildTimeEnableTelemetry = true;
+#else
+  static const bool BuildTimeEnableTelemetry = false;
+#endif

labath wrote:

I'd also make this constexpr for good measure

https://github.com/llvm/llvm-project/pull/128534
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Pavel Labath via lldb-commits]

https://github.com/labath commented:

For me the biggest question is what to do with tests when the setting is turned 
off, as this makes them run but not succeed. Not running them is one option, 
but maybe there's a way to disable this in a sufficiently decisive manner while 
leaving a back door open for tests? (question both for you and @JDevlieghere)

https://github.com/llvm/llvm-project/pull/128534
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [WIP] [lldb][TypeSystemClang] Create clang::SourceLocation from DWARF and attach to AST (PR #127829)

[Pavel Labath via lldb-commits]

labath wrote:

Yeah, I expect this could be a bit of a problem on several fronts:
- even if the memory usage is low enough to not matter, the fact that we have 
to hit the filesystem to load the file is most likely going to slow things 
down, particularly when you're using a remote filesystem (like we are). 
Normally, all one needs to parse debug info is the DWARF data, but now we would 
be fetching potentially thousands of files without actually using most most of 
them
- this setup also makes it hard to display the source files in the same way 
that the rest of lldb does. In particular the `target.source-map` setting is 
inaccessible to the debug info code, and if I'm not mistaken the source manager 
in lldb always takes the current value of the setting (whereas the mapping in 
the clang ast is fixed at construction time)
- mmapping the files (I suspect this is the reason this doesn't have a big 
impact on memory footprint) also has some unfortunate side effects on various 
systems. On windows, it prevents the file from being deleted (some editors 
implement "editing" as deleting and recreating a file), and on most unix 
systems (with the exception of Darwin which has `MAP_RESILIENT_MEDIA`) 
accessing the file after it has been truncated (which is the other way to 
implement "editing a file") can cause a SIGBUS.
- I wonder whether we can exhaust the SourceLocation space in this way (and 
what happens if we do). 4GB  sounds like a lot, but I know that some people 
manage to do that with just a single compile unit (I think it involves 
`#include`ing the same file repeatedly), so I wouldn't be surprised if we hit 
it after placing the entire large binary into a single AST context. (Maybe it's 
not as acute as we're not modelling `#include`s, but still..)

With the current implementation, I expect that we would need this to be 
controlled by some sort of a setting, although I'd really rather not do that. I 
wonder if we could cheat by pointing all files to some fake memory buffer 
(containing a bunch of newlines or something), and then hijacking the error 
printing logic to look up the "real" contents of the file (taking into account 
the current source map, file checksums and everything).

https://github.com/llvm/llvm-project/pull/127829
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Use existing lldb::IOObjectSP for DAP IO (NFC). (PR #128750)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere edited 
https://github.com/llvm/llvm-project/pull/128750
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Use existing lldb::IOObjectSP for DAP IO (NFC). (PR #128750)

[Jonas Devlieghere via lldb-commits]

@@ -17,6 +17,7 @@
 #include "lldb/API/SBListener.h"
 #include "lldb/API/SBProcess.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/Utility/IOObject.h" // IWYU pragma: keep

JDevlieghere wrote:

```suggestion
#include "lldb/Utility/IOObject.h"
```

https://github.com/llvm/llvm-project/pull/128750
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][HostInfoMacOSX] Search CommandLineTools directory when looking up SDK paths (PR #128712)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere edited 
https://github.com/llvm/llvm-project/pull/128712
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Modernize ABI-based unwind plan creation (PR #128505)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere approved this pull request.

LGTM. This is a lot cleaner and much less error prone. 

https://github.com/llvm/llvm-project/pull/128505
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Use existing lldb::IOObjectSP for DAP IO (NFC). (PR #128750)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere commented:

Is it worth keeping `InputStream` and `OutputStream` around? Can we use a 
`(Stream)File` directly? 

https://github.com/llvm/llvm-project/pull/128750
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Use existing lldb::IOObjectSP for DAP IO (NFC). (PR #128750)

[Jonas Devlieghere via lldb-commits]

@@ -7,125 +7,34 @@
 
//===--===//
 
 #include "IOStream.h"
+#include "lldb/Utility/IOObject.h" // IWYU pragma: keep

JDevlieghere wrote:

```suggestion
#include "lldb/Utility/IOObject.h"
```

https://github.com/llvm/llvm-project/pull/128750
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Use existing lldb::IOObjectSP for DAP IO (NFC). (PR #128750)

[John Harrison via lldb-commits]

https://github.com/ashgti updated 
https://github.com/llvm/llvm-project/pull/128750

>From b3f60a464deedae59d902f7417c74a64c5cbf5a1 Mon Sep 17 00:00:00 2001
From: John Harrison 
Date: Mon, 24 Feb 2025 15:07:13 -0800
Subject: [PATCH 1/3] [lldb-dap] Use existing lldb::IOObjectSP for IO (NFC).

This simplifies the IOStream.cpp implementation by building on top of the 
existing lldb::IOObjectSP.

Additionally, this should help ensure clients connected of a `--connection` 
specifier properly detect shutdown requests when the Socket is closed. 
Previously, the StreamDescriptor was just accessing the underyling native 
handle and was not aware of the `Close()` call to the Socket itself.
---
 lldb/tools/lldb-dap/DAP.cpp  |   3 +-
 lldb/tools/lldb-dap/DAP.h|   9 +--
 lldb/tools/lldb-dap/IOStream.cpp | 117 ---
 lldb/tools/lldb-dap/IOStream.h   |  38 ++
 lldb/tools/lldb-dap/lldb-dap.cpp |  36 +-
 5 files changed, 45 insertions(+), 158 deletions(-)

diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 01f294e14de6a..95f2e5c6521c2 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -17,6 +17,7 @@
 #include "lldb/API/SBListener.h"
 #include "lldb/API/SBProcess.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/Utility/IOObject.h" // IWYU pragma: keep
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h"
@@ -59,7 +60,7 @@ const char DEV_NULL[] = "/dev/null";
 namespace lldb_dap {
 
 DAP::DAP(std::string name, llvm::StringRef path, std::ofstream *log,
- StreamDescriptor input, StreamDescriptor output, ReplMode repl_mode,
+ lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode,
  std::vector pre_init_commands)
 : name(std::move(name)), debug_adaptor_path(path), log(log),
   input(std::move(input)), output(std::move(output)),
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index f87841a56f4d3..b4e77b78c5273 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -29,6 +29,7 @@
 #include "lldb/API/SBThread.h"
 #include "lldb/API/SBValue.h"
 #include "lldb/API/SBValueList.h"
+#include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
@@ -125,21 +126,21 @@ struct Variables {
 
 struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {};
+  explicit StartDebuggingRequestHandler(DAP &d) : dap(d){};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
  lldb::SBCommandReturnObject &result) override;
 };
 
 struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit ReplModeRequestHandler(DAP &d) : dap(d) {};
+  explicit ReplModeRequestHandler(DAP &d) : dap(d){};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
  lldb::SBCommandReturnObject &result) override;
 };
 
 struct SendEventRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit SendEventRequestHandler(DAP &d) : dap(d) {};
+  explicit SendEventRequestHandler(DAP &d) : dap(d){};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
  lldb::SBCommandReturnObject &result) override;
 };
@@ -211,7 +212,7 @@ struct DAP {
   std::string last_nonempty_var_expression;
 
   DAP(std::string name, llvm::StringRef path, std::ofstream *log,
-  StreamDescriptor input, StreamDescriptor output, ReplMode repl_mode,
+  lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode,
   std::vector pre_init_commands);
   ~DAP();
   DAP(const DAP &rhs) = delete;
diff --git a/lldb/tools/lldb-dap/IOStream.cpp b/lldb/tools/lldb-dap/IOStream.cpp
index 7d0f363440f53..d8543b560c050 100644
--- a/lldb/tools/lldb-dap/IOStream.cpp
+++ b/lldb/tools/lldb-dap/IOStream.cpp
@@ -7,125 +7,34 @@
 
//===--===//
 
 #include "IOStream.h"
+#include "lldb/Utility/IOObject.h" // IWYU pragma: keep
+#include "lldb/Utility/Status.h"
 #include 
 #include 
 
-#if defined(_WIN32)
-#include 
-#else
-#include 
-#include 
-#include 
-#endif
-
 using namespace lldb_dap;
 
-StreamDescriptor::StreamDescriptor() = default;
-
-StreamDescriptor::StreamDescriptor(StreamDescriptor &&other) {
-  *this = std::move(other);
-}
-
-StreamDescriptor::~StreamDescriptor() {
-  if (!m_close)
-return;
-
-  if (m_is_socket)
-#if defined(_WIN32)
-::closesocket(m_socket);
-#else
-::close(m_socket);
-#endif
-  else
-::close(m_fd);
-}
-
-StreamDescriptor &StreamDescriptor::operator=(StreamDescriptor &&other) {
-  m_close = other.m_close;
-  other.m_close = false;
-  m_is_socket = other.m_is_socket;
-  if (m_is_socket)
-m_socket = other.m_socket;
-  else
-m_fd = other.m_fd;
-  return *this;
-}
-
-StreamDescriptor Strea

[Lldb-commits] [lldb] [lldb] Add missing calls to SetThreadHitBreakpointSite/DetectThreadStoppedAtUnexecutedBP (PR #128726)

[Jason Molenda via lldb-commits]

https://github.com/jasonmolenda approved this pull request.

LGTM.

https://github.com/llvm/llvm-project/pull/128726
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [WIP] [lldb][TypeSystemClang] Create clang::SourceLocation from DWARF and attach to AST (PR #127829)

[via lldb-commits]

jimingham wrote:

Why do we need to touch source files to add a source file attribution from 
debug info to a declaration?  We don't require the actual presence of source 
files in order to make source file attributions anywhere else in the debug info 
handling.

Jim

> On Feb 25, 2025, at 2:06 AM, Pavel Labath ***@***.***> wrote:
> 
> 
> labath
>  left a comment 
> (llvm/llvm-project#127829)
> Yeah, I expect this could be a bit of a problem on several fronts:
> 
> even if the memory usage is low enough to not matter, the fact that we have 
> to hit the filesystem to load the file is most likely going to slow things 
> down, particularly when you're using a remote filesystem (like we are). 
> Normally, all one needs to parse debug info is the DWARF data, but now we 
> would be fetching potentially thousands of files without actually using most 
> most of them
> this setup also makes it hard to display the source files in the same way 
> that the rest of lldb does. In particular the target.source-map setting is 
> inaccessible to the debug info code, and if I'm not mistaken the source 
> manager in lldb always takes the current value of the setting (whereas the 
> mapping in the clang ast is fixed at construction time)
> mmapping the files (I suspect this is the reason this doesn't have a big 
> impact on memory footprint) also has some unfortunate side effects on various 
> systems. On windows, it prevents the file from being deleted (some editors 
> implement "editing" as deleting and recreating a file), and on most unix 
> systems (with the exception of Darwin which has MAP_RESILIENT_MEDIA) 
> accessing the file after it has been truncated (which is the other way to 
> implement "editing a file") can cause a SIGBUS.
> I wonder whether we can exhaust the SourceLocation space in this way (and 
> what happens if we do). 4GB sounds like a lot, but I know that some people 
> manage to do that with just a single compile unit (I think it involves 
> #includeing the same file repeatedly), so I wouldn't be surprised if we hit 
> it after placing the entire large binary into a single AST context. (Maybe 
> it's not as acute as we're not modelling #includes, but still..)
> With the current implementation, I expect that we would need this to be 
> controlled by some sort of a setting, although I'd really rather not do that. 
> I wonder if we could cheat by pointing all files to some fake memory buffer 
> (containing a bunch of newlines or something), and then hijacking the error 
> printing logic to look up the "real" contents of the file (taking into 
> account the current source map, file checksums and everything).
> 
> —
> Reply to this email directly, view it on GitHub, or unsubscribe.
> You are receiving this because you are on a team that was mentioned.
> 
>   
> 
> 
> labath
>  left a comment 
> (llvm/llvm-project#127829)
>  
> Yeah, I expect this could be a bit of a problem on several fronts:
> 
> even if the memory usage is low enough to not matter, the fact that we have 
> to hit the filesystem to load the file is most likely going to slow things 
> down, particularly when you're using a remote filesystem (like we are). 
> Normally, all one needs to parse debug info is the DWARF data, but now we 
> would be fetching potentially thousands of files without actually using most 
> most of them
> this setup also makes it hard to display the source files in the same way 
> that the rest of lldb does. In particular the target.source-map setting is 
> inaccessible to the debug info code, and if I'm not mistaken the source 
> manager in lldb always takes the current value of the setting (whereas the 
> mapping in the clang ast is fixed at construction time)
> mmapping the files (I suspect this is the reason this doesn't have a big 
> impact on memory footprint) also has some unfortunate side effects on various 
> systems. On windows, it prevents the file from being deleted (some editors 
> implement "editing" as deleting and recreating a file), and on most unix 
> systems (with the exception of Darwin which has MAP_RESILIENT_MEDIA) 
> accessing the file after it has been truncated (which is the other way to 
> implement "editing a file") can cause a SIGBUS.
> I wonder whether we can exhaust the SourceLocation space in this way (and 
> what happens if we do). 4GB sounds like a lot, but I know that some people 
> manage to do that with just a single compile unit (I think it involves 
> #includeing the same file repeatedly), so I wouldn't be surprised if we hit 
> it after placing the entire large binary into a single AST context. (Maybe 
> it's not as acute as we're not modelling #includes, but still..)
> With the c

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Vy Nguyen via lldb-commits]

https://github.com/oontvoo updated 
https://github.com/llvm/llvm-project/pull/128534

>From 236ab76b7c9433f8fed6586a4ee18351779cc191 Mon Sep 17 00:00:00 2001
From: Vy Nguyen 
Date: Mon, 24 Feb 2025 11:32:03 -0500
Subject: [PATCH 1/6] [llvm][telemetr]Change Telemetry-disabling mechanism.

Details:
- Previously, we used the LLVM_BUILD_TELEMETRY flag to control whether any 
Telemetry code will be built.
This has proven to cause more nuisance to both users of the Telemetry and any 
further extension of it.
(Eg., we needed to put #ifdef around caller/user code)

- So the new approach is to:
 + Remove this flag and introduce LLVM_ENABLE_TELEMETRY which would be true by 
default.
 + If LLVM_ENABLE_TELEMETRY is set to FALSE (at buildtime), the library would 
still be built BUT Telemetry cannot be enabled.
   And no data can be collected.

The benefit of this is that it simplifies user (and extension) code since we 
just need to put the check on Config::EnableTelemetry.
Besides, the Telemetry library itself is very small, hence the additional code 
to be built would not cause any difference in build performance.
---
 lldb/source/Core/CMakeLists.txt  |  4 +---
 lldb/source/Core/Telemetry.cpp   |  7 ++-
 lldb/unittests/Core/CMakeLists.txt   |  6 +++---
 llvm/CMakeLists.txt  |  3 ++-
 llvm/cmake/modules/LLVMConfig.cmake.in   |  1 +
 llvm/include/llvm/Config/llvm-config.h.cmake |  2 ++
 llvm/include/llvm/Telemetry/Telemetry.h  | 12 ++--
 llvm/lib/CMakeLists.txt  |  5 ++---
 llvm/unittests/CMakeLists.txt|  4 +---
 .../gn/secondary/llvm/include/llvm/Config/BUILD.gn   |  1 +
 utils/bazel/llvm_configs/llvm-config.h.cmake |  2 ++
 11 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index 82fb5f42f9f4b..a3c12e4c1bdbc 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -16,9 +16,7 @@ if (LLDB_ENABLE_CURSES)
   endif()
 endif()
 
-if (LLVM_BUILD_TELEMETRY)
-   set(TELEMETRY_DEPS Telemetry)
-endif()
+set(TELEMETRY_DEPS Telemetry)
 
 # TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore
 add_lldb_library(lldbCore
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 5222f76704f91..51a860ea5313b 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -8,8 +8,6 @@
 
 #include "llvm/Config/llvm-config.h"
 
-#ifdef LLVM_BUILD_TELEMETRY
-
 #include "lldb/Core/Telemetry.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Utility/LLDBLog.h"
@@ -57,7 +55,8 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) 
const {
 }
 
 TelemetryManager::TelemetryManager(std::unique_ptr config)
-: m_config(std::move(config)) {}
+: m_config(std::move(config))
+}
 
 llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
   // Do nothing for now.
@@ -75,5 +74,3 @@ void 
TelemetryManager::setInstance(std::unique_ptr manager) {
 
 } // namespace telemetry
 } // namespace lldb_private
-
-#endif // LLVM_BUILD_TELEMETRY
diff --git a/lldb/unittests/Core/CMakeLists.txt 
b/lldb/unittests/Core/CMakeLists.txt
index d4d3764b67ae3..e8df299631e2e 100644
--- a/lldb/unittests/Core/CMakeLists.txt
+++ b/lldb/unittests/Core/CMakeLists.txt
@@ -1,6 +1,6 @@
-if (LLVM_BUILD_TELEMETRY)
-  set(TELEMETRY_DEPS Telemetry)
-endif()
+
+set(TELEMETRY_DEPS Telemetry)
+
 
 add_lldb_unittest(LLDBCoreTests
   CommunicationTest.cpp
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 88512d0f1dd96..9188fb93b7846 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -835,7 +835,8 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm 
API documentation." OF
 option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF)
 option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON)
 option (LLVM_ENABLE_BINDINGS "Build bindings." ON)
-option (LLVM_BUILD_TELEMETRY "Build the telemetry library. This does not 
enable telemetry." ON)
+option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable 
the telemetry library. If set to OFF, library cannot be enabled after build 
(eg., at runtime)" ON)
+option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF, 
library cannot be enabled after build (eg., at runtime)" ON)
 
 set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html"
 CACHE STRING "Doxygen-generated HTML documentation install directory")
diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in 
b/llvm/cmake/modules/LLVMConfig.cmake.in
index 28655ee3ab87d..134d107ce79ba 100644
--- a/llvm/cmake/modules/LLVMConfig.cmake.in
+++ b/llvm/cmake/modules/LLVMConfig.cmake.in
@@ -101,6 +101,7 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@)
 set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@)
 
 set(LLVM_BUILD_TELEMETRY @LLVM_B

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Vy Nguyen via lldb-commits]

@@ -21,6 +21,8 @@ void TelemetryInfo::serialize(Serializer &serializer) const {
 }
 
 Error Manager::dispatch(TelemetryInfo *Entry) {
+  assert(Config::BuildTimeEnableTelemetry &&
+ "Telemetry should have been enabled");

oontvoo wrote:

Is there a way to say "if LLVM_ENABLE_TELEMETRY is not set, then skip the 
tests)?

https://github.com/llvm/llvm-project/pull/128534
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 568106c - [lldb][NFC] Fix comment in lldb/Core/ModuleList.h (#128602)

[via lldb-commits]

Author: Julian Lettner
Date: 2025-02-25T08:52:03-08:00
New Revision: 568106c2150f4442ad39d9c58493b962c87763bd

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

LOG: [lldb][NFC] Fix comment in lldb/Core/ModuleList.h (#128602)

Added: 


Modified: 
lldb/include/lldb/Core/ModuleList.h

Removed: 




diff  --git a/lldb/include/lldb/Core/ModuleList.h 
b/lldb/include/lldb/Core/ModuleList.h
index 29b87de88520d..909ee08f9ba62 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -326,11 +326,11 @@ class ModuleList {
   void FindGlobalVariables(const RegularExpression ®ex, size_t max_matches,
VariableList &variable_list) const;
 
-  /// Finds the first module whose file specification matches \a file_spec.
+  /// Finds modules whose file specification matches \a module_spec.
   ///
   /// \param[in] module_spec
   /// A file specification object to match against the Module's
-  /// file specifications. If \a file_spec does not have
+  /// file specifications. If \a module_spec does not have
   /// directory information, matches will occur by matching only
   /// the basename of any modules in this list. If this value is
   /// NULL, then file specifications won't be compared when
@@ -351,6 +351,7 @@ class ModuleList {
   // UUID values is very efficient and accurate.
   lldb::ModuleSP FindModule(const UUID &uuid) const;
 
+  /// Finds the first module whose file specification matches \a module_spec.
   lldb::ModuleSP FindFirstModule(const ModuleSpec &module_spec) const;
 
   void FindSymbolsWithNameAndType(ConstString name,



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

[Lldb-commits] [lldb] [lldb] Assorted improvements to the Pipe class (PR #128719)

[John Harrison via lldb-commits]

@@ -268,29 +267,24 @@ PipeWindows::GetReadNativeHandle() { return m_read; }
 HANDLE
 PipeWindows::GetWriteNativeHandle() { return m_write; }
 
-Status PipeWindows::ReadWithTimeout(void *buf, size_t size,
-const std::chrono::microseconds &duration,
-size_t &bytes_read) {
+llvm::Expected PipeWindows::Read(void *buf, size_t size,
+ const Timeout &timeout) {
   if (!CanRead())
-return Status(ERROR_INVALID_HANDLE, eErrorTypeWin32);
+return Status(ERROR_INVALID_HANDLE, eErrorTypeWin32).takeError();
 
-  bytes_read = 0;
-  DWORD sys_bytes_read = 0;
-  BOOL result =
-  ::ReadFile(m_read, buf, size, &sys_bytes_read, &m_read_overlapped);
-  if (result) {
-bytes_read = sys_bytes_read;
-return Status();
-  }
+  DWORD bytes_read = 0;
+  BOOL result = ::ReadFile(m_read, buf, size, &bytes_read, &m_read_overlapped);
+  if (result)
+return bytes_read;
 
   DWORD failure_error = ::GetLastError();
   if (failure_error != ERROR_IO_PENDING)
-return Status(failure_error, eErrorTypeWin32);
+return Status(failure_error, eErrorTypeWin32).takeError();
 
-  DWORD timeout = (duration == std::chrono::microseconds::zero())
-  ? INFINITE
-  : duration.count() / 1000;
-  DWORD wait_result = ::WaitForSingleObject(m_read_overlapped.hEvent, timeout);
+  DWORD timeout_msec =
+  timeout ? ceil(*timeout).count() : INFINITE;
+  DWORD wait_result =
+  ::WaitForSingleObject(m_read_overlapped.hEvent, timeout_msec);

ashgti wrote:

More for my understanding, but could we use `WaitForSingleObject` / 
`WaitForMultipleObjects` to emulate select on a FileHandle? That could make 
`SelectHelper` work on files and sockets on all platforms instead of just 
sockets on Windows.

https://github.com/llvm/llvm-project/pull/128719
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Assorted improvements to the Pipe class (PR #128719)

[John Harrison via lldb-commits]

@@ -300,70 +304,51 @@ void PipePosix::CloseWriteFileDescriptorUnlocked() {
   }
 }
 
-Status PipePosix::ReadWithTimeout(void *buf, size_t size,
-  const std::chrono::microseconds &timeout,
-  size_t &bytes_read) {
+llvm::Expected PipePosix::Read(void *buf, size_t size,
+   const Timeout &timeout) {
   std::lock_guard guard(m_read_mutex);
-  bytes_read = 0;
   if (!CanReadUnlocked())
-return Status(EINVAL, eErrorTypePOSIX);
+return llvm::errorCodeToError(
+std::make_error_code(std::errc::invalid_argument));
 
   const int fd = GetReadFileDescriptorUnlocked();
 
   SelectHelper select_helper;
-  select_helper.SetTimeout(timeout);
+  if (timeout)
+select_helper.SetTimeout(*timeout);
   select_helper.FDSetRead(fd);
 
-  Status error;
-  while (error.Success()) {
-error = select_helper.Select();
-if (error.Success()) {
-  auto result =
-  ::read(fd, static_cast(buf) + bytes_read, size - bytes_read);
-  if (result != -1) {
-bytes_read += result;
-if (bytes_read == size || result == 0)
-  break;
-  } else if (errno == EINTR) {
-continue;
-  } else {
-error = Status::FromErrno();
-break;
-  }
-}
-  }
-  return error;
+  if (llvm::Error error = select_helper.Select().takeError())
+return error;
+
+  ssize_t result = ::read(fd, buf, size);
+  if (result == -1)
+return llvm::errorCodeToError(
+std::error_code(errno, std::generic_category()));
+
+  return result;
 }
 
-Status PipePosix::WriteWithTimeout(const void *buf, size_t size,
-   const std::chrono::microseconds &timeout,
-   size_t &bytes_written) {
+llvm::Expected PipePosix::Write(const void *buf, size_t size,
+const Timeout &timeout) {
   std::lock_guard guard(m_write_mutex);
-  bytes_written = 0;
   if (!CanWriteUnlocked())
-return Status(EINVAL, eErrorTypePOSIX);
+return llvm::errorCodeToError(
+std::make_error_code(std::errc::invalid_argument));
 
   const int fd = GetWriteFileDescriptorUnlocked();
   SelectHelper select_helper;
-  select_helper.SetTimeout(timeout);
+  if (timeout)
+select_helper.SetTimeout(*timeout);
   select_helper.FDSetWrite(fd);
 
-  Status error;
-  while (error.Success()) {
-error = select_helper.Select();
-if (error.Success()) {
-  auto result = ::write(fd, static_cast(buf) + bytes_written,
-size - bytes_written);
-  if (result != -1) {
-bytes_written += result;
-if (bytes_written == size)
-  break;
-  } else if (errno == EINTR) {
-continue;
-  } else {
-error = Status::FromErrno();
-  }
-}
-  }
-  return error;
+  if (llvm::Error error = select_helper.Select().takeError())
+return error;
+
+  ssize_t result = ::write(fd, buf, size);
+  if (result == -1)
+return llvm::errorCodeToError(
+std::error_code(errno, std::generic_category()));

ashgti wrote:

Should we check for `EINTR`? (or RetryAfterSignal)?

https://github.com/llvm/llvm-project/pull/128719
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Assorted improvements to the Pipe class (PR #128719)

[John Harrison via lldb-commits]

@@ -1154,17 +1156,25 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
   if (socket_pipe.CanWrite())
 socket_pipe.CloseWriteFileDescriptor();
   if (socket_pipe.CanRead()) {
-// The port number may be up to "65535\0".
-char port_cstr[6] = {0};
-size_t num_bytes = sizeof(port_cstr);
 // Read port from pipe with 10 second timeout.
-error = socket_pipe.ReadWithTimeout(
-port_cstr, num_bytes, std::chrono::seconds{10}, num_bytes);
+std::string port_str;
+while (error.Success()) {
+  char buf[10];

ashgti wrote:

Does this need to be initialized to zero? `= {0};`?

https://github.com/llvm/llvm-project/pull/128719
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Assorted improvements to the Pipe class (PR #128719)

[John Harrison via lldb-commits]

@@ -300,70 +304,51 @@ void PipePosix::CloseWriteFileDescriptorUnlocked() {
   }
 }
 
-Status PipePosix::ReadWithTimeout(void *buf, size_t size,
-  const std::chrono::microseconds &timeout,
-  size_t &bytes_read) {
+llvm::Expected PipePosix::Read(void *buf, size_t size,
+   const Timeout &timeout) {
   std::lock_guard guard(m_read_mutex);
-  bytes_read = 0;
   if (!CanReadUnlocked())
-return Status(EINVAL, eErrorTypePOSIX);
+return llvm::errorCodeToError(
+std::make_error_code(std::errc::invalid_argument));
 
   const int fd = GetReadFileDescriptorUnlocked();
 
   SelectHelper select_helper;
-  select_helper.SetTimeout(timeout);
+  if (timeout)
+select_helper.SetTimeout(*timeout);
   select_helper.FDSetRead(fd);
 
-  Status error;
-  while (error.Success()) {
-error = select_helper.Select();
-if (error.Success()) {
-  auto result =
-  ::read(fd, static_cast(buf) + bytes_read, size - bytes_read);
-  if (result != -1) {
-bytes_read += result;
-if (bytes_read == size || result == 0)
-  break;
-  } else if (errno == EINTR) {
-continue;
-  } else {
-error = Status::FromErrno();
-break;
-  }
-}
-  }
-  return error;
+  if (llvm::Error error = select_helper.Select().takeError())
+return error;
+
+  ssize_t result = ::read(fd, buf, size);
+  if (result == -1)
+return llvm::errorCodeToError(
+std::error_code(errno, std::generic_category()));

ashgti wrote:

Should we check for EINTR? (or RetryAfterSignal)?

https://github.com/llvm/llvm-project/pull/128719
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][nfc] Improve duplicated code in unexecuted breakpoint detection (PR #128724)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Felipe de Azevedo Piovezan (felipepiovezan)


Changes

This code is replicated in multiple places, and a subsequent commit would 
introduce another copy of it in ThreadMemory.

---
Full diff: https://github.com/llvm/llvm-project/pull/128724.diff


5 Files Affected:

- (modified) lldb/include/lldb/Target/Thread.h (+6-5) 
- (modified) lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp 
(+1-5) 
- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+1-4) 
- (modified) lldb/source/Plugins/Process/scripted/ScriptedThread.cpp (+1-10) 
- (modified) lldb/source/Target/Thread.cpp (+13) 


``diff
diff --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Thread.h
index 1d1e3dcfc1dc6..d1988407965da 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -385,21 +385,22 @@ class Thread : public 
std::enable_shared_from_this,
   virtual void SetQueueLibdispatchQueueAddress(lldb::addr_t dispatch_queue_t) 
{}
 
   /// When a thread stops at an enabled BreakpointSite that has not executed,
-  /// the Process plugin should call SetThreadStoppedAtUnexecutedBP(pc).
+  /// the Process plugin should call DetectThreadStoppedAtUnexecutedBP().
   /// If that BreakpointSite was actually triggered (the instruction was
   /// executed, for a software breakpoint), regardless of whether the
   /// breakpoint is valid for this thread, SetThreadHitBreakpointSite()
   /// should be called to record that fact.
   ///
   /// Depending on the structure of the Process plugin, it may be easiest
-  /// to call SetThreadStoppedAtUnexecutedBP(pc) unconditionally when at
+  /// to call DetectThreadStoppedAtUnexecutedBP() unconditionally when at
   /// a BreakpointSite, and later when it is known that it was triggered,
   /// SetThreadHitBreakpointSite() can be called.  These two methods
   /// overwrite the same piece of state in the Thread, the last one
   /// called on a Thread wins.
-  void SetThreadStoppedAtUnexecutedBP(lldb::addr_t pc) {
-m_stopped_at_unexecuted_bp = pc;
-  }
+  ///
+  /// Returns the BreakpointSite this thread is stopped at, if any.
+  lldb::BreakpointSiteSP DetectThreadStoppedAtUnexecutedBP();
+
   void SetThreadHitBreakpointSite() {
 m_stopped_at_unexecuted_bp = LLDB_INVALID_ADDRESS;
   }
diff --git a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp 
b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
index 29a64a2a03bf0..daa7ccc2086c3 100644
--- a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
+++ b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
@@ -591,11 +591,7 @@ StopInfoSP 
StopInfoMachException::CreateStopReasonWithMachException(
   // breakpoint at 0x100.
   // The fact that the pc may be off by one at this point
   // (for an x86 KDP breakpoint hit) is not a problem.
-  addr_t pc = reg_ctx_sp->GetPC();
-  BreakpointSiteSP bp_site_sp =
-  process_sp->GetBreakpointSiteList().FindByAddress(pc);
-  if (bp_site_sp && bp_site_sp->IsEnabled())
-thread.SetThreadStoppedAtUnexecutedBP(pc);
+  BreakpointSiteSP bp_site_sp = thread.DetectThreadStoppedAtUnexecutedBP();
 
   switch (exc_type) {
   case 1: // EXC_BAD_ACCESS
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 8a8c0f92fbbc2..4ce543f8810a3 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1706,11 +1706,8 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
   if (!thread_sp->StopInfoIsUpToDate()) {
 thread_sp->SetStopInfo(StopInfoSP());
 
-addr_t pc = thread_sp->GetRegisterContext()->GetPC();
 BreakpointSiteSP bp_site_sp =
-thread_sp->GetProcess()->GetBreakpointSiteList().FindByAddress(pc);
-if (bp_site_sp && bp_site_sp->IsEnabled())
-  thread_sp->SetThreadStoppedAtUnexecutedBP(pc);
+thread_sp->DetectThreadStoppedAtUnexecutedBP();
 
 if (exc_type != 0) {
   // For thread plan async interrupt, creating stop info on the
diff --git a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp 
b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
index d0d1508e85172..12474402af909 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -229,16 +229,7 @@ bool ScriptedThread::CalculateStopInfo() {
 LLVM_PRETTY_FUNCTION, "Failed to get scripted thread stop info.", 
error,
 LLDBLog::Thread);
 
-  // If we're at a BreakpointSite, mark that we stopped there and
-  // need to hit the breakpoint when we resume.  This will be cleared
-  // if we CreateStopReasonWithBreakpointSiteID.
-  if (RegisterContextSP reg_ctx_sp = GetRegisterContext()) {
-addr_t pc = reg_ctx_sp->GetPC();
-if (BreakpointSiteSP bp_site_sp =
-GetProcess()->GetBreakpoi

[Lldb-commits] [lldb] [lldb]HostInfoMacOSX] Search CommandLineTools directory when looking up SDK paths (PR #128712)

[Jonas Devlieghere via lldb-commits]

@@ -15,11 +15,14 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Timer.h"
 
+#include "clang/Basic/DarwinSDKInfo.h"

JDevlieghere wrote:

Yeah, this goes against all the work of localizing the places we depend on 
clang. 

https://github.com/llvm/llvm-project/pull/128712
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFC] Fix comment in lldb/Core/ModuleList.h (PR #128602)

[Julian Lettner via lldb-commits]

https://github.com/yln closed https://github.com/llvm/llvm-project/pull/128602
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 83c6b1a - [lldb] Add fzf_history command to examples (#128571)

[via lldb-commits]

Author: Dave Lee
Date: 2025-02-25T08:10:09-08:00
New Revision: 83c6b1a88852ac6462e2ae58cb4e5ebdeb0eadd3

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

LOG: [lldb] Add fzf_history command to examples (#128571)

Adds a `fzf_history` to the examples directory.

This python command invokes [fzf](https://github.com/junegunn/fzf) to
select from lldb's command history.

Tighter integration is available on macOS, via commands for copy and
paste. The user's chosen history entry back is pasted into the lldb
console (via AppleScript). By pasting it, users have the opportunity to
edit it before running it. This matches how fzf's history search works.

Without copy and paste, the user's chosen history entry is printed to
screen and then run automatically.

Added: 
lldb/examples/python/fzf_history.py

Modified: 


Removed: 




diff  --git a/lldb/examples/python/fzf_history.py 
b/lldb/examples/python/fzf_history.py
new file mode 100644
index 0..546edb2ed2712
--- /dev/null
+++ b/lldb/examples/python/fzf_history.py
@@ -0,0 +1,110 @@
+import os
+import re
+import sys
+import subprocess
+import tempfile
+
+import lldb
+
+
+@lldb.command()
+def fzf_history(debugger, cmdstr, ctx, result, _):
+"""Use fzf to search and select from lldb command history."""
+history_file = os.path.expanduser("~/.lldb/lldb-widehistory")
+if not os.path.exists(history_file):
+result.SetError("history file does not exist")
+return
+history = _load_history(history_file)
+
+if sys.platform != "darwin":
+# The ability to integrate fzf's result into lldb uses copy and paste.
+# In absense of copy and paste, run the selected command directly.
+temp_file = tempfile.NamedTemporaryFile("r")
+fzf_command = (
+"fzf",
+"--no-sort",
+f"--query={cmdstr}",
+f"--bind=enter:execute-silent(echo -n {{}} > 
{temp_file.name})+accept",
+)
+subprocess.run(fzf_command, input=history, text=True)
+command = temp_file.read()
+debugger.HandleCommand(command)
+return
+
+# Capture the current pasteboard contents to restore after overwriting.
+paste_snapshot = subprocess.run("pbpaste", text=True, 
capture_output=True).stdout
+
+# On enter, copy the selected history entry into the pasteboard.
+fzf_command = (
+"fzf",
+"--no-sort",
+f"--query={cmdstr}",
+"--bind=enter:execute-silent(echo -n {} | pbcopy)+close",
+)
+completed = subprocess.run(fzf_command, input=history, text=True)
+# 130 is used for CTRL-C or ESC.
+if completed.returncode not in (0, 130):
+result.SetError("fzf failed")
+return
+
+# Get the user's selected history entry.
+selected_command = subprocess.run("pbpaste", text=True, 
capture_output=True).stdout
+if selected_command == paste_snapshot:
+# Nothing was selected, no cleanup needed.
+return
+
+_handle_command(debugger, selected_command)
+
+# Restore the pasteboard's contents.
+subprocess.run("pbcopy", input=paste_snapshot, text=True)
+
+
+def _handle_command(debugger, command):
+"""Try pasting the command, and failing that, run it directly."""
+if not command:
+return
+
+# Use applescript to paste the selected result into lldb's console.
+paste_command = (
+"osascript",
+"-e",
+'tell application "System Events" to keystroke "v" using command down',
+)
+completed = subprocess.run(paste_command, capture_output=True)
+
+if completed.returncode != 0:
+# The above applescript requires the "control your computer" 
permission.
+# Settings > Private & Security > Accessibility
+# If not enabled, fallback to running the command.
+debugger.HandleCommand(command)
+
+
+def _load_history(history_file):
+"""Load, decode, parse, and prepare an lldb history file for fzf."""
+with open(history_file) as f:
+history_contents = f.read()
+
+history_decoded = re.sub(r"\\0([0-7][0-7])", _decode_char, 
history_contents)
+history_lines = history_decoded.splitlines()
+
+# Skip the header line (_HiStOrY_V2_)
+del history_lines[0]
+# Reverse to show latest first.
+history_lines.reverse()
+
+history_commands = []
+history_seen = set()
+for line in history_lines:
+line = line.strip()
+# Skip empty lines, single character commands, and duplicates.
+if line and len(line) > 1 and line not in history_seen:
+history_commands.append(line)
+history_seen.add(line)
+
+return "\n".join(history_commands)
+
+
+def _decode_char(match):
+"""Decode octal strings ('\0NN') into a single charact

[Lldb-commits] [lldb] [lldb-dap] Use existing lldb::IOObjectSP for DAP IO (NFC). (PR #128750)

[John Harrison via lldb-commits]

https://github.com/ashgti updated 
https://github.com/llvm/llvm-project/pull/128750

>From b3f60a464deedae59d902f7417c74a64c5cbf5a1 Mon Sep 17 00:00:00 2001
From: John Harrison 
Date: Mon, 24 Feb 2025 15:07:13 -0800
Subject: [PATCH 1/2] [lldb-dap] Use existing lldb::IOObjectSP for IO (NFC).

This simplifies the IOStream.cpp implementation by building on top of the 
existing lldb::IOObjectSP.

Additionally, this should help ensure clients connected of a `--connection` 
specifier properly detect shutdown requests when the Socket is closed. 
Previously, the StreamDescriptor was just accessing the underyling native 
handle and was not aware of the `Close()` call to the Socket itself.
---
 lldb/tools/lldb-dap/DAP.cpp  |   3 +-
 lldb/tools/lldb-dap/DAP.h|   9 +--
 lldb/tools/lldb-dap/IOStream.cpp | 117 ---
 lldb/tools/lldb-dap/IOStream.h   |  38 ++
 lldb/tools/lldb-dap/lldb-dap.cpp |  36 +-
 5 files changed, 45 insertions(+), 158 deletions(-)

diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 01f294e14de6a..95f2e5c6521c2 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -17,6 +17,7 @@
 #include "lldb/API/SBListener.h"
 #include "lldb/API/SBProcess.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/Utility/IOObject.h" // IWYU pragma: keep
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h"
@@ -59,7 +60,7 @@ const char DEV_NULL[] = "/dev/null";
 namespace lldb_dap {
 
 DAP::DAP(std::string name, llvm::StringRef path, std::ofstream *log,
- StreamDescriptor input, StreamDescriptor output, ReplMode repl_mode,
+ lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode,
  std::vector pre_init_commands)
 : name(std::move(name)), debug_adaptor_path(path), log(log),
   input(std::move(input)), output(std::move(output)),
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index f87841a56f4d3..b4e77b78c5273 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -29,6 +29,7 @@
 #include "lldb/API/SBThread.h"
 #include "lldb/API/SBValue.h"
 #include "lldb/API/SBValueList.h"
+#include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
@@ -125,21 +126,21 @@ struct Variables {
 
 struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {};
+  explicit StartDebuggingRequestHandler(DAP &d) : dap(d){};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
  lldb::SBCommandReturnObject &result) override;
 };
 
 struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit ReplModeRequestHandler(DAP &d) : dap(d) {};
+  explicit ReplModeRequestHandler(DAP &d) : dap(d){};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
  lldb::SBCommandReturnObject &result) override;
 };
 
 struct SendEventRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit SendEventRequestHandler(DAP &d) : dap(d) {};
+  explicit SendEventRequestHandler(DAP &d) : dap(d){};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
  lldb::SBCommandReturnObject &result) override;
 };
@@ -211,7 +212,7 @@ struct DAP {
   std::string last_nonempty_var_expression;
 
   DAP(std::string name, llvm::StringRef path, std::ofstream *log,
-  StreamDescriptor input, StreamDescriptor output, ReplMode repl_mode,
+  lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode,
   std::vector pre_init_commands);
   ~DAP();
   DAP(const DAP &rhs) = delete;
diff --git a/lldb/tools/lldb-dap/IOStream.cpp b/lldb/tools/lldb-dap/IOStream.cpp
index 7d0f363440f53..d8543b560c050 100644
--- a/lldb/tools/lldb-dap/IOStream.cpp
+++ b/lldb/tools/lldb-dap/IOStream.cpp
@@ -7,125 +7,34 @@
 
//===--===//
 
 #include "IOStream.h"
+#include "lldb/Utility/IOObject.h" // IWYU pragma: keep
+#include "lldb/Utility/Status.h"
 #include 
 #include 
 
-#if defined(_WIN32)
-#include 
-#else
-#include 
-#include 
-#include 
-#endif
-
 using namespace lldb_dap;
 
-StreamDescriptor::StreamDescriptor() = default;
-
-StreamDescriptor::StreamDescriptor(StreamDescriptor &&other) {
-  *this = std::move(other);
-}
-
-StreamDescriptor::~StreamDescriptor() {
-  if (!m_close)
-return;
-
-  if (m_is_socket)
-#if defined(_WIN32)
-::closesocket(m_socket);
-#else
-::close(m_socket);
-#endif
-  else
-::close(m_fd);
-}
-
-StreamDescriptor &StreamDescriptor::operator=(StreamDescriptor &&other) {
-  m_close = other.m_close;
-  other.m_close = false;
-  m_is_socket = other.m_is_socket;
-  if (m_is_socket)
-m_socket = other.m_socket;
-  else
-m_fd = other.m_fd;
-  return *this;
-}
-
-StreamDescriptor Strea

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Vy Nguyen via lldb-commits]

https://github.com/oontvoo updated 
https://github.com/llvm/llvm-project/pull/128534

>From 236ab76b7c9433f8fed6586a4ee18351779cc191 Mon Sep 17 00:00:00 2001
From: Vy Nguyen 
Date: Mon, 24 Feb 2025 11:32:03 -0500
Subject: [PATCH 1/9] [llvm][telemetr]Change Telemetry-disabling mechanism.

Details:
- Previously, we used the LLVM_BUILD_TELEMETRY flag to control whether any 
Telemetry code will be built.
This has proven to cause more nuisance to both users of the Telemetry and any 
further extension of it.
(Eg., we needed to put #ifdef around caller/user code)

- So the new approach is to:
 + Remove this flag and introduce LLVM_ENABLE_TELEMETRY which would be true by 
default.
 + If LLVM_ENABLE_TELEMETRY is set to FALSE (at buildtime), the library would 
still be built BUT Telemetry cannot be enabled.
   And no data can be collected.

The benefit of this is that it simplifies user (and extension) code since we 
just need to put the check on Config::EnableTelemetry.
Besides, the Telemetry library itself is very small, hence the additional code 
to be built would not cause any difference in build performance.
---
 lldb/source/Core/CMakeLists.txt  |  4 +---
 lldb/source/Core/Telemetry.cpp   |  7 ++-
 lldb/unittests/Core/CMakeLists.txt   |  6 +++---
 llvm/CMakeLists.txt  |  3 ++-
 llvm/cmake/modules/LLVMConfig.cmake.in   |  1 +
 llvm/include/llvm/Config/llvm-config.h.cmake |  2 ++
 llvm/include/llvm/Telemetry/Telemetry.h  | 12 ++--
 llvm/lib/CMakeLists.txt  |  5 ++---
 llvm/unittests/CMakeLists.txt|  4 +---
 .../gn/secondary/llvm/include/llvm/Config/BUILD.gn   |  1 +
 utils/bazel/llvm_configs/llvm-config.h.cmake |  2 ++
 11 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index 82fb5f42f9f4b..a3c12e4c1bdbc 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -16,9 +16,7 @@ if (LLDB_ENABLE_CURSES)
   endif()
 endif()
 
-if (LLVM_BUILD_TELEMETRY)
-   set(TELEMETRY_DEPS Telemetry)
-endif()
+set(TELEMETRY_DEPS Telemetry)
 
 # TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore
 add_lldb_library(lldbCore
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 5222f76704f91..51a860ea5313b 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -8,8 +8,6 @@
 
 #include "llvm/Config/llvm-config.h"
 
-#ifdef LLVM_BUILD_TELEMETRY
-
 #include "lldb/Core/Telemetry.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Utility/LLDBLog.h"
@@ -57,7 +55,8 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) 
const {
 }
 
 TelemetryManager::TelemetryManager(std::unique_ptr config)
-: m_config(std::move(config)) {}
+: m_config(std::move(config))
+}
 
 llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
   // Do nothing for now.
@@ -75,5 +74,3 @@ void 
TelemetryManager::setInstance(std::unique_ptr manager) {
 
 } // namespace telemetry
 } // namespace lldb_private
-
-#endif // LLVM_BUILD_TELEMETRY
diff --git a/lldb/unittests/Core/CMakeLists.txt 
b/lldb/unittests/Core/CMakeLists.txt
index d4d3764b67ae3..e8df299631e2e 100644
--- a/lldb/unittests/Core/CMakeLists.txt
+++ b/lldb/unittests/Core/CMakeLists.txt
@@ -1,6 +1,6 @@
-if (LLVM_BUILD_TELEMETRY)
-  set(TELEMETRY_DEPS Telemetry)
-endif()
+
+set(TELEMETRY_DEPS Telemetry)
+
 
 add_lldb_unittest(LLDBCoreTests
   CommunicationTest.cpp
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 88512d0f1dd96..9188fb93b7846 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -835,7 +835,8 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm 
API documentation." OF
 option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF)
 option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON)
 option (LLVM_ENABLE_BINDINGS "Build bindings." ON)
-option (LLVM_BUILD_TELEMETRY "Build the telemetry library. This does not 
enable telemetry." ON)
+option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable 
the telemetry library. If set to OFF, library cannot be enabled after build 
(eg., at runtime)" ON)
+option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF, 
library cannot be enabled after build (eg., at runtime)" ON)
 
 set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html"
 CACHE STRING "Doxygen-generated HTML documentation install directory")
diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in 
b/llvm/cmake/modules/LLVMConfig.cmake.in
index 28655ee3ab87d..134d107ce79ba 100644
--- a/llvm/cmake/modules/LLVMConfig.cmake.in
+++ b/llvm/cmake/modules/LLVMConfig.cmake.in
@@ -101,6 +101,7 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@)
 set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@)
 
 set(LLVM_BUILD_TELEMETRY @LLVM_B

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Vy Nguyen via lldb-commits]

@@ -21,6 +21,8 @@ void TelemetryInfo::serialize(Serializer &serializer) const {
 }
 
 Error Manager::dispatch(TelemetryInfo *Entry) {
+  assert(Config::BuildTimeEnableTelemetry &&
+ "Telemetry should have been enabled");

oontvoo wrote:

P.S: How about this? Adding GTEST_SKIP if the static flag is false.


https://github.com/llvm/llvm-project/pull/128534
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Vy Nguyen via lldb-commits]

https://github.com/oontvoo updated 
https://github.com/llvm/llvm-project/pull/128534

>From 236ab76b7c9433f8fed6586a4ee18351779cc191 Mon Sep 17 00:00:00 2001
From: Vy Nguyen 
Date: Mon, 24 Feb 2025 11:32:03 -0500
Subject: [PATCH 01/10] [llvm][telemetr]Change Telemetry-disabling mechanism.

Details:
- Previously, we used the LLVM_BUILD_TELEMETRY flag to control whether any 
Telemetry code will be built.
This has proven to cause more nuisance to both users of the Telemetry and any 
further extension of it.
(Eg., we needed to put #ifdef around caller/user code)

- So the new approach is to:
 + Remove this flag and introduce LLVM_ENABLE_TELEMETRY which would be true by 
default.
 + If LLVM_ENABLE_TELEMETRY is set to FALSE (at buildtime), the library would 
still be built BUT Telemetry cannot be enabled.
   And no data can be collected.

The benefit of this is that it simplifies user (and extension) code since we 
just need to put the check on Config::EnableTelemetry.
Besides, the Telemetry library itself is very small, hence the additional code 
to be built would not cause any difference in build performance.
---
 lldb/source/Core/CMakeLists.txt  |  4 +---
 lldb/source/Core/Telemetry.cpp   |  7 ++-
 lldb/unittests/Core/CMakeLists.txt   |  6 +++---
 llvm/CMakeLists.txt  |  3 ++-
 llvm/cmake/modules/LLVMConfig.cmake.in   |  1 +
 llvm/include/llvm/Config/llvm-config.h.cmake |  2 ++
 llvm/include/llvm/Telemetry/Telemetry.h  | 12 ++--
 llvm/lib/CMakeLists.txt  |  5 ++---
 llvm/unittests/CMakeLists.txt|  4 +---
 .../gn/secondary/llvm/include/llvm/Config/BUILD.gn   |  1 +
 utils/bazel/llvm_configs/llvm-config.h.cmake |  2 ++
 11 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index 82fb5f42f9f4b..a3c12e4c1bdbc 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -16,9 +16,7 @@ if (LLDB_ENABLE_CURSES)
   endif()
 endif()
 
-if (LLVM_BUILD_TELEMETRY)
-   set(TELEMETRY_DEPS Telemetry)
-endif()
+set(TELEMETRY_DEPS Telemetry)
 
 # TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore
 add_lldb_library(lldbCore
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 5222f76704f91..51a860ea5313b 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -8,8 +8,6 @@
 
 #include "llvm/Config/llvm-config.h"
 
-#ifdef LLVM_BUILD_TELEMETRY
-
 #include "lldb/Core/Telemetry.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Utility/LLDBLog.h"
@@ -57,7 +55,8 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) 
const {
 }
 
 TelemetryManager::TelemetryManager(std::unique_ptr config)
-: m_config(std::move(config)) {}
+: m_config(std::move(config))
+}
 
 llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
   // Do nothing for now.
@@ -75,5 +74,3 @@ void 
TelemetryManager::setInstance(std::unique_ptr manager) {
 
 } // namespace telemetry
 } // namespace lldb_private
-
-#endif // LLVM_BUILD_TELEMETRY
diff --git a/lldb/unittests/Core/CMakeLists.txt 
b/lldb/unittests/Core/CMakeLists.txt
index d4d3764b67ae3..e8df299631e2e 100644
--- a/lldb/unittests/Core/CMakeLists.txt
+++ b/lldb/unittests/Core/CMakeLists.txt
@@ -1,6 +1,6 @@
-if (LLVM_BUILD_TELEMETRY)
-  set(TELEMETRY_DEPS Telemetry)
-endif()
+
+set(TELEMETRY_DEPS Telemetry)
+
 
 add_lldb_unittest(LLDBCoreTests
   CommunicationTest.cpp
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 88512d0f1dd96..9188fb93b7846 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -835,7 +835,8 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm 
API documentation." OF
 option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF)
 option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON)
 option (LLVM_ENABLE_BINDINGS "Build bindings." ON)
-option (LLVM_BUILD_TELEMETRY "Build the telemetry library. This does not 
enable telemetry." ON)
+option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable 
the telemetry library. If set to OFF, library cannot be enabled after build 
(eg., at runtime)" ON)
+option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF, 
library cannot be enabled after build (eg., at runtime)" ON)
 
 set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html"
 CACHE STRING "Doxygen-generated HTML documentation install directory")
diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in 
b/llvm/cmake/modules/LLVMConfig.cmake.in
index 28655ee3ab87d..134d107ce79ba 100644
--- a/llvm/cmake/modules/LLVMConfig.cmake.in
+++ b/llvm/cmake/modules/LLVMConfig.cmake.in
@@ -101,6 +101,7 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@)
 set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@)
 
 set(LLVM_BUILD_TELEMETRY @LLVM

[Lldb-commits] [lldb] [lldb] Assorted improvements to the Pipe class (PR #128719)

[John Harrison via lldb-commits]

@@ -55,8 +55,6 @@ TEST_F(PipeTest, OpenAsReader) {
 }
 #endif
 
-// This test is flaky on Windows on Arm.
-#ifndef _WIN32
 TEST_F(PipeTest, WriteWithTimeout) {

ashgti wrote:

Should we add some tests for the Reader as well?

Something along the lines of:

```
* Read(buf[100], timeout=10s)
* Write("hello world") (under the buffer size)
* AssertEqual(buf, "hello world")

* Read(buf[100], timeout=10s)
* Write("hello world" * 200) (over the buffer size)
* AssertEqual(buf, ("hello world" * 200).slice(0, 100))
* Read(buf[100]) // read the rest
```

https://github.com/llvm/llvm-project/pull/128719
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb]HostInfoMacOSX] Search CommandLineTools directory when looking up SDK paths (PR #128712)

[Jonas Devlieghere via lldb-commits]

@@ -581,6 +626,21 @@ static bool ResolveAndVerifyCandidateSupportDir(FileSpec 
&path) {
"XcodeSDK not specified");
   XcodeSDK sdk = *options.XcodeSDKSelection;
   auto key = sdk.GetString();
+
+  // xcrun doesn't search SDKs in the CommandLineTools (CLT) directory. So if
+  // a program was compiled against a CLT SDK, but that SDK wasn't present in
+  // any of the Xcode installations, then xcrun would fail to find the SDK
+  // (which is expensive). To avoid this we first try to find the specified SDK
+  // in the CLT directory.
+  auto clt_root_dir = find_cached_path(g_sdk_path, g_sdk_path_mutex, key, [&] {
+return GetCommandLineToolsSDKRoot(sdk.GetVersion());

JDevlieghere wrote:

Yeah, you don't know you need the CLT until you know it's not in Xcode, which 
is exactly the problem this is trying to avoid. 

https://github.com/llvm/llvm-project/pull/128712
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Use existing lldb::IOObjectSP for DAP IO (NFC). (PR #128750)

[John Harrison via lldb-commits]

https://github.com/ashgti ready_for_review 
https://github.com/llvm/llvm-project/pull/128750
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Use existing lldb::IOObjectSP for DAP IO (NFC). (PR #128750)

[John Harrison via lldb-commits]

https://github.com/ashgti created 
https://github.com/llvm/llvm-project/pull/128750

This simplifies the IOStream.cpp implementation by building on top of the 
existing lldb::IOObjectSP.

Additionally, this should help ensure clients connected of a `--connection` 
specifier properly detect shutdown requests when the Socket is closed. 
Previously, the StreamDescriptor was just accessing the underlying native 
handle and was not aware of the `Close()` call to the Socket itself.

This is both nice to have for simplifying the existing code and this unblocks 
an upcoming refactor to support the cancel request.

>From b3f60a464deedae59d902f7417c74a64c5cbf5a1 Mon Sep 17 00:00:00 2001
From: John Harrison 
Date: Mon, 24 Feb 2025 15:07:13 -0800
Subject: [PATCH] [lldb-dap] Use existing lldb::IOObjectSP for IO (NFC).

This simplifies the IOStream.cpp implementation by building on top of the 
existing lldb::IOObjectSP.

Additionally, this should help ensure clients connected of a `--connection` 
specifier properly detect shutdown requests when the Socket is closed. 
Previously, the StreamDescriptor was just accessing the underyling native 
handle and was not aware of the `Close()` call to the Socket itself.
---
 lldb/tools/lldb-dap/DAP.cpp  |   3 +-
 lldb/tools/lldb-dap/DAP.h|   9 +--
 lldb/tools/lldb-dap/IOStream.cpp | 117 ---
 lldb/tools/lldb-dap/IOStream.h   |  38 ++
 lldb/tools/lldb-dap/lldb-dap.cpp |  36 +-
 5 files changed, 45 insertions(+), 158 deletions(-)

diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 01f294e14de6a..95f2e5c6521c2 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -17,6 +17,7 @@
 #include "lldb/API/SBListener.h"
 #include "lldb/API/SBProcess.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/Utility/IOObject.h" // IWYU pragma: keep
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h"
@@ -59,7 +60,7 @@ const char DEV_NULL[] = "/dev/null";
 namespace lldb_dap {
 
 DAP::DAP(std::string name, llvm::StringRef path, std::ofstream *log,
- StreamDescriptor input, StreamDescriptor output, ReplMode repl_mode,
+ lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode,
  std::vector pre_init_commands)
 : name(std::move(name)), debug_adaptor_path(path), log(log),
   input(std::move(input)), output(std::move(output)),
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index f87841a56f4d3..b4e77b78c5273 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -29,6 +29,7 @@
 #include "lldb/API/SBThread.h"
 #include "lldb/API/SBValue.h"
 #include "lldb/API/SBValueList.h"
+#include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
@@ -125,21 +126,21 @@ struct Variables {
 
 struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {};
+  explicit StartDebuggingRequestHandler(DAP &d) : dap(d){};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
  lldb::SBCommandReturnObject &result) override;
 };
 
 struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit ReplModeRequestHandler(DAP &d) : dap(d) {};
+  explicit ReplModeRequestHandler(DAP &d) : dap(d){};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
  lldb::SBCommandReturnObject &result) override;
 };
 
 struct SendEventRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit SendEventRequestHandler(DAP &d) : dap(d) {};
+  explicit SendEventRequestHandler(DAP &d) : dap(d){};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
  lldb::SBCommandReturnObject &result) override;
 };
@@ -211,7 +212,7 @@ struct DAP {
   std::string last_nonempty_var_expression;
 
   DAP(std::string name, llvm::StringRef path, std::ofstream *log,
-  StreamDescriptor input, StreamDescriptor output, ReplMode repl_mode,
+  lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode,
   std::vector pre_init_commands);
   ~DAP();
   DAP(const DAP &rhs) = delete;
diff --git a/lldb/tools/lldb-dap/IOStream.cpp b/lldb/tools/lldb-dap/IOStream.cpp
index 7d0f363440f53..d8543b560c050 100644
--- a/lldb/tools/lldb-dap/IOStream.cpp
+++ b/lldb/tools/lldb-dap/IOStream.cpp
@@ -7,125 +7,34 @@
 
//===--===//
 
 #include "IOStream.h"
+#include "lldb/Utility/IOObject.h" // IWYU pragma: keep
+#include "lldb/Utility/Status.h"
 #include 
 #include 
 
-#if defined(_WIN32)
-#include 
-#else
-#include 
-#include 
-#include 
-#endif
-
 using namespace lldb_dap;
 
-StreamDescriptor::StreamDescriptor() = default;
-
-StreamDescriptor::StreamDescriptor(StreamDescriptor &&other) {
-  *this = std::mov

[Lldb-commits] [lldb] [lldb-dap] Refactor reverse request response handlers (NFC) (PR #128594)

[John Harrison via lldb-commits]

https://github.com/ashgti approved this pull request.

LGTM, this looks much nicer and consistent with the other RequestHandlers.

https://github.com/llvm/llvm-project/pull/128594
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Use existing lldb::IOObjectSP for DAP IO (NFC). (PR #128750)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: John Harrison (ashgti)


Changes

This simplifies the IOStream.cpp implementation by building on top of the 
existing lldb::IOObjectSP.

Additionally, this should help ensure clients connected of a `--connection` 
specifier properly detect shutdown requests when the Socket is closed. 
Previously, the StreamDescriptor was just accessing the underlying native 
handle and was not aware of the `Close()` call to the Socket itself.

This is both nice to have for simplifying the existing code and this unblocks 
an upcoming refactor to support the cancel request.

---
Full diff: https://github.com/llvm/llvm-project/pull/128750.diff


5 Files Affected:

- (modified) lldb/tools/lldb-dap/DAP.cpp (+2-1) 
- (modified) lldb/tools/lldb-dap/DAP.h (+5-4) 
- (modified) lldb/tools/lldb-dap/IOStream.cpp (+13-104) 
- (modified) lldb/tools/lldb-dap/IOStream.h (+5-33) 
- (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+20-16) 


``diff
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 01f294e14de6a..95f2e5c6521c2 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -17,6 +17,7 @@
 #include "lldb/API/SBListener.h"
 #include "lldb/API/SBProcess.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/Utility/IOObject.h" // IWYU pragma: keep
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h"
@@ -59,7 +60,7 @@ const char DEV_NULL[] = "/dev/null";
 namespace lldb_dap {
 
 DAP::DAP(std::string name, llvm::StringRef path, std::ofstream *log,
- StreamDescriptor input, StreamDescriptor output, ReplMode repl_mode,
+ lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode,
  std::vector pre_init_commands)
 : name(std::move(name)), debug_adaptor_path(path), log(log),
   input(std::move(input)), output(std::move(output)),
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index f87841a56f4d3..b4e77b78c5273 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -29,6 +29,7 @@
 #include "lldb/API/SBThread.h"
 #include "lldb/API/SBValue.h"
 #include "lldb/API/SBValueList.h"
+#include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
@@ -125,21 +126,21 @@ struct Variables {
 
 struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {};
+  explicit StartDebuggingRequestHandler(DAP &d) : dap(d){};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
  lldb::SBCommandReturnObject &result) override;
 };
 
 struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit ReplModeRequestHandler(DAP &d) : dap(d) {};
+  explicit ReplModeRequestHandler(DAP &d) : dap(d){};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
  lldb::SBCommandReturnObject &result) override;
 };
 
 struct SendEventRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit SendEventRequestHandler(DAP &d) : dap(d) {};
+  explicit SendEventRequestHandler(DAP &d) : dap(d){};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
  lldb::SBCommandReturnObject &result) override;
 };
@@ -211,7 +212,7 @@ struct DAP {
   std::string last_nonempty_var_expression;
 
   DAP(std::string name, llvm::StringRef path, std::ofstream *log,
-  StreamDescriptor input, StreamDescriptor output, ReplMode repl_mode,
+  lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode,
   std::vector pre_init_commands);
   ~DAP();
   DAP(const DAP &rhs) = delete;
diff --git a/lldb/tools/lldb-dap/IOStream.cpp b/lldb/tools/lldb-dap/IOStream.cpp
index 7d0f363440f53..d8543b560c050 100644
--- a/lldb/tools/lldb-dap/IOStream.cpp
+++ b/lldb/tools/lldb-dap/IOStream.cpp
@@ -7,125 +7,34 @@
 
//===--===//
 
 #include "IOStream.h"
+#include "lldb/Utility/IOObject.h" // IWYU pragma: keep
+#include "lldb/Utility/Status.h"
 #include 
 #include 
 
-#if defined(_WIN32)
-#include 
-#else
-#include 
-#include 
-#include 
-#endif
-
 using namespace lldb_dap;
 
-StreamDescriptor::StreamDescriptor() = default;
-
-StreamDescriptor::StreamDescriptor(StreamDescriptor &&other) {
-  *this = std::move(other);
-}
-
-StreamDescriptor::~StreamDescriptor() {
-  if (!m_close)
-return;
-
-  if (m_is_socket)
-#if defined(_WIN32)
-::closesocket(m_socket);
-#else
-::close(m_socket);
-#endif
-  else
-::close(m_fd);
-}
-
-StreamDescriptor &StreamDescriptor::operator=(StreamDescriptor &&other) {
-  m_close = other.m_close;
-  other.m_close = false;
-  m_is_socket = other.m_is_socket;
-  if (m_is_socket)
-m_socket = other.m_socket;
-  else
-m_fd = other.m_fd;
-  return *this;
-}
-
-StreamDescriptor StreamDescriptor::from_sock

[Lldb-commits] [lldb] [lldb-dap] Use existing lldb::IOObjectSP for DAP IO (NFC). (PR #128750)

[via lldb-commits]

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 041b7f508533417bcda4feaa03d6c16ff85275f5 
b3f60a464deedae59d902f7417c74a64c5cbf5a1 --extensions cpp,h -- 
lldb/tools/lldb-dap/DAP.cpp lldb/tools/lldb-dap/DAP.h 
lldb/tools/lldb-dap/IOStream.cpp lldb/tools/lldb-dap/IOStream.h 
lldb/tools/lldb-dap/lldb-dap.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index b4e77b78c5..9831308f9e 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -126,21 +126,21 @@ struct Variables {
 
 struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit StartDebuggingRequestHandler(DAP &d) : dap(d){};
+  explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
  lldb::SBCommandReturnObject &result) override;
 };
 
 struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit ReplModeRequestHandler(DAP &d) : dap(d){};
+  explicit ReplModeRequestHandler(DAP &d) : dap(d) {};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
  lldb::SBCommandReturnObject &result) override;
 };
 
 struct SendEventRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
-  explicit SendEventRequestHandler(DAP &d) : dap(d){};
+  explicit SendEventRequestHandler(DAP &d) : dap(d) {};
   bool DoExecute(lldb::SBDebugger debugger, char **command,
  lldb::SBCommandReturnObject &result) override;
 };

``




https://github.com/llvm/llvm-project/pull/128750
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][nfc] Improve duplicated code in unexecuted breakpoint detection (PR #128724)

[Jason Molenda via lldb-commits]

https://github.com/jasonmolenda approved this pull request.

Good idea. I think you need to also update ProcessWindows.cpp?

I'm still a little unsatisfied with my approach of "a thread calls 
`DetectThreadStoppedAtUnexecutedBP()` and if it turns out it DID execute the 
breakpoint, then calls `SetThreadHitBreakpointSite()`.  And in fact, if you do 
it the other way around -- if a method ends up calling 
`SetThreadHitBreakpointSite()` and then `DetectThreadStoppedAtUnexecutedBP()`, 
it will lose the fact that it hit the breakpoint and well hit it again on 
resume, another thing I'm not thrilled about.  But none of that is relevant to 
this change here.

https://github.com/llvm/llvm-project/pull/128724
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Vy Nguyen via lldb-commits]

https://github.com/oontvoo updated 
https://github.com/llvm/llvm-project/pull/128534

>From 236ab76b7c9433f8fed6586a4ee18351779cc191 Mon Sep 17 00:00:00 2001
From: Vy Nguyen 
Date: Mon, 24 Feb 2025 11:32:03 -0500
Subject: [PATCH 1/9] [llvm][telemetr]Change Telemetry-disabling mechanism.

Details:
- Previously, we used the LLVM_BUILD_TELEMETRY flag to control whether any 
Telemetry code will be built.
This has proven to cause more nuisance to both users of the Telemetry and any 
further extension of it.
(Eg., we needed to put #ifdef around caller/user code)

- So the new approach is to:
 + Remove this flag and introduce LLVM_ENABLE_TELEMETRY which would be true by 
default.
 + If LLVM_ENABLE_TELEMETRY is set to FALSE (at buildtime), the library would 
still be built BUT Telemetry cannot be enabled.
   And no data can be collected.

The benefit of this is that it simplifies user (and extension) code since we 
just need to put the check on Config::EnableTelemetry.
Besides, the Telemetry library itself is very small, hence the additional code 
to be built would not cause any difference in build performance.
---
 lldb/source/Core/CMakeLists.txt  |  4 +---
 lldb/source/Core/Telemetry.cpp   |  7 ++-
 lldb/unittests/Core/CMakeLists.txt   |  6 +++---
 llvm/CMakeLists.txt  |  3 ++-
 llvm/cmake/modules/LLVMConfig.cmake.in   |  1 +
 llvm/include/llvm/Config/llvm-config.h.cmake |  2 ++
 llvm/include/llvm/Telemetry/Telemetry.h  | 12 ++--
 llvm/lib/CMakeLists.txt  |  5 ++---
 llvm/unittests/CMakeLists.txt|  4 +---
 .../gn/secondary/llvm/include/llvm/Config/BUILD.gn   |  1 +
 utils/bazel/llvm_configs/llvm-config.h.cmake |  2 ++
 11 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index 82fb5f42f9f4b..a3c12e4c1bdbc 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -16,9 +16,7 @@ if (LLDB_ENABLE_CURSES)
   endif()
 endif()
 
-if (LLVM_BUILD_TELEMETRY)
-   set(TELEMETRY_DEPS Telemetry)
-endif()
+set(TELEMETRY_DEPS Telemetry)
 
 # TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore
 add_lldb_library(lldbCore
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 5222f76704f91..51a860ea5313b 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -8,8 +8,6 @@
 
 #include "llvm/Config/llvm-config.h"
 
-#ifdef LLVM_BUILD_TELEMETRY
-
 #include "lldb/Core/Telemetry.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Utility/LLDBLog.h"
@@ -57,7 +55,8 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) 
const {
 }
 
 TelemetryManager::TelemetryManager(std::unique_ptr config)
-: m_config(std::move(config)) {}
+: m_config(std::move(config))
+}
 
 llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
   // Do nothing for now.
@@ -75,5 +74,3 @@ void 
TelemetryManager::setInstance(std::unique_ptr manager) {
 
 } // namespace telemetry
 } // namespace lldb_private
-
-#endif // LLVM_BUILD_TELEMETRY
diff --git a/lldb/unittests/Core/CMakeLists.txt 
b/lldb/unittests/Core/CMakeLists.txt
index d4d3764b67ae3..e8df299631e2e 100644
--- a/lldb/unittests/Core/CMakeLists.txt
+++ b/lldb/unittests/Core/CMakeLists.txt
@@ -1,6 +1,6 @@
-if (LLVM_BUILD_TELEMETRY)
-  set(TELEMETRY_DEPS Telemetry)
-endif()
+
+set(TELEMETRY_DEPS Telemetry)
+
 
 add_lldb_unittest(LLDBCoreTests
   CommunicationTest.cpp
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 88512d0f1dd96..9188fb93b7846 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -835,7 +835,8 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm 
API documentation." OF
 option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF)
 option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON)
 option (LLVM_ENABLE_BINDINGS "Build bindings." ON)
-option (LLVM_BUILD_TELEMETRY "Build the telemetry library. This does not 
enable telemetry." ON)
+option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable 
the telemetry library. If set to OFF, library cannot be enabled after build 
(eg., at runtime)" ON)
+option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF, 
library cannot be enabled after build (eg., at runtime)" ON)
 
 set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html"
 CACHE STRING "Doxygen-generated HTML documentation install directory")
diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in 
b/llvm/cmake/modules/LLVMConfig.cmake.in
index 28655ee3ab87d..134d107ce79ba 100644
--- a/llvm/cmake/modules/LLVMConfig.cmake.in
+++ b/llvm/cmake/modules/LLVMConfig.cmake.in
@@ -101,6 +101,7 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@)
 set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@)
 
 set(LLVM_BUILD_TELEMETRY @LLVM_B

[Lldb-commits] [lldb] [lldb]HostInfoMacOSX] Search CommandLineTools directory when looking up SDK paths (PR #128712)

[Adrian Prantl via lldb-commits]

@@ -581,6 +626,21 @@ static bool ResolveAndVerifyCandidateSupportDir(FileSpec 
&path) {
"XcodeSDK not specified");
   XcodeSDK sdk = *options.XcodeSDKSelection;
   auto key = sdk.GetString();
+
+  // xcrun doesn't search SDKs in the CommandLineTools (CLT) directory. So if
+  // a program was compiled against a CLT SDK, but that SDK wasn't present in
+  // any of the Xcode installations, then xcrun would fail to find the SDK
+  // (which is expensive). To avoid this we first try to find the specified SDK
+  // in the CLT directory.
+  auto clt_root_dir = find_cached_path(g_sdk_path, g_sdk_path_mutex, key, [&] {
+return GetCommandLineToolsSDKRoot(sdk.GetVersion());

adrian-prantl wrote:

That's my only concern with this, but if we wanted to avoid this behavior, then 
we would need to reimplement all of xcrun's logic to find SDKs in Xcode. That's 
also not something I'm excited about.

https://github.com/llvm/llvm-project/pull/128712
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb]HostInfoMacOSX] Search CommandLineTools directory when looking up SDK paths (PR #128712)

[Jonas Devlieghere via lldb-commits]

@@ -569,10 +572,52 @@ static bool ResolveAndVerifyCandidateSupportDir(FileSpec 
&path) {
 cache.insert({key, {error, true}});
 return llvm::createStringError(llvm::inconvertibleErrorCode(), error);
   }
+
+  if (path_or_err->empty())
+return llvm::createStringError("Empty path determined for '%s'",
+   key.data());
+
   auto it_new = cache.insert({key, {*path_or_err, false}});
   return it_new.first->second.str;
 }
 
+static llvm::Expected
+GetCommandLineToolsSDKRoot(llvm::VersionTuple version) {
+  std::string clt_root_dir;
+  FileSystem::Instance().EnumerateDirectory(
+  "/Library/Developer/CommandLineTools/SDKs/", /*find_directories=*/true,
+  /*find_files=*/false, /*find_other=*/false,
+  [&](void *baton, llvm::sys::fs::file_type file_type,
+  llvm::StringRef name) {
+assert(file_type == llvm::sys::fs::file_type::directory_file);
+
+if (!name.ends_with(".sdk"))
+  return FileSystem::eEnumerateDirectoryResultNext;
+
+llvm::Expected> sdk_info =

JDevlieghere wrote:

I think it should support both. I'm sure it can write binary property lists. I 
would assume it can read them too. 

https://github.com/llvm/llvm-project/pull/128712
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 66af492 - [lldb-dap] Refactor reverse request response handlers (NFC) (#128594)

[via lldb-commits]

Author: Jonas Devlieghere
Date: 2025-02-25T13:00:26-06:00
New Revision: 66af4923ce245a0fd9427db8e4861354576d0866

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

LOG: [lldb-dap] Refactor reverse request response handlers (NFC) (#128594)

This refactors the response handlers for reverse request to follow the
same architecture as the request handlers. With only two implementation
that might be overkill, but it reduces code duplication and improves
error reporting by storing the sequence ID. This PR also fixes an
unchecked Expected in the old callback for unknown sequence IDs.

Added: 
lldb/tools/lldb-dap/Handler/ResponseHandler.cpp
lldb/tools/lldb-dap/Handler/ResponseHandler.h

Modified: 
lldb/tools/lldb-dap/CMakeLists.txt
lldb/tools/lldb-dap/DAP.cpp
lldb/tools/lldb-dap/DAP.h
lldb/tools/lldb-dap/Handler/RequestHandler.cpp

Removed: 




diff  --git a/lldb/tools/lldb-dap/CMakeLists.txt 
b/lldb/tools/lldb-dap/CMakeLists.txt
index 804dd8e4cc2a0..8b3c520ec4360 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -37,6 +37,7 @@ add_lldb_tool(lldb-dap
   SourceBreakpoint.cpp
   Watchpoint.cpp
 
+  Handler/ResponseHandler.cpp
   Handler/AttachRequestHandler.cpp
   Handler/BreakpointLocationsHandler.cpp
   Handler/CompileUnitsRequestHandler.cpp

diff  --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 01f294e14de6a..37bc1f68e4b08 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "DAP.h"
+#include "Handler/ResponseHandler.h"
 #include "JSONUtils.h"
 #include "LLDBUtils.h"
 #include "OutputRedirector.h"
@@ -34,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -769,10 +771,8 @@ bool DAP::HandleObject(const llvm::json::Object &object) {
 
   if (packet_type == "response") {
 auto id = GetSigned(object, "request_seq", 0);
-ResponseCallback response_handler = [](llvm::Expected) {
-  llvm::errs() << "Unhandled response\n";
-};
 
+std::unique_ptr response_handler;
 {
   std::lock_guard locker(call_mutex);
   auto inflight = inflight_reverse_requests.find(id);
@@ -782,19 +782,22 @@ bool DAP::HandleObject(const llvm::json::Object &object) {
   }
 }
 
+if (!response_handler)
+  response_handler = std::make_unique("", id);
+
 // Result should be given, use null if not.
 if (GetBoolean(object, "success", false)) {
   llvm::json::Value Result = nullptr;
   if (auto *B = object.get("body")) {
 Result = std::move(*B);
   }
-  response_handler(Result);
+  (*response_handler)(Result);
 } else {
   llvm::StringRef message = GetString(object, "message");
   if (message.empty()) {
 message = "Unknown error, response failed";
   }
-  response_handler(llvm::createStringError(
+  (*response_handler)(llvm::createStringError(
   std::error_code(-1, std::generic_category()), message));
 }
 
@@ -875,24 +878,6 @@ llvm::Error DAP::Loop() {
   return llvm::Error::success();
 }
 
-void DAP::SendReverseRequest(llvm::StringRef command,
- llvm::json::Value arguments,
- ResponseCallback callback) {
-  int64_t id;
-  {
-std::lock_guard locker(call_mutex);
-id = ++reverse_request_seq;
-inflight_reverse_requests.emplace(id, std::move(callback));
-  }
-
-  SendJSON(llvm::json::Object{
-  {"type", "request"},
-  {"seq", id},
-  {"command", command},
-  {"arguments", std::move(arguments)},
-  });
-}
-
 lldb::SBError DAP::WaitForProcessToStop(uint32_t seconds) {
   lldb::SBError error;
   lldb::SBProcess process = target.GetProcess();
@@ -1007,17 +992,10 @@ bool StartDebuggingRequestHandler::DoExecute(
 return false;
   }
 
-  dap.SendReverseRequest(
+  dap.SendReverseRequest(
   "startDebugging",
   llvm::json::Object{{"request", request},
- {"configuration", std::move(*configuration)}},
-  [](llvm::Expected value) {
-if (!value) {
-  llvm::Error err = value.takeError();
-  llvm::errs() << "reverse start debugging request failed: "
-   << llvm::toString(std::move(err)) << "\n";
-}
-  });
+ {"configuration", std::move(*configuration)}});
 
   result.SetStatus(lldb::eReturnStatusSuccessFinishNoResult);
 

diff  --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index f87841a56f4d3..7ceb1d114a57d 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -13,6 +13,7 @@
 #include "ExceptionBreakpoint.h"
 #include "FunctionBreakpoint.h"
 #inc

[Lldb-commits] [lldb] [lldb-dap] Refactor reverse request response handlers (NFC) (PR #128594)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere closed 
https://github.com/llvm/llvm-project/pull/128594
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Vy Nguyen via lldb-commits]

https://github.com/oontvoo updated 
https://github.com/llvm/llvm-project/pull/128534

>From 236ab76b7c9433f8fed6586a4ee18351779cc191 Mon Sep 17 00:00:00 2001
From: Vy Nguyen 
Date: Mon, 24 Feb 2025 11:32:03 -0500
Subject: [PATCH 1/4] [llvm][telemetr]Change Telemetry-disabling mechanism.

Details:
- Previously, we used the LLVM_BUILD_TELEMETRY flag to control whether any 
Telemetry code will be built.
This has proven to cause more nuisance to both users of the Telemetry and any 
further extension of it.
(Eg., we needed to put #ifdef around caller/user code)

- So the new approach is to:
 + Remove this flag and introduce LLVM_ENABLE_TELEMETRY which would be true by 
default.
 + If LLVM_ENABLE_TELEMETRY is set to FALSE (at buildtime), the library would 
still be built BUT Telemetry cannot be enabled.
   And no data can be collected.

The benefit of this is that it simplifies user (and extension) code since we 
just need to put the check on Config::EnableTelemetry.
Besides, the Telemetry library itself is very small, hence the additional code 
to be built would not cause any difference in build performance.
---
 lldb/source/Core/CMakeLists.txt  |  4 +---
 lldb/source/Core/Telemetry.cpp   |  7 ++-
 lldb/unittests/Core/CMakeLists.txt   |  6 +++---
 llvm/CMakeLists.txt  |  3 ++-
 llvm/cmake/modules/LLVMConfig.cmake.in   |  1 +
 llvm/include/llvm/Config/llvm-config.h.cmake |  2 ++
 llvm/include/llvm/Telemetry/Telemetry.h  | 12 ++--
 llvm/lib/CMakeLists.txt  |  5 ++---
 llvm/unittests/CMakeLists.txt|  4 +---
 .../gn/secondary/llvm/include/llvm/Config/BUILD.gn   |  1 +
 utils/bazel/llvm_configs/llvm-config.h.cmake |  2 ++
 11 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index 82fb5f42f9f4b..a3c12e4c1bdbc 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -16,9 +16,7 @@ if (LLDB_ENABLE_CURSES)
   endif()
 endif()
 
-if (LLVM_BUILD_TELEMETRY)
-   set(TELEMETRY_DEPS Telemetry)
-endif()
+set(TELEMETRY_DEPS Telemetry)
 
 # TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore
 add_lldb_library(lldbCore
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 5222f76704f91..51a860ea5313b 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -8,8 +8,6 @@
 
 #include "llvm/Config/llvm-config.h"
 
-#ifdef LLVM_BUILD_TELEMETRY
-
 #include "lldb/Core/Telemetry.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Utility/LLDBLog.h"
@@ -57,7 +55,8 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) 
const {
 }
 
 TelemetryManager::TelemetryManager(std::unique_ptr config)
-: m_config(std::move(config)) {}
+: m_config(std::move(config))
+}
 
 llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
   // Do nothing for now.
@@ -75,5 +74,3 @@ void 
TelemetryManager::setInstance(std::unique_ptr manager) {
 
 } // namespace telemetry
 } // namespace lldb_private
-
-#endif // LLVM_BUILD_TELEMETRY
diff --git a/lldb/unittests/Core/CMakeLists.txt 
b/lldb/unittests/Core/CMakeLists.txt
index d4d3764b67ae3..e8df299631e2e 100644
--- a/lldb/unittests/Core/CMakeLists.txt
+++ b/lldb/unittests/Core/CMakeLists.txt
@@ -1,6 +1,6 @@
-if (LLVM_BUILD_TELEMETRY)
-  set(TELEMETRY_DEPS Telemetry)
-endif()
+
+set(TELEMETRY_DEPS Telemetry)
+
 
 add_lldb_unittest(LLDBCoreTests
   CommunicationTest.cpp
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 88512d0f1dd96..9188fb93b7846 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -835,7 +835,8 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm 
API documentation." OF
 option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF)
 option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON)
 option (LLVM_ENABLE_BINDINGS "Build bindings." ON)
-option (LLVM_BUILD_TELEMETRY "Build the telemetry library. This does not 
enable telemetry." ON)
+option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable 
the telemetry library. If set to OFF, library cannot be enabled after build 
(eg., at runtime)" ON)
+option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF, 
library cannot be enabled after build (eg., at runtime)" ON)
 
 set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html"
 CACHE STRING "Doxygen-generated HTML documentation install directory")
diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in 
b/llvm/cmake/modules/LLVMConfig.cmake.in
index 28655ee3ab87d..134d107ce79ba 100644
--- a/llvm/cmake/modules/LLVMConfig.cmake.in
+++ b/llvm/cmake/modules/LLVMConfig.cmake.in
@@ -101,6 +101,7 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@)
 set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@)
 
 set(LLVM_BUILD_TELEMETRY @LLVM_B

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Vy Nguyen via lldb-commits]

https://github.com/oontvoo updated 
https://github.com/llvm/llvm-project/pull/128534

>From 236ab76b7c9433f8fed6586a4ee18351779cc191 Mon Sep 17 00:00:00 2001
From: Vy Nguyen 
Date: Mon, 24 Feb 2025 11:32:03 -0500
Subject: [PATCH 1/5] [llvm][telemetr]Change Telemetry-disabling mechanism.

Details:
- Previously, we used the LLVM_BUILD_TELEMETRY flag to control whether any 
Telemetry code will be built.
This has proven to cause more nuisance to both users of the Telemetry and any 
further extension of it.
(Eg., we needed to put #ifdef around caller/user code)

- So the new approach is to:
 + Remove this flag and introduce LLVM_ENABLE_TELEMETRY which would be true by 
default.
 + If LLVM_ENABLE_TELEMETRY is set to FALSE (at buildtime), the library would 
still be built BUT Telemetry cannot be enabled.
   And no data can be collected.

The benefit of this is that it simplifies user (and extension) code since we 
just need to put the check on Config::EnableTelemetry.
Besides, the Telemetry library itself is very small, hence the additional code 
to be built would not cause any difference in build performance.
---
 lldb/source/Core/CMakeLists.txt  |  4 +---
 lldb/source/Core/Telemetry.cpp   |  7 ++-
 lldb/unittests/Core/CMakeLists.txt   |  6 +++---
 llvm/CMakeLists.txt  |  3 ++-
 llvm/cmake/modules/LLVMConfig.cmake.in   |  1 +
 llvm/include/llvm/Config/llvm-config.h.cmake |  2 ++
 llvm/include/llvm/Telemetry/Telemetry.h  | 12 ++--
 llvm/lib/CMakeLists.txt  |  5 ++---
 llvm/unittests/CMakeLists.txt|  4 +---
 .../gn/secondary/llvm/include/llvm/Config/BUILD.gn   |  1 +
 utils/bazel/llvm_configs/llvm-config.h.cmake |  2 ++
 11 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index 82fb5f42f9f4b..a3c12e4c1bdbc 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -16,9 +16,7 @@ if (LLDB_ENABLE_CURSES)
   endif()
 endif()
 
-if (LLVM_BUILD_TELEMETRY)
-   set(TELEMETRY_DEPS Telemetry)
-endif()
+set(TELEMETRY_DEPS Telemetry)
 
 # TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore
 add_lldb_library(lldbCore
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 5222f76704f91..51a860ea5313b 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -8,8 +8,6 @@
 
 #include "llvm/Config/llvm-config.h"
 
-#ifdef LLVM_BUILD_TELEMETRY
-
 #include "lldb/Core/Telemetry.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Utility/LLDBLog.h"
@@ -57,7 +55,8 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) 
const {
 }
 
 TelemetryManager::TelemetryManager(std::unique_ptr config)
-: m_config(std::move(config)) {}
+: m_config(std::move(config))
+}
 
 llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
   // Do nothing for now.
@@ -75,5 +74,3 @@ void 
TelemetryManager::setInstance(std::unique_ptr manager) {
 
 } // namespace telemetry
 } // namespace lldb_private
-
-#endif // LLVM_BUILD_TELEMETRY
diff --git a/lldb/unittests/Core/CMakeLists.txt 
b/lldb/unittests/Core/CMakeLists.txt
index d4d3764b67ae3..e8df299631e2e 100644
--- a/lldb/unittests/Core/CMakeLists.txt
+++ b/lldb/unittests/Core/CMakeLists.txt
@@ -1,6 +1,6 @@
-if (LLVM_BUILD_TELEMETRY)
-  set(TELEMETRY_DEPS Telemetry)
-endif()
+
+set(TELEMETRY_DEPS Telemetry)
+
 
 add_lldb_unittest(LLDBCoreTests
   CommunicationTest.cpp
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 88512d0f1dd96..9188fb93b7846 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -835,7 +835,8 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm 
API documentation." OF
 option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF)
 option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON)
 option (LLVM_ENABLE_BINDINGS "Build bindings." ON)
-option (LLVM_BUILD_TELEMETRY "Build the telemetry library. This does not 
enable telemetry." ON)
+option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable 
the telemetry library. If set to OFF, library cannot be enabled after build 
(eg., at runtime)" ON)
+option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF, 
library cannot be enabled after build (eg., at runtime)" ON)
 
 set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html"
 CACHE STRING "Doxygen-generated HTML documentation install directory")
diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in 
b/llvm/cmake/modules/LLVMConfig.cmake.in
index 28655ee3ab87d..134d107ce79ba 100644
--- a/llvm/cmake/modules/LLVMConfig.cmake.in
+++ b/llvm/cmake/modules/LLVMConfig.cmake.in
@@ -101,6 +101,7 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@)
 set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@)
 
 set(LLVM_BUILD_TELEMETRY @LLVM_B

[Lldb-commits] [lldb] [lldb][nfc] Improve duplicated code in unexecuted breakpoint detection (PR #128724)

[Felipe de Azevedo Piovezan via lldb-commits]

https://github.com/felipepiovezan created 
https://github.com/llvm/llvm-project/pull/128724

This code is replicated in multiple places, and a subsequent commit would 
introduce another copy of it in ThreadMemory.

>From e53d5e606da5703152759fa927ac066575b4ab11 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Mon, 24 Feb 2025 15:33:20 -0800
Subject: [PATCH] [lldb][nfc] Improve duplicated code in unexecuted breakpoint
 detection

This code is replicated in multiple places, and a subsequent commit
would introduce another copy of it in ThreadMemory.
---
 lldb/include/lldb/Target/Thread.h   | 11 ++-
 .../Process/Utility/StopInfoMachException.cpp   |  6 +-
 .../Plugins/Process/gdb-remote/ProcessGDBRemote.cpp |  5 +
 .../Plugins/Process/scripted/ScriptedThread.cpp | 11 +--
 lldb/source/Target/Thread.cpp   | 13 +
 5 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Thread.h
index 1d1e3dcfc1dc6..d1988407965da 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -385,21 +385,22 @@ class Thread : public 
std::enable_shared_from_this,
   virtual void SetQueueLibdispatchQueueAddress(lldb::addr_t dispatch_queue_t) 
{}
 
   /// When a thread stops at an enabled BreakpointSite that has not executed,
-  /// the Process plugin should call SetThreadStoppedAtUnexecutedBP(pc).
+  /// the Process plugin should call DetectThreadStoppedAtUnexecutedBP().
   /// If that BreakpointSite was actually triggered (the instruction was
   /// executed, for a software breakpoint), regardless of whether the
   /// breakpoint is valid for this thread, SetThreadHitBreakpointSite()
   /// should be called to record that fact.
   ///
   /// Depending on the structure of the Process plugin, it may be easiest
-  /// to call SetThreadStoppedAtUnexecutedBP(pc) unconditionally when at
+  /// to call DetectThreadStoppedAtUnexecutedBP() unconditionally when at
   /// a BreakpointSite, and later when it is known that it was triggered,
   /// SetThreadHitBreakpointSite() can be called.  These two methods
   /// overwrite the same piece of state in the Thread, the last one
   /// called on a Thread wins.
-  void SetThreadStoppedAtUnexecutedBP(lldb::addr_t pc) {
-m_stopped_at_unexecuted_bp = pc;
-  }
+  ///
+  /// Returns the BreakpointSite this thread is stopped at, if any.
+  lldb::BreakpointSiteSP DetectThreadStoppedAtUnexecutedBP();
+
   void SetThreadHitBreakpointSite() {
 m_stopped_at_unexecuted_bp = LLDB_INVALID_ADDRESS;
   }
diff --git a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp 
b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
index 29a64a2a03bf0..daa7ccc2086c3 100644
--- a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
+++ b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
@@ -591,11 +591,7 @@ StopInfoSP 
StopInfoMachException::CreateStopReasonWithMachException(
   // breakpoint at 0x100.
   // The fact that the pc may be off by one at this point
   // (for an x86 KDP breakpoint hit) is not a problem.
-  addr_t pc = reg_ctx_sp->GetPC();
-  BreakpointSiteSP bp_site_sp =
-  process_sp->GetBreakpointSiteList().FindByAddress(pc);
-  if (bp_site_sp && bp_site_sp->IsEnabled())
-thread.SetThreadStoppedAtUnexecutedBP(pc);
+  BreakpointSiteSP bp_site_sp = thread.DetectThreadStoppedAtUnexecutedBP();
 
   switch (exc_type) {
   case 1: // EXC_BAD_ACCESS
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 8a8c0f92fbbc2..4ce543f8810a3 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1706,11 +1706,8 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
   if (!thread_sp->StopInfoIsUpToDate()) {
 thread_sp->SetStopInfo(StopInfoSP());
 
-addr_t pc = thread_sp->GetRegisterContext()->GetPC();
 BreakpointSiteSP bp_site_sp =
-thread_sp->GetProcess()->GetBreakpointSiteList().FindByAddress(pc);
-if (bp_site_sp && bp_site_sp->IsEnabled())
-  thread_sp->SetThreadStoppedAtUnexecutedBP(pc);
+thread_sp->DetectThreadStoppedAtUnexecutedBP();
 
 if (exc_type != 0) {
   // For thread plan async interrupt, creating stop info on the
diff --git a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp 
b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
index d0d1508e85172..12474402af909 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -229,16 +229,7 @@ bool ScriptedThread::CalculateStopInfo() {
 LLVM_PRETTY_FUNCTION, "Failed to get scripted thread stop info.", 
error,
 LLDBLog::Thread);
 
-  // If we're at a BreakpointSite, mark that we stopped there and
-  // need to hit

[Lldb-commits] [lldb] [lldb] Add missing calls to SetThreadHitBreakpointSite/DetectThreadStoppedAtUnexecutedBP (PR #128726)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Felipe de Azevedo Piovezan (felipepiovezan)


Changes

* One of the overloads of CreateStopReasonWithBreakpointSiteID was missing a 
call to SetThreadHitBreakpointSite.
* ThreadMemory was missing a "DetectThreadStoppedAtUnexecutedBP" in its 
CalculateStopInfo.
* When OS plugins are involved, they will sometimes calculate the stop info for 
the backing thread, and then "grab" it from them (see for example 
`ThreadMemory::CalculateStopInfo`. So the stop info is created on the backing 
thread (e.g. a GDBRemoteThread), and then we call SetStopInfo on the OS plugin 
thread. As a result, we never call `SetThreadHitBreakpointSite" for 
MemoryThreads, as this is done at the StopInfo creation. To address this, we 
add a check in SetStopInfo.)

---
Full diff: https://github.com/llvm/llvm-project/pull/128726.diff


3 Files Affected:

- (modified) lldb/source/Plugins/Process/Utility/ThreadMemory.cpp (+1) 
- (modified) lldb/source/Target/StopInfo.cpp (+2) 
- (modified) lldb/source/Target/Thread.cpp (+4) 


``diff
diff --git a/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp 
b/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
index 550b53688fd39..9643d721b8501 100644
--- a/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
+++ b/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
@@ -66,6 +66,7 @@ ThreadMemory::CreateRegisterContextForFrame(StackFrame 
*frame) {
 }
 
 bool ThreadMemory::CalculateStopInfo() {
+  DetectThreadStoppedAtUnexecutedBP();
   if (m_backing_thread_sp) {
 lldb::StopInfoSP backing_stop_info_sp(
 m_backing_thread_sp->GetPrivateStopInfo());
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 1c9ecbfe70c3c..1999216f8d2e9 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -1451,6 +1451,8 @@ StopInfoSP 
StopInfo::CreateStopReasonWithBreakpointSiteID(Thread &thread,
 StopInfoSP StopInfo::CreateStopReasonWithBreakpointSiteID(Thread &thread,
   break_id_t break_id,
   bool should_stop) {
+  thread.SetThreadHitBreakpointSite();
+
   return StopInfoSP(new StopInfoBreakpoint(thread, break_id, should_stop));
 }
 
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index 50f7c73f2c4c1..71ea76ebfa0dc 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -462,6 +462,10 @@ void Thread::ResetStopInfo() {
 }
 
 void Thread::SetStopInfo(const lldb::StopInfoSP &stop_info_sp) {
+  if (stop_info_sp &&
+  stop_info_sp->GetStopReason() == lldb::eStopReasonBreakpoint)
+SetThreadHitBreakpointSite();
+
   m_stop_info_sp = stop_info_sp;
   if (m_stop_info_sp) {
 m_stop_info_sp->MakeStopInfoValid();

``




https://github.com/llvm/llvm-project/pull/128726
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add missing calls to SetThreadHitBreakpointSite/DetectThreadStoppedAtUnexecutedBP (PR #128726)

[Felipe de Azevedo Piovezan via lldb-commits]

https://github.com/felipepiovezan created 
https://github.com/llvm/llvm-project/pull/128726

* One of the overloads of CreateStopReasonWithBreakpointSiteID was missing a 
call to SetThreadHitBreakpointSite.
* ThreadMemory was missing a "DetectThreadStoppedAtUnexecutedBP" in its 
CalculateStopInfo.
* When OS plugins are involved, they will sometimes calculate the stop info for 
the backing thread, and then "grab" it from them (see for example 
`ThreadMemory::CalculateStopInfo`. So the stop info is created on the backing 
thread (e.g. a GDBRemoteThread), and then we call SetStopInfo on the OS plugin 
thread. As a result, we never call `SetThreadHitBreakpointSite" for 
MemoryThreads, as this is done at the StopInfo creation. To address this, we 
add a check in SetStopInfo.)

>From a373f8a4523f22901307d474d19ff39c4fa931f0 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Tue, 25 Feb 2025 07:01:17 -0800
Subject: [PATCH] [lldb] Add missing calls to
 SetThreadHitBreakpointSite/DetectThreadStoppedAtUnexecutedBP

* One of the overloads of CreateStopReasonWithBreakpointSiteID was
  missing a call to SetThreadHitBreakpointSite.
* ThreadMemory was missing a "DetectThreadStoppedAtUnexecutedBP" in its
  CalculateStopInfo.
* When OS plugins are involved, they will sometimes calculate the stop
  info for the backing thread, and then "grab" it from them (see for
  example `ThreadMemory::CalculateStopInfo`. So the stop info is created
  on the backing thread (e.g. a GDBRemoteThread), and then we call
  SetStopInfo on the OS plugin thread. As a result, we never call
  `SetThreadHitBreakpointSite" for MemoryThreads, as this is done at the
  StopInfo creation. To address this, we add a check in SetStopInfo.
---
 lldb/source/Plugins/Process/Utility/ThreadMemory.cpp | 1 +
 lldb/source/Target/StopInfo.cpp  | 2 ++
 lldb/source/Target/Thread.cpp| 4 
 3 files changed, 7 insertions(+)

diff --git a/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp 
b/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
index 550b53688fd39..9643d721b8501 100644
--- a/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
+++ b/lldb/source/Plugins/Process/Utility/ThreadMemory.cpp
@@ -66,6 +66,7 @@ ThreadMemory::CreateRegisterContextForFrame(StackFrame 
*frame) {
 }
 
 bool ThreadMemory::CalculateStopInfo() {
+  DetectThreadStoppedAtUnexecutedBP();
   if (m_backing_thread_sp) {
 lldb::StopInfoSP backing_stop_info_sp(
 m_backing_thread_sp->GetPrivateStopInfo());
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 1c9ecbfe70c3c..1999216f8d2e9 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -1451,6 +1451,8 @@ StopInfoSP 
StopInfo::CreateStopReasonWithBreakpointSiteID(Thread &thread,
 StopInfoSP StopInfo::CreateStopReasonWithBreakpointSiteID(Thread &thread,
   break_id_t break_id,
   bool should_stop) {
+  thread.SetThreadHitBreakpointSite();
+
   return StopInfoSP(new StopInfoBreakpoint(thread, break_id, should_stop));
 }
 
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index 50f7c73f2c4c1..71ea76ebfa0dc 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -462,6 +462,10 @@ void Thread::ResetStopInfo() {
 }
 
 void Thread::SetStopInfo(const lldb::StopInfoSP &stop_info_sp) {
+  if (stop_info_sp &&
+  stop_info_sp->GetStopReason() == lldb::eStopReasonBreakpoint)
+SetThreadHitBreakpointSite();
+
   m_stop_info_sp = stop_info_sp;
   if (m_stop_info_sp) {
 m_stop_info_sp->MakeStopInfoValid();

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

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Vy Nguyen via lldb-commits]

https://github.com/oontvoo updated 
https://github.com/llvm/llvm-project/pull/128534

>From 236ab76b7c9433f8fed6586a4ee18351779cc191 Mon Sep 17 00:00:00 2001
From: Vy Nguyen 
Date: Mon, 24 Feb 2025 11:32:03 -0500
Subject: [PATCH 1/6] [llvm][telemetr]Change Telemetry-disabling mechanism.

Details:
- Previously, we used the LLVM_BUILD_TELEMETRY flag to control whether any 
Telemetry code will be built.
This has proven to cause more nuisance to both users of the Telemetry and any 
further extension of it.
(Eg., we needed to put #ifdef around caller/user code)

- So the new approach is to:
 + Remove this flag and introduce LLVM_ENABLE_TELEMETRY which would be true by 
default.
 + If LLVM_ENABLE_TELEMETRY is set to FALSE (at buildtime), the library would 
still be built BUT Telemetry cannot be enabled.
   And no data can be collected.

The benefit of this is that it simplifies user (and extension) code since we 
just need to put the check on Config::EnableTelemetry.
Besides, the Telemetry library itself is very small, hence the additional code 
to be built would not cause any difference in build performance.
---
 lldb/source/Core/CMakeLists.txt  |  4 +---
 lldb/source/Core/Telemetry.cpp   |  7 ++-
 lldb/unittests/Core/CMakeLists.txt   |  6 +++---
 llvm/CMakeLists.txt  |  3 ++-
 llvm/cmake/modules/LLVMConfig.cmake.in   |  1 +
 llvm/include/llvm/Config/llvm-config.h.cmake |  2 ++
 llvm/include/llvm/Telemetry/Telemetry.h  | 12 ++--
 llvm/lib/CMakeLists.txt  |  5 ++---
 llvm/unittests/CMakeLists.txt|  4 +---
 .../gn/secondary/llvm/include/llvm/Config/BUILD.gn   |  1 +
 utils/bazel/llvm_configs/llvm-config.h.cmake |  2 ++
 11 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index 82fb5f42f9f4b..a3c12e4c1bdbc 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -16,9 +16,7 @@ if (LLDB_ENABLE_CURSES)
   endif()
 endif()
 
-if (LLVM_BUILD_TELEMETRY)
-   set(TELEMETRY_DEPS Telemetry)
-endif()
+set(TELEMETRY_DEPS Telemetry)
 
 # TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore
 add_lldb_library(lldbCore
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 5222f76704f91..51a860ea5313b 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -8,8 +8,6 @@
 
 #include "llvm/Config/llvm-config.h"
 
-#ifdef LLVM_BUILD_TELEMETRY
-
 #include "lldb/Core/Telemetry.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Utility/LLDBLog.h"
@@ -57,7 +55,8 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) 
const {
 }
 
 TelemetryManager::TelemetryManager(std::unique_ptr config)
-: m_config(std::move(config)) {}
+: m_config(std::move(config))
+}
 
 llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
   // Do nothing for now.
@@ -75,5 +74,3 @@ void 
TelemetryManager::setInstance(std::unique_ptr manager) {
 
 } // namespace telemetry
 } // namespace lldb_private
-
-#endif // LLVM_BUILD_TELEMETRY
diff --git a/lldb/unittests/Core/CMakeLists.txt 
b/lldb/unittests/Core/CMakeLists.txt
index d4d3764b67ae3..e8df299631e2e 100644
--- a/lldb/unittests/Core/CMakeLists.txt
+++ b/lldb/unittests/Core/CMakeLists.txt
@@ -1,6 +1,6 @@
-if (LLVM_BUILD_TELEMETRY)
-  set(TELEMETRY_DEPS Telemetry)
-endif()
+
+set(TELEMETRY_DEPS Telemetry)
+
 
 add_lldb_unittest(LLDBCoreTests
   CommunicationTest.cpp
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 88512d0f1dd96..9188fb93b7846 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -835,7 +835,8 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm 
API documentation." OF
 option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF)
 option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON)
 option (LLVM_ENABLE_BINDINGS "Build bindings." ON)
-option (LLVM_BUILD_TELEMETRY "Build the telemetry library. This does not 
enable telemetry." ON)
+option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable 
the telemetry library. If set to OFF, library cannot be enabled after build 
(eg., at runtime)" ON)
+option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF, 
library cannot be enabled after build (eg., at runtime)" ON)
 
 set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html"
 CACHE STRING "Doxygen-generated HTML documentation install directory")
diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in 
b/llvm/cmake/modules/LLVMConfig.cmake.in
index 28655ee3ab87d..134d107ce79ba 100644
--- a/llvm/cmake/modules/LLVMConfig.cmake.in
+++ b/llvm/cmake/modules/LLVMConfig.cmake.in
@@ -101,6 +101,7 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@)
 set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@)
 
 set(LLVM_BUILD_TELEMETRY @LLVM_B

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Vy Nguyen via lldb-commits]

oontvoo wrote:

> I'm fairly ambivalent about the mechanism used to disable telemetry at build 
> time, so no complaints here, though I do wonder what the point is: if there's 
> no concrete implementation of the framework in the build tree, then telemetry 
> is disabled at build time anyway. I suppose there could be downstream cases 
> with a telemetry implementation that want to have the option not to build it 
> at build-time, but in that situation, I don't think we need upstream support. 
> Maybe I'm missing something though?

I agree! In current state, the upstream telemetry is mostly empty and doesn't 
collect anything. 
But I think the worry (from someone who's not familiar with how this is 
structured) is that they don't know if this would change and that they want 
clear guarantee for privacy reasons. (Placebo effect, maybe? )

> My only other observation is that we need to be careful about testing, in 
> that if telemetry is disabled at build time, a class of tests become 
> impossible to implement.

It is disabled but still built. Can we simply not run those tests? 


https://github.com/llvm/llvm-project/pull/128534
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Vy Nguyen via lldb-commits]

@@ -202,6 +202,10 @@
 #cmakedefine LLVM_HAS_LOGF128
 
 /* Define if building LLVM with LLVM_BUILD_TELEMETRY */
+/* DEPRECATED - use LLVM_ENABLE_TELEMETRY */

oontvoo wrote:

@rupprecht Hi, do you know if there's any other place (bazel builds?) that need 
updating? Or can I just remove the flag directly w/o deprecating it? Thanks!

https://github.com/llvm/llvm-project/pull/128534
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb]HostInfoMacOSX] Search CommandLineTools directory when looking up SDK paths (PR #128712)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere commented:

This feels like it's working around a shortcoming of `xcrun`. Since that's 
somewhat within our control, have we asked the team to look into the CLT for an 
SDK? They already do for binaries so I'm not sure why the SDK would need to 
behave differently. 

https://github.com/llvm/llvm-project/pull/128712
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb]HostInfoMacOSX] Search CommandLineTools directory when looking up SDK paths (PR #128712)

[Jonas Devlieghere via lldb-commits]

@@ -581,6 +626,21 @@ static bool ResolveAndVerifyCandidateSupportDir(FileSpec 
&path) {
"XcodeSDK not specified");
   XcodeSDK sdk = *options.XcodeSDKSelection;
   auto key = sdk.GetString();
+
+  // xcrun doesn't search SDKs in the CommandLineTools (CLT) directory. So if
+  // a program was compiled against a CLT SDK, but that SDK wasn't present in
+  // any of the Xcode installations, then xcrun would fail to find the SDK
+  // (which is expensive). To avoid this we first try to find the specified SDK
+  // in the CLT directory.
+  auto clt_root_dir = find_cached_path(g_sdk_path, g_sdk_path_mutex, key, [&] {
+return GetCommandLineToolsSDKRoot(sdk.GetVersion());

JDevlieghere wrote:

Does this mean we're going to prefer the CommandLineTools SDK over the Xcode 
one when both exist? I know they should be the same, but I'd really prefer to 
avoid the CLT whenever we can. 

https://github.com/llvm/llvm-project/pull/128712
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb]HostInfoMacOSX] Search CommandLineTools directory when looking up SDK paths (PR #128712)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere edited 
https://github.com/llvm/llvm-project/pull/128712
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Assorted improvements to the Pipe class (PR #128719)

[David Spickett via lldb-commits]

@@ -55,8 +55,6 @@ TEST_F(PipeTest, OpenAsReader) {
 }
 #endif
 
-// This test is flaky on Windows on Arm.

DavidSpickett wrote:

Likely me that disabled it, but I doubt I had tested it on x86 Windows since we 
don't run a buildbot for it. Hopefully it'll be fine on WoA now.

https://github.com/llvm/llvm-project/pull/128719
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Assorted improvements to the Pipe class (PR #128719)

[David Spickett via lldb-commits]

@@ -93,15 +93,13 @@ Status SharedSocket::CompleteSending(lldb::pid_t child_pid) 
{
 "WSADuplicateSocket() failed, error: %d", last_error);
   }
 
-  size_t num_bytes;
-  Status error =
-  m_socket_pipe.WriteWithTimeout(&protocol_info, sizeof(protocol_info),
- std::chrono::seconds(10), num_bytes);
-  if (error.Fail())
-return error;
-  if (num_bytes != sizeof(protocol_info))
+  llvm::Expected num_bytes = m_socket_pipe.Write(
+  &protocol_info, sizeof(protocol_info), std::chrono::seconds(10));
+  if (!num_bytes)
+return Status::FromError(num_bytes.takeError());
+  if (*num_bytes != sizeof(protocol_info))
 return Status::FromErrorStringWithFormatv(
-"WriteWithTimeout(WSAPROTOCOL_INFO) failed: {0} bytes", num_bytes);
+"Write(WSAPROTOCOL_INFO) failed: {0} bytes", *num_bytes);

DavidSpickett wrote:

This and the read error below are a bit lacking. How about:
Write(WSAPROTOCOL_INFO) failed: expcted to write {} bytes, wrote {} bytes

https://github.com/llvm/llvm-project/pull/128719
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Assorted improvements to the Pipe class (PR #128719)

[David Spickett via lldb-commits]

@@ -1154,17 +1156,25 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
   if (socket_pipe.CanWrite())
 socket_pipe.CloseWriteFileDescriptor();
   if (socket_pipe.CanRead()) {
-// The port number may be up to "65535\0".
-char port_cstr[6] = {0};
-size_t num_bytes = sizeof(port_cstr);
 // Read port from pipe with 10 second timeout.
-error = socket_pipe.ReadWithTimeout(
-port_cstr, num_bytes, std::chrono::seconds{10}, num_bytes);
+std::string port_str;
+while (error.Success()) {
+  char buf[10];
+  if (llvm::Expected num_bytes = socket_pipe.Read(
+  buf, std::size(buf), std::chrono::seconds(10))) {
+port_str.append(buf, *num_bytes);
+if (*num_bytes == 0)

DavidSpickett wrote:

Also shouldn't there be something here that stops us reading more than the buf 
size?

If I read 9 bytes in one call, then 10 in the next, it'll overflow. Should 
there be some tracking of how much we have already read?

https://github.com/llvm/llvm-project/pull/128719
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Assorted improvements to the Pipe class (PR #128719)

[David Spickett via lldb-commits]

@@ -1154,17 +1156,25 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
   if (socket_pipe.CanWrite())
 socket_pipe.CloseWriteFileDescriptor();
   if (socket_pipe.CanRead()) {
-// The port number may be up to "65535\0".
-char port_cstr[6] = {0};
-size_t num_bytes = sizeof(port_cstr);
 // Read port from pipe with 10 second timeout.
-error = socket_pipe.ReadWithTimeout(
-port_cstr, num_bytes, std::chrono::seconds{10}, num_bytes);
+std::string port_str;
+while (error.Success()) {
+  char buf[10];
+  if (llvm::Expected num_bytes = socket_pipe.Read(
+  buf, std::size(buf), std::chrono::seconds(10))) {
+port_str.append(buf, *num_bytes);
+if (*num_bytes == 0)

DavidSpickett wrote:

If num_bytes is 0 then there's nothing to copy, so do the break check first?

https://github.com/llvm/llvm-project/pull/128719
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Assorted improvements to the Pipe class (PR #128719)

[David Spickett via lldb-commits]

@@ -1154,17 +1156,25 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
   if (socket_pipe.CanWrite())
 socket_pipe.CloseWriteFileDescriptor();
   if (socket_pipe.CanRead()) {
-// The port number may be up to "65535\0".
-char port_cstr[6] = {0};
-size_t num_bytes = sizeof(port_cstr);
 // Read port from pipe with 10 second timeout.
-error = socket_pipe.ReadWithTimeout(
-port_cstr, num_bytes, std::chrono::seconds{10}, num_bytes);
+std::string port_str;
+while (error.Success()) {
+  char buf[10];
+  if (llvm::Expected num_bytes = socket_pipe.Read(
+  buf, std::size(buf), std::chrono::seconds(10))) {
+port_str.append(buf, *num_bytes);
+if (*num_bytes == 0)

DavidSpickett wrote:

And, maybe related, why do you not reserve the string length and use its 
storage as the buffer? Is this because the port number may not be 10 
characters, it could be any number of characters?

https://github.com/llvm/llvm-project/pull/128719
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Vy Nguyen via lldb-commits]

https://github.com/oontvoo updated 
https://github.com/llvm/llvm-project/pull/128534

>From 236ab76b7c9433f8fed6586a4ee18351779cc191 Mon Sep 17 00:00:00 2001
From: Vy Nguyen 
Date: Mon, 24 Feb 2025 11:32:03 -0500
Subject: [PATCH 1/7] [llvm][telemetr]Change Telemetry-disabling mechanism.

Details:
- Previously, we used the LLVM_BUILD_TELEMETRY flag to control whether any 
Telemetry code will be built.
This has proven to cause more nuisance to both users of the Telemetry and any 
further extension of it.
(Eg., we needed to put #ifdef around caller/user code)

- So the new approach is to:
 + Remove this flag and introduce LLVM_ENABLE_TELEMETRY which would be true by 
default.
 + If LLVM_ENABLE_TELEMETRY is set to FALSE (at buildtime), the library would 
still be built BUT Telemetry cannot be enabled.
   And no data can be collected.

The benefit of this is that it simplifies user (and extension) code since we 
just need to put the check on Config::EnableTelemetry.
Besides, the Telemetry library itself is very small, hence the additional code 
to be built would not cause any difference in build performance.
---
 lldb/source/Core/CMakeLists.txt  |  4 +---
 lldb/source/Core/Telemetry.cpp   |  7 ++-
 lldb/unittests/Core/CMakeLists.txt   |  6 +++---
 llvm/CMakeLists.txt  |  3 ++-
 llvm/cmake/modules/LLVMConfig.cmake.in   |  1 +
 llvm/include/llvm/Config/llvm-config.h.cmake |  2 ++
 llvm/include/llvm/Telemetry/Telemetry.h  | 12 ++--
 llvm/lib/CMakeLists.txt  |  5 ++---
 llvm/unittests/CMakeLists.txt|  4 +---
 .../gn/secondary/llvm/include/llvm/Config/BUILD.gn   |  1 +
 utils/bazel/llvm_configs/llvm-config.h.cmake |  2 ++
 11 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index 82fb5f42f9f4b..a3c12e4c1bdbc 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -16,9 +16,7 @@ if (LLDB_ENABLE_CURSES)
   endif()
 endif()
 
-if (LLVM_BUILD_TELEMETRY)
-   set(TELEMETRY_DEPS Telemetry)
-endif()
+set(TELEMETRY_DEPS Telemetry)
 
 # TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore
 add_lldb_library(lldbCore
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 5222f76704f91..51a860ea5313b 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -8,8 +8,6 @@
 
 #include "llvm/Config/llvm-config.h"
 
-#ifdef LLVM_BUILD_TELEMETRY
-
 #include "lldb/Core/Telemetry.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Utility/LLDBLog.h"
@@ -57,7 +55,8 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) 
const {
 }
 
 TelemetryManager::TelemetryManager(std::unique_ptr config)
-: m_config(std::move(config)) {}
+: m_config(std::move(config))
+}
 
 llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
   // Do nothing for now.
@@ -75,5 +74,3 @@ void 
TelemetryManager::setInstance(std::unique_ptr manager) {
 
 } // namespace telemetry
 } // namespace lldb_private
-
-#endif // LLVM_BUILD_TELEMETRY
diff --git a/lldb/unittests/Core/CMakeLists.txt 
b/lldb/unittests/Core/CMakeLists.txt
index d4d3764b67ae3..e8df299631e2e 100644
--- a/lldb/unittests/Core/CMakeLists.txt
+++ b/lldb/unittests/Core/CMakeLists.txt
@@ -1,6 +1,6 @@
-if (LLVM_BUILD_TELEMETRY)
-  set(TELEMETRY_DEPS Telemetry)
-endif()
+
+set(TELEMETRY_DEPS Telemetry)
+
 
 add_lldb_unittest(LLDBCoreTests
   CommunicationTest.cpp
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 88512d0f1dd96..9188fb93b7846 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -835,7 +835,8 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm 
API documentation." OF
 option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF)
 option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON)
 option (LLVM_ENABLE_BINDINGS "Build bindings." ON)
-option (LLVM_BUILD_TELEMETRY "Build the telemetry library. This does not 
enable telemetry." ON)
+option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable 
the telemetry library. If set to OFF, library cannot be enabled after build 
(eg., at runtime)" ON)
+option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF, 
library cannot be enabled after build (eg., at runtime)" ON)
 
 set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html"
 CACHE STRING "Doxygen-generated HTML documentation install directory")
diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in 
b/llvm/cmake/modules/LLVMConfig.cmake.in
index 28655ee3ab87d..134d107ce79ba 100644
--- a/llvm/cmake/modules/LLVMConfig.cmake.in
+++ b/llvm/cmake/modules/LLVMConfig.cmake.in
@@ -101,6 +101,7 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@)
 set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@)
 
 set(LLVM_BUILD_TELEMETRY @LLVM_B

[Lldb-commits] [lldb] [lldb-dap] Addressing the order of events during disconnect to flush output. (PR #128583)

[Pavel Labath via lldb-commits]

labath wrote:

It looks like it's still flaky: 
https://lab.llvm.org/buildbot/#/builders/18/builds/11957

I think there's still at least one possible way this can race (return without 
reading all data):

1. read thread returns from the read call (reads some random data) and gets 
suspended
2. we begin shutting down: set the flag, and write the sentinel to the pipe
3. read thread wakes up, checks the flag and decides to exit (without reading 
our sentinel, or any other remaining data)

I think that one way to improve this would be to do a non-blocking read to 
drain the pipe before returning from the read thread.. which reminds me that I 
wanted to fix the problem which prevented you from using the Pipe class  ~~> 
#128719

https://github.com/llvm/llvm-project/pull/128583
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb]HostInfoMacOSX] Search CommandLineTools directory when looking up SDK paths (PR #128712)

[Michael Buch via lldb-commits]

@@ -15,11 +15,14 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Timer.h"
 
+#include "clang/Basic/DarwinSDKInfo.h"

Michael137 wrote:

Is this a dependency we can pull in? Probably not?

https://github.com/llvm/llvm-project/pull/128712
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb]HostInfoMacOSX] Search CommandLineTools directory when looking up SDK paths (PR #128712)

[Michael Buch via lldb-commits]

https://github.com/Michael137 created 
https://github.com/llvm/llvm-project/pull/128712

`GetSDKRoot` uses `xcrun` to find an SDK root path for a given SDK version 
string. But if the SDK doesn't exist in the Xcode installations, but instead 
lives in the `CommandLineTools`, `xcrun` will fail to find it. Negative 
searches for an SDK path cost a lot (a few seconds) each time `xcrun` is 
invoked. We do cache negative results in `find_cached_path` inside LLDB, but we 
would still pay the price on every new debug session the first time we evaluate 
an expression. This doesn't only cause a noticable delay in running the 
expression, but also generates following error:
```
error: Error while searching for Xcode SDK: timed out waiting for shell command 
to complete
(int) $0 = 42
```

To avoid this `xcrun` penalty, we search `CommandLineTools` for a matching SDK 
ourselves, and only if we don't find it, do we fall back to calling `xcrun`.

rdar://113619904
rdar://113619723

>From 2665e747d5866480f2f704d5a72fe016d258f200 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Tue, 25 Feb 2025 13:11:33 +
Subject: [PATCH] [lldb]HostInfoMacOSX] Search CommandLineTools directory when
 looking up SDK paths

`GetSDKRoot` uses `xcrun` to find an SDK root path for a given SDK
version string. But if the SDK doesn't exist in the Xcode installations,
but instead lives in the `CommandLineTools`, `xcrun` will fail to find
it. Negative searches for an SDK path cost a lot (a few seconds) each
time `xcrun` is invoked. We do cache negative results in
`find_cached_path` inside LLDB, but we would still pay the price on
every new debug session the first time we evaluate an expression. This
doesn't only cause a noticable delay in running the expression, but also
generates following error:
```
error: Error while searching for Xcode SDK: timed out waiting for shell command 
to complete
(int) $0 = 42
```

To avoid this `xcrun` penalty, we search `CommandLineTools` for a
matching SDK ourselves, and only if we don't find it, do we fall back to
calling `xcrun`.

rdar://113619904
rdar://113619723
---
 lldb/include/lldb/Host/FileSystem.h   |  5 +-
 .../Host/macosx/objcxx/HostInfoMacOSX.mm  | 60 +++
 2 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/lldb/include/lldb/Host/FileSystem.h 
b/lldb/include/lldb/Host/FileSystem.h
index 640f3846e448c..4128d7b012041 100644
--- a/lldb/include/lldb/Host/FileSystem.h
+++ b/lldb/include/lldb/Host/FileSystem.h
@@ -183,8 +183,9 @@ class FileSystem {
 eEnumerateDirectoryResultQuit
   };
 
-  typedef EnumerateDirectoryResult (*EnumerateDirectoryCallbackType)(
-  void *baton, llvm::sys::fs::file_type file_type, llvm::StringRef);
+  typedef std::function
+  EnumerateDirectoryCallbackType;
 
   typedef std::function
diff --git a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm 
b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
index 6e924fdc684cf..a94fd3b57f9d6 100644
--- a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -15,11 +15,14 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Timer.h"
 
+#include "clang/Basic/DarwinSDKInfo.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/VersionTuple.h"
 #include "llvm/Support/raw_ostream.h"
 
 // C++ Includes
@@ -569,10 +572,52 @@ static bool ResolveAndVerifyCandidateSupportDir(FileSpec 
&path) {
 cache.insert({key, {error, true}});
 return llvm::createStringError(llvm::inconvertibleErrorCode(), error);
   }
+
+  if (path_or_err->empty())
+return llvm::createStringError("Empty path determined for '%s'",
+   key.data());
+
   auto it_new = cache.insert({key, {*path_or_err, false}});
   return it_new.first->second.str;
 }
 
+static llvm::Expected
+GetCommandLineToolsSDKRoot(llvm::VersionTuple version) {
+  std::string clt_root_dir;
+  FileSystem::Instance().EnumerateDirectory(
+  "/Library/Developer/CommandLineTools/SDKs/", /*find_directories=*/true,
+  /*find_files=*/false, /*find_other=*/false,
+  [&](void *baton, llvm::sys::fs::file_type file_type,
+  llvm::StringRef name) {
+assert(file_type == llvm::sys::fs::file_type::directory_file);
+
+if (!name.ends_with(".sdk"))
+  return FileSystem::eEnumerateDirectoryResultNext;
+
+llvm::Expected> sdk_info =
+clang::parseDarwinSDKInfo(
+*FileSystem::Instance().GetVirtualFileSystem(), name);
+if (!sdk_info) {
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Expressions), sdk_info.takeError(),
+ "Error while parsing {1}: {0}", name);
+  return FileSystem::eEnumerateDirectoryResultNext;
+}
+
+if (!*sdk_info)
+  return FileSystem::eEnumerateDirectoryResult

[Lldb-commits] [lldb] [lldb][nfc] Improve duplicated code in unexecuted breakpoint detection (PR #128724)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere approved this pull request.

Nice cleanup!

https://github.com/llvm/llvm-project/pull/128724
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Assorted improvements to the Pipe class (PR #128719)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere commented:

I don't feel qualified to express an opinion on the implementation, but I 
really appreciate the new interfaces. 

https://github.com/llvm/llvm-project/pull/128719
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb]HostInfoMacOSX] Search CommandLineTools directory when looking up SDK paths (PR #128712)

[Michael Buch via lldb-commits]

@@ -569,10 +572,52 @@ static bool ResolveAndVerifyCandidateSupportDir(FileSpec 
&path) {
 cache.insert({key, {error, true}});
 return llvm::createStringError(llvm::inconvertibleErrorCode(), error);
   }
+
+  if (path_or_err->empty())
+return llvm::createStringError("Empty path determined for '%s'",
+   key.data());
+
   auto it_new = cache.insert({key, {*path_or_err, false}});
   return it_new.first->second.str;
 }
 
+static llvm::Expected
+GetCommandLineToolsSDKRoot(llvm::VersionTuple version) {
+  std::string clt_root_dir;
+  FileSystem::Instance().EnumerateDirectory(
+  "/Library/Developer/CommandLineTools/SDKs/", /*find_directories=*/true,
+  /*find_files=*/false, /*find_other=*/false,
+  [&](void *baton, llvm::sys::fs::file_type file_type,
+  llvm::StringRef name) {
+assert(file_type == llvm::sys::fs::file_type::directory_file);
+
+if (!name.ends_with(".sdk"))
+  return FileSystem::eEnumerateDirectoryResultNext;
+
+llvm::Expected> sdk_info =

Michael137 wrote:

Hmm but looks like it's only able to parse the XML version, not the binary one?

https://github.com/llvm/llvm-project/pull/128712
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb]HostInfoMacOSX] Search CommandLineTools directory when looking up SDK paths (PR #128712)

[Michael Buch via lldb-commits]

@@ -569,10 +572,52 @@ static bool ResolveAndVerifyCandidateSupportDir(FileSpec 
&path) {
 cache.insert({key, {error, true}});
 return llvm::createStringError(llvm::inconvertibleErrorCode(), error);
   }
+
+  if (path_or_err->empty())
+return llvm::createStringError("Empty path determined for '%s'",
+   key.data());
+
   auto it_new = cache.insert({key, {*path_or_err, false}});
   return it_new.first->second.str;
 }
 
+static llvm::Expected
+GetCommandLineToolsSDKRoot(llvm::VersionTuple version) {
+  std::string clt_root_dir;
+  FileSystem::Instance().EnumerateDirectory(
+  "/Library/Developer/CommandLineTools/SDKs/", /*find_directories=*/true,
+  /*find_files=*/false, /*find_other=*/false,
+  [&](void *baton, llvm::sys::fs::file_type file_type,
+  llvm::StringRef name) {
+assert(file_type == llvm::sys::fs::file_type::directory_file);
+
+if (!name.ends_with(".sdk"))
+  return FileSystem::eEnumerateDirectoryResultNext;
+
+llvm::Expected> sdk_info =

Michael137 wrote:

I guess worst case we could read the json ourselves and get the key we're 
looking for (Clang does a bit more than that, it looks at VersionMappings etc.)

https://github.com/llvm/llvm-project/pull/128712
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Assorted improvements to the Pipe class (PR #128719)

[Pavel Labath via lldb-commits]

https://github.com/labath created 
https://github.com/llvm/llvm-project/pull/128719

The main motivation for this was the inconsistency in handling of partial 
reads/writes between the windows and posix implementations (windows was 
returning partial reads, posix was trying to fill the buffer completely). I 
settle on the windows implementation, as that's the more common behavior, and 
the "eager" version can be implemented on top of that (in most cases, it isn't 
necessary, since we're writing just a single byte).

Since this also required auditing the callers to make sure they're handling 
partial reads/writes correctly, I used the opportunity to modernize the 
function signatures as a forcing function. They now use the `Timeout` class 
(basically an `optional`) to support both polls (timeout=0) and 
blocking (timeout=nullopt) operations in a single function, and use an 
`Expected` instead of a by-ref result to return the number of bytes 
read/written.

As a drive-by, I also fix a problem with the windows implementation where we 
were rounding the timeout value down, which meant that calls could time out 
slightly sooner than expected.

>From 9116f3110146965d18eab7e50465855954c87f26 Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Tue, 25 Feb 2025 13:30:12 +0100
Subject: [PATCH] [lldb] Assorted improvements to the Pipe class

The main motivation for this was the inconsistency in handling of
partial reads/writes between the windows and posix implementations
(windows was returning partial reads, posix was trying to fill the
buffer completely). I settle on the windows implementation, as that's
the more common behavior, and the "eager" version can be implemented on
top of that (in most cases, it isn't necessary, since we're writing just
a single byte).

Since this also required auditing the callers to make sure they're
handling partial reads/writes correctly, I used the opportunity to
modernize the function signatures as a forcing function. They now use
the `Timeout` class (basically an `optional`) to support both
polls (timeout=0) and blocking (timeout=nullopt) operations in a single
function, and use an `Expected` instead of a by-ref result to return the
number of bytes read/written.

As a drive-by, I also fix a problem with the windows implementation
where we were rounding the timeout value down, which meant that calls
could time out slightly sooner than expected.
---
 lldb/include/lldb/Host/PipeBase.h |  27 ++---
 lldb/include/lldb/Host/posix/PipePosix.h  |  19 ++--
 lldb/include/lldb/Host/windows/PipeWindows.h  |  18 +--
 lldb/source/Host/common/PipeBase.cpp  |  16 ---
 lldb/source/Host/common/Socket.cpp|  29 ++---
 .../posix/ConnectionFileDescriptorPosix.cpp   |  16 +--
 lldb/source/Host/posix/MainLoopPosix.cpp  |   6 +-
 lldb/source/Host/posix/PipePosix.cpp  | 107 --
 lldb/source/Host/windows/PipeWindows.cpp  |  87 ++
 .../gdb-remote/GDBRemoteCommunication.cpp |  24 ++--
 lldb/source/Target/Process.cpp|  17 +--
 lldb/tools/lldb-server/lldb-gdbserver.cpp |  28 +++--
 lldb/unittests/Host/PipeTest.cpp  |  61 +-
 13 files changed, 203 insertions(+), 252 deletions(-)

diff --git a/lldb/include/lldb/Host/PipeBase.h 
b/lldb/include/lldb/Host/PipeBase.h
index d51d0cd54e036..ed8df6bf1e511 100644
--- a/lldb/include/lldb/Host/PipeBase.h
+++ b/lldb/include/lldb/Host/PipeBase.h
@@ -10,12 +10,11 @@
 #ifndef LLDB_HOST_PIPEBASE_H
 #define LLDB_HOST_PIPEBASE_H
 
-#include 
-#include 
-
 #include "lldb/Utility/Status.h"
+#include "lldb/Utility/Timeout.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
 
 namespace lldb_private {
 class PipeBase {
@@ -32,10 +31,9 @@ class PipeBase {
   virtual Status OpenAsReader(llvm::StringRef name,
   bool child_process_inherit) = 0;
 
-  Status OpenAsWriter(llvm::StringRef name, bool child_process_inherit);
-  virtual Status
-  OpenAsWriterWithTimeout(llvm::StringRef name, bool child_process_inherit,
-  const std::chrono::microseconds &timeout) = 0;
+  virtual llvm::Error OpenAsWriter(llvm::StringRef name,
+   bool child_process_inherit,
+   const Timeout &timeout) = 0;
 
   virtual bool CanRead() const = 0;
   virtual bool CanWrite() const = 0;
@@ -56,14 +54,13 @@ class PipeBase {
   // Delete named pipe.
   virtual Status Delete(llvm::StringRef name) = 0;
 
-  virtual Status WriteWithTimeout(const void *buf, size_t size,
-  const std::chrono::microseconds &timeout,
-  size_t &bytes_written) = 0;
-  Status Write(const void *buf, size_t size, size_t &bytes_written);
-  virtual Status ReadWithTimeout(void *buf, size_t size,
- const std::chrono::microseconds &timeout,
-   

[Lldb-commits] [lldb] [lldb] Assorted improvements to the Pipe class (PR #128719)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)


Changes

The main motivation for this was the inconsistency in handling of partial 
reads/writes between the windows and posix implementations (windows was 
returning partial reads, posix was trying to fill the buffer completely). I 
settle on the windows implementation, as that's the more common behavior, and 
the "eager" version can be implemented on top of that (in most cases, it isn't 
necessary, since we're writing just a single byte).

Since this also required auditing the callers to make sure they're handling 
partial reads/writes correctly, I used the opportunity to modernize the 
function signatures as a forcing function. They now use the `Timeout` class 
(basically an `optional`) to support both polls (timeout=0) and 
blocking (timeout=nullopt) operations in a single function, and use an 
`Expected` instead of a by-ref result to return the number of bytes 
read/written.

As a drive-by, I also fix a problem with the windows implementation where we 
were rounding the timeout value down, which meant that calls could time out 
slightly sooner than expected.

---

Patch is 32.98 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/128719.diff


13 Files Affected:

- (modified) lldb/include/lldb/Host/PipeBase.h (+12-15) 
- (modified) lldb/include/lldb/Host/posix/PipePosix.h (+10-9) 
- (modified) lldb/include/lldb/Host/windows/PipeWindows.h (+9-9) 
- (modified) lldb/source/Host/common/PipeBase.cpp (-16) 
- (modified) lldb/source/Host/common/Socket.cpp (+12-17) 
- (modified) lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp (+6-10) 
- (modified) lldb/source/Host/posix/MainLoopPosix.cpp (+1-5) 
- (modified) lldb/source/Host/posix/PipePosix.cpp (+46-61) 
- (modified) lldb/source/Host/windows/PipeWindows.cpp (+37-50) 
- (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp 
(+17-7) 
- (modified) lldb/source/Target/Process.cpp (+9-8) 
- (modified) lldb/tools/lldb-server/lldb-gdbserver.cpp (+18-10) 
- (modified) lldb/unittests/Host/PipeTest.cpp (+26-35) 


``diff
diff --git a/lldb/include/lldb/Host/PipeBase.h 
b/lldb/include/lldb/Host/PipeBase.h
index d51d0cd54e036..ed8df6bf1e511 100644
--- a/lldb/include/lldb/Host/PipeBase.h
+++ b/lldb/include/lldb/Host/PipeBase.h
@@ -10,12 +10,11 @@
 #ifndef LLDB_HOST_PIPEBASE_H
 #define LLDB_HOST_PIPEBASE_H
 
-#include 
-#include 
-
 #include "lldb/Utility/Status.h"
+#include "lldb/Utility/Timeout.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
 
 namespace lldb_private {
 class PipeBase {
@@ -32,10 +31,9 @@ class PipeBase {
   virtual Status OpenAsReader(llvm::StringRef name,
   bool child_process_inherit) = 0;
 
-  Status OpenAsWriter(llvm::StringRef name, bool child_process_inherit);
-  virtual Status
-  OpenAsWriterWithTimeout(llvm::StringRef name, bool child_process_inherit,
-  const std::chrono::microseconds &timeout) = 0;
+  virtual llvm::Error OpenAsWriter(llvm::StringRef name,
+   bool child_process_inherit,
+   const Timeout &timeout) = 0;
 
   virtual bool CanRead() const = 0;
   virtual bool CanWrite() const = 0;
@@ -56,14 +54,13 @@ class PipeBase {
   // Delete named pipe.
   virtual Status Delete(llvm::StringRef name) = 0;
 
-  virtual Status WriteWithTimeout(const void *buf, size_t size,
-  const std::chrono::microseconds &timeout,
-  size_t &bytes_written) = 0;
-  Status Write(const void *buf, size_t size, size_t &bytes_written);
-  virtual Status ReadWithTimeout(void *buf, size_t size,
- const std::chrono::microseconds &timeout,
- size_t &bytes_read) = 0;
-  Status Read(void *buf, size_t size, size_t &bytes_read);
+  virtual llvm::Expected
+  Write(const void *buf, size_t size,
+const Timeout &timeout = std::nullopt) = 0;
+
+  virtual llvm::Expected
+  Read(void *buf, size_t size,
+   const Timeout &timeout = std::nullopt) = 0;
 };
 }
 
diff --git a/lldb/include/lldb/Host/posix/PipePosix.h 
b/lldb/include/lldb/Host/posix/PipePosix.h
index 2e291160817c4..effd33fba7eb0 100644
--- a/lldb/include/lldb/Host/posix/PipePosix.h
+++ b/lldb/include/lldb/Host/posix/PipePosix.h
@@ -8,6 +8,7 @@
 
 #ifndef LLDB_HOST_POSIX_PIPEPOSIX_H
 #define LLDB_HOST_POSIX_PIPEPOSIX_H
+
 #include "lldb/Host/PipeBase.h"
 #include 
 
@@ -38,9 +39,8 @@ class PipePosix : public PipeBase {
   llvm::SmallVectorImpl &name) override;
   Status OpenAsReader(llvm::StringRef name,
   bool child_process_inherit) override;
-  Status
-  OpenAsWriterWithTimeout(llvm::StringRef name, bool child_process_inherit,
-  const std::chrono::microseconds &timeout) overr

[Lldb-commits] [lldb] [lldb]HostInfoMacOSX] Search CommandLineTools directory when looking up SDK paths (PR #128712)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)


Changes

`GetSDKRoot` uses `xcrun` to find an SDK root path for a given SDK version 
string. But if the SDK doesn't exist in the Xcode installations, but instead 
lives in the `CommandLineTools`, `xcrun` will fail to find it. Negative 
searches for an SDK path cost a lot (a few seconds) each time `xcrun` is 
invoked. We do cache negative results in `find_cached_path` inside LLDB, but we 
would still pay the price on every new debug session the first time we evaluate 
an expression. This doesn't only cause a noticable delay in running the 
expression, but also generates following error:
```
error: Error while searching for Xcode SDK: timed out waiting for shell command 
to complete
(int) $0 = 42
```

To avoid this `xcrun` penalty, we search `CommandLineTools` for a matching SDK 
ourselves, and only if we don't find it, do we fall back to calling `xcrun`.

rdar://113619904
rdar://113619723

---
Full diff: https://github.com/llvm/llvm-project/pull/128712.diff


2 Files Affected:

- (modified) lldb/include/lldb/Host/FileSystem.h (+3-2) 
- (modified) lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm (+60) 


``diff
diff --git a/lldb/include/lldb/Host/FileSystem.h 
b/lldb/include/lldb/Host/FileSystem.h
index 640f3846e448c..4128d7b012041 100644
--- a/lldb/include/lldb/Host/FileSystem.h
+++ b/lldb/include/lldb/Host/FileSystem.h
@@ -183,8 +183,9 @@ class FileSystem {
 eEnumerateDirectoryResultQuit
   };
 
-  typedef EnumerateDirectoryResult (*EnumerateDirectoryCallbackType)(
-  void *baton, llvm::sys::fs::file_type file_type, llvm::StringRef);
+  typedef std::function
+  EnumerateDirectoryCallbackType;
 
   typedef std::function
diff --git a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm 
b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
index 6e924fdc684cf..a94fd3b57f9d6 100644
--- a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -15,11 +15,14 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Timer.h"
 
+#include "clang/Basic/DarwinSDKInfo.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/VersionTuple.h"
 #include "llvm/Support/raw_ostream.h"
 
 // C++ Includes
@@ -569,10 +572,52 @@ static bool ResolveAndVerifyCandidateSupportDir(FileSpec 
&path) {
 cache.insert({key, {error, true}});
 return llvm::createStringError(llvm::inconvertibleErrorCode(), error);
   }
+
+  if (path_or_err->empty())
+return llvm::createStringError("Empty path determined for '%s'",
+   key.data());
+
   auto it_new = cache.insert({key, {*path_or_err, false}});
   return it_new.first->second.str;
 }
 
+static llvm::Expected
+GetCommandLineToolsSDKRoot(llvm::VersionTuple version) {
+  std::string clt_root_dir;
+  FileSystem::Instance().EnumerateDirectory(
+  "/Library/Developer/CommandLineTools/SDKs/", /*find_directories=*/true,
+  /*find_files=*/false, /*find_other=*/false,
+  [&](void *baton, llvm::sys::fs::file_type file_type,
+  llvm::StringRef name) {
+assert(file_type == llvm::sys::fs::file_type::directory_file);
+
+if (!name.ends_with(".sdk"))
+  return FileSystem::eEnumerateDirectoryResultNext;
+
+llvm::Expected> sdk_info =
+clang::parseDarwinSDKInfo(
+*FileSystem::Instance().GetVirtualFileSystem(), name);
+if (!sdk_info) {
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Expressions), sdk_info.takeError(),
+ "Error while parsing {1}: {0}", name);
+  return FileSystem::eEnumerateDirectoryResultNext;
+}
+
+if (!*sdk_info)
+  return FileSystem::eEnumerateDirectoryResultNext;
+
+if (version == (*sdk_info)->getVersion()) {
+  clt_root_dir = name;
+  return FileSystem::eEnumerateDirectoryResultQuit;
+}
+
+return FileSystem::eEnumerateDirectoryResultNext;
+  },
+  /*baton=*/nullptr);
+
+  return clt_root_dir;
+}
+
 llvm::Expected HostInfoMacOSX::GetSDKRoot(SDKOptions options) 
{
   static llvm::StringMap g_sdk_path;
   static std::mutex g_sdk_path_mutex;
@@ -581,6 +626,21 @@ static bool ResolveAndVerifyCandidateSupportDir(FileSpec 
&path) {
"XcodeSDK not specified");
   XcodeSDK sdk = *options.XcodeSDKSelection;
   auto key = sdk.GetString();
+
+  // xcrun doesn't search SDKs in the CommandLineTools (CLT) directory. So if
+  // a program was compiled against a CLT SDK, but that SDK wasn't present in
+  // any of the Xcode installations, then xcrun would fail to find the SDK
+  // (which is expensive). To avoid this we first try to find the specified SDK
+  // in the CLT directory.
+  auto clt_root_dir = find_cached_path(g_sdk_path, g

[Lldb-commits] [lldb] [lldb]HostInfoMacOSX] Search CommandLineTools directory when looking up SDK paths (PR #128712)

[Michael Buch via lldb-commits]

@@ -569,10 +572,52 @@ static bool ResolveAndVerifyCandidateSupportDir(FileSpec 
&path) {
 cache.insert({key, {error, true}});
 return llvm::createStringError(llvm::inconvertibleErrorCode(), error);
   }
+
+  if (path_or_err->empty())
+return llvm::createStringError("Empty path determined for '%s'",
+   key.data());
+
   auto it_new = cache.insert({key, {*path_or_err, false}});
   return it_new.first->second.str;
 }
 
+static llvm::Expected
+GetCommandLineToolsSDKRoot(llvm::VersionTuple version) {
+  std::string clt_root_dir;
+  FileSystem::Instance().EnumerateDirectory(
+  "/Library/Developer/CommandLineTools/SDKs/", /*find_directories=*/true,
+  /*find_files=*/false, /*find_other=*/false,
+  [&](void *baton, llvm::sys::fs::file_type file_type,
+  llvm::StringRef name) {
+assert(file_type == llvm::sys::fs::file_type::directory_file);
+
+if (!name.ends_with(".sdk"))
+  return FileSystem::eEnumerateDirectoryResultNext;
+
+llvm::Expected> sdk_info =

Michael137 wrote:

Alternatively we can scan the `.plist` using LLDB's `ApplePropertyList`, and 
extract the SDK version/name from there. That would allow us not to depend on 
Clang here

https://github.com/llvm/llvm-project/pull/128712
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add fzf_history command to examples (PR #128571)

[Dave Lee via lldb-commits]

https://github.com/kastiglione edited 
https://github.com/llvm/llvm-project/pull/128571
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)

[Vy Nguyen via lldb-commits]

https://github.com/oontvoo updated 
https://github.com/llvm/llvm-project/pull/128534

>From 236ab76b7c9433f8fed6586a4ee18351779cc191 Mon Sep 17 00:00:00 2001
From: Vy Nguyen 
Date: Mon, 24 Feb 2025 11:32:03 -0500
Subject: [PATCH 1/8] [llvm][telemetr]Change Telemetry-disabling mechanism.

Details:
- Previously, we used the LLVM_BUILD_TELEMETRY flag to control whether any 
Telemetry code will be built.
This has proven to cause more nuisance to both users of the Telemetry and any 
further extension of it.
(Eg., we needed to put #ifdef around caller/user code)

- So the new approach is to:
 + Remove this flag and introduce LLVM_ENABLE_TELEMETRY which would be true by 
default.
 + If LLVM_ENABLE_TELEMETRY is set to FALSE (at buildtime), the library would 
still be built BUT Telemetry cannot be enabled.
   And no data can be collected.

The benefit of this is that it simplifies user (and extension) code since we 
just need to put the check on Config::EnableTelemetry.
Besides, the Telemetry library itself is very small, hence the additional code 
to be built would not cause any difference in build performance.
---
 lldb/source/Core/CMakeLists.txt  |  4 +---
 lldb/source/Core/Telemetry.cpp   |  7 ++-
 lldb/unittests/Core/CMakeLists.txt   |  6 +++---
 llvm/CMakeLists.txt  |  3 ++-
 llvm/cmake/modules/LLVMConfig.cmake.in   |  1 +
 llvm/include/llvm/Config/llvm-config.h.cmake |  2 ++
 llvm/include/llvm/Telemetry/Telemetry.h  | 12 ++--
 llvm/lib/CMakeLists.txt  |  5 ++---
 llvm/unittests/CMakeLists.txt|  4 +---
 .../gn/secondary/llvm/include/llvm/Config/BUILD.gn   |  1 +
 utils/bazel/llvm_configs/llvm-config.h.cmake |  2 ++
 11 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index 82fb5f42f9f4b..a3c12e4c1bdbc 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -16,9 +16,7 @@ if (LLDB_ENABLE_CURSES)
   endif()
 endif()
 
-if (LLVM_BUILD_TELEMETRY)
-   set(TELEMETRY_DEPS Telemetry)
-endif()
+set(TELEMETRY_DEPS Telemetry)
 
 # TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore
 add_lldb_library(lldbCore
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 5222f76704f91..51a860ea5313b 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -8,8 +8,6 @@
 
 #include "llvm/Config/llvm-config.h"
 
-#ifdef LLVM_BUILD_TELEMETRY
-
 #include "lldb/Core/Telemetry.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Utility/LLDBLog.h"
@@ -57,7 +55,8 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) 
const {
 }
 
 TelemetryManager::TelemetryManager(std::unique_ptr config)
-: m_config(std::move(config)) {}
+: m_config(std::move(config))
+}
 
 llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
   // Do nothing for now.
@@ -75,5 +74,3 @@ void 
TelemetryManager::setInstance(std::unique_ptr manager) {
 
 } // namespace telemetry
 } // namespace lldb_private
-
-#endif // LLVM_BUILD_TELEMETRY
diff --git a/lldb/unittests/Core/CMakeLists.txt 
b/lldb/unittests/Core/CMakeLists.txt
index d4d3764b67ae3..e8df299631e2e 100644
--- a/lldb/unittests/Core/CMakeLists.txt
+++ b/lldb/unittests/Core/CMakeLists.txt
@@ -1,6 +1,6 @@
-if (LLVM_BUILD_TELEMETRY)
-  set(TELEMETRY_DEPS Telemetry)
-endif()
+
+set(TELEMETRY_DEPS Telemetry)
+
 
 add_lldb_unittest(LLDBCoreTests
   CommunicationTest.cpp
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 88512d0f1dd96..9188fb93b7846 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -835,7 +835,8 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm 
API documentation." OF
 option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF)
 option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON)
 option (LLVM_ENABLE_BINDINGS "Build bindings." ON)
-option (LLVM_BUILD_TELEMETRY "Build the telemetry library. This does not 
enable telemetry." ON)
+option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable 
the telemetry library. If set to OFF, library cannot be enabled after build 
(eg., at runtime)" ON)
+option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF, 
library cannot be enabled after build (eg., at runtime)" ON)
 
 set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html"
 CACHE STRING "Doxygen-generated HTML documentation install directory")
diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in 
b/llvm/cmake/modules/LLVMConfig.cmake.in
index 28655ee3ab87d..134d107ce79ba 100644
--- a/llvm/cmake/modules/LLVMConfig.cmake.in
+++ b/llvm/cmake/modules/LLVMConfig.cmake.in
@@ -101,6 +101,7 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@)
 set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@)
 
 set(LLVM_BUILD_TELEMETRY @LLVM_B

[Lldb-commits] [lldb] [lldb] Add fzf_history command to examples (PR #128571)

[Dave Lee via lldb-commits]

https://github.com/kastiglione closed 
https://github.com/llvm/llvm-project/pull/128571
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits