[Lldb-commits] [lldb] 59f0827 - [clang] Instantiate alias templates with sugar

[Matheus Izvekov via lldb-commits]

Author: Matheus Izvekov
Date: 2022-10-27T06:18:52+02:00
New Revision: 59f0827e2cf3755834620e7e0b6d946832440f80

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

LOG: [clang] Instantiate alias templates with sugar

This makes use of the changes introduced in D134604, in order to
instantiate alias templates witn a final sugared substitution.

This comes at no additional relevant cost.
Since we don't track / unique them in specializations, we wouldn't be
able to resugar them later anyway.

Signed-off-by: Matheus Izvekov 

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

Added: 


Modified: 
clang-tools-extra/clangd/unittests/HoverTests.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/test/AST/ast-dump-template-decls.cpp
clang/test/CXX/temp/temp.deduct.guide/p3.cpp
clang/test/Misc/diag-template-diffing.cpp
clang/test/SemaCXX/sizeless-1.cpp
clang/test/SemaTemplate/make_integer_seq.cpp
clang/test/SemaTemplate/temp_arg_nontype.cpp

lldb/test/API/commands/expression/import-std-module/shared_ptr/TestSharedPtrFromStdModule.py

lldb/test/API/commands/expression/import-std-module/weak_ptr-dbg-info-content/TestDbgInfoContentWeakPtrFromStdModule.py

lldb/test/API/commands/expression/import-std-module/weak_ptr/TestWeakPtrFromStdModule.py

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp 
b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 9f64defaf898f..8bca0dc974539 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1067,7 +1067,7 @@ class Foo final {})cpp";
  HI.LocalScope = "";
  HI.Kind = index::SymbolKind::TypeAlias;
  HI.Definition = "template  using AA = A";
- HI.Type = {"A", "type-parameter-0-0"}; // FIXME: should be 'T'
+ HI.Type = {"A", "T"};
  HI.TemplateParameters = {{{"typename"}, std::string("T"), 
llvm::None}};
}},
   {// Constant array

diff  --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 37eaa2a5795aa..c5a86486b2dea 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -3854,8 +3854,8 @@ QualType Sema::CheckTemplateIdType(TemplateName Name,
 
 // Only substitute for the innermost template argument list.
 MultiLevelTemplateArgumentList TemplateArgLists;
-TemplateArgLists.addOuterTemplateArguments(Template, CanonicalConverted,
-   /*Final=*/false);
+TemplateArgLists.addOuterTemplateArguments(Template, SugaredConverted,
+   /*Final=*/true);
 TemplateArgLists.addOuterRetainedLevels(
 AliasTemplate->getTemplateParameters()->getDepth());
 

diff  --git a/clang/test/AST/ast-dump-template-decls.cpp 
b/clang/test/AST/ast-dump-template-decls.cpp
index 49760d17c7f22..3bc0357d1b51f 100644
--- a/clang/test/AST/ast-dump-template-decls.cpp
+++ b/clang/test/AST/ast-dump-template-decls.cpp
@@ -120,8 +120,6 @@ using type2 = typename C::type1;
 // CHECK-NEXT: TemplateArgument type 'void'
 // CHECK-NEXT: BuiltinType 0x{{[^ ]*}} 'void'
 // CHECK-NEXT: FunctionProtoType 0x{{[^ ]*}} 'void (int)' cdecl
-// CHECK-NEXT: SubstTemplateTypeParmType 0x{{[^ ]*}} 'void' sugar class depth 
0 index 0 U
-// CHECK-NEXT: TypeAliasTemplate 0x{{[^ ]*}} 'type1'
 // CHECK-NEXT: BuiltinType 0x{{[^ ]*}} 'void'
 // CHECK-NEXT: SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar class depth 0 
index 0 T
 // CHECK-NEXT: ClassTemplateSpecialization 0x{{[^ ]*}} 'C'
@@ -129,38 +127,24 @@ using type2 = typename C::type1;
 } // namespace PR55886
 
 namespace PR56099 {
-template  struct Y;
-template  using Z = Y;
-template  struct foo {
-  template  using bind = Z;
-};
-using t1 = foo::bind;
-// CHECK:  TemplateSpecializationType 0x{{[^ ]*}} 'Y' sugar Y
-// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'char' sugar typename 
depth 0 index 0 ... Bs pack_index 3
-// CHECK-NEXT: TypeAliasTemplate 0x{{[^ ]*}} 'Z'
-// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'float' sugar typename 
depth 0 index 0 ... Bs pack_index 2
-// CHECK-NEXT: TypeAliasTemplate 0x{{[^ ]*}} 'Z'
-// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar typename 
depth 0 index 0 ... Bs pack_index 1
-// CHECK-NEXT: TypeAliasTemplate 0x{{[^ ]*}} 'Z'
-// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'short' sugar typename 
depth 0 index 0 ... Bs pack_index 0
-// CHECK-NEXT: TypeAliasTemplate 0x{{[^ ]*}} 'Z'
-
 template  struct D {
-  template  using B = int(int (*...p)(T, U));
+  template  struct bind {
+using bound_type = int(int (*...p)(T, U));
+  };
 };
-using t2 = D::B;
-// CHECK:  TemplateSpecializationType 0x{{[^ ]*}} 'B' sugar 
alias B

[Lldb-commits] [PATCH] D136801: [intelpt] Update Python tests to account for new errrors

[walter erquinigo via Phabricator via lldb-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136801

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

[Lldb-commits] [lldb] 7bbd0fb - Revert "[lldb-vscode] Send Statistics Dump in terminated event"

[Wanyi Ye via lldb-commits]

Author: Wanyi Ye
Date: 2022-10-26T19:49:03-07:00
New Revision: 7bbd0fba986c241162b77b7e424ad82bc7e17b41

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

LOG: Revert "[lldb-vscode] Send Statistics Dump in terminated event"

This reverts commit c8a26f8c6de30dbd814546f02e4c89a4fcb2b4ef.

Returning full statistics result in "terminated" (DAP) event could result in 
delay in the UI when debugging from VSCode.

If the program run to exit and debug session terminates. The DAP event order 
will be: exited event --> terminateCommands --> terminated event --> disconnect 
request --> disconnect response.

The debugging UI in VSCode corresponds to "disconnect" request/response. If the 
terminated event is taking long to process, the IDE won't quit debugging UI 
until it's done.

For big binary (tested example has 29 GB of debug info), it can cause ~15s 
delay in terminated event itself. And the UI could take ~20s to reflect.

This may cause confusion in debug sessions. We should persuit a more 
lightweight return or other solution to return such info.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/tools/lldb-vscode/JSONUtils.cpp
lldb/tools/lldb-vscode/JSONUtils.h
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 
lldb/test/API/tools/lldb-vscode/terminated-event/Makefile

lldb/test/API/tools/lldb-vscode/terminated-event/TestVSCode_terminatedEvent.py
lldb/test/API/tools/lldb-vscode/terminated-event/foo.cpp
lldb/test/API/tools/lldb-vscode/terminated-event/foo.h
lldb/test/API/tools/lldb-vscode/terminated-event/main.cpp



diff  --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
index c2de4ad5c7d9a..d6a6abca53e38 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -369,13 +369,7 @@ def wait_for_stopped(self, timeout=None):
 def wait_for_exited(self):
 event_dict = self.wait_for_event('exited')
 if event_dict is None:
-raise ValueError("didn't get exited event")
-return event_dict
-
-def wait_for_terminated(self):
-event_dict = self.wait_for_event('terminated')
-if event_dict is None:
-raise ValueError("didn't get terminated event")
+raise ValueError("didn't get stopped event")
 return event_dict
 
 def get_initialize_value(self, key):

diff  --git a/lldb/test/API/tools/lldb-vscode/terminated-event/Makefile 
b/lldb/test/API/tools/lldb-vscode/terminated-event/Makefile
deleted file mode 100644
index b30baf48b972e..0
--- a/lldb/test/API/tools/lldb-vscode/terminated-event/Makefile
+++ /dev/null
@@ -1,17 +0,0 @@
-DYLIB_NAME := foo
-DYLIB_CXX_SOURCES := foo.cpp
-CXX_SOURCES := main.cpp
-
-LD_EXTRAS := -Wl,-rpath "-Wl,$(shell pwd)"
-USE_LIBDL :=1
-
-include Makefile.rules
-
-all: a.out.stripped
-
-a.out.stripped:
-   strip -o a.out.stripped a.out
-
-ifneq "$(CODESIGN)" ""
-   $(CODESIGN) -fs - a.out.stripped
-endif
\ No newline at end of file

diff  --git 
a/lldb/test/API/tools/lldb-vscode/terminated-event/TestVSCode_terminatedEvent.py
 
b/lldb/test/API/tools/lldb-vscode/terminated-event/TestVSCode_terminatedEvent.py
deleted file mode 100644
index 0a9fe6ac5a719..0
--- 
a/lldb/test/API/tools/lldb-vscode/terminated-event/TestVSCode_terminatedEvent.py
+++ /dev/null
@@ -1,60 +0,0 @@
-"""
-Test lldb-vscode terminated event
-"""
-
-import vscode
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-import lldbvscode_testcase
-import re
-import json
-
-class TestVSCode_terminatedEvent(lldbvscode_testcase.VSCodeTestCaseBase):
-
-@skipIfWindows
-@skipIfRemote
-def test_terminated_event(self):
-'''
-Terminated Event
-Now contains the statistics of a debug session:
-metatdata:
-totalDebugInfoByteSize > 0
-totalDebugInfoEnabled > 0
-totalModuleCountHasDebugInfo > 0
-...
-targetInfo:
-totalBreakpointResolveTime > 0
-breakpoints:
-recognize function breakpoint
-recognize source line breakpoint
-It should contains the breakpoints info: function bp & source line 
bp
-'''
-
-program_basename = "a.out.stripped"
-program = self.getBuildArtifact(program_basename)
-self.build_and_launch(program)
-# Set breakpoints
-functions = ['foo']
-breakpoint_ids = self.set_function_breakpoints(functions)
-

[Lldb-commits] [PATCH] D136697: Add formatting support for VSCode logpoints message

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:29
+lldb::SBError BreakpointBase::AppendLogMessagePart(llvm::StringRef part,
+  bool is_expr) {
+  if (is_expr) {

indentation is off here. 



Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:45
+lldb::SBError BreakpointBase::FormatLogText(llvm::StringRef text,
+   std::string ) {
+  lldb::SBError error;

indentation is off here



Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:53
+}
+assert(backslash_pos >=0 && backslash_pos < text.size());
+

Remove assert, this should be already tested in StringRef::find(...)



Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:55
+
+formatted += text.substr(0, backslash_pos + 1).str();
+// Skip the characters before (and include) '\'.

Is this going to append the backslash character as well? Seems like a bug. 
Above suggested code did this correctly I think with:

```
  // Append anything before the backslash character.
  if (backslash_pos > 0)
formatted += text.drop_front(backslash_pos).str();
```
Which was always then followed by:
```
  text = text.drop_front(); // Drop the backslash
```



Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:306
   }
+  output.push_back('\n'); // Ensure log message has line break.
   g_vsc.SendOutput(OutputType::Console, output.c_str());

I would still only push one if the output doesn't end with one and add a test 
for this.
```
if (!output.empty() && output.back() != '\n')
  output.push_back('\n'); // Ensure log message has line break.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136697

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

[Lldb-commits] [PATCH] D126014: Show error message for optimized variables

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D126014#3886996 , @jgorbe wrote:

> Hi! I've just debugged an issue that brought me to this commit. I'll start 
> preparing a patch tomorrow (not sure how to test it yet) but since it's a 
> recent-ish change I figured I'd ping the commit thread to give you a heads up.
>
> `v.GetSummary()` returns a char* that is backed by a member `std::string 
> m_summary_str` in SBValue, and we put the result in a StringRef that just 
> points there. I've found that in some cases the call to `v.GetError()` 
> invalidates the cached summary and causes the `m_summary_str` member to be 
> cleared.
>
> So, if we have a summary provider that returns "Summary string", it ends up 
> being displayed as "ummary string", because clearing 
> `m_summary_str` sets the first byte to 0 but we are still holding on to a 
> StringRef with the old length.
>
> Calling `v.GetError()` first seems to avoid the issue. Another option to fix 
> it would be to keep the summary in a local `std::string` instead. Any 
> preference?

I would vote that we modify our lldb::SBValue to not clear the cached 
m_summary_str when it is called. Doesn't make any sense for it to do this. 
Since we are returning "const char *" from the SBValue object methods, those 
strings need to live for the lifetime of the object and any methods on SBValue 
shouldn't be causing others to be cleared, unless someone holds onto a SBValue 
object and it actually gets updated due to a process resuming and then stopping.

See inlined code suggestion as a way to fix this a bit more safely.

If you want to use std::string objects you have to watch our for NULL being 
returned for any "const char *". If NULL is returned the std::string 
constructor will crash lldb-vscode.




Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:136-156
   llvm::StringRef value = v.GetValue();
   llvm::StringRef summary = v.GetSummary();
   llvm::StringRef type_name = v.GetType().GetDisplayTypeName();
+  lldb::SBError error = v.GetError();
 
   std::string result;
   llvm::raw_string_ostream strm(result);

Another way to fix this is to re-org the code a bit. We don't need the value, 
summary or typename if there is an error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126014

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

[Lldb-commits] [PATCH] D132734: [lldb] Fix member access in GetExpressionPath

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Thanks Will, let me apply this locally and see if that addresses the issue for 
which I reverted the original patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132734

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

[Lldb-commits] [PATCH] D136798: [lldb] Explicitly open file to write with utf-8 encoding in crashlog.py

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.

LGTM with @rupprecht's comments addressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136798

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

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 470975.
JDevlieghere added a comment.

Remove spurious `lldb` subdir


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

https://reviews.llvm.org/D134991

Files:
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/Utility/Diagnostics.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Initialization/SystemInitializerCommon.cpp
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/Diagnostics.cpp
  lldb/tools/driver/Driver.cpp

Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -788,6 +788,10 @@
<< '\n';
 return 1;
   }
+
+  // Setup LLDB signal handlers once the debugger has been initialized.
+  SBDebugger::PrintDiagnosticsOnError();
+
   SBHostOS::ThreadCreated("");
 
   signal(SIGINT, sigint_handler);
Index: lldb/source/Utility/Diagnostics.cpp
===
--- /dev/null
+++ lldb/source/Utility/Diagnostics.cpp
@@ -0,0 +1,74 @@
+//===-- Diagnostics.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Utility/Diagnostics.h"
+#include "lldb/Utility/LLDBAssert.h"
+
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace lldb_private;
+using namespace lldb;
+using namespace llvm;
+
+void Diagnostics::Initialize() {
+  lldbassert(!InstanceImpl() && "Already initialized.");
+  InstanceImpl().emplace();
+}
+
+void Diagnostics::Terminate() {
+  lldbassert(InstanceImpl() && "Already terminated.");
+  InstanceImpl().reset();
+}
+
+Optional ::InstanceImpl() {
+  static Optional g_diagnostics;
+  return g_diagnostics;
+}
+
+Diagnostics ::Instance() { return *InstanceImpl(); }
+
+Diagnostics::Diagnostics() {}
+
+Diagnostics::~Diagnostics() {}
+
+void Diagnostics::AddCallback(Callback callback) {
+  std::lock_guard guard(m_callbacks_mutex);
+  m_callbacks.push_back(callback);
+}
+
+bool Diagnostics::Dump(raw_ostream ) {
+  SmallString<128> diagnostics_dir;
+  std::error_code ec =
+  sys::fs::createUniqueDirectory("diagnostics", diagnostics_dir);
+  if (ec) {
+stream << "unable to create diagnostic dir: "
+   << toString(errorCodeToError(ec)) << '\n';
+return false;
+  }
+
+  stream << "LLDB diagnostics written to " << diagnostics_dir << "\n";
+  stream << "Please include the directory content when filing a bug report\n";
+
+  Error error = Create(FileSpec(diagnostics_dir.str()));
+  if (error) {
+stream << toString(std::move(error)) << '\n';
+return false;
+  }
+
+  return true;
+}
+
+Error Diagnostics::Create(const FileSpec ) {
+  for (Callback c : m_callbacks) {
+if (Error err = c(dir))
+  return err;
+  }
+  return Error::success();
+}
Index: lldb/source/Utility/CMakeLists.txt
===
--- lldb/source/Utility/CMakeLists.txt
+++ lldb/source/Utility/CMakeLists.txt
@@ -35,6 +35,7 @@
   DataBufferLLVM.cpp
   DataEncoder.cpp
   DataExtractor.cpp
+  Diagnostics.cpp
   Environment.cpp
   Event.cpp
   FileSpec.cpp
Index: lldb/source/Initialization/SystemInitializerCommon.cpp
===
--- lldb/source/Initialization/SystemInitializerCommon.cpp
+++ lldb/source/Initialization/SystemInitializerCommon.cpp
@@ -12,6 +12,8 @@
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/Socket.h"
+#include "lldb/Target/Statistics.h"
+#include "lldb/Utility/Diagnostics.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Timer.h"
 #include "lldb/Version/Version.h"
@@ -63,6 +65,7 @@
 
   InitializeLldbChannel();
 
+  Diagnostics::Initialize();
   FileSystem::Initialize();
   HostInfo::Initialize(m_shlib_dir_helper);
 
@@ -95,4 +98,5 @@
   HostInfo::Terminate();
   Log::DisableAllLogChannels();
   FileSystem::Terminate();
+  Diagnostics::Terminate();
 }
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -44,6 +44,7 @@
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadList.h"
 #include "lldb/Utility/AnsiTerminal.h"
+#include "lldb/Utility/Diagnostics.h"
 #include "lldb/Utility/Event.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Listener.h"
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -51,6 

[Lldb-commits] [PATCH] D126014: Show error message for optimized variables

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe added a comment.

Hi! I've just debugged an issue that brought me to this commit. I'll start 
preparing a patch tomorrow (not sure how to test it yet) but since it's a 
recent-ish change I figured I'd ping the commit thread to give you a heads up.

`v.GetSummary()` returns a char* that is backed by a member `std::string 
m_summary_str` in SBValue, and we put the result in a StringRef that just 
points there. I've found that in some cases the call to `v.GetError()` 
invalidates the cached summary and causes the `m_summary_str` member to be 
cleared.

So, if we have a summary provider that returns "Summary string", it ends up 
being displayed as "ummary string", because clearing `m_summary_str` 
sets the first byte to 0 but we are still holding on to a StringRef with the 
old length.

Calling `v.GetError()` first seems to avoid the issue. Another option to fix it 
would be to keep the summary in a local `std::string` instead. Any preference?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126014

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

[Lldb-commits] [PATCH] D136719: change debugserver to return an empty platform name for unknown platforms, instead of crashing

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda updated this revision to Diff 470972.
jasonmolenda added a comment.

Update patch to pass std::string's in the optionals, and use std::nullopt.  
Thanks for all the feedback on this one, Thorsten.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136719

Files:
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNB.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm
  lldb/tools/debugserver/source/RNBRemote.cpp

Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -6258,12 +6258,12 @@
 
 bool is_executable = true;
 uint32_t major_version, minor_version, patch_version;
-auto *platform =
+std::optional platform =
 DNBGetDeploymentInfo(pid, is_executable, lc, load_command_addr,
  major_version, minor_version, patch_version);
-if (platform) {
+if (platform.has_value()) {
   os_handled = true;
-  rep << "ostype:" << platform << ";";
+  rep << "ostype:" << platform.value() << ";";
   break;
 }
 load_command_addr = load_command_addr + lc.cmdsize;
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -720,7 +720,9 @@
   return info;
 }
 
-const char *MachProcess::GetPlatformString(unsigned char platform) {
+std::optional
+MachProcess::GetPlatformString(unsigned char platform) {
+  std::optional platform_string;
   switch (platform) {
   case PLATFORM_MACOS:
 return "macosx";
@@ -742,8 +744,10 @@
 return "bridgeos";
   case PLATFORM_DRIVERKIT:
 return "driverkit";
+  default:
+DNBLogError("Unknown platform %u found for one binary", platform);
+return std::nullopt;
   }
-  return nullptr;
 }
 
 static bool mach_header_validity_test(uint32_t magic, uint32_t cputype) {
@@ -867,7 +871,8 @@
 }
 if (DeploymentInfo deployment_info = GetDeploymentInfo(
 lc, load_cmds_p, inf.mach_header.filetype == MH_EXECUTE)) {
-  const char *lc_platform = GetPlatformString(deployment_info.platform);
+  std::optional lc_platform =
+  GetPlatformString(deployment_info.platform);
   if (dyld_platform != PLATFORM_MACCATALYST &&
   inf.min_version_os_name == "macosx") {
 // macCatalyst support.
@@ -882,7 +887,7 @@
 // processed, ignore this one, which is presumed to be a
 // PLATFORM_MACCATALYST one.
   } else {
-inf.min_version_os_name = lc_platform;
+inf.min_version_os_name = lc_platform.value_or("");
 inf.min_version_os_version = "";
 inf.min_version_os_version +=
 std::to_string(deployment_info.major_version);
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.h
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.h
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.h
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -252,7 +253,7 @@
   DeploymentInfo GetDeploymentInfo(const struct load_command &,
uint64_t load_command_address,
bool is_executable);
-  static const char *GetPlatformString(unsigned char platform);
+  static std::optional GetPlatformString(unsigned char platform);
   bool GetMachOInformationFromMemory(uint32_t platform,
  nub_addr_t mach_o_header_addr,
  int wordsize,
Index: lldb/tools/debugserver/source/DNB.h
===
--- lldb/tools/debugserver/source/DNB.h
+++ lldb/tools/debugserver/source/DNB.h
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #define DNB_EXPORT __attribute__((visibility("default")))
@@ -134,12 +135,11 @@
 nub_size_t
 DNBProcessGetSharedLibraryInfo(nub_process_t pid, nub_bool_t only_changed,
DNBExecutableImageInfo **image_infos) DNB_EXPORT;
-const char *DNBGetDeploymentInfo(nub_process_t pid, bool is_executable,
- const struct load_command ,
- uint64_t load_command_address,
- uint32_t _version,
- uint32_t _version,
- uint32_t _version);
+std::optional
+DNBGetDeploymentInfo(nub_process_t pid, bool is_executable,
+ const struct load_command ,
+  

[Lldb-commits] [PATCH] D136798: [lldb] Explicitly open file to write with utf-8 encoding in crashlog.py

[Jordan Rupprecht via Phabricator via lldb-commits]

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

Two other places you might need to update as well




Comment at: lldb/examples/python/crashlog.py:452
 
 with open(path, 'r') as f:
 buffer = f.read()

Do you need to specify encoding here too?



Comment at: lldb/examples/python/crashlog.py:647
 def parse(self):
 with open(self.path,'r') as f:
 lines = f.read().splitlines()

(and here)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136798

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

[Lldb-commits] [PATCH] D136801: [intelpt] Update Python tests to account for new errrors

[Jakob Johnson via Phabricator via lldb-commits]

jj10306 created this revision.
jj10306 added reviewers: wallace, persona0220.
Herald added a project: All.
jj10306 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Update the Python tests (ie tests run via `lldb-dotest -p TestTrace`) to
handle new error introduced in D136610 .

Test Plan:
`lldb-dotest -p TestTrace`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136801

Files:
  lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py
  lldb/test/API/commands/trace/TestTraceLoad.py

Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -13,11 +13,11 @@
 trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace.json")
 self.traceLoad(traceDescriptionFilePath=trace_description_file_path, substrs=["intel-pt"])
 self.expect("thread trace dump instructions 2 -t",
-  substrs=["19532: [20456513.000 ns] (error) expected tracing enabled event",
+  substrs=["19526: [19691636.212 ns] (error) decoding truncated: TSC 40450075478109270 exceeds maximum TSC value 40450075477704372, will skip decoding the remaining data of the PSB (skipping 774 of 825 bytes)",
"m.out`foo() + 65 at multi_thread.cpp:12:21",
-   "9524: [19691630.226 ns] 0x00400ba7jg 0x400bb3"])
+   "9524: [19691632.221 ns] 0x00400ba7jg 0x400bb3"])
 self.expect("thread trace dump instructions 3 -t",
-  substrs=["61831: [19736134.073 ns] 0x00400bd7addl   $0x1, -0x4(%rbp)",
+  substrs=["61831: [19736132.088 ns] 0x00400bd7addl   $0x1, -0x4(%rbp)",
"m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
 self.expect("thread trace dump info --json",
@@ -62,20 +62,20 @@
   "traceTechnology": "intel-pt",
   "threadStats": {
 "tid": 3497496,
-"traceItemsCount": 19533,
+"traceItemsCount": 19527,
 "memoryUsage": {
-  "totalInBytes": "176065",
-  "avgPerItemInBytes": 9.''', '''},
+  "totalInBytes": "175819",
+  "avgPerItemInBytes": 9.0038920469094084''', '''},
 "timingInSeconds": {
   "Decoding instructions": ''', '''
 },
 "events": {
-  "totalCount": 11,
+  "totalCount": 5,
   "individualCounts": {
 "software disabled tracing": 1,
 "trace synchronization point": 1,
 "CPU core changed": 1,
-"HW clock tick": 8
+"HW clock tick": 2
   }
 },
 "errors": {
@@ -116,11 +116,12 @@
 # we'll load the compact trace and make sure it works
 self.traceLoad(os.path.join(compact_trace_bundle_dir, "trace.json"), substrs=["intel-pt"])
 self.expect("thread trace dump instructions 2 -t",
-  substrs=["19532: [20456513.000 ns] (error) expected tracing enabled event",
+  substrs=["19526: [19691636.212 ns] (error) decoding truncated: TSC 40450075478109270 exceeds maximum TSC value 40450075477704372, will skip decoding the remaining data of the PSB (skipping 774 of 825 bytes)",
"m.out`foo() + 65 at multi_thread.cpp:12:21",
-   "19524: [19691630.226 ns] 0x00400ba7jg 0x400bb3"])
+   "19524: [19691632.221 ns] 0x00400ba7jg 0x400bb3"])
 self.expect("thread trace dump instructions 3 -t",
-  substrs=["61831: [19736134.073 ns] 0x00400bd7addl   $0x1, -0x4(%rbp)",
+  substrs=["61833: [19736136.079 ns] (error) decoding truncated: TSC 40450075478174268 exceeds maximum TSC value 40450075477820383, will skip decoding the remaining data of the PSB (skipping 296 of 297 bytes)",
+   "61831: [19736132.088 ns] 0x00400bd7addl   $0x1, -0x4(%rbp)",
"m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
 # This reduced the number of continuous executions to look at
@@ -135,11 +136,11 @@
 trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_with_string_numbers.json")
 self.traceLoad(traceDescriptionFilePath=trace_description_file_path, substrs=["intel-pt"])
 self.expect("thread trace dump instructions 2 -t",
-  substrs=["19532: [20456513.000 ns] (error) expected tracing enabled event",
+  substrs=["19526: [19691636.212 ns] (error) decoding truncated: TSC 40450075478109270 exceeds maximum TSC value 40450075477704372, will skip decoding the remaining data of the PSB (skipping 774 of 825 bytes)",
"m.out`foo() + 65 at multi_thread.cpp:12:21",
-   "19524: [19691630.226 ns] 0x00400ba7jg 0x400bb3"])
+   "19524: [19691632.221 ns] 0x00400ba7jg 0x400bb3"])
 

[Lldb-commits] [PATCH] D136798: [lldb] Explicitly open file to write with utf-8 encoding in crashlog.py

[Augusto Noronha via Phabricator via lldb-commits]

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

The python "open" function will use the default encoding for the
locale (the result of "locale.getpreferredencoding()"). Explicitly set
the locale to utf-8 when opening the crashlog for writing, as there may
be non-ascii symbols in there (for example, Swift uses "τ" to indicate
generic parameters).

rdar://101402755


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136798

Files:
  lldb/examples/python/crashlog.py


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -865,7 +865,7 @@
 result.PutCString(
 "error: invalid arguments, a single output file is the only valid 
argument")
 return
-out_file = open(args[0], 'w')
+out_file = open(args[0], 'w', encoding='utf-8')
 if not out_file:
 result.PutCString(
 "error: failed to open file '%s' for writing...",


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -865,7 +865,7 @@
 result.PutCString(
 "error: invalid arguments, a single output file is the only valid argument")
 return
-out_file = open(args[0], 'w')
+out_file = open(args[0], 'w', encoding='utf-8')
 if not out_file:
 result.PutCString(
 "error: failed to open file '%s' for writing...",
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D136795: [LLDB] Mark placeholder modules in `target module list` output.

[Zequan Wu via Phabricator via lldb-commits]

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

Minidump may contain modules that are created from placeholder object files if
they are not loaded. Add a `(placeholder)` suffix to indicate they are not
loaded, which is more clear than checking if the address is 0x0 in
`target module list -o`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136795

Files:
  lldb/source/Commands/CommandObjectTarget.cpp


Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -3110,11 +3110,14 @@
 DumpModuleArchitecture(strm, module, true, width);
 break;
 
-  case 'f':
+  case 'f': {
 DumpFullpath(strm, >GetFileSpec(), width);
 dump_object_name = true;
+ObjectFile *objfile = module->GetObjectFile();
+if (objfile && objfile->GetPluginName() == "placeholder")
+  strm.Printf(" (placeholder)");
 break;
-
+  }
   case 'd':
 DumpDirectory(strm, >GetFileSpec(), width);
 break;


Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -3110,11 +3110,14 @@
 DumpModuleArchitecture(strm, module, true, width);
 break;
 
-  case 'f':
+  case 'f': {
 DumpFullpath(strm, >GetFileSpec(), width);
 dump_object_name = true;
+ObjectFile *objfile = module->GetObjectFile();
+if (objfile && objfile->GetPluginName() == "placeholder")
+  strm.Printf(" (placeholder)");
 break;
-
+  }
   case 'd':
 DumpDirectory(strm, >GetFileSpec(), width);
 break;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] cb0eb9d - [test] Fix LLDB tests with just-built libcxx when using a target directory.

[Jordan Rupprecht via lldb-commits]

Author: Jordan Rupprecht
Date: 2022-10-26T12:07:22-07:00
New Revision: cb0eb9d8dd5a74ed33d5dd5c62686fb6342b10bc

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

LOG: [test] Fix LLDB tests with just-built libcxx when using a target directory.

In certain configurations, libc++ headers all exist in the same directory, and 
libc++ binaries exist in the same directory as lldb libs. When 
`LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` is enabled (*and* the host is not Apple, 
which is why I assume this wasn't caught by others?), this is not the case: 
most headers will exist in the usual `include/c++/v1` directory, but 
`__config_site` exists in `include/$TRIPLE/c++/v1`. Likewise, the libc++.so 
binary exists in `lib/$TRIPLE/`, not `lib/` (where LLDB libraries reside).

This also adds the just-built-libcxx functionality to the lldb-dotest tool.

The `LIBCXX_` cmake config is borrowed from `libcxx/CMakeLists.txt`. I could 
not figure out a way to share the cmake config; ideally we would reuse the same 
config instead of copy/paste.

Reviewed By: JDevlieghere, fdeazeve

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/builders/builder.py
lldb/packages/Python/lldbsuite/test/configuration.py
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/packages/Python/lldbsuite/test/dotest_args.py
lldb/packages/Python/lldbsuite/test/make/Makefile.rules
lldb/test/API/lit.cfg.py
lldb/test/API/lit.site.cfg.py.in
lldb/test/CMakeLists.txt
lldb/utils/lldb-dotest/CMakeLists.txt
lldb/utils/lldb-dotest/lldb-dotest.in

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/builders/builder.py 
b/lldb/packages/Python/lldbsuite/test/builders/builder.py
index 3cd48378435e5..4817daea52026 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/builder.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/builder.py
@@ -122,8 +122,12 @@ def getModuleCacheSpec(self):
 
 def getLibCxxArgs(self):
 if configuration.libcxx_include_dir and 
configuration.libcxx_library_dir:
-return 
["LIBCPP_INCLUDE_DIR={}".format(configuration.libcxx_include_dir),
-
"LIBCPP_LIBRARY_DIR={}".format(configuration.libcxx_library_dir)]
+libcpp_args = 
["LIBCPP_INCLUDE_DIR={}".format(configuration.libcxx_include_dir),
+   
"LIBCPP_LIBRARY_DIR={}".format(configuration.libcxx_library_dir)]
+if configuration.libcxx_include_target_dir:
+libcpp_args.append("LIBCPP_INCLUDE_TARGET_DIR={}".format(
+configuration.libcxx_include_target_dir))
+return libcpp_args
 return []
 
 def _getDebugInfoArgs(self, debug_info):

diff  --git a/lldb/packages/Python/lldbsuite/test/configuration.py 
b/lldb/packages/Python/lldbsuite/test/configuration.py
index 2a8d125f2ee94..266d785971ee7 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -124,6 +124,7 @@
 lldb_libs_dir = None
 
 libcxx_include_dir = None
+libcxx_include_target_dir = None
 libcxx_library_dir = None
 
 # A plugin whose tests will be enabled, like intel-pt.

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index b4b1a365bbdfe..818e8f618afa7 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -280,17 +280,15 @@ def parseOptionsAndInitTestdirs():
 logging.warning('No valid FileCheck executable; some tests may 
fail...')
 logging.warning('(Double-check the --llvm-tools-dir argument to 
dotest.py)')
 
-configuration.libcxx_include_dir = args.libcxx_include_dir
-configuration.libcxx_library_dir = args.libcxx_library_dir
 if args.libcxx_include_dir or args.libcxx_library_dir:
 if args.lldb_platform_name:
 logging.warning('Custom libc++ is not supported for remote runs: 
ignoring --libcxx arguments')
-elif args.libcxx_include_dir and args.libcxx_library_dir:
-configuration.libcxx_include_dir = args.libcxx_include_dir
-configuration.libcxx_library_dir = args.libcxx_library_dir
-else:
+elif not (args.libcxx_include_dir and args.libcxx_library_dir):
 logging.error('Custom libc++ requires both --libcxx-include-dir 
and --libcxx-library-dir')
 sys.exit(-1)
+configuration.libcxx_include_dir = args.libcxx_include_dir
+configuration.libcxx_include_target_dir = args.libcxx_include_target_dir
+configuration.libcxx_library_dir = args.libcxx_library_dir
 
 if args.channels:
 lldbtest_config.channels = args.channels

diff  --git 

[Lldb-commits] [PATCH] D133973: [test] Fix LLDB tests with just-built libcxx when using a target directory.

[Jordan Rupprecht via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcb0eb9d8dd5a: [test] Fix LLDB tests with just-built libcxx 
when using a target directory. (authored by rupprecht).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133973

Files:
  lldb/packages/Python/lldbsuite/test/builders/builder.py
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules
  lldb/test/API/lit.cfg.py
  lldb/test/API/lit.site.cfg.py.in
  lldb/test/CMakeLists.txt
  lldb/utils/lldb-dotest/CMakeLists.txt
  lldb/utils/lldb-dotest/lldb-dotest.in

Index: lldb/utils/lldb-dotest/lldb-dotest.in
===
--- lldb/utils/lldb-dotest/lldb-dotest.in
+++ lldb/utils/lldb-dotest/lldb-dotest.in
@@ -13,6 +13,10 @@
 lldb_framework_dir = "@LLDB_FRAMEWORK_DIR_CONFIGURED@"
 lldb_libs_dir = "@LLDB_LIBS_DIR_CONFIGURED@"
 llvm_tools_dir = "@LLVM_TOOLS_DIR_CONFIGURED@"
+has_libcxx = @LLDB_HAS_LIBCXX@
+libcxx_libs_dir = "@LIBCXX_LIBRARY_DIR@"
+libcxx_include_dir = "@LIBCXX_GENERATED_INCLUDE_DIR@"
+libcxx_include_target_dir = "@LIBCXX_GENERATED_INCLUDE_TARGET_DIR@"
 
 if __name__ == '__main__':
 wrapper_args = sys.argv[1:]
@@ -31,6 +35,11 @@
 cmd.extend(['--dsymutil', dsymutil])
 cmd.extend(['--lldb-libs-dir', lldb_libs_dir])
 cmd.extend(['--llvm-tools-dir', llvm_tools_dir])
+if has_libcxx:
+cmd.extend(['--libcxx-include-dir', libcxx_include_dir])
+if libcxx_include_target_dir:
+cmd.extend(['--libcxx-include-target-dir', libcxx_include_target_dir])
+cmd.extend(['--libcxx-library-dir', libcxx_libs_dir])
 if lldb_framework_dir:
 cmd.extend(['--framework', lldb_framework_dir])
 if lldb_build_intel_pt == "1":
Index: lldb/utils/lldb-dotest/CMakeLists.txt
===
--- lldb/utils/lldb-dotest/CMakeLists.txt
+++ lldb/utils/lldb-dotest/CMakeLists.txt
@@ -9,8 +9,21 @@
 
 llvm_canonicalize_cmake_booleans(
   LLDB_BUILD_INTEL_PT
+  LLDB_HAS_LIBCXX
 )
 
+if ("libcxx" IN_LIST LLVM_ENABLE_RUNTIMES)
+  set(LLDB_HAS_LIBCXX ON)
+  if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
+set(LIBCXX_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE})
+set(LIBCXX_GENERATED_INCLUDE_DIR "${LLVM_BINARY_DIR}/include/c++/v1")
+set(LIBCXX_GENERATED_INCLUDE_TARGET_DIR "${LLVM_BINARY_DIR}/include/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
+  else()
+set(LIBCXX_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBCXX_LIBDIR_SUFFIX})
+set(LIBCXX_GENERATED_INCLUDE_DIR "${CMAKE_BINARY_DIR}/include/c++/v1")
+  endif()
+endif()
+
 set(LLVM_TOOLS_DIR "${LLVM_TOOLS_BINARY_DIR}")
 set(vars
   LLDB_TEST_COMMON_ARGS
@@ -23,8 +36,13 @@
   LLDB_TEST_DSYMUTIL
   LLDB_LIBS_DIR
   LLVM_TOOLS_DIR
+  LIBCXX_LIBRARY_DIR
+  LIBCXX_GENERATED_INCLUDE_DIR
+  LIBCXX_GENERATED_INCLUDE_TARGET_DIR
   )
 
+llvm_canonicalize_cmake_booleans(LLDB_HAS_LIBCXX)
+
 # Generate lldb-dotest Python driver script for each build mode.
 if(LLDB_BUILT_STANDALONE)
   set(config_types ".")
Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -97,6 +97,14 @@
 
   if (TARGET libcxx OR ("libcxx" IN_LIST LLVM_ENABLE_RUNTIMES))
 set(LLDB_HAS_LIBCXX ON)
+if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
+  set(LIBCXX_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE})
+  set(LIBCXX_GENERATED_INCLUDE_DIR "${LLVM_BINARY_DIR}/include/c++/v1")
+  set(LIBCXX_GENERATED_INCLUDE_TARGET_DIR "${LLVM_BINARY_DIR}/include/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
+else()
+  set(LIBCXX_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBCXX_LIBDIR_SUFFIX})
+  set(LIBCXX_GENERATED_INCLUDE_DIR "${CMAKE_BINARY_DIR}/include/c++/v1")
+endif()
 add_lldb_test_dependency(cxx)
   endif()
 
Index: lldb/test/API/lit.site.cfg.py.in
===
--- lldb/test/API/lit.site.cfg.py.in
+++ lldb/test/API/lit.site.cfg.py.in
@@ -32,6 +32,9 @@
 config.test_compiler = lit_config.substitute('@LLDB_TEST_COMPILER@')
 config.dsymutil = lit_config.substitute('@LLDB_TEST_DSYMUTIL@')
 config.has_libcxx = @LLDB_HAS_LIBCXX@
+config.libcxx_libs_dir = "@LIBCXX_LIBRARY_DIR@"
+config.libcxx_include_dir = "@LIBCXX_GENERATED_INCLUDE_DIR@"
+config.libcxx_include_target_dir = "@LIBCXX_GENERATED_INCLUDE_TARGET_DIR@"
 # The API tests use their own module caches.
 config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-api")
 config.clang_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_CLANG@", "lldb-api")
Index: lldb/test/API/lit.cfg.py

[Lldb-commits] [PATCH] D136719: change debugserver to return an empty platform name for unknown platforms, instead of crashing

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

In D136719#3886254 , @tschuett wrote:

> Looks great, but I would have expected `std::optional` and a 
> `nullopt`.

Yeah I'm fine with that, it does seems more natural.  In this case we're 
returning the address of a half-dozen const strings in the text section, so 
there wasn't any lifetime worries or anything, I left it as a char* after 
thinking about it for a few seconds.  I'll update to use a std::string, this is 
not a hot code path, a small object allocation doesn't matter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136719

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

[Lldb-commits] [PATCH] D136719: change debugserver to return an empty platform name for unknown platforms, instead of crashing

[Thorsten via Phabricator via lldb-commits]

tschuett added a comment.

Looks great, but I would have expected `std::optional` and a 
`nullopt`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136719

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

[Lldb-commits] [PATCH] D136610: [trace][intelpt] Fix multi CPU decoding TSC assertion error

[Jakob Johnson via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf6eb089734dd: [trace][intelpt] Fix multi CPU decoding TSC 
assertion error (authored by jj10306).

Changed prior to commit:
  https://reviews.llvm.org/D136610?vs=470472=470887#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136610

Files:
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp

Index: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
+++ lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
@@ -329,12 +329,18 @@
   /// \param[in] decoded_thread
   /// A \a DecodedThread object where the decoded instructions will be
   /// appended to. It might have already some instructions.
+  ///
+  /// \param[in] tsc_upper_bound
+  ///   Maximum allowed value of TSCs decoded from this PSB block.
+  ///   Any of this PSB's data occurring after this TSC will be excluded.
   PSBBlockDecoder(PtInsnDecoderUP &_up, const PSBBlock _block,
   Optional next_block_ip,
-  DecodedThread _thread, TraceIntelPT _intel_pt)
+  DecodedThread _thread, TraceIntelPT _intel_pt,
+  llvm::Optional tsc_upper_bound)
   : m_decoder_up(std::move(decoder_up)), m_psb_block(psb_block),
 m_next_block_ip(next_block_ip), m_decoded_thread(decoded_thread),
-m_anomaly_detector(*m_decoder_up, trace_intel_pt, decoded_thread) {}
+m_anomaly_detector(*m_decoder_up, trace_intel_pt, decoded_thread),
+m_tsc_upper_bound(tsc_upper_bound) {}
 
   /// \param[in] trace_intel_pt
   /// The main Trace object that own the PSB block.
@@ -362,14 +368,15 @@
   static Expected
   Create(TraceIntelPT _intel_pt, const PSBBlock _block,
  ArrayRef buffer, Process ,
- Optional next_block_ip, DecodedThread _thread) {
+ Optional next_block_ip, DecodedThread _thread,
+ llvm::Optional tsc_upper_bound) {
 Expected decoder_up =
 CreateInstructionDecoder(trace_intel_pt, buffer, process);
 if (!decoder_up)
   return decoder_up.takeError();
 
 return PSBBlockDecoder(std::move(*decoder_up), psb_block, next_block_ip,
-   decoded_thread, trace_intel_pt);
+   decoded_thread, trace_intel_pt, tsc_upper_bound);
   }
 
   void DecodePSBBlock() {
@@ -451,6 +458,41 @@
 }
   }
 
+  /// Process the TSC of a decoded PT event. Specifically, check if this TSC
+  /// is below the TSC upper bound for this PSB. If the TSC exceeds the upper
+  /// bound, return an error to abort decoding. Otherwise add the it to the
+  /// underlying DecodedThread and decoding should continue as expected.
+  ///
+  /// \param[in] tsc
+  ///   The TSC of the a decoded event.
+  Error ProcessPTEventTSC(DecodedThread::TSC tsc) {
+if (m_tsc_upper_bound && tsc >= *m_tsc_upper_bound) {
+  // This event and all the remaining events of this PSB have a TSC
+  // outside the range of the "owning" ThreadContinuousExecution. For
+  // now we drop all of these events/instructions, future work can
+  // improve upon this by determining the "owning"
+  // ThreadContinuousExecution of the remaining PSB data.
+  std::string err_msg = formatv("decoding truncated: TSC {0} exceeds "
+"maximum TSC value {1}, will skip decoding"
+" the remaining data of the PSB",
+tsc, *m_tsc_upper_bound)
+.str();
+
+  uint64_t offset;
+  int status = pt_insn_get_offset(m_decoder_up.get(), );
+  if (!IsLibiptError(status)) {
+err_msg = formatv("{2} (skipping {0} of {1} bytes)", offset,
+  m_psb_block.size, err_msg)
+  .str();
+  }
+  m_decoded_thread.AppendCustomError(err_msg);
+  return createStringError(inconvertibleErrorCode(), err_msg);
+} else {
+  m_decoded_thread.NotifyTsc(tsc);
+  return Error::success();
+}
+  }
+
   /// Before querying instructions, we need to query the events associated with
   /// that instruction, e.g. timing and trace disablement events.
   ///
@@ -471,8 +513,12 @@
 return status;
   }
 
-  if (event.has_tsc)
-m_decoded_thread.NotifyTsc(event.tsc);
+  if (event.has_tsc) {
+if (Error err = ProcessPTEventTSC(event.tsc)) {
+  consumeError(std::move(err));
+  return -pte_internal;
+}
+  }
 
   switch (event.type) {
   case ptev_disabled:
@@ -506,6 +552,7 @@
   Optional m_next_block_ip;
   DecodedThread _decoded_thread;
   PSBBlockAnomalyDetector m_anomaly_detector;
+  llvm::Optional m_tsc_upper_bound;
 };
 
 Error lldb_private::trace_intel_pt::DecodeSingleTraceForThread(
@@ 

[Lldb-commits] [lldb] f6eb089 - [trace][intelpt] Fix multi CPU decoding TSC assertion error

[Jakob Johnson via lldb-commits]

Author: Jakob Johnson
Date: 2022-10-26T11:37:30-07:00
New Revision: f6eb089734ddbd7f9b9935a122ff4ad658f06360

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

LOG: [trace][intelpt] Fix multi CPU decoding TSC assertion error

Occasionally the assertion that enforces increasing TSC values in 
`DecodedThread::NotifyTsc`
would get tripped during large multi CPU trace decoding.
The root cause of this issue was an assumption that all the data of a
PSB will fit within the start,end TSC of the "owning"
`ThreadContinuousExecution`. After investigating, this is not the case
because PSBs can have multiple TSCs.
This diff works around this issue by introducing a TSC upper bound for
each `PSBBlockDecoder`. This fixes the assertion failure by simply
"dropping" the remaining data of PSB whenever the TSC upper bound is
exceeded during decoding.
Future work will do a larger refactor of the multi CPU decoding to
remove the dependencies on this incorrect assumption so that PSB blocks
that span multiple `ThreadContinuousExecutions` are correctly handled.
correctly

Test Plan:

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

Added: 


Modified: 
lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp 
b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
index 10f0bbe42b274..b85c4f3bacf3d 100644
--- a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
@@ -329,12 +329,18 @@ class PSBBlockDecoder {
   /// \param[in] decoded_thread
   /// A \a DecodedThread object where the decoded instructions will be
   /// appended to. It might have already some instructions.
+  ///
+  /// \param[in] tsc_upper_bound
+  ///   Maximum allowed value of TSCs decoded from this PSB block.
+  ///   Any of this PSB's data occurring after this TSC will be excluded.
   PSBBlockDecoder(PtInsnDecoderUP &_up, const PSBBlock _block,
   Optional next_block_ip,
-  DecodedThread _thread, TraceIntelPT _intel_pt)
+  DecodedThread _thread, TraceIntelPT _intel_pt,
+  llvm::Optional tsc_upper_bound)
   : m_decoder_up(std::move(decoder_up)), m_psb_block(psb_block),
 m_next_block_ip(next_block_ip), m_decoded_thread(decoded_thread),
-m_anomaly_detector(*m_decoder_up, trace_intel_pt, decoded_thread) {}
+m_anomaly_detector(*m_decoder_up, trace_intel_pt, decoded_thread),
+m_tsc_upper_bound(tsc_upper_bound) {}
 
   /// \param[in] trace_intel_pt
   /// The main Trace object that own the PSB block.
@@ -362,14 +368,15 @@ class PSBBlockDecoder {
   static Expected
   Create(TraceIntelPT _intel_pt, const PSBBlock _block,
  ArrayRef buffer, Process ,
- Optional next_block_ip, DecodedThread _thread) {
+ Optional next_block_ip, DecodedThread _thread,
+ llvm::Optional tsc_upper_bound) {
 Expected decoder_up =
 CreateInstructionDecoder(trace_intel_pt, buffer, process);
 if (!decoder_up)
   return decoder_up.takeError();
 
 return PSBBlockDecoder(std::move(*decoder_up), psb_block, next_block_ip,
-   decoded_thread, trace_intel_pt);
+   decoded_thread, trace_intel_pt, tsc_upper_bound);
   }
 
   void DecodePSBBlock() {
@@ -451,6 +458,41 @@ class PSBBlockDecoder {
 }
   }
 
+  /// Process the TSC of a decoded PT event. Specifically, check if this TSC
+  /// is below the TSC upper bound for this PSB. If the TSC exceeds the upper
+  /// bound, return an error to abort decoding. Otherwise add the it to the
+  /// underlying DecodedThread and decoding should continue as expected.
+  ///
+  /// \param[in] tsc
+  ///   The TSC of the a decoded event.
+  Error ProcessPTEventTSC(DecodedThread::TSC tsc) {
+if (m_tsc_upper_bound && tsc >= *m_tsc_upper_bound) {
+  // This event and all the remaining events of this PSB have a TSC
+  // outside the range of the "owning" ThreadContinuousExecution. For
+  // now we drop all of these events/instructions, future work can
+  // improve upon this by determining the "owning"
+  // ThreadContinuousExecution of the remaining PSB data.
+  std::string err_msg = formatv("decoding truncated: TSC {0} exceeds "
+"maximum TSC value {1}, will skip decoding"
+" the remaining data of the PSB",
+tsc, *m_tsc_upper_bound)
+.str();
+
+  uint64_t offset;
+  int status = pt_insn_get_offset(m_decoder_up.get(), );
+  if (!IsLibiptError(status)) {
+err_msg = formatv("{2} (skipping {0} of {1} 

[Lldb-commits] [PATCH] D136719: change debugserver to return an empty platform name for unknown platforms, instead of crashing

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda updated this revision to Diff 470885.
jasonmolenda added a comment.

Update MachProcess::GetPlatformString to return a std::optional platform 
string, if the platform enumeration is recognized.  Update callers to handle 
the absence of a value appropriately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136719

Files:
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNB.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm
  lldb/tools/debugserver/source/RNBRemote.cpp

Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -6258,12 +6258,12 @@
 
 bool is_executable = true;
 uint32_t major_version, minor_version, patch_version;
-auto *platform =
+std::optional platform =
 DNBGetDeploymentInfo(pid, is_executable, lc, load_command_addr,
  major_version, minor_version, patch_version);
-if (platform) {
+if (platform.has_value()) {
   os_handled = true;
-  rep << "ostype:" << platform << ";";
+  rep << "ostype:" << platform.value() << ";";
   break;
 }
 load_command_addr = load_command_addr + lc.cmdsize;
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -720,30 +720,44 @@
   return info;
 }
 
-const char *MachProcess::GetPlatformString(unsigned char platform) {
+std::optional
+MachProcess::GetPlatformString(unsigned char platform) {
+  std::optional platform_string;
   switch (platform) {
   case PLATFORM_MACOS:
-return "macosx";
+platform_string = "macosx";
+break;
   case PLATFORM_MACCATALYST:
-return "maccatalyst";
+platform_string = "maccatalyst";
+break;
   case PLATFORM_IOS:
-return "ios";
+platform_string = "ios";
+break;
   case PLATFORM_IOSSIMULATOR:
-return "iossimulator";
+platform_string = "iossimulator";
+break;
   case PLATFORM_TVOS:
-return "tvos";
+platform_string = "tvos";
+break;
   case PLATFORM_TVOSSIMULATOR:
-return "tvossimulator";
+platform_string = "tvossimulator";
+break;
   case PLATFORM_WATCHOS:
-return "watchos";
+platform_string = "watchos";
+break;
   case PLATFORM_WATCHOSSIMULATOR:
-return "watchossimulator";
+platform_string = "watchossimulator";
+break;
   case PLATFORM_BRIDGEOS:
-return "bridgeos";
+platform_string = "bridgeos";
+break;
   case PLATFORM_DRIVERKIT:
-return "driverkit";
+platform_string = "driverkit";
+break;
+  default:
+DNBLogError("Unknown platform %u found for one binary", platform);
   }
-  return nullptr;
+  return platform_string;
 }
 
 static bool mach_header_validity_test(uint32_t magic, uint32_t cputype) {
@@ -867,7 +881,8 @@
 }
 if (DeploymentInfo deployment_info = GetDeploymentInfo(
 lc, load_cmds_p, inf.mach_header.filetype == MH_EXECUTE)) {
-  const char *lc_platform = GetPlatformString(deployment_info.platform);
+  std::optional lc_platform =
+  GetPlatformString(deployment_info.platform);
   if (dyld_platform != PLATFORM_MACCATALYST &&
   inf.min_version_os_name == "macosx") {
 // macCatalyst support.
@@ -882,7 +897,7 @@
 // processed, ignore this one, which is presumed to be a
 // PLATFORM_MACCATALYST one.
   } else {
-inf.min_version_os_name = lc_platform;
+inf.min_version_os_name = lc_platform.value_or("");
 inf.min_version_os_version = "";
 inf.min_version_os_version +=
 std::to_string(deployment_info.major_version);
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.h
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.h
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.h
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -252,7 +253,7 @@
   DeploymentInfo GetDeploymentInfo(const struct load_command &,
uint64_t load_command_address,
bool is_executable);
-  static const char *GetPlatformString(unsigned char platform);
+  static std::optional GetPlatformString(unsigned char platform);
   bool GetMachOInformationFromMemory(uint32_t platform,
  nub_addr_t mach_o_header_addr,
  int wordsize,
Index: lldb/tools/debugserver/source/DNB.h

[Lldb-commits] [PATCH] D135983: [lldb] Fix a -Wdeprecated-declarations warning

[Nico Weber via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0af7280a5ee0: [lldb] Fix a -Wdeprecated-declarations warning 
(authored by thakis).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135983

Files:
  lldb/source/Symbol/LocateSymbolFileMacOSX.cpp


Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -630,8 +630,8 @@
   command_output.size(), kCFAllocatorNull));
 
   CFCReleaser plist(
-  (CFDictionaryRef)::CFPropertyListCreateFromXMLData(
-  NULL, data.get(), kCFPropertyListImmutable, NULL));
+  (CFDictionaryRef)::CFPropertyListCreateWithData(
+  NULL, data.get(), kCFPropertyListImmutable, NULL, NULL));
 
   if (!plist.get()) {
 LLDB_LOGF(log, "'%s' failed: output is not a valid plist",


Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -630,8 +630,8 @@
   command_output.size(), kCFAllocatorNull));
 
   CFCReleaser plist(
-  (CFDictionaryRef)::CFPropertyListCreateFromXMLData(
-  NULL, data.get(), kCFPropertyListImmutable, NULL));
+  (CFDictionaryRef)::CFPropertyListCreateWithData(
+  NULL, data.get(), kCFPropertyListImmutable, NULL, NULL));
 
   if (!plist.get()) {
 LLDB_LOGF(log, "'%s' failed: output is not a valid plist",
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 0af7280 - [lldb] Fix a -Wdeprecated-declarations warning

[Nico Weber via lldb-commits]

Author: Nico Weber
Date: 2022-10-26T13:34:56-04:00
New Revision: 0af7280a5ee005737fbf1b709909d9600e2b5c75

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

LOG: [lldb] Fix a -Wdeprecated-declarations warning

Fixes:

../../lldb/source/Symbol/LocateSymbolFileMacOSX.cpp:633:26:
warning: 'CFPropertyListCreateFromXMLData' is deprecated:
 first deprecated in macOS 10.10 -
 Use CFPropertyListCreateWithData instead.
 [-Wdeprecated-declarations]
(CFDictionaryRef)::CFPropertyListCreateFromXMLData(
   ^

Hopefully no behavior change.

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

Added: 


Modified: 
lldb/source/Symbol/LocateSymbolFileMacOSX.cpp

Removed: 




diff  --git a/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp 
b/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
index a95d7375bf5f6..7d24905be3504 100644
--- a/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ b/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -630,8 +630,8 @@ bool Symbols::DownloadObjectAndSymbolFile(ModuleSpec 
_spec,
   command_output.size(), kCFAllocatorNull));
 
   CFCReleaser plist(
-  (CFDictionaryRef)::CFPropertyListCreateFromXMLData(
-  NULL, data.get(), kCFPropertyListImmutable, NULL));
+  (CFDictionaryRef)::CFPropertyListCreateWithData(
+  NULL, data.get(), kCFPropertyListImmutable, NULL, NULL));
 
   if (!plist.get()) {
 LLDB_LOGF(log, "'%s' failed: output is not a valid plist",



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

[Lldb-commits] [PATCH] D136719: change debugserver to return an empty platform name for unknown platforms, instead of crashing

[Thorsten via Phabricator via lldb-commits]

tschuett added a comment.

In D136719#3885974 , @jasonmolenda 
wrote:

> In D136719#3884836 , @tschuett 
> wrote:
>
>> How about an optional instead, if GetPlatformString is fallible.
>
> Ah, interesting idea.  debugserver doesn't use any llvm, but I believe we're 
> building with C++17 these days so I could do it with std::optional.

Yep.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136719

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

[Lldb-commits] [PATCH] D136719: change debugserver to return an empty platform name for unknown platforms, instead of crashing

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

In D136719#3884836 , @tschuett wrote:

> How about an optional instead, if GetPlatformString is fallible.

Ah, interesting idea.  debugserver doesn't use any llvm, but I believe we're 
building with C++17 these days so I could do it with std::optional.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136719

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

[Lldb-commits] [PATCH] D136697: Add formatting support for VSCode logpoints message

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan updated this revision to Diff 470837.
yinghuitan added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136697

Files:
  lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_logpoints.py
  lldb/tools/lldb-vscode/BreakpointBase.cpp
  lldb/tools/lldb-vscode/BreakpointBase.h

Index: lldb/tools/lldb-vscode/BreakpointBase.h
===
--- lldb/tools/lldb-vscode/BreakpointBase.h
+++ lldb/tools/lldb-vscode/BreakpointBase.h
@@ -22,7 +22,7 @@
   struct LogMessagePart {
 LogMessagePart(llvm::StringRef text, bool is_expr)
 : text(text), is_expr(is_expr) {}
-llvm::StringRef text;
+std::string text;
 bool is_expr;
   };
   // An optional expression for conditional breakpoints.
@@ -45,6 +45,13 @@
   void SetHitCondition();
   void SetLogMessage();
   void UpdateBreakpoint(const BreakpointBase _bp);
+
+  // Format \param text and return formatted text in \param formatted.
+  // \return any formatting failures.
+  lldb::SBError FormatLogText(llvm::StringRef text, std::string );
+  lldb::SBError AppendLogMessagePart(llvm::StringRef part, bool is_expr);
+  void NotifyLogMessageError(llvm::StringRef error);
+
   static const char *GetBreakpointLabel();
   static bool BreakpointHitCallback(void *baton, lldb::SBProcess ,
 lldb::SBThread ,
Index: lldb/tools/lldb-vscode/BreakpointBase.cpp
===
--- lldb/tools/lldb-vscode/BreakpointBase.cpp
+++ lldb/tools/lldb-vscode/BreakpointBase.cpp
@@ -25,6 +25,145 @@
 bp.SetIgnoreCount(hitCount - 1);
 }
 
+lldb::SBError BreakpointBase::AppendLogMessagePart(llvm::StringRef part,
+  bool is_expr) {
+  if (is_expr) {
+logMessageParts.emplace_back(part, is_expr);
+  } else {
+std::string formatted;
+lldb::SBError error = FormatLogText(part, formatted);
+if (error.Fail())
+  return error;
+logMessageParts.emplace_back(formatted, is_expr);
+  }
+  return lldb::SBError();
+}
+
+// TODO: consolidate this code with the implementation in
+// FormatEntity::ParseInternal().
+lldb::SBError BreakpointBase::FormatLogText(llvm::StringRef text,
+   std::string ) {
+  lldb::SBError error;
+  while (!text.empty()) {
+size_t backslash_pos = text.find_first_of('\\');
+if (backslash_pos == std::string::npos) {
+  formatted += text.str();
+  return error;
+}
+assert(backslash_pos >=0 && backslash_pos < text.size());
+
+formatted += text.substr(0, backslash_pos + 1).str();
+// Skip the characters before (and include) '\'.
+text = text.drop_front(backslash_pos + 1);
+
+if (text.empty()) {
+  error.SetErrorString(
+  "'\\' character was not followed by another character");
+  return error;
+}
+
+const char desens_char = text[0];
+text = text.drop_front(); // Skip the desensitized char character
+switch (desens_char) {
+case 'a':
+  formatted.push_back('\a');
+  break;
+case 'b':
+  formatted.push_back('\b');
+  break;
+case 'f':
+  formatted.push_back('\f');
+  break;
+case 'n':
+  formatted.push_back('\n');
+  break;
+case 'r':
+  formatted.push_back('\r');
+  break;
+case 't':
+  formatted.push_back('\t');
+  break;
+case 'v':
+  formatted.push_back('\v');
+  break;
+case '\'':
+  formatted.push_back('\'');
+  break;
+case '\\':
+  formatted.push_back('\\');
+  break;
+case '0':
+  // 1 to 3 octal chars
+  {
+if (text.empty()) {
+  error.SetErrorString("missing octal number following '\\0'");
+  return error;
+}
+
+// Make a string that can hold onto the initial zero char, up to 3
+// octal digits, and a terminating NULL.
+char oct_str[5] = {0, 0, 0, 0, 0};
+
+size_t i;
+for (i = 0;
+ i < text.size() && i < 4 && (text[i] >= '0' && text[i] <= '7');
+ ++i) {
+  oct_str[i] = text[i];
+}
+
+text = text.drop_front(i);
+unsigned long octal_value = ::strtoul(oct_str, nullptr, 8);
+if (octal_value <= UINT8_MAX) {
+  formatted.push_back((char)octal_value);
+} else {
+  error.SetErrorString("octal number is larger than a single byte");
+  return error;
+}
+  }
+  break;
+
+case 'x': {
+  if (text.empty()) {
+error.SetErrorString("missing hex number following '\\x'");
+return error;
+  }
+  // hex number in the text
+  if (isxdigit(text[0])) {
+// Make a string that can hold onto two hex chars plus a
+// NULL terminator
+char hex_str[3] = {0, 0, 0};
+

[Lldb-commits] [PATCH] D136697: Add formatting support for VSCode logpoints message

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan added inline comments.



Comment at: 
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_logpoints.py:52
 logMessage_prefix = "This is log message for { -- "
-# Trailing newline is needed for splitlines()
-logMessage = logMessage_prefix + "{i + 3}\n"
+logMessage = logMessage_prefix + "{i + 3}"
 [loop_breakpoint_id, post_loop_breakpoint_id] = 
self.set_source_breakpoints(

clayborg wrote:
> Are we now auto appending a newline if there isn't one? If so we need to test 
> this functionality. We should append on if the string doesn't end with "\n" 
> and not if it does?
Yes, we are always appending a newline now. I decided not to detect if string 
ends with "\n" or not but always append so that the behavior can be consistent. 

By removing the trailing the trailing newline in testcase here we are already 
testing the newline behavior right? Otherwise the testcase would fail.



Comment at: lldb/tools/lldb-vscode/BreakpointBase.cpp:47-52
+  while (!text.empty()) {
+if (text[0] != '\\') {
+  formatted.push_back(text[0]);
+  text = text.drop_front();
+  continue;
+}

clayborg wrote:
> Please use StringRef::find(char). Something like:
> 
> Please use StringRef::find_first_of(...) instead of iterating per character. 
> Something like:
> 
> ```
> formatted.clear();
> while (!text.empty()) {
>   size_t backslash_pos = text.find_first_of('\\');
>   if (backslash_pos = StringRef::npos) {
> // No more backslash chars append the rest of the string
> formatted += text.str();
> return error;
>   }
>   // Append anything before the backslash character.
>   if (backslash_pos > 0)
> formatted += text.drop_front(backslash_pos).str();
> ```
Ah, definitely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136697

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

[Lldb-commits] [PATCH] D136761: [lldb][FormatEntity] Fix closing parenthesis for function.name-with-args frame format

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Core/FormatEntity.cpp:1673
 if (open_paren)
-  close_paren = strchr(open_paren, ')');
+  close_paren = strrchr(open_paren, ')');
   } else

labath wrote:
> Michael137 wrote:
> > labath wrote:
> > > What if there are multiple function arguments? Won't this find the end of 
> > > the last one?
> > Had a local test-case that worked iirc. A well-formed demangled function 
> > name always has a final closing parenthesis after all the function 
> > arguments. So unless I missed something it should find the last parenthesis 
> > correctly
> > 
> > I will add a test-case for this to make sure
> Ok, I see. Given your description above, I believe this will work OK. 
> However, I suspect it will go wrong in the part where it tries skip over the 
> template arguments (and gets confused by things like `operator<<` and `foo<(2 
> > 1)>`).
Good catch with the `operator<<`. Indeed this solution wouldn't quite work. 
Added a test for this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136761

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

[Lldb-commits] [PATCH] D136761: [lldb][FormatEntity] Fix closing parenthesis for function.name-with-args frame format

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D136761#3885557 , @Michael137 
wrote:

> In D136761#3885529 , @labath wrote:
>
>> Wow, another name parser I knew nothing about. :/
>>
>> I'm probably being naive, but I don't suppose there's an easy a way to 
>> reuse/repurpose the parser in the C++ language plugin for this (?) This is 
>> the first time I see this code, so it's hard to construct counter-examples, 
>> but I'd be surprised if this is correct.
>
> Hehe I agree this function could use some clean-up
> I tried to avoid going down that rabbit hole at the moment but I'll double 
> check :)
> I wouldn't be shocked if there were edge-cases that don't work.
>
> The algorithm iiuc works as follows:
>
> 1. Find the opening function parenthesis (`open_paren`) and print out the 
> string up to that point (effectively the function name)
> 2. For each variable in scope print out: `=`
> 3. Find closing parenthesis
> 4. Print everything from closing parenthesis onward (I imagine this is to 
> preserve function qualifiers)

Ok, so it basically takes the function base name, and then manually prints the 
list of function arguments inside parenthesis.  That sounds like it could be 
easy to do with the other parser. Keep the argument printing part, and replace 
the basename parsing with a call to the language parser ?

I wouldn't say its mandatory but, unlike removing the language parser, this 
could actually be achievable.




Comment at: lldb/source/Core/FormatEntity.cpp:1673
 if (open_paren)
-  close_paren = strchr(open_paren, ')');
+  close_paren = strrchr(open_paren, ')');
   } else

Michael137 wrote:
> labath wrote:
> > What if there are multiple function arguments? Won't this find the end of 
> > the last one?
> Had a local test-case that worked iirc. A well-formed demangled function name 
> always has a final closing parenthesis after all the function arguments. So 
> unless I missed something it should find the last parenthesis correctly
> 
> I will add a test-case for this to make sure
Ok, I see. Given your description above, I believe this will work OK. However, 
I suspect it will go wrong in the part where it tries skip over the template 
arguments (and gets confused by things like `operator<<` and `foo<(2 > 1)>`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136761

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

[Lldb-commits] [PATCH] D136766: [lldb][Docs][NFC] Fix sphinx warnings/errors for LLDB docs

[Michael Buch via Phabricator via lldb-commits]

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

Ran `ninja docs-lldb-html` and made sure the docs are fixed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136766

Files:
  lldb/bindings/interface/SBModule.i
  lldb/bindings/interface/SBTarget.i
  lldb/bindings/interface/SBType.i
  lldb/docs/resources/build.rst
  lldb/docs/resources/test.rst
  lldb/docs/use/intel_pt.rst

Index: lldb/docs/use/intel_pt.rst
===
--- lldb/docs/use/intel_pt.rst
+++ lldb/docs/use/intel_pt.rst
@@ -39,8 +39,8 @@
   $ cd libipt-build
   $ make
 
-This will generate a few files in the `/lib`
-and `/libipt/include` directories.
+This will generate a few files in the ``/lib``
+and ``/libipt/include`` directories.
 
 Configure and build LLDB with Intel PT support
 
Index: lldb/docs/resources/test.rst
===
--- lldb/docs/resources/test.rst
+++ lldb/docs/resources/test.rst
@@ -229,7 +229,7 @@
 time (e.g., C and C++) there is also usually no process necessary to test
 the `SBType`-related parts of the API. With those languages it's also
 possible to test `SBValue` by running expressions with
-`SBTarget.EvaluateExpression` or the `expect_expr` testing utility.
+`SBTarget.EvaluateExpression` or the ``expect_expr`` testing utility.
 
 Functionality that always requires a running process is everything that
 tests the `SBProcess`, `SBThread`, and `SBFrame` classes. The same is true
@@ -315,27 +315,27 @@
 self.expect_expr("2 + 2", result_value="0")
 
 **Prefer using specific asserts over the generic assertTrue/assertFalse.**.
-The `self.assertTrue`/`self.assertFalse` functions should always be your
+The ``self.assertTrue``/``self.assertFalse`` functions should always be your
 last option as they give non-descriptive error messages. The test class has
-several expressive asserts such as `self.assertIn` that automatically
+several expressive asserts such as ``self.assertIn`` that automatically
 generate an explanation how the received values differ from the expected
-ones. Check the documentation of Python's `unittest` module to see what
+ones. Check the documentation of Python's ``unittest`` module to see what
 asserts are available. LLDB also has a few custom asserts that are tailored
 to our own data types.
 
 +---+-+
-| **Assert**| **Description**   |
+| **Assert**| **Description** |
 +---+-+
-| ``assertSuccess`` | Assert that an ``lldb.SBError`` is in the "success" state.|
+| ``assertSuccess`` | Assert that an ``lldb.SBError`` is in the "success" state.  |
 +---+-+
-| ``assertState``   | Assert that two states (``lldb.eState*``) are equal.  |
+| ``assertState``   | Assert that two states (``lldb.eState*``) are equal.|
 +---+-+
 | ``assertStopReason``  | Assert that two stop reasons (``lldb.eStopReason*``) are equal. |
 +---+-+
 
 If you can't find a specific assert that fits your needs and you fall back
 to a generic assert, make sure you put useful information into the assert's
-`msg` argument that helps explain the failure.
+``msg`` argument that helps explain the failure.
 
 ::
 
@@ -599,8 +599,6 @@
 
alias lldb self.dbg.HandleCommand("%*")
 
-::
-
 Debugging Test Failures on Windows
 ``
 
Index: lldb/docs/resources/build.rst
===
--- lldb/docs/resources/build.rst
+++ lldb/docs/resources/build.rst
@@ -99,7 +99,7 @@
   or even write new tests at all, PTVS is an indispensable debugging
   extension to VS that enables full editing and debugging support for Python
   (including mixed native/managed debugging).
-* `SWIG for Windows _`
+* `SWIG for Windows `_
 
 The steps outlined here describes how to set up your system and 

[Lldb-commits] [PATCH] D136650: Add a check for TypeSystem use-after-free problems

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The Halloween theme is nice, but I think it'd be more traditional if we kept a 
list of active instances, instead of inactive ones. That said, a part of me 
dies every time I see a class keeping a list of its instances.

I don't know if you've seen it, but I think the `Module::m_old_symfiles` member 
exists for the very same reason, and it could be why this does not happen so 
often outside of swift (which, I believe, stores most of its types in the 
target type system instead of the module systems).




Comment at: lldb/source/Symbol/CompilerType.cpp:35
+  bool unused;
+  if (GetTypeSystemGraveyard().Lookup(m_type_system, unused)) {
+lldbassert(false && "CompilerType belongs to deleted Typesystem");

aprantl wrote:
> hawkinsw wrote:
> > I am sorry if this is obvious, but is `CompilerType` used in a 
> > multithreaded environment? So, is there a possibility that we could pass 
> > the check on line 32 but become invalid by the use of `m_type_system` here 
> > and fall victim to an attempt (in `Lookup`, perhaps?) to dereference a 
> > `NULL` pointer? Again, I am sorry if that is a stupid question!
> Yes, CompilerType could be used on any thread and this will not catch a 
> use-after-free if the TypeSystem is deleted between the IsValid check and its 
> use.
> 
> A solution would be to store a TypeSystemSP/WP in CompilerType, but I'm 
> expecting this to be rather expensive. The intention of this patch is as a 
> bug-finding tool. It's not intended to make holding on to CompilerType 
> objects safe. That said, I'm open to explore ideas for how to efficiently 
> make this safe in a future patch.
Well.. if you're only interested in SB uses, then I'd say that the SBType 
object should hold an SP/WP on the type system or some parent of it.


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

https://reviews.llvm.org/D136650

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

[Lldb-commits] [PATCH] D136761: [lldb][FormatEntity] Fix closing parenthesis for function.name-with-args frame format

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Core/FormatEntity.cpp:1673
 if (open_paren)
-  close_paren = strchr(open_paren, ')');
+  close_paren = strrchr(open_paren, ')');
   } else

labath wrote:
> What if there are multiple function arguments? Won't this find the end of the 
> last one?
Had a local test-case that worked iirc. A well-formed demangled function name 
always has a final closing parenthesis after all the function arguments. So 
unless I missed something it should find the last parenthesis correctly

I will add a test-case for this to make sure


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136761

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

[Lldb-commits] [PATCH] D136761: [lldb][FormatEntity] Fix closing parenthesis for function.name-with-args frame format

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D136761#3885529 , @labath wrote:

> Wow, another name parser I knew nothing about. :/
>
> I'm probably being naive, but I don't suppose there's an easy a way to 
> reuse/repurpose the parser in the C++ language plugin for this (?) This is 
> the first time I see this code, so it's hard to construct counter-examples, 
> but I'd be surprised if this is correct.

Hehe I agree this function could use some clean-up
I tried to avoid going down that rabbit hole at the moment :)
I wouldn't be shocked if there were edge-cases that don't work.

The algorithm iiuc works as follows:

1. Find the opening function parenthesis (`open_paren`) and print out the 
string up to that point (effectively the function name)
2. For each variable in scope print out: `=`
3. Find closing parenthesis
4. Print everything from closing parenthesis onward (I imagine this is to 
preserve function qualifiers)

Where the reverse scan might go wrong is with `noexecept(...)` specifications. 
Will check...

An alternative is to actually implement the 
`Language::GetFunctionDisplayName(Language::FunctionNameRepresentation::eNameWithNoArgs)`
 API for `CPlusPlusLanguage`. Currently it's a no-op (which is why we end up in 
this hand-rolled parser)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136761

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

[Lldb-commits] [PATCH] D136761: [lldb][FormatEntity] Fix closing parenthesis for function.name-with-args frame format

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Wow, another name parser I knew nothing about. :/

I'm probably being naive, but I don't suppose there's an easy a way to 
reuse/repurpose the parser in the C++ language plugin for this (?) This is the 
first time I see this code, so it's hard to construct counter-examples, but I'd 
be surprised if this is correct.




Comment at: lldb/source/Core/FormatEntity.cpp:1673
 if (open_paren)
-  close_paren = strchr(open_paren, ')');
+  close_paren = strrchr(open_paren, ')');
   } else

What if there are multiple function arguments? Won't this find the end of the 
last one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136761

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

[Lldb-commits] [PATCH] D132734: [lldb] Fix member access in GetExpressionPath

[Will Hawkins via Phabricator via lldb-commits]

hawkinsw added a comment.

If you would like, I'd be happy to reopen this patch as a separate issue if 
that's the better way to handle it! Sorry for the SPAM!

Will


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132734

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

[Lldb-commits] [PATCH] D136761: [lldb][FormatEntity] Fix closing parenthesis for function.name-with-args frame format

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 470791.
Michael137 added a comment.

- Add more test cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136761

Files:
  lldb/source/Core/FormatEntity.cpp
  lldb/test/Shell/Settings/Inputs/names.cpp
  lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test


Index: lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test
===
--- /dev/null
+++ lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test
@@ -0,0 +1,19 @@
+# RUN: %clangxx_host -g -O0 %S/Inputs/names.cpp -std=c++17 -o %t.out
+# RUN: %lldb -b -s %s %t.out | FileCheck %s
+settings set -f frame-format "frame ${function.name-with-args}\n"
+break set -n foo
+run
+# CHECK: frame int foo(t={{.*}})
+c
+# CHECK: frame int foo>(t= Function = bar() )
+c
+# CHECK: frame int foo<(anonymous namespace)::$_0>(t={{.*}})
+c
+# CHECK: frame int foo>(t= Function = (anonymous 
namespace)::anon_bar() )
+c
+# CHECK: frame int foo const&) 
const noexcept>(t={{.*}})
+c
+# CHECK: frame void Foo::foo>(this={{.*}}, t= 
Function = bar() ) const
+c
+# CHECK: frame void Foo::foo>(this={{.*}}, t= 
Function = (anonymous namespace)::anon_bar() ) const
+q
Index: lldb/test/Shell/Settings/Inputs/names.cpp
===
--- /dev/null
+++ lldb/test/Shell/Settings/Inputs/names.cpp
@@ -0,0 +1,26 @@
+#include 
+
+struct Foo {
+  template  void foo(T const ) const noexcept(true) {}
+};
+
+template  int foo(T const ) { return 0; }
+
+int bar() { return 1; }
+
+namespace {
+int anon_bar() { return 1; }
+auto anon_lambda = [](std::function) mutable {};
+} // namespace
+
+int main() {
+  foo(bar);
+  foo(std::function{bar});
+  foo(anon_lambda);
+  foo(std::function{anon_bar});
+  foo(::foo>);
+  Foo f;
+  f.foo(std::function{bar});
+  f.foo(std::function{anon_bar});
+  return 0;
+}
Index: lldb/source/Core/FormatEntity.cpp
===
--- lldb/source/Core/FormatEntity.cpp
+++ lldb/source/Core/FormatEntity.cpp
@@ -1670,9 +1670,9 @@
 open_paren =
 strchr(open_paren + strlen("(anonymous namespace)"), '(');
 if (open_paren)
-  close_paren = strchr(open_paren, ')');
+  close_paren = strrchr(open_paren, ')');
   } else
-close_paren = strchr(open_paren, ')');
+close_paren = strrchr(open_paren, ')');
 }
 
 if (open_paren)


Index: lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test
===
--- /dev/null
+++ lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test
@@ -0,0 +1,19 @@
+# RUN: %clangxx_host -g -O0 %S/Inputs/names.cpp -std=c++17 -o %t.out
+# RUN: %lldb -b -s %s %t.out | FileCheck %s
+settings set -f frame-format "frame ${function.name-with-args}\n"
+break set -n foo
+run
+# CHECK: frame int foo(t={{.*}})
+c
+# CHECK: frame int foo>(t= Function = bar() )
+c
+# CHECK: frame int foo<(anonymous namespace)::$_0>(t={{.*}})
+c
+# CHECK: frame int foo>(t= Function = (anonymous namespace)::anon_bar() )
+c
+# CHECK: frame int foo const&) const noexcept>(t={{.*}})
+c
+# CHECK: frame void Foo::foo>(this={{.*}}, t= Function = bar() ) const
+c
+# CHECK: frame void Foo::foo>(this={{.*}}, t= Function = (anonymous namespace)::anon_bar() ) const
+q
Index: lldb/test/Shell/Settings/Inputs/names.cpp
===
--- /dev/null
+++ lldb/test/Shell/Settings/Inputs/names.cpp
@@ -0,0 +1,26 @@
+#include 
+
+struct Foo {
+  template  void foo(T const ) const noexcept(true) {}
+};
+
+template  int foo(T const ) { return 0; }
+
+int bar() { return 1; }
+
+namespace {
+int anon_bar() { return 1; }
+auto anon_lambda = [](std::function) mutable {};
+} // namespace
+
+int main() {
+  foo(bar);
+  foo(std::function{bar});
+  foo(anon_lambda);
+  foo(std::function{anon_bar});
+  foo(::foo>);
+  Foo f;
+  f.foo(std::function{bar});
+  f.foo(std::function{anon_bar});
+  return 0;
+}
Index: lldb/source/Core/FormatEntity.cpp
===
--- lldb/source/Core/FormatEntity.cpp
+++ lldb/source/Core/FormatEntity.cpp
@@ -1670,9 +1670,9 @@
 open_paren =
 strchr(open_paren + strlen("(anonymous namespace)"), '(');
 if (open_paren)
-  close_paren = strchr(open_paren, ')');
+  close_paren = strrchr(open_paren, ')');
   } else
-close_paren = strchr(open_paren, ')');
+close_paren = strrchr(open_paren, ')');
 }
 
 if (open_paren)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org

[Lldb-commits] [lldb] 6cc5bcc - [LLDB] Correct env vars for Android port selection

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-10-26T12:31:27Z
New Revision: 6cc5bcc12d3095376fa28b794b3bbb09ea6a9e4e

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

LOG: [LLDB] Correct env vars for Android port selection

These should have "LOCAL" in them.

First added in 1e210abf9925ad08fb7c79894b4ec5ef8f0ef173.

Added: 


Modified: 
lldb/docs/use/remote.rst

Removed: 




diff  --git a/lldb/docs/use/remote.rst b/lldb/docs/use/remote.rst
index ef8847a7c062..1649becea78f 100644
--- a/lldb/docs/use/remote.rst
+++ b/lldb/docs/use/remote.rst
@@ -138,7 +138,7 @@ further using the platform shell command.
 When using the "remote-android" platform, the client LLDB forwards two ports, 
one
 for connecting to the platform, and another for connecting to the gdbserver.
 The client ports are configurable through the environment variables
-ANDROID_PLATFORM_PORT and ANDROID_PLATFORM_GDB_PORT, respectively.
+ANDROID_PLATFORM_LOCAL_PORT and ANDROID_PLATFORM_LOCAL_GDB_PORT, respectively.
 
 Launching a locally built process on the remote machine
 ---



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

[Lldb-commits] [PATCH] D136761: [lldb][FormatEntity] Fix closing parenthesis for function.name-with-args frame format

[Michael Buch via Phabricator via lldb-commits]

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

Previously we would try to find the closing parenthesis by doing
a forward scan through the demangled function name. However,
for arguments that contain parenthesis (e.g., function pointers)
this would leave garbage in the frame function name.

This patch addresses this by doing a reverse scan to find the closing
parenthesis.

**Example**

For following function:

  int foo(std::function const& func) { return 1; }

Before patch:

  frame #0: 0x0001151c a.out`foo(func= Function = bar() )> const&) at 
sample.cpp:11:49

After patch:

  frame #0: 0x0001151c a.out`foo(func= Function = bar() ) at 
sample.cpp:11:49

**Testing**

- Added shell test


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136761

Files:
  lldb/source/Core/FormatEntity.cpp
  lldb/test/Shell/Settings/Inputs/names.cpp
  lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test


Index: lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test
===
--- /dev/null
+++ lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test
@@ -0,0 +1,13 @@
+# RUN: %clangxx_host -g -O0 %S/Inputs/names.cpp -std=c++17 -o %t.out
+# RUN: %lldb -b -s %s %t.out | FileCheck %s
+settings set -f frame-format "frame ${function.name-with-args}\n"
+break set -n foo
+run
+# CHECK: frame int foo(t={{.*}})
+c
+# CHECK: frame int foo>(t= Function = bar() )
+c
+# CHECK: frame int foo<(anonymous namespace)::$_0>(t={{.*}})
+c
+# CHECK: frame int foo>(t= Function = (anonymous 
namespace)::anon_bar() )
+q
Index: lldb/test/Shell/Settings/Inputs/names.cpp
===
--- /dev/null
+++ lldb/test/Shell/Settings/Inputs/names.cpp
@@ -0,0 +1,18 @@
+#include 
+
+template  int foo(T const ) { return 0; }
+
+int bar() { return 1; }
+
+namespace {
+int anon_bar() { return 1; }
+auto anon_lambda = [](std::function) mutable {};
+} // namespace
+
+int main() {
+  foo(bar);
+  foo(std::function{bar});
+  foo(anon_lambda);
+  foo(std::function{anon_bar});
+  return 0;
+}
Index: lldb/source/Core/FormatEntity.cpp
===
--- lldb/source/Core/FormatEntity.cpp
+++ lldb/source/Core/FormatEntity.cpp
@@ -1670,9 +1670,9 @@
 open_paren =
 strchr(open_paren + strlen("(anonymous namespace)"), '(');
 if (open_paren)
-  close_paren = strchr(open_paren, ')');
+  close_paren = strrchr(open_paren, ')');
   } else
-close_paren = strchr(open_paren, ')');
+close_paren = strrchr(open_paren, ')');
 }
 
 if (open_paren)


Index: lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test
===
--- /dev/null
+++ lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test
@@ -0,0 +1,13 @@
+# RUN: %clangxx_host -g -O0 %S/Inputs/names.cpp -std=c++17 -o %t.out
+# RUN: %lldb -b -s %s %t.out | FileCheck %s
+settings set -f frame-format "frame ${function.name-with-args}\n"
+break set -n foo
+run
+# CHECK: frame int foo(t={{.*}})
+c
+# CHECK: frame int foo>(t= Function = bar() )
+c
+# CHECK: frame int foo<(anonymous namespace)::$_0>(t={{.*}})
+c
+# CHECK: frame int foo>(t= Function = (anonymous namespace)::anon_bar() )
+q
Index: lldb/test/Shell/Settings/Inputs/names.cpp
===
--- /dev/null
+++ lldb/test/Shell/Settings/Inputs/names.cpp
@@ -0,0 +1,18 @@
+#include 
+
+template  int foo(T const ) { return 0; }
+
+int bar() { return 1; }
+
+namespace {
+int anon_bar() { return 1; }
+auto anon_lambda = [](std::function) mutable {};
+} // namespace
+
+int main() {
+  foo(bar);
+  foo(std::function{bar});
+  foo(anon_lambda);
+  foo(std::function{anon_bar});
+  return 0;
+}
Index: lldb/source/Core/FormatEntity.cpp
===
--- lldb/source/Core/FormatEntity.cpp
+++ lldb/source/Core/FormatEntity.cpp
@@ -1670,9 +1670,9 @@
 open_paren =
 strchr(open_paren + strlen("(anonymous namespace)"), '(');
 if (open_paren)
-  close_paren = strchr(open_paren, ')');
+  close_paren = strrchr(open_paren, ')');
   } else
-close_paren = strchr(open_paren, ')');
+close_paren = strrchr(open_paren, ')');
 }
 
 if (open_paren)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D136719: change debugserver to return an empty platform name for unknown platforms, instead of crashing

[Thorsten via Phabricator via lldb-commits]

tschuett added a comment.

How about an optional instead, if GetPlatformString is fallible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136719

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

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Thanks for your efforts here, very much appreciated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136465

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

[Lldb-commits] [lldb] a58d83b - Revert "[clang] Instantiate alias templates with sugar"

[Matheus Izvekov via lldb-commits]

Author: Matheus Izvekov
Date: 2022-10-26T10:14:21+02:00
New Revision: a58d83b2c97cd480a8533b11b86c7cd709c48176

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

LOG: Revert "[clang] Instantiate alias templates with sugar"

This reverts commit 4c44c91ad980304c5cca3792115349e68cfafd2b.

Added: 


Modified: 
clang-tools-extra/clangd/unittests/HoverTests.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/test/AST/ast-dump-template-decls.cpp
clang/test/CXX/temp/temp.deduct.guide/p3.cpp
clang/test/Misc/diag-template-diffing.cpp
clang/test/SemaCXX/sizeless-1.cpp
clang/test/SemaTemplate/make_integer_seq.cpp
clang/test/SemaTemplate/temp_arg_nontype.cpp

lldb/test/API/commands/expression/import-std-module/shared_ptr/TestSharedPtrFromStdModule.py

lldb/test/API/commands/expression/import-std-module/weak_ptr-dbg-info-content/TestDbgInfoContentWeakPtrFromStdModule.py

lldb/test/API/commands/expression/import-std-module/weak_ptr/TestWeakPtrFromStdModule.py

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp 
b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 8bca0dc974539..9f64defaf898f 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1067,7 +1067,7 @@ class Foo final {})cpp";
  HI.LocalScope = "";
  HI.Kind = index::SymbolKind::TypeAlias;
  HI.Definition = "template  using AA = A";
- HI.Type = {"A", "T"};
+ HI.Type = {"A", "type-parameter-0-0"}; // FIXME: should be 'T'
  HI.TemplateParameters = {{{"typename"}, std::string("T"), 
llvm::None}};
}},
   {// Constant array

diff  --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index c5a86486b2dea..37eaa2a5795aa 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -3854,8 +3854,8 @@ QualType Sema::CheckTemplateIdType(TemplateName Name,
 
 // Only substitute for the innermost template argument list.
 MultiLevelTemplateArgumentList TemplateArgLists;
-TemplateArgLists.addOuterTemplateArguments(Template, SugaredConverted,
-   /*Final=*/true);
+TemplateArgLists.addOuterTemplateArguments(Template, CanonicalConverted,
+   /*Final=*/false);
 TemplateArgLists.addOuterRetainedLevels(
 AliasTemplate->getTemplateParameters()->getDepth());
 

diff  --git a/clang/test/AST/ast-dump-template-decls.cpp 
b/clang/test/AST/ast-dump-template-decls.cpp
index 3bc0357d1b51f..49760d17c7f22 100644
--- a/clang/test/AST/ast-dump-template-decls.cpp
+++ b/clang/test/AST/ast-dump-template-decls.cpp
@@ -120,6 +120,8 @@ using type2 = typename C::type1;
 // CHECK-NEXT: TemplateArgument type 'void'
 // CHECK-NEXT: BuiltinType 0x{{[^ ]*}} 'void'
 // CHECK-NEXT: FunctionProtoType 0x{{[^ ]*}} 'void (int)' cdecl
+// CHECK-NEXT: SubstTemplateTypeParmType 0x{{[^ ]*}} 'void' sugar class depth 
0 index 0 U
+// CHECK-NEXT: TypeAliasTemplate 0x{{[^ ]*}} 'type1'
 // CHECK-NEXT: BuiltinType 0x{{[^ ]*}} 'void'
 // CHECK-NEXT: SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar class depth 0 
index 0 T
 // CHECK-NEXT: ClassTemplateSpecialization 0x{{[^ ]*}} 'C'
@@ -127,24 +129,38 @@ using type2 = typename C::type1;
 } // namespace PR55886
 
 namespace PR56099 {
+template  struct Y;
+template  using Z = Y;
+template  struct foo {
+  template  using bind = Z;
+};
+using t1 = foo::bind;
+// CHECK:  TemplateSpecializationType 0x{{[^ ]*}} 'Y' sugar Y
+// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'char' sugar typename 
depth 0 index 0 ... Bs pack_index 3
+// CHECK-NEXT: TypeAliasTemplate 0x{{[^ ]*}} 'Z'
+// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'float' sugar typename 
depth 0 index 0 ... Bs pack_index 2
+// CHECK-NEXT: TypeAliasTemplate 0x{{[^ ]*}} 'Z'
+// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar typename 
depth 0 index 0 ... Bs pack_index 1
+// CHECK-NEXT: TypeAliasTemplate 0x{{[^ ]*}} 'Z'
+// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'short' sugar typename 
depth 0 index 0 ... Bs pack_index 0
+// CHECK-NEXT: TypeAliasTemplate 0x{{[^ ]*}} 'Z'
+
 template  struct D {
-  template  struct bind {
-using bound_type = int(int (*...p)(T, U));
-  };
+  template  using B = int(int (*...p)(T, U));
 };
-template struct D::bind;
-// CHECK:  TypeAliasDecl 0x{{[^ ]*}}  col:11 
bound_type 'int (int (*)(float, int), int (*)(char, short))'
+using t2 = D::B;
+// CHECK:  TemplateSpecializationType 0x{{[^ ]*}} 'B' sugar 
alias B
 // CHECK:  FunctionProtoType 0x{{[^ ]*}} 'int (int (*)(float, int), int 
(*)(char, short))' cdecl
 // CHECK:  FunctionProtoType 0x{{[^ ]*}} 'int (float, 

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Landed as 
https://github.com/llvm/llvm-project/commit/1e210abf9925ad08fb7c79894b4ec5ef8f0ef173.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136465

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

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1e210abf9925: [LLDB] Make remote-android local ports 
configurable (authored by mark2185, committed by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136465

Files:
  lldb/docs/use/remote.rst
  lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h

Index: lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h
===
--- lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h
+++ lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h
@@ -49,7 +49,8 @@
 
   void DeleteForwardPort(lldb::pid_t pid);
 
-  Status MakeConnectURL(const lldb::pid_t pid, const uint16_t remote_port,
+  Status MakeConnectURL(const lldb::pid_t pid, const uint16_t local_port,
+const uint16_t remote_port,
 llvm::StringRef remote_socket_name,
 std::string _url);
 
Index: lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
===
--- lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
+++ lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
@@ -90,8 +90,13 @@
 
   Log *log = GetLog(LLDBLog::Platform);
 
-  auto error =
-  MakeConnectURL(pid, remote_port, socket_name.c_str(), connect_url);
+  uint16_t local_port = 0;
+  const char *gdbstub_port = std::getenv("ANDROID_PLATFORM_LOCAL_GDB_PORT");
+  if (gdbstub_port)
+local_port = std::stoi(gdbstub_port);
+
+  auto error = MakeConnectURL(pid, local_port, remote_port, socket_name.c_str(),
+  connect_url);
   if (error.Success() && log)
 LLDB_LOGF(log, "gdbserver connect URL: %s", connect_url.c_str());
 
@@ -126,10 +131,15 @@
   else if (parsed_url->scheme == "unix-abstract-connect")
 m_socket_namespace = AdbClient::UnixSocketNamespaceAbstract;
 
+  uint16_t local_port = 0;
+  const char *platform_local_port = std::getenv("ANDROID_PLATFORM_LOCAL_PORT");
+  if (platform_local_port)
+local_port = std::stoi(platform_local_port);
+
   std::string connect_url;
-  auto error =
-  MakeConnectURL(g_remote_platform_pid, parsed_url->port.value_or(0),
- parsed_url->path, connect_url);
+  auto error = MakeConnectURL(g_remote_platform_pid, local_port,
+  parsed_url->port.value_or(0), parsed_url->path,
+  connect_url);
 
   if (error.Fail())
 return error;
@@ -170,11 +180,28 @@
 }
 
 Status PlatformAndroidRemoteGDBServer::MakeConnectURL(
-const lldb::pid_t pid, const uint16_t remote_port,
-llvm::StringRef remote_socket_name, std::string _url) {
+const lldb::pid_t pid, const uint16_t local_port,
+const uint16_t remote_port, llvm::StringRef remote_socket_name,
+std::string _url) {
   static const int kAttempsNum = 5;
 
   Status error;
+
+  auto forward = [&](const uint16_t local, const uint16_t remote) {
+error = ForwardPortWithAdb(local, remote, remote_socket_name,
+   m_socket_namespace, m_device_id);
+if (error.Success()) {
+  m_port_forwards[pid] = local;
+  std::ostringstream url_str;
+  url_str << "connect://127.0.0.1:" << local;
+  connect_url = url_str.str();
+}
+return error;
+  };
+
+  if (local_port != 0)
+return forward(local_port, remote_port);
+
   // There is a race possibility that somebody will occupy a port while we're
   // in between FindUnusedPort and ForwardPortWithAdb - adding the loop to
   // mitigate such problem.
@@ -184,15 +211,8 @@
 if (error.Fail())
   return error;
 
-error = ForwardPortWithAdb(local_port, remote_port, remote_socket_name,
-   m_socket_namespace, m_device_id);
-if (error.Success()) {
-  m_port_forwards[pid] = local_port;
-  std::ostringstream url_str;
-  url_str << "connect://127.0.0.1:" << local_port;
-  connect_url = url_str.str();
+if (forward(local_port, remote_port).Success())
   break;
-}
   }
 
   return error;
@@ -216,7 +236,7 @@
   }
 
   std::string new_connect_url;
-  error = MakeConnectURL(s_remote_gdbserver_fake_pid--,
+  error = MakeConnectURL(s_remote_gdbserver_fake_pid--, 0,
  parsed_url->port.value_or(0), parsed_url->path,
  new_connect_url);
   if (error.Fail())
Index: lldb/docs/use/remote.rst
===
--- lldb/docs/use/remote.rst
+++ lldb/docs/use/remote.rst
@@ -135,6 +135,11 @@
 commands: get-file, put-file, mkdir, etc. The environment can be prepared
 further using the platform shell 

[Lldb-commits] [lldb] 1e210ab - [LLDB] Make remote-android local ports configurable

[David Spickett via lldb-commits]

Author: Luka Markušić
Date: 2022-10-26T08:14:22Z
New Revision: 1e210abf9925ad08fb7c79894b4ec5ef8f0ef173

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

LOG: [LLDB] Make remote-android local ports configurable

The local ports for `platform connect` and `attach` were always random, this 
allows the user to configure them.
This is useful for debugging a truly remote android (when the android in 
question is connected to a remote server).

There is a lengthier discussion on github - 
https://github.com/llvm/llvm-project/issues/58114

Reviewed By: clayborg

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

Added: 


Modified: 
lldb/docs/use/remote.rst
lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h

Removed: 




diff  --git a/lldb/docs/use/remote.rst b/lldb/docs/use/remote.rst
index 0eb43fbb82fca..ef8847a7c062e 100644
--- a/lldb/docs/use/remote.rst
+++ b/lldb/docs/use/remote.rst
@@ -135,6 +135,11 @@ application needs additional files, you can transfer them 
using the platform
 commands: get-file, put-file, mkdir, etc. The environment can be prepared
 further using the platform shell command.
 
+When using the "remote-android" platform, the client LLDB forwards two ports, 
one
+for connecting to the platform, and another for connecting to the gdbserver.
+The client ports are configurable through the environment variables
+ANDROID_PLATFORM_PORT and ANDROID_PLATFORM_GDB_PORT, respectively.
+
 Launching a locally built process on the remote machine
 ---
 

diff  --git 
a/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp 
b/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
index 2727450846d53..7c694390b7266 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
@@ -90,8 +90,13 @@ bool 
PlatformAndroidRemoteGDBServer::LaunchGDBServer(lldb::pid_t ,
 
   Log *log = GetLog(LLDBLog::Platform);
 
-  auto error =
-  MakeConnectURL(pid, remote_port, socket_name.c_str(), connect_url);
+  uint16_t local_port = 0;
+  const char *gdbstub_port = std::getenv("ANDROID_PLATFORM_LOCAL_GDB_PORT");
+  if (gdbstub_port)
+local_port = std::stoi(gdbstub_port);
+
+  auto error = MakeConnectURL(pid, local_port, remote_port, 
socket_name.c_str(),
+  connect_url);
   if (error.Success() && log)
 LLDB_LOGF(log, "gdbserver connect URL: %s", connect_url.c_str());
 
@@ -126,10 +131,15 @@ Status PlatformAndroidRemoteGDBServer::ConnectRemote(Args 
) {
   else if (parsed_url->scheme == "unix-abstract-connect")
 m_socket_namespace = AdbClient::UnixSocketNamespaceAbstract;
 
+  uint16_t local_port = 0;
+  const char *platform_local_port = std::getenv("ANDROID_PLATFORM_LOCAL_PORT");
+  if (platform_local_port)
+local_port = std::stoi(platform_local_port);
+
   std::string connect_url;
-  auto error =
-  MakeConnectURL(g_remote_platform_pid, parsed_url->port.value_or(0),
- parsed_url->path, connect_url);
+  auto error = MakeConnectURL(g_remote_platform_pid, local_port,
+  parsed_url->port.value_or(0), parsed_url->path,
+  connect_url);
 
   if (error.Fail())
 return error;
@@ -170,11 +180,28 @@ void 
PlatformAndroidRemoteGDBServer::DeleteForwardPort(lldb::pid_t pid) {
 }
 
 Status PlatformAndroidRemoteGDBServer::MakeConnectURL(
-const lldb::pid_t pid, const uint16_t remote_port,
-llvm::StringRef remote_socket_name, std::string _url) {
+const lldb::pid_t pid, const uint16_t local_port,
+const uint16_t remote_port, llvm::StringRef remote_socket_name,
+std::string _url) {
   static const int kAttempsNum = 5;
 
   Status error;
+
+  auto forward = [&](const uint16_t local, const uint16_t remote) {
+error = ForwardPortWithAdb(local, remote, remote_socket_name,
+   m_socket_namespace, m_device_id);
+if (error.Success()) {
+  m_port_forwards[pid] = local;
+  std::ostringstream url_str;
+  url_str << "connect://127.0.0.1:" << local;
+  connect_url = url_str.str();
+}
+return error;
+  };
+
+  if (local_port != 0)
+return forward(local_port, remote_port);
+
   // There is a race possibility that somebody will occupy a port while we're
   // in between FindUnusedPort and ForwardPortWithAdb - adding the loop to
   // mitigate such problem.
@@ -184,15 +211,8 @@ Status PlatformAndroidRemoteGDBServer::MakeConnectURL(
 if (error.Fail())
   return error;
 
-error = ForwardPortWithAdb(local_port, 

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

[Luka Markušić via Phabricator via lldb-commits]

mark2185 added a comment.

In D136465#3884755 , @DavidSpickett 
wrote:

>> Great! I'd just like to note that I do not have commit access, per the 
>> guide's instructions.
>
> What name/email address do you want on the commit?

Luka Markušić (markusicl...@gmail.com)

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136465

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

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> Great! I'd just like to note that I do not have commit access, per the 
> guide's instructions.

What name/email address do you want on the commit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136465

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