[Lldb-commits] [PATCH] D122684: [lldb] Use the selected and host platform to disambiguate between fat binary architectures

2022-03-30 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.

Just this morning, I was (again) contemplating the idea of introducing some 
sort of numeric platform priorities. (We currently have a sort of a ternary 
system -- exact match, compatible match, no match). That would allow us to 
express pretty much arbitrary relationships ("I know this binary looks like it 
could be run on the host, but I _really_ want to run it through my super fancy 
platform").

Nevertheless, the current implementation and the priorities set within it look 
reasonable to me.


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

https://reviews.llvm.org/D122684

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


[Lldb-commits] [lldb] c484857 - [lldb] Use =default in the ValueList class

2022-03-30 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2022-03-30T09:12:59+02:00
New Revision: c484857b2e77721a4235b0e2d53d335c09fc6af3

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

LOG: [lldb] Use =default in the ValueList class

Added: 


Modified: 
lldb/include/lldb/Core/Value.h
lldb/source/Core/Value.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Value.h b/lldb/include/lldb/Core/Value.h
index 9f00a14251e20..fdda9f1db491a 100644
--- a/lldb/include/lldb/Core/Value.h
+++ b/lldb/include/lldb/Core/Value.h
@@ -156,13 +156,11 @@ class Value {
 
 class ValueList {
 public:
-  ValueList() {}
-
-  ValueList(const ValueList &rhs);
-
+  ValueList() = default;
   ~ValueList() = default;
 
-  const ValueList &operator=(const ValueList &rhs);
+  ValueList(const ValueList &rhs) = default;
+  ValueList &operator=(const ValueList &rhs) = default;
 
   // void InsertValue (Value *value, size_t idx);
   void PushValue(const Value &value);

diff  --git a/lldb/source/Core/Value.cpp b/lldb/source/Core/Value.cpp
index 2dec3373b4667..2b60dd6f00fa4 100644
--- a/lldb/source/Core/Value.cpp
+++ b/lldb/source/Core/Value.cpp
@@ -662,13 +662,6 @@ void Value::ConvertToLoadAddress(Module *module, Target 
*target) {
   GetScalar() = load_addr;
 }
 
-ValueList::ValueList(const ValueList &rhs) { m_values = rhs.m_values; }
-
-const ValueList &ValueList::operator=(const ValueList &rhs) {
-  m_values = rhs.m_values;
-  return *this;
-}
-
 void ValueList::PushValue(const Value &value) { m_values.push_back(value); }
 
 size_t ValueList::GetSize() { return m_values.size(); }



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


[Lldb-commits] [PATCH] D121844: Applying clang-tidy modernize-use-equals-default over LLDB

2022-03-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/source/Core/Value.cpp:667
 
-const ValueList &ValueList::operator=(const ValueList &rhs) {
+const ValueList &ValueList::operator=(const ValueList &rhs) {  // 
NOLINT(modernize-use-equals-default)
   m_values = rhs.m_values;

shafik wrote:
> labath wrote:
> > shafik wrote:
> > > I have to look into why we return a `const &` here. We do this in a few 
> > > other places too.
> > I don't think there's a good reason for that. Most people aren't aware that 
> > built-in assignment operators return lvalues. And some of the people who 
> > are aware of that think that it's a bad idea, so they make sure their 
> > operators don't do it...
> It produced build errors, so some of the users are relying on this. I didn't 
> want to plumb into this since it was orthogonal to the change.
I was curious to see what kind of errors could be produced by that change -- I 
didn't get any, so I committed (c484857b2e77721a4235b0e2d53d335c09fc6af3) my 
version. :)



Comment at: lldb/source/Host/macosx/cfcpp/CFCMutableArray.cpp:18
 CFCMutableArray::CFCMutableArray(const CFCMutableArray &rhs)
-: CFCReleaser(rhs) // NOTE: this won't make a copy of 
the
+: CFCReleaser(rhs) // 
NOLINT(modernize-use-equals-default)
+  // NOTE: this won't make a copy of 
the

shafik wrote:
> labath wrote:
> > Why suppress this?
> I wanted to preserve the comment since someone thought it was important.
In that case, maybe you can just put the comment next to the `=default`  
clause. Now, this is beginning to look like there is something very subtle 
going on -- subtle enough to confuse the clang-tidy check. Which, of course, 
isn't true...


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

https://reviews.llvm.org/D121844

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


[Lldb-commits] [lldb] 1410a48 - [lldb] Remove vasprintf windows-compat implementation

2022-03-30 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2022-03-30T09:32:35+02:00
New Revision: 1410a4860eb2355f0c6e3274a937f7e382a59e5c

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

LOG: [lldb] Remove vasprintf windows-compat implementation

We already have a VASprintf function for this purpose, so I'm switching
the remaining few users to that.

Added: 


Modified: 
lldb/include/lldb/Host/windows/PosixApi.h
lldb/source/Host/CMakeLists.txt
lldb/source/Host/common/File.cpp
lldb/source/Target/RegisterContextUnwind.cpp

Removed: 
lldb/source/Host/windows/Windows.cpp



diff  --git a/lldb/include/lldb/Host/windows/PosixApi.h 
b/lldb/include/lldb/Host/windows/PosixApi.h
index 85e828c80eef1..981261a3ea358 100644
--- a/lldb/include/lldb/Host/windows/PosixApi.h
+++ b/lldb/include/lldb/Host/windows/PosixApi.h
@@ -86,10 +86,6 @@ typedef uint32_t pid_t;
 
 #endif // _MSC_VER
 
-// Various useful posix functions that are not present in Windows.  We provide
-// custom implementations.
-int vasprintf(char **ret, const char *fmt, va_list ap);
-
 // empty functions
 inline int posix_openpt(int flag) { LLVM_BUILTIN_UNREACHABLE; }
 

diff  --git a/lldb/source/Host/CMakeLists.txt b/lldb/source/Host/CMakeLists.txt
index 4374abca05066..70942e0772aa6 100644
--- a/lldb/source/Host/CMakeLists.txt
+++ b/lldb/source/Host/CMakeLists.txt
@@ -66,7 +66,6 @@ if (CMAKE_SYSTEM_NAME MATCHES "Windows")
 windows/PipeWindows.cpp
 windows/ProcessLauncherWindows.cpp
 windows/ProcessRunLock.cpp
-windows/Windows.cpp
 )
 else()
   add_host_subdirectory(posix

diff  --git a/lldb/source/Host/common/File.cpp 
b/lldb/source/Host/common/File.cpp
index daac1fef2f36d..760fb98fb496e 100644
--- a/lldb/source/Host/common/File.cpp
+++ b/lldb/source/Host/common/File.cpp
@@ -23,17 +23,17 @@
 #include 
 #endif
 
-#include "llvm/Support/ConvertUTF.h"
-#include "llvm/Support/Errno.h"
-#include "llvm/Support/FileSystem.h"
-#include "llvm/Support/Process.h"
-
 #include "lldb/Host/Config.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
+#include "lldb/Utility/VASPrintf.h"
+#include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/Errno.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Process.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -215,18 +215,13 @@ size_t File::Printf(const char *format, ...) {
 }
 
 size_t File::PrintfVarArg(const char *format, va_list args) {
-  size_t result = 0;
-  char *s = nullptr;
-  result = vasprintf(&s, format, args);
-  if (s != nullptr) {
-if (result > 0) {
-  size_t s_len = result;
-  Write(s, s_len);
-  result = s_len;
-}
-free(s);
+  llvm::SmallString<0> s;
+  if (VASprintf(s, format, args)) {
+size_t written = s.size();;
+Write(s.data(), written);
+return written;
   }
-  return result;
+  return 0;
 }
 
 Expected File::GetOptions() const {

diff  --git a/lldb/source/Host/windows/Windows.cpp 
b/lldb/source/Host/windows/Windows.cpp
deleted file mode 100644
index a74858301ee5a..0
--- a/lldb/source/Host/windows/Windows.cpp
+++ /dev/null
@@ -1,72 +0,0 @@
-//===-- Windows.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
-//
-//===--===//
-
-// This file provides Windows support functions
-
-#include "lldb/Host/PosixApi.h"
-#include "lldb/Host/windows/windows.h"
-
-#include "llvm/Support/ConvertUTF.h"
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-int vasprintf(char **ret, const char *fmt, va_list ap) {
-  char *buf;
-  int len;
-  size_t buflen;
-  va_list ap2;
-
-  va_copy(ap2, ap);
-  len = vsnprintf(NULL, 0, fmt, ap2);
-
-  if (len >= 0 &&
-  (buf = (char *)malloc((buflen = (size_t)(len + 1 != NULL) {
-len = vsnprintf(buf, buflen, fmt, ap);
-*ret = buf;
-  } else {
-*ret = NULL;
-len = -1;
-  }
-
-  va_end(ap2);
-  return len;
-}
-
-#ifdef _MSC_VER
-
-#if _MSC_VER < 1900
-namespace lldb_private {
-int vsnprintf(char *buffer, size_t count, const char *format, va_list argptr) {
-  int old_errno = errno;
-  int r = ::vsnprintf(buffer, count, format, argptr);
-  int new_errno = errno;
-  buffer[count - 1] = '\0';
-  if (r == -1 || r == count) {
-FILE *nul = fopen("nul", "w");
-int bytes_written = ::vfprintf(nul, format, argptr);
-fclose(nul);
-if (bytes_written < count)
-  errno = new_errno;
-else {
-  errn

[Lldb-commits] [PATCH] D122710: [lldb-vscode] Avoid a -Wunused-but-set-variable warning. NFC.

2022-03-30 Thread Martin Storsjö via Phabricator via lldb-commits
mstorsjo created this revision.
mstorsjo added reviewers: labath, clayborg.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122710

Files:
  lldb/tools/lldb-vscode/lldb-vscode.cpp


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -3246,7 +3246,6 @@
 g_vsc.output.descriptor = StreamDescriptor::from_file(new_stdout_fd, 
false);
   }
 
-  uint32_t packet_idx = 0;
   while (!g_vsc.sent_terminated_event) {
 llvm::json::Object object;
 lldb_vscode::PacketStatus status = g_vsc.GetNextObject(object);
@@ -3257,7 +3256,6 @@
 
 if (!g_vsc.HandleObject(object))
   return 1;
-++packet_idx;
   }
 
   return EXIT_SUCCESS;


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -3246,7 +3246,6 @@
 g_vsc.output.descriptor = StreamDescriptor::from_file(new_stdout_fd, false);
   }
 
-  uint32_t packet_idx = 0;
   while (!g_vsc.sent_terminated_event) {
 llvm::json::Object object;
 lldb_vscode::PacketStatus status = g_vsc.GetNextObject(object);
@@ -3257,7 +3256,6 @@
 
 if (!g_vsc.HandleObject(object))
   return 1;
-++packet_idx;
   }
 
   return EXIT_SUCCESS;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D122710: [lldb-vscode] Avoid a -Wunused-but-set-variable warning. NFC.

2022-03-30 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: JDevlieghere.

I think this qualifies as an obvious fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122710

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


[Lldb-commits] [PATCH] D122716: [lldb/linux] Handle main thread exits

2022-03-30 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: mgorny, DavidSpickett.
Herald added a subscriber: krytarowski.
Herald added a project: All.
labath requested review of this revision.
Herald added a project: LLDB.

This patch handles the situation where the main thread exits (through
the SYS_exit syscall). In this case, the process as a whole continues
running until all of the other threads exit, or one of them issues an
exit_group syscall.

The patch consists of two changes:

- a moderate redesign of the handling of thread exit (WIFEXITED) events. 
Previously, we were removing (forgetting) a thread once we received the 
WIFEXITED (or WIFSIGNALED) event. This was problematic for the main thread, 
since the main thread WIFEXITED event (which is better thought of as a 
process-wide event) gets reported only after the entire process exits. This 
resulted in deadlocks, where we were waiting for the process to stop (because 
we still considered the main thread "live").

  This patch changes the logic such that the main thread is removed as soon as 
its PTRACE_EVENT_EXIT (the pre-exit) event is received. At this point we can 
consider the thread gone (for most purposes). As a corrolary, I needed to add 
special logic to catch process-wide exit events in the cases where we don't 
have the main thread around.

- The second part of the patch is the removal of the assumptions that the main 
thread is always available. This generally meant replacing the uses of 
GetThreadByID(process_id) with GetCurrentThread() in various process-wide 
operations (such as memory reads).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122716

Files:
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/test/API/functionalities/thread/main_thread_exit/Makefile
  lldb/test/API/functionalities/thread/main_thread_exit/TestMainThreadExit.py
  lldb/test/API/functionalities/thread/main_thread_exit/main.cpp

Index: lldb/test/API/functionalities/thread/main_thread_exit/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/thread/main_thread_exit/main.cpp
@@ -0,0 +1,23 @@
+#include 
+
+#ifdef __linux__
+#include 
+#include 
+
+void exit_thread(int result) { syscall(SYS_exit, result); }
+#else
+#error Needs OS-specific implementation
+#endif
+
+int call_me() { return 12345; }
+
+void thread() {
+  std::this_thread::sleep_for(
+  std::chrono::seconds(10)); // Let the main thread exit.
+  exit_thread(42);   // break here
+}
+
+int main() {
+  std::thread(thread).detach();
+  exit_thread(47);
+}
Index: lldb/test/API/functionalities/thread/main_thread_exit/TestMainThreadExit.py
===
--- /dev/null
+++ lldb/test/API/functionalities/thread/main_thread_exit/TestMainThreadExit.py
@@ -0,0 +1,31 @@
+"""
+Test handling of the situation where the main thread exits but the other threads
+in the process keep running.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class ThreadExitTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+# Needs os-specific implementation in the inferior
+@skipIf(oslist=no_match(["linux"]))
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here",
+lldb.SBFileSpec("main.cpp"))
+
+# There should be one (non-main) thread left
+self.assertEquals(self.process().GetNumThreads(), 1)
+
+# Ensure we can evaluate_expressions in this state
+self.expect_expr("call_me()", result_value="12345")
+
+self.runCmd("continue")
+self.assertEquals(self.process().GetExitStatus(), 47)
Index: lldb/test/API/functionalities/thread/main_thread_exit/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/thread/main_thread_exit/Makefile
@@ -0,0 +1,3 @@
+ENABLE_THREADS := YES
+CXX_SOURCES := main.cpp
+include Makefile.rules
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -447,13 +447,7 @@
 // This is a thread that exited.  Ensure we're not tracking it anymore.
 StopTrackingThread(thread);
 
-if (is_main_thread) {
-  // The main thread exited.  We're done monitoring.  Report to delegate.
-  SetExitStatus(status, true);
-
-  // Notify delegate that our process has exited.
-  SetState(StateType::eStateExited, true);
-}
+assert(!is_main_thread && "Main thread exits handled elsewhere");
 return;
   }
 
@@ -611,6 +605,13 @@
 }
 ResumeThread(thread, state, LLDB_INVALID_SIGNAL_NUMBER);
 
+if 

[Lldb-commits] [lldb] 21c5bb0 - Recommit [lldb/test] Make category-skipping logic "platform"-independent

2022-03-30 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2022-03-30T17:16:37+02:00
New Revision: 21c5bb0a636c23ec75b13681c0a6fdb03ecd9c0d

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

LOG: Recommit [lldb/test] Make category-skipping logic "platform"-independent

This recommits dddf4ce03, which was reverted because of a couple of test
failures on macos. The reason behind the failures was that the patch
inadvertenly changed the value returned by the host platform from
"macosx" to "darwin". The new version fixes that.

Original commit message was:

The decision which categories are relevant for a particular test run
happen very early in the test setup process. They use the SBPlatform
object to determine which categories should be skipped. The platform
object created for this purpose transcends individual test runs.

This setup is not compatible with the direction discussed in

-- when platform objects are tied to a specific (SB)Debugger, they need
to be created alongside it, which currently happens in the test setUp
method.

This patch is the first step in that direction -- it rewrites the
category skipping logic to avoid depending on a global SBPlatform
object. Fortunately, the skipping logic is fairly simple (and I believe
it outght to stay that way) and mainly consists of comparing the
platform name against some hardcoded lists. This patch bases this
comparison on the platform name instead of the os part of the triple (as
reported by the platform).

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
lldb/test/API/functionalities/paths/TestPaths.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index ce01146055b2c..5cf75391d8440 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -822,9 +822,9 @@ def checkObjcSupport():
 configuration.skip_categories.append("objc")
 
 def checkDebugInfoSupport():
-import lldb
+from lldbsuite.test import lldbplatformutil
 
-platform = lldb.selected_platform.GetTriple().split('-')[2]
+platform = lldbplatformutil.getPlatform()
 compiler = configuration.compiler
 for cat in test_categories.debug_info_categories:
 if cat in configuration.categories_list:
@@ -840,14 +840,14 @@ def checkDebugServerSupport():
 skip_msg = "Skipping %s tests, as they are not compatible with remote 
testing on this platform"
 if lldbplatformutil.platformIsDarwin():
 configuration.skip_categories.append("llgs")
-if lldb.remote_platform:
+if configuration.lldb_platform_name:
 # 
 configuration.skip_categories.append("debugserver")
 if configuration.verbose:
 print(skip_msg%"debugserver");
 else:
 configuration.skip_categories.append("debugserver")
-if lldb.remote_platform and lldbplatformutil.getPlatform() == 
"windows":
+if configuration.lldb_platform_name and lldbplatformutil.getPlatform() 
== "windows":
 configuration.skip_categories.append("llgs")
 if configuration.verbose:
 print(skip_msg%"lldb-server");
@@ -881,7 +881,16 @@ def run_suite():
 import lldb
 lldb.SBDebugger.Initialize()
 
+checkLibcxxSupport()
+checkLibstdcxxSupport()
+checkWatchpointSupport()
+checkDebugInfoSupport()
+checkDebugServerSupport()
+checkObjcSupport()
+checkForkVForkSupport()
+
 # Use host platform by default.
+lldb.remote_platform = None
 lldb.selected_platform = lldb.SBPlatform.GetHostPlatform()
 
 # Now we can also import lldbutil
@@ -938,14 +947,6 @@ def run_suite():
 # Note that it's not dotest's job to clean this directory.
 lldbutil.mkdir_p(configuration.test_build_dir)
 
-checkLibcxxSupport()
-checkLibstdcxxSupport()
-checkWatchpointSupport()
-checkDebugInfoSupport()
-checkDebugServerSupport()
-checkObjcSupport()
-checkForkVForkSupport()
-
 print("Skipping the following test categories: 
{}".format(configuration.skip_categories))
 
 for testdir in configuration.testdirs:

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py 
b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
index 94b133589dcc5..e14c4f8c0d383 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
@@ -57,12 +57,7 @@ def _run_adb_command(cmd, device_id):
 
 
 def target_is_android():
-if not hasattr(target_is_android, 

[Lldb-commits] [PATCH] D117559: [lldb] Remove forward-connect ability from lldb-server tests

2022-03-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

(Thanks for trying this out, I'm still thinking about the best way to move this 
forward.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117559

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


[Lldb-commits] [PATCH] D122684: [lldb] Use the selected and host platform to disambiguate between fat binary architectures

2022-03-30 Thread Jim Ingham via Phabricator via lldb-commits
jingham added inline comments.



Comment at: lldb/source/Target/Platform.cpp:1252
+if (selected_platform_sp) {
+  if (selected_platform_sp->IsCompatibleArchitecture(
+  arch, process_host_arch, false, nullptr)) {

Why are you passing process_host_arch here?  This is the "selected_platform" so 
you have no way of knowing a priori that this is the host platform or has the 
same architecture as the host system.  In the old version, this selected 
platform part of the processing passed {} instead of the process_host_arch, 
which seems more correct.


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

https://reviews.llvm.org/D122684

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


[Lldb-commits] [PATCH] D122684: [lldb] Use the selected and host platform to disambiguate between fat binary architectures

2022-03-30 Thread Jim Ingham via Phabricator via lldb-commits
jingham added inline comments.



Comment at: lldb/source/Target/Platform.cpp:1252
+if (selected_platform_sp) {
+  if (selected_platform_sp->IsCompatibleArchitecture(
+  arch, process_host_arch, false, nullptr)) {

jingham wrote:
> Why are you passing process_host_arch here?  This is the "selected_platform" 
> so you have no way of knowing a priori that this is the host platform or has 
> the same architecture as the host system.  In the old version, this selected 
> platform part of the processing passed {} instead of the process_host_arch, 
> which seems more correct.
Note, the old code made what seems like the opposite mistake, and DIDN'T pass 
process_host_arch in the Host Platform section of the code.


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

https://reviews.llvm.org/D122684

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


[Lldb-commits] [PATCH] D122684: [lldb] Use the selected and host platform to disambiguate between fat binary architectures

2022-03-30 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.



Comment at: lldb/source/Target/Platform.cpp:1252
+if (selected_platform_sp) {
+  if (selected_platform_sp->IsCompatibleArchitecture(
+  arch, process_host_arch, false, nullptr)) {

jingham wrote:
> jingham wrote:
> > Why are you passing process_host_arch here?  This is the 
> > "selected_platform" so you have no way of knowing a priori that this is the 
> > host platform or has the same architecture as the host system.  In the old 
> > version, this selected platform part of the processing passed {} instead of 
> > the process_host_arch, which seems more correct.
> Note, the old code made what seems like the opposite mistake, and DIDN'T pass 
> process_host_arch in the Host Platform section of the code.
The `process_host_arch` argument was added to 
`Platform::IsCompatibleArchitecture` for 
https://reviews.llvm.org/rGc22c7a61b6d9c90d5d4292205c63cd576f4fd05b. You're 
correct that we don't have a process host arch from where this function is 
being called. So I could have omitted it, but I decided not to for consistency 
with `Platform::GetPlatformForArchitecture` just above.


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

https://reviews.llvm.org/D122684

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


[Lldb-commits] [PATCH] D122684: [lldb] Use the selected and host platform to disambiguate between fat binary architectures

2022-03-30 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: lldb/source/Target/Platform.cpp:1252
+if (selected_platform_sp) {
+  if (selected_platform_sp->IsCompatibleArchitecture(
+  arch, process_host_arch, false, nullptr)) {

JDevlieghere wrote:
> jingham wrote:
> > jingham wrote:
> > > Why are you passing process_host_arch here?  This is the 
> > > "selected_platform" so you have no way of knowing a priori that this is 
> > > the host platform or has the same architecture as the host system.  In 
> > > the old version, this selected platform part of the processing passed {} 
> > > instead of the process_host_arch, which seems more correct.
> > Note, the old code made what seems like the opposite mistake, and DIDN'T 
> > pass process_host_arch in the Host Platform section of the code.
> The `process_host_arch` argument was added to 
> `Platform::IsCompatibleArchitecture` for 
> https://reviews.llvm.org/rGc22c7a61b6d9c90d5d4292205c63cd576f4fd05b. You're 
> correct that we don't have a process host arch from where this function is 
> being called. So I could have omitted it, but I decided not to for 
> consistency with `Platform::GetPlatformForArchitecture` just above.
The new code in TargetList.cpp:179 still passes `{}` to 
GetPlatformForArchitectures as the process host arch. 


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

https://reviews.llvm.org/D122684

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


[Lldb-commits] [PATCH] D122753: Fix NSIndexPathSyntheticFrontEnd::Impl::Clear() to only clear the active union member

2022-03-30 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik created this revision.
shafik added reviewers: labath, aprantl, JDevlieghere.
Herald added a subscriber: arphaman.
Herald added a project: All.
shafik requested review of this revision.

`NSIndexPathSyntheticFrontEnd::Impl::Clear()` currently calls `Clear()` on both 
unions members regardless of which one is active. I modified it to only call 
`Clear()` on the active member.


https://reviews.llvm.org/D122753

Files:
  lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp


Index: lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
+++ lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
@@ -282,9 +282,17 @@
 };
 
 void Clear() {
+  switch (m_mode) {
+  case Mode::Inlined:
+m_inlined.Clear();
+break;
+  case Mode::Outsourced:
+m_outsourced.Clear();
+break;
+  case Mode::Invalid:
+break;
+  }
   m_mode = Mode::Invalid;
-  m_inlined.Clear();
-  m_outsourced.Clear();
 }
 
 Impl() {}


Index: lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
+++ lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
@@ -282,9 +282,17 @@
 };
 
 void Clear() {
+  switch (m_mode) {
+  case Mode::Inlined:
+m_inlined.Clear();
+break;
+  case Mode::Outsourced:
+m_outsourced.Clear();
+break;
+  case Mode::Invalid:
+break;
+  }
   m_mode = Mode::Invalid;
-  m_inlined.Clear();
-  m_outsourced.Clear();
 }
 
 Impl() {}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D122753: Fix NSIndexPathSyntheticFrontEnd::Impl::Clear() to only clear the active union member

2022-03-30 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added a comment.

I believe that `NSIndexPathSyntheticFrontEnd::Update()` also needs a fix in 
order to properly set the active member but I will do that as a separate fix 
since.


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

https://reviews.llvm.org/D122753

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


[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info

2022-03-30 Thread jeffrey tan via Phabricator via lldb-commits
yinghuitan updated this revision to Diff 419226.
yinghuitan added a comment.

Support debug map dwarf mode.
Use zero symbol ability to skip SymbolFileOnDemand wrapping.
Add new API tests for symbol on-demand which support multi-platform.
Remove linux-only shell tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121631

Files:
  lldb/docs/use/ondemand.rst
  lldb/include/lldb/Core/ModuleList.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolFileOnDemand.h
  lldb/include/lldb/Target/Statistics.h
  lldb/include/lldb/Utility/LLDBLog.h
  lldb/source/Core/CoreProperties.td
  lldb/source/Core/Module.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  lldb/source/Symbol/CMakeLists.txt
  lldb/source/Symbol/CompileUnit.cpp
  lldb/source/Symbol/SymbolFile.cpp
  lldb/source/Symbol/SymbolFileOnDemand.cpp
  lldb/source/Target/Statistics.cpp
  lldb/source/Utility/LLDBLog.cpp
  lldb/test/API/symbol_ondemand/breakpoint_language/Makefile
  lldb/test/API/symbol_ondemand/breakpoint_language/TestBreakpointLanguage.py
  lldb/test/API/symbol_ondemand/breakpoint_language/c_lang.c
  lldb/test/API/symbol_ondemand/breakpoint_language/cpp_lang.cpp
  lldb/test/API/symbol_ondemand/breakpoint_language/main.cpp
  lldb/test/API/symbol_ondemand/breakpoint_source_regex/Makefile
  
lldb/test/API/symbol_ondemand/breakpoint_source_regex/TestSourceTextRegexBreakpoint.py
  lldb/test/API/symbol_ondemand/breakpoint_source_regex/main.cpp
  lldb/test/API/symbol_ondemand/shared_library/Makefile
  lldb/test/API/symbol_ondemand/shared_library/TestSharedLib.py
  lldb/test/API/symbol_ondemand/shared_library/foo.c
  lldb/test/API/symbol_ondemand/shared_library/foo.h
  lldb/test/API/symbol_ondemand/shared_library/shared.c
  lldb/test/Shell/SymbolFile/OnDemand/Inputs/basic.cpp
  lldb/test/Shell/SymbolFile/OnDemand/source-breakpoint.test
  lldb/test/Shell/SymbolFile/OnDemand/symbolic-breakpoint.test

Index: lldb/test/Shell/SymbolFile/OnDemand/symbolic-breakpoint.test
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/OnDemand/symbolic-breakpoint.test
@@ -0,0 +1,24 @@
+
+# Test shows that symbolic function breakpoint works with LLDB on demand symbol loading.
+
+# RUN: mkdir -p %t
+# RUN: cd %t
+# RUN: %build %p/Inputs/basic.cpp -o basic.out
+# RUN: %lldb -b -O "settings set symbols.load-on-demand true" -s %s basic.out | FileCheck %s
+
+breakpoint list
+# CHECK: No breakpoints currently set
+
+b bar
+# CHECK: where = {{.*}}`bar(int, int) + {{.*}} at basic.cpp:1
+
+breakpoint list
+# CHECK: where = {{.*}}`bar(int, int) + {{.*}} at basic.cpp:1
+
+run
+# CHECK: stop reason = breakpoint
+
+bt
+# CHECK: {{.*}}`bar(x=33, y=78) at basic.cpp:1
+# CHECK: {{.*}}`foo(x=33, y=78) at basic.cpp:3
+# CHECK: {{.*}}`main(argc=1, argv={{.*}}) at basic.cpp:5
Index: lldb/test/Shell/SymbolFile/OnDemand/source-breakpoint.test
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/OnDemand/source-breakpoint.test
@@ -0,0 +1,23 @@
+# Test shows that source line breakpoint works with LLDB on demand symbol loading.
+
+# RUN: mkdir -p %t
+# RUN: cd %t
+# RUN: %build %p/Inputs/basic.cpp -o basic.out
+# RUN: %lldb -b -O "settings set symbols.load-on-demand true" -s %s basic.out | FileCheck %s
+
+breakpoint list
+# CHECK: No breakpoints currently set
+
+breakpoint set -f basic.cpp -l 1
+# CHECK: where = {{.*}}`bar(int, int) + {{.*}} at basic.cpp:1
+
+breakpoint list
+# CHECK: file = 'basic.cpp'
+
+run
+# CHECK: stop reason = breakpoint
+
+bt
+# CHECK: {{.*}}`bar(x=33, y=78) at basic.cpp:1
+# CHECK: {{.*}}`foo(x=33, y=78) at basic.cpp:3
+# CHECK: {{.*}}`main(argc=1, argv={{.*}}) at basic.cpp:5
Index: lldb/test/Shell/SymbolFile/OnDemand/Inputs/basic.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/OnDemand/Inputs/basic.cpp
@@ -0,0 +1,5 @@
+int bar(int x, int y) { return x + y + 5; }
+
+int foo(int x, int y) { return bar(x, y) + 12; }
+
+int main(int argc, char **argv) { return foo(33, 78); }
Index: lldb/test/API/symbol_ondemand/shared_library/shared.c
===
--- /dev/null
+++ lldb/test/API/symbol_ondemand/shared_library/shared.c
@@ -0,0 +1,9 @@
+#include "foo.h"
+#include 
+
+int global_shared = 897;
+int main(void) {
+  puts("This is a shared library test...");
+  foo(); // Set breakpoint 0 here.
+  return 0;
+}
Index: lldb/test/API/symbol_ondemand/shared_library/foo.h
===
--- /dev/null
+++ lldb/test/API/symbol_ondemand/shared_library/foo.h
@@ -0,0 +1,6 @@
+

[Lldb-commits] [PATCH] D122710: [lldb-vscode] Avoid a -Wunused-but-set-variable warning. NFC.

2022-03-30 Thread Martin Storsjö via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa37cb5ece513: [lldb-vscode] Avoid a 
-Wunused-but-set-variable warning. NFC. (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122710

Files:
  lldb/tools/lldb-vscode/lldb-vscode.cpp


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -3246,7 +3246,6 @@
 g_vsc.output.descriptor = StreamDescriptor::from_file(new_stdout_fd, 
false);
   }
 
-  uint32_t packet_idx = 0;
   while (!g_vsc.sent_terminated_event) {
 llvm::json::Object object;
 lldb_vscode::PacketStatus status = g_vsc.GetNextObject(object);
@@ -3257,7 +3256,6 @@
 
 if (!g_vsc.HandleObject(object))
   return 1;
-++packet_idx;
   }
 
   return EXIT_SUCCESS;


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -3246,7 +3246,6 @@
 g_vsc.output.descriptor = StreamDescriptor::from_file(new_stdout_fd, false);
   }
 
-  uint32_t packet_idx = 0;
   while (!g_vsc.sent_terminated_event) {
 llvm::json::Object object;
 lldb_vscode::PacketStatus status = g_vsc.GetNextObject(object);
@@ -3257,7 +3256,6 @@
 
 if (!g_vsc.HandleObject(object))
   return 1;
-++packet_idx;
   }
 
   return EXIT_SUCCESS;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] a37cb5e - [lldb-vscode] Avoid a -Wunused-but-set-variable warning. NFC.

2022-03-30 Thread Martin Storsjö via lldb-commits

Author: Martin Storsjö
Date: 2022-03-31T00:10:05+03:00
New Revision: a37cb5ece513cbed9059880a45838331460e42fb

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

LOG: [lldb-vscode] Avoid a -Wunused-but-set-variable warning. NFC.

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

Added: 


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

Removed: 




diff  --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp 
b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index 28c0dfbac40f8..83a9762df6040 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -3246,7 +3246,6 @@ int main(int argc, char *argv[]) {
 g_vsc.output.descriptor = StreamDescriptor::from_file(new_stdout_fd, 
false);
   }
 
-  uint32_t packet_idx = 0;
   while (!g_vsc.sent_terminated_event) {
 llvm::json::Object object;
 lldb_vscode::PacketStatus status = g_vsc.GetNextObject(object);
@@ -3257,7 +3256,6 @@ int main(int argc, char *argv[]) {
 
 if (!g_vsc.HandleObject(object))
   return 1;
-++packet_idx;
   }
 
   return EXIT_SUCCESS;



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


[Lldb-commits] [PATCH] D122767: [lldb] Improve documentation for some of the platform functions

2022-03-30 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added a reviewer: jingham.
Herald added a project: All.
JDevlieghere requested review of this revision.

Improve the documentation for the platform functions that take a process host 
architecture as input.


https://reviews.llvm.org/D122767

Files:
  lldb/include/lldb/Target/Platform.h


Index: lldb/include/lldb/Target/Platform.h
===
--- lldb/include/lldb/Target/Platform.h
+++ lldb/include/lldb/Target/Platform.h
@@ -94,6 +94,24 @@
   /// attaching to processes unless another platform is specified.
   static lldb::PlatformSP GetHostPlatform();
 
+  /// Get the platform for the given architecture. See Platform::Create for how
+  /// a platform is chosen for a given architecture.
+  ///
+  /// \param[in] arch
+  /// The architecture for which we are getting a platform.
+  ///
+  /// \param[in] process_host_arch
+  /// The process host architecture if it's known. An invalid ArchSpec
+  /// represents that the process host architecture is unknown.
+  ///
+  /// \param[out] platform_arch_ptr
+  /// The architecture which was used to pick the returned platform. This
+  /// can be different from the input architecture if we're looking for a
+  /// compatible match instead of an exact match.
+  ///
+  /// \return
+  /// Return the platform that matches the input architecture or the
+  /// default (invalid) platform if none could be found.
   static lldb::PlatformSP
   GetPlatformForArchitecture(const ArchSpec &arch,
  const ArchSpec &process_host_arch = {},
@@ -111,6 +129,26 @@
   /// candidates output argument differentiates between either no platforms
   /// supporting the given architecture or multiple platforms supporting the
   /// given architecture.
+  ///
+  /// \param[in] arch
+  /// The architecture for which we are getting a platform.
+  ///
+  /// \param[in] process_host_arch
+  /// The process host architecture if it's known. An invalid ArchSpec
+  /// represents that the process host architecture is unknown.
+  ///
+  /// \param[in] selected_platform_sp
+  /// The currently selected platform.
+  ///
+  /// \param[out] candidates
+  /// A list of candidate platforms in case multiple platforms could
+  /// support the given list of architectures. If no platforms match the
+  /// given architecture the list will be empty.
+  ///
+  /// \return
+  /// Return the platform that matches the input architectures or the
+  /// default (invalid) platform if no platforms support the given
+  /// architectures or multiple platforms support the given architecture.
   static lldb::PlatformSP
   GetPlatformForArchitectures(std::vector archs,
   const ArchSpec &process_host_arch,
@@ -331,6 +369,10 @@
 
   /// Get the platform's supported architectures in the order in which they
   /// should be searched.
+  ///
+  /// \param[in] process_host_arch
+  /// The process host architecture if it's known. An invalid ArchSpec
+  /// represents that the process host architecture is unknown.
   virtual std::vector
   GetSupportedArchitectures(const ArchSpec &process_host_arch) = 0;
 


Index: lldb/include/lldb/Target/Platform.h
===
--- lldb/include/lldb/Target/Platform.h
+++ lldb/include/lldb/Target/Platform.h
@@ -94,6 +94,24 @@
   /// attaching to processes unless another platform is specified.
   static lldb::PlatformSP GetHostPlatform();
 
+  /// Get the platform for the given architecture. See Platform::Create for how
+  /// a platform is chosen for a given architecture.
+  ///
+  /// \param[in] arch
+  /// The architecture for which we are getting a platform.
+  ///
+  /// \param[in] process_host_arch
+  /// The process host architecture if it's known. An invalid ArchSpec
+  /// represents that the process host architecture is unknown.
+  ///
+  /// \param[out] platform_arch_ptr
+  /// The architecture which was used to pick the returned platform. This
+  /// can be different from the input architecture if we're looking for a
+  /// compatible match instead of an exact match.
+  ///
+  /// \return
+  /// Return the platform that matches the input architecture or the
+  /// default (invalid) platform if none could be found.
   static lldb::PlatformSP
   GetPlatformForArchitecture(const ArchSpec &arch,
  const ArchSpec &process_host_arch = {},
@@ -111,6 +129,26 @@
   /// candidates output argument differentiates between either no platforms
   /// supporting the given architecture or multiple platforms supporting the
   /// given architecture.
+  ///
+  /// \param[in] arch
+  /// The architecture for which we are getting a platform.
+  ///
+  /// \param[in] process_host_arch
+  /// The process host architecture if it's known. An invalid ArchSpec
+  ///

[Lldb-commits] [PATCH] D122680: Add a setting to force overwriting commands in "command script add"

2022-03-30 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122680

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


[Lldb-commits] [lldb] a114ec0 - [lldb] Change the way we pick a platform for fat binaries

2022-03-30 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2022-03-30T15:30:05-07:00
New Revision: a114ec0c6dc052832ec3dc1f65c9e221e3272925

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

LOG: [lldb] Change the way we pick a platform for fat binaries

Currently, when creating a target for a fat binary, we error out if more
than one platforms can support the different architectures in the
binary. There are situations where it makes sense for multiple platforms
to support the same architectures: for example the host and
remote-macosx platform on Darwin.

The only way to currently disambiguate between them is to specify the
architecture. This patch changes that to take into account the selected
and host platform. The new algorithm works a follows:

1. Pick the selected platform if it matches any of the architectures.
2. Pick the host platform if it matches any of the architectures.
3. If there's one platform that works for all architectures, pick that.

If none of the above apply then we either have no platform supporting
the architectures in the fat binary or multiple platforms with no good
way to disambiguate between them.

I've added a bunch of unit tests to codify this new behavior.

rdar://90360204

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

Added: 
lldb/unittests/Platform/PlatformTest.cpp

Modified: 
lldb/include/lldb/Target/Platform.h
lldb/source/Target/Platform.cpp
lldb/source/Target/TargetList.cpp
lldb/unittests/Platform/CMakeLists.txt

Removed: 




diff  --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index 9136989fea0bb..eee7e3116beea 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -99,6 +99,24 @@ class Platform : public PluginInterface {
  const ArchSpec &process_host_arch = {},
  ArchSpec *platform_arch_ptr = nullptr);
 
+  /// Get the platform for the given list of architectures.
+  ///
+  /// The algorithm works a follows:
+  ///
+  /// 1. Returns the selected platform if it matches any of the architectures.
+  /// 2. Returns the host platform if it matches any of the architectures.
+  /// 3. Returns the platform that matches all the architectures.
+  ///
+  /// If none of the above apply, this function returns a default platform. The
+  /// candidates output argument 
diff erentiates between either no platforms
+  /// supporting the given architecture or multiple platforms supporting the
+  /// given architecture.
+  static lldb::PlatformSP
+  GetPlatformForArchitectures(std::vector archs,
+  const ArchSpec &process_host_arch,
+  lldb::PlatformSP selected_platform_sp,
+  std::vector &candidates);
+
   static const char *GetHostPlatformName();
 
   static void SetHostPlatform(const lldb::PlatformSP &platform_sp);
@@ -871,6 +889,9 @@ class Platform : public PluginInterface {
   virtual CompilerType GetSiginfoType(const llvm::Triple &triple);
 
 protected:
+  /// For unit testing purposes only.
+  static void Clear();
+
   /// Create a list of ArchSpecs with the given OS and a architectures. The
   /// vendor field is left as an "unspecified unknown".
   static std::vector

diff  --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index 2c3bdf3a6bbb1..74e3e38953608 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -38,6 +38,7 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/StructuredData.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 
@@ -156,6 +157,8 @@ void Platform::Terminate() {
   }
 }
 
+void Platform::Clear() { GetPlatformList().clear(); }
+
 PlatformProperties &Platform::GetGlobalPlatformProperties() {
   static PlatformProperties g_settings;
   return g_settings;
@@ -1216,6 +1219,61 @@ Platform::GetPlatformForArchitecture(const ArchSpec 
&arch,
   return platform_sp;
 }
 
+lldb::PlatformSP Platform::GetPlatformForArchitectures(
+std::vector archs, const ArchSpec &process_host_arch,
+lldb::PlatformSP selected_platform_sp,
+std::vector &candidates) {
+  candidates.clear();
+  candidates.reserve(archs.size());
+
+  if (archs.empty())
+return nullptr;
+
+  PlatformSP host_platform_sp = Platform::GetHostPlatform();
+
+  // Prefer the selected platform if it matches at least one architecture.
+  if (selected_platform_sp) {
+for (const ArchSpec &arch : archs) {
+  if (selected_platform_sp->IsCompatibleArchitecture(
+  arch, process_host_arch, false, nullptr))
+return selected_platform_sp;
+}
+  }
+
+  // Prefer the host platform if it matche

[Lldb-commits] [PATCH] D122684: [lldb] Use the selected and host platform to disambiguate between fat binary architectures

2022-03-30 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa114ec0c6dc0: [lldb] Change the way we pick a platform for 
fat binaries (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D122684?vs=419005&id=419280#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122684

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/Target/Platform.cpp
  lldb/source/Target/TargetList.cpp
  lldb/unittests/Platform/CMakeLists.txt
  lldb/unittests/Platform/PlatformTest.cpp

Index: lldb/unittests/Platform/PlatformTest.cpp
===
--- /dev/null
+++ lldb/unittests/Platform/PlatformTest.cpp
@@ -0,0 +1,162 @@
+//===-- PlatformTest.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/Platform/POSIX/PlatformPOSIX.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/Platform.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class TestPlatform : public PlatformPOSIX {
+public:
+  TestPlatform() : PlatformPOSIX(false) {}
+  using Platform::Clear;
+};
+
+class PlatformArm : public TestPlatform {
+public:
+  PlatformArm() = default;
+
+  std::vector
+  GetSupportedArchitectures(const ArchSpec &process_host_arch) override {
+return {ArchSpec("arm64-apple-ps4")};
+  }
+
+  llvm::StringRef GetPluginName() override { return "arm"; }
+  llvm::StringRef GetDescription() override { return "arm"; }
+};
+
+class PlatformIntel : public TestPlatform {
+public:
+  PlatformIntel() = default;
+
+  std::vector
+  GetSupportedArchitectures(const ArchSpec &process_host_arch) override {
+return {ArchSpec("x86_64-apple-ps4")};
+  }
+
+  llvm::StringRef GetPluginName() override { return "intel"; }
+  llvm::StringRef GetDescription() override { return "intel"; }
+};
+
+class PlatformThumb : public TestPlatform {
+public:
+  static void Initialize() {
+PluginManager::RegisterPlugin("thumb", "thumb",
+  PlatformThumb::CreateInstance);
+  }
+  static void Terminate() {
+PluginManager::UnregisterPlugin(PlatformThumb::CreateInstance);
+  }
+
+  static PlatformSP CreateInstance(bool force, const ArchSpec *arch) {
+return std::make_shared();
+  }
+
+  std::vector
+  GetSupportedArchitectures(const ArchSpec &process_host_arch) override {
+return {ArchSpec("thumbv7-apple-ps4"), ArchSpec("thumbv7f-apple-ps4")};
+  }
+
+  llvm::StringRef GetPluginName() override { return "thumb"; }
+  llvm::StringRef GetDescription() override { return "thumb"; }
+};
+
+class PlatformTest : public ::testing::Test {
+  SubsystemRAII subsystems;
+  void SetUp() override { TestPlatform::Clear(); }
+};
+
+TEST_F(PlatformTest, GetPlatformForArchitecturesHost) {
+  const PlatformSP host_platform_sp = std::make_shared();
+  Platform::SetHostPlatform(host_platform_sp);
+  ASSERT_EQ(Platform::GetHostPlatform(), host_platform_sp);
+
+  const std::vector archs = {ArchSpec("arm64-apple-ps4"),
+   ArchSpec("arm64e-apple-ps4")};
+  std::vector candidates;
+
+  // The host platform matches all architectures.
+  PlatformSP platform_sp =
+  Platform::GetPlatformForArchitectures(archs, {}, {}, candidates);
+  ASSERT_TRUE(platform_sp);
+  EXPECT_EQ(platform_sp, host_platform_sp);
+}
+
+TEST_F(PlatformTest, GetPlatformForArchitecturesSelected) {
+  const PlatformSP host_platform_sp = std::make_shared();
+  Platform::SetHostPlatform(host_platform_sp);
+  ASSERT_EQ(Platform::GetHostPlatform(), host_platform_sp);
+
+  const std::vector archs = {ArchSpec("arm64-apple-ps4"),
+   ArchSpec("arm64e-apple-ps4")};
+  std::vector candidates;
+
+  // The host platform matches no architectures. No selected platform.
+  PlatformSP platform_sp =
+  Platform::GetPlatformForArchitectures(archs, {}, {}, candidates);
+  ASSERT_FALSE(platform_sp);
+
+  // The selected platform matches all architectures.
+  const PlatformSP selected_platform_sp = std::make_shared();
+  platform_sp = Platform::GetPlatformForArchitectures(
+  archs, {}, selected_platform_sp, candidates);
+  ASSERT_TRUE(platform_sp);
+  EXPECT_EQ(platform_sp, selected_platform_sp);
+}
+
+TEST_F(PlatformTest, GetPlatformForArchitecturesSelectedOverHost) {
+  const PlatformSP host_platform_sp = std::make_shared();
+  Platform::SetHostPlatform(host_platform_sp);
+  ASSERT_EQ(Platform::GetHostPlatform(), host

[Lldb-commits] [PATCH] D122753: Fix NSIndexPathSyntheticFrontEnd::Impl::Clear() to only clear the active union member

2022-03-30 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D122753

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


[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

2022-03-30 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121967

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


[Lldb-commits] [lldb] 14cad95 - [LLDB] Fix NSIndexPathSyntheticFrontEnd::Impl::Clear() to only clear the active union member

2022-03-30 Thread Shafik Yaghmour via lldb-commits

Author: Shafik Yaghmour
Date: 2022-03-30T18:00:37-07:00
New Revision: 14cad95d38235df6c5fd5dd3da84b91fa69e7e74

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

LOG: [LLDB] Fix NSIndexPathSyntheticFrontEnd::Impl::Clear() to only clear the 
active union member

NSIndexPathSyntheticFrontEnd::Impl::Clear() currently calls Clear() on both
unions members regardless of which one is active. I modified it to only call
Clear() on the active member.

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

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp 
b/lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
index 8db75716e1862..429633e5e6b87 100644
--- a/lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
+++ b/lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
@@ -282,9 +282,17 @@ class NSIndexPathSyntheticFrontEnd : public 
SyntheticChildrenFrontEnd {
 };
 
 void Clear() {
+  switch (m_mode) {
+  case Mode::Inlined:
+m_inlined.Clear();
+break;
+  case Mode::Outsourced:
+m_outsourced.Clear();
+break;
+  case Mode::Invalid:
+break;
+  }
   m_mode = Mode::Invalid;
-  m_inlined.Clear();
-  m_outsourced.Clear();
 }
 
 Impl() {}



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


[Lldb-commits] [PATCH] D122753: Fix NSIndexPathSyntheticFrontEnd::Impl::Clear() to only clear the active union member

2022-03-30 Thread Shafik Yaghmour via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG14cad95d3823: [LLDB] Fix 
NSIndexPathSyntheticFrontEnd::Impl::Clear() to only clear the active… (authored 
by shafik).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122753

Files:
  lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp


Index: lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
+++ lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
@@ -282,9 +282,17 @@
 };
 
 void Clear() {
+  switch (m_mode) {
+  case Mode::Inlined:
+m_inlined.Clear();
+break;
+  case Mode::Outsourced:
+m_outsourced.Clear();
+break;
+  case Mode::Invalid:
+break;
+  }
   m_mode = Mode::Invalid;
-  m_inlined.Clear();
-  m_outsourced.Clear();
 }
 
 Impl() {}


Index: lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
+++ lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
@@ -282,9 +282,17 @@
 };
 
 void Clear() {
+  switch (m_mode) {
+  case Mode::Inlined:
+m_inlined.Clear();
+break;
+  case Mode::Outsourced:
+m_outsourced.Clear();
+break;
+  case Mode::Invalid:
+break;
+  }
   m_mode = Mode::Invalid;
-  m_inlined.Clear();
-  m_outsourced.Clear();
 }
 
 Impl() {}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

2022-03-30 Thread walter erquinigo via Phabricator via lldb-commits
wallace requested changes to this revision.
wallace added a comment.
This revision now requires changes to proceed.

Some calculations are wrong, but overall this is good. We are very close!




Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:112-113
+DecodedThread::CalculateTscRange(size_t insn_index) const {
+  if (m_instruction_timestamps.empty())
+return None;
+

now that I think of this, you can delete this, because if the map is empty, 
this function will return in line 117



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:135-138
+: m_thread_sp(thread_sp), m_last_tsc(None) {}
 
 DecodedThread::DecodedThread(ThreadSP thread_sp, Error &&error)
+: m_thread_sp(thread_sp), m_last_tsc(None) {

undo these two lines



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:159
+DecodedThread::TscRange::TscRange(std::map::const_iterator 
it,
+  const DecodedThread &ref)
+: m_it(it), m_decoded_thread(ref) {

decoded_thread instead of ref



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:161-162
+: m_it(it), m_decoded_thread(ref) {
+  m_start_index = it->first;
+  m_tsc = it->second;
+

delete



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:164-168
+  auto end = m_decoded_thread.m_instruction_timestamps.end();
+  if (it != end)
+m_end_index = (++it--)->first - 1;
+  else
+m_end_index = end->first;

seeing ++ and -- is very hard to read. I also prefer not to modify the `it` 
variable for cleanness. Also doing end->first might crash the program. I'm 
writing here a correct version



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:173
+
+size_t DecodedThread::TscRange::GetStart() const { return m_start_index; }
+





Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:178
+bool DecodedThread::TscRange::InRange(size_t insn_index) {
+  if (insn_index < m_end_index && insn_index > m_start_index)
+return true;

The comparison is not right. let's use <= in a specific order to make it easier 
to read



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:186
+return None;
+  return TscRange(++m_it, m_decoded_thread);
+}

As m_it is valid, doing the comparison `m_it == 
m_decoded_thread.m_instruction_timestamps.end()` will always return false. 
Remember that .end() will return a fake iterator that points to no value.

Besides that, don't modify m_it. Let's better create a new iterator



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:190-192
+  if (m_it == m_decoded_thread.m_instruction_timestamps.end())
+return None;
+  return TscRange(--m_it, m_decoded_thread);

Similarly, this has to be improved. I also like to put `--it` statements in 
their own line to make it easier to read.



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:142
+  /// \class TscRange
+  /// Class that represents the instruction range associated with a given TSC.
+  /// It provides efficient iteration to the previous or next TSC range in the





Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:147-148
+  /// TSC timestamps are emitted by the decoder infrequently, which means
+  /// that each TSC covers a range of instruction indices, which we can use to
+  /// speed up TSC lookups.
+  class TscRange {





Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:149
+  /// speed up TSC lookups.
+  class TscRange {
+  public:

Move this class to the beginning of the public section of DecodedThread for 
easier discoverability



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:151
+  public:
+// Check if current TSC range covers given instruction index.
+bool InRange(size_t insn_index);





Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:167-173
+TscRange &operator=(const TscRange &r) {
+  m_it = r.m_it;
+  m_tsc = r.m_tsc;
+  m_start_index = r.m_start_index;
+  m_end_index = r.m_end_index;
+  return *this;
+}

let's better delete this. It adds some maintenance cost with little benefits



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:178
+
+// Construct TscRange respecting bounds of timestamp map in thread
+TscRange(std::map::const_iterator it,

This comment is hard to follow. Let's just delete it because it's a private 
constructor



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:185-188
+/// The TSC v

[Lldb-commits] [PATCH] D121844: Applying clang-tidy modernize-use-equals-default over LLDB

2022-03-30 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik marked 3 inline comments as done.
shafik added inline comments.



Comment at: lldb/source/Core/Value.cpp:667
 
-const ValueList &ValueList::operator=(const ValueList &rhs) {
+const ValueList &ValueList::operator=(const ValueList &rhs) {  // 
NOLINT(modernize-use-equals-default)
   m_values = rhs.m_values;

labath wrote:
> shafik wrote:
> > labath wrote:
> > > shafik wrote:
> > > > I have to look into why we return a `const &` here. We do this in a few 
> > > > other places too.
> > > I don't think there's a good reason for that. Most people aren't aware 
> > > that built-in assignment operators return lvalues. And some of the people 
> > > who are aware of that think that it's a bad idea, so they make sure their 
> > > operators don't do it...
> > It produced build errors, so some of the users are relying on this. I 
> > didn't want to plumb into this since it was orthogonal to the change.
> I was curious to see what kind of errors could be produced by that change -- 
> I didn't get any, so I committed (c484857b2e77721a4235b0e2d53d335c09fc6af3) 
> my version. :)
I see that, I am not sure what I did wrong that I got a bunch of errors the 
other day.


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

https://reviews.llvm.org/D121844

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


[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

2022-03-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D121967#3403886 , @zequanwu wrote:

>> Can you say (in english) what are the properties that you are trying to 
>> check there? Maybe we can find a better way to do that...
>
> I'm trying to check for local variables values in inline functions by 
> printing them (see inline comment on `PdbAstBuilder::ParseBlockChildren`). 
> Since this test only runs on x64, I think I will change the live debugging 
> test to a cpp file.

Sorry about the delay. This dropped off my radar.

If the goal is checking for local variables, then I'd say the test is way too 
strict for that. In particular, moving through code (even unoptimized code) 
with step instructions is very sensitive to codegen changes. And the hardcoded 
line numbers everywhere will make life hard for whoever needs to update this 
tests.

Please see inline comments on how I'd approach a local variable test. I use the 
`always_inline` attribute, which allows you to create inline functions in 
unoptimized builds. This greatly reduces our exposure to codegen cleverness. If 
building with -O0, then the optnone attribute is not necessary, but I've left 
it there to demonstrate how that can be used to keep a variable live in 
optimized builds. (It also provides a good anchor to place a breakpoint). I am 
also using breakpoints for moving through the code. Breakpoints are more 
reliable as they're usually not affected by instruction reordering (mainly 
important for optimized code, but a good practice in any case). I set the 
breakpoint's via markers left in the source code, which allows me to avoid 
using any line numbers. I am deliberately not matching the entire block that 
lldb prints when the process stops, as that is not relevant for this test.

I wasn't sure what was the exact setup you wanted to check (one problem with 
overly strict tests), so I've created something simple but still fairly similar 
to your setup. It should be fairly easy to tweak the test to do check other 
things as well.




Comment at: lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp:7
+// RUN: %p/Inputs/inline_sites_live.lldbinit 2>&1 | FileCheck %s
+
+inline int bar(int bar_param) {

```
void __attribute__((optnone)) use(int) {}

void __attribute__((always_inline)) bar(int param) {
  use(param); // BP_bar
}

void __attribute__((always_inline)) foo(int param) {
  int local = param+1;
  bar(local);
  use(param); // BP_foo
  use(local);
}

int main(int argc) {
  foo(argc);
}
```




Comment at: lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp:24
+
+// CHECK:  (lldb) b main
+// CHECK-NEXT: Breakpoint 1: where = {{.*}}`main + 4 [inlined] foo at {{.*}}:14

```
br set -p BP_bar
br set -p BP_foo
run
(CHECK: stop reason = breakpoint 1)
(CHECK: frame #0: {{.*}}`main [inlined] bar)
p param
continue
(CHECK: stop reason = breakpoint 2)
(CHECK: frame #0: {{.*}}`main [inlined] foo)
p param
p local
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121967

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