[Lldb-commits] [PATCH] D121030: [LLDB][NativePDB] Don't complete static members' types when completing a record type.

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp:8-26
+class B;
+class A {
+public:
+static const A constA;
+static A a;
+static B b;
+int val = 1;

zequanwu wrote:
> labath wrote:
> > Would it be possible to put this into a new test case, one that does not 
> > require running the binary? If we're able to read global variables without 
> > a running process (we can do it with elf+dwarf, but I don't know what's the 
> > state with coff+pdb), then we could run this test on non-windows hosts as 
> > well (which is great for test coverage).
> Printing doesn't work with any expression involves `::` in NativePDB. So, I 
> changed the test to `image lookup -type` which also tries to complete given 
> type.
That's cool, but I see the new test still has a `REQUIRES: system-windows`. 
Would it be possible to drop that requirement (if not, then why)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121030

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

[Lldb-commits] [PATCH] D121511: [lldb] Report debugger diagnostics as events

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Heh, I didn't realize that "debugger-less" events are implemented by iterating 
over all debugger instances, but since we're doing that for progress events 
anyway, we might as well copy the pattern.

Any chance of a test case for some of this stuff? Maybe a unittest which 
creates a Debugger object and uses it to send/receive some events?




Comment at: lldb/include/lldb/Core/DebuggerEvents.h:73
+
+class WarningEventData : public DiagnosticEventData {
+public:

Do we really need separate types for this? The handling of the two events is 
completely identical (except for the "error"/"warning" prefix, so it seems that 
an enum would suffice. The classes won't make it to the SB api (I think), so 
it's not like we're locking ourselves into a particular design.



Comment at: lldb/source/Core/Debugger.cpp:1665-1671
   listener_sp->StartListeningForEvents(&m_broadcaster,
Debugger::eBroadcastBitProgress);
+  listener_sp->StartListeningForEvents(&m_broadcaster,
+   Debugger::eBroadcastBitWarning);
+  listener_sp->StartListeningForEvents(&m_broadcaster,
+   Debugger::eBroadcastBitError);
 

StartListeningForEvents(eBroadcastBitProgress | eBroadcastBitWarning | 
eBroadcastBitError) ?



Comment at: lldb/source/Core/Debugger.cpp:1866
+
+  Stream &stream = *GetAsyncErrorStream();
+  stream << "warning: " << data->GetMessage() << '\n';

GetAsyncErrorStream returns a (shared -- it should probably be a unique_ptr 
instead) pointer to a new object. You can't just throw it away here.


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

https://reviews.llvm.org/D121511

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

[Lldb-commits] [PATCH] D121444: [lldb] Fix platform selection on Apple Silicon (again)

[Pavel Labath via Phabricator via lldb-commits]

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

Cool.


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

https://reviews.llvm.org/D121444

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

[Lldb-commits] [PATCH] D121537: [lldb] Synchronize the output through the IOHandler

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/include/lldb/Interpreter/CommandInterpreter.h:659
+  void PrintCommandOutput(IOHandler &io_handler, llvm::StringRef str,
+  bool is_stdout0);
 




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

https://reviews.llvm.org/D121537

___
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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I agree with everything Jim said.

One of the interesting 'strategy' questions would be the interaction between 
this feature and the `target.preload-symbols` feature. I don't think the 
interaction between the two is obvious or intuitive, and I'm even sure that 
supporting all possible combinations is useful. We could e.g. consider 
replacing the two settings with one ternary setting with values like "load 
everything upfront", "load everything the first time it's needed" and "load 
everything when the module is accessed in a specific way".

Also, I don't think that "run everything with the setting turned on" is a good 
testing strategy. Nearly all of our tests consist of a single binary and the 
very first action they do is set a file/line or a function breakpoint. That 
will immediately trigger a 'hydration' and the rest of the test will just check 
that the forwarders really forward all calls. Rerunning all tests to catch the 
few testcases that do something interesting is not a good tradeoff.

We should identify the interesting/important scenarios and write targeted tests 
for that. The tests in this patch are a pretty good start. My only complaint is 
that the shared library tests are linux-only. We currently don't have 
infrastructure in place to write shell tests with shared libraries portably. It 
may be worth rewriting those into api tests to make use of the existing shared 
library facilities.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121631

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

[Lldb-commits] [lldb] dddf4ce - [lldb/test] Make category-skipping logic "platform"-independent

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-03-15T13:38:13+01:00
New Revision: dddf4ce034a8e06cc1351492dceece3fa2344c14

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

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

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

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index ce01146055b2c..2d8bf5311a699 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
@@ -892,6 +901,7 @@ def run_suite():
   (configuration.lldb_platform_name))
 lldb.remote_platform = lldb.SBPlatform(
 configuration.lldb_platform_name)
+lldb.selected_platform = lldb.remote_platform
 if not lldb.remote_platform.IsValid():
 print(
 "error: unable to create the LLDB platform named '%s'." %
@@ -938,14 +948,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..6b93dcf38bbe6 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 

[Lldb-commits] [PATCH] D121605: [lldb/test] Make category-skipping logic "platform"-independent

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdddf4ce034a8: [lldb/test] Make category-skipping logic 
"platform"-independent (authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121605

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/lldbplatformutil.py

Index: lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
===
--- lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
+++ lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
@@ -57,12 +57,7 @@
 
 
 def target_is_android():
-if not hasattr(target_is_android, 'result'):
-triple = lldb.selected_platform.GetTriple()
-match = re.match(".*-.*-.*-android", triple)
-target_is_android.result = match is not None
-return target_is_android.result
-
+return configuration.lldb_platform_name == "remote-android"
 
 def android_device_api():
 if not hasattr(android_device_api, 'result'):
@@ -139,18 +134,16 @@
 platform = 'ios'
 return platform
 
-# Use the triple to determine the platform if set.
-triple = lldb.selected_platform.GetTriple()
-if triple:
-platform = triple.split('-')[2]
-if platform.startswith('freebsd'):
-platform = 'freebsd'
-elif platform.startswith('netbsd'):
-platform = 'netbsd'
-return platform
-
-# It still might be an unconnected remote platform.
-return ''
+platform = configuration.lldb_platform_name
+if platform is None:
+platform = "host"
+if platform == "qemu-user":
+platform = "host"
+if platform == "host":
+return getHostPlatform()
+if platform.startswith("remote-"):
+return platform[7:]
+return platform
 
 
 def platformIsDarwin():
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -822,9 +822,9 @@
 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 @@
 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 @@
 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
@@ -892,6 +901,7 @@
   (configuration.lldb_platform_name))
 lldb.remote_platform = lldb.SBPlatform(
 configuration.lldb_platform_name)
+lldb.selected_platform = lldb.remote_platform
 if not lldb.remote_platform.IsValid():
 print(
 "error: unable to create the LLDB platform named '%s'." %
@@ -938,14 +948,6 @@
 # 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:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D121408: Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 415440.
shafik marked an inline comment as done.
shafik added a comment.

Removing dead code.


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

https://reviews.llvm.org/D121408

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s
@@ -3,7 +3,7 @@
 
 # CHECK: (lldb) target variable ug
 # CHECK: (U) ug = {
-# CHECK:   raw = 0 
+# CHECK:   raw = 0
 # CHECK:= (a = 0, b = 0, c = 0, d = 0, e = 0, f = 0)
 # CHECK: }
 
@@ -48,7 +48,7 @@
 #   clang -O1 -gdwarf-4 dw_op_deref_size_test.c -S -o dw_op_deref_size_test.s
 #
 # Hand modified .s file to remoce various section that are not needed for this
-# test 
+# test
 
 .zerofill __DATA,__bss,_ug.0,1,2## @ug.0
 .zerofill __DATA,__bss,__type_anchor,4,2 ## @_type_anchor
@@ -199,7 +199,7 @@
 	.long	105 ## DW_AT_name
 	.long	129 ## DW_AT_LLVM_sysroot
 	.long	185 ## DW_AT_APPLE_sdk
-	.long 0	
+	.long 0
 	.long	200 ## DW_AT_comp_dir
 ## DW_AT_APPLE_optimized
 	.byte	2   ## Abbrev [2] 0x26:0x15 DW_TAG_variable
Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -944,6 +944,80 @@
 }
 } // namespace
 
+/// Helper function to move common code used to resolve a file address and turn
+/// into a load address.
+///
+/// \param exe_ctx Pointer to the execution context
+/// \param module_sp shared_ptr contains the module if we have one
+/// \param error_ptr pointer to Status object if we have one
+/// \param dw_op_type C-style string used to vary the error output
+/// \param file_addr the file address we are trying to resolve and turn into a
+///  load address
+/// \param so_addr out parameter, will be set to load addresss or section offset
+/// \param check_sectionoffset bool which determines if having a section offset
+///but not a load address is considerd a success
+/// \returns llvm::Optional containing the load address if resolving and getting
+///  the load address succeed or an empty Optinal otherwise. If
+///  check_sectionoffset is true we consider LLDB_INVALID_ADDRESS a
+///  success if so_addr.IsSectionOffset() is true.
+static llvm::Optional
+ResolveAndLoadFileAddress(ExecutionContext *exe_ctx, lldb::ModuleSP module_sp,
+  Status *error_ptr, const char *dw_op_type,
+  lldb::addr_t file_addr, Address &so_addr,
+  bool check_sectionoffset = false) {
+  if (!module_sp) {
+if (error_ptr)
+  error_ptr->SetErrorStringWithFormat(
+  "need module to resolve file address for %s", dw_op_type);
+return {};
+  }
+
+  if (!module_sp->ResolveFileAddress(file_addr, so_addr)) {
+if (error_ptr)
+  error_ptr->SetErrorString("failed to resolve file address in module");
+return {};
+  }
+
+  addr_t load_addr = so_addr.GetLoadAddress(exe_ctx->GetTargetPtr());
+
+  if (load_addr == LLDB_INVALID_ADDRESS &&
+  (check_sectionoffset && !so_addr.IsSectionOffset())) {
+if (error_ptr)
+  error_ptr->SetErrorString("failed to resolve load address");
+return {};
+  }
+
+  return load_addr;
+}
+
+/// Helper function to move common code used to load sized data from a uint8_t
+/// buffer.
+///
+/// \param addr_bytes uint8_t buffer containg raw data
+/// \param size_addr_bytes how large is the underlying raw data
+/// \param byte_order what is the byter order of the underlyig data
+/// \param size How much of the underlying data we want to use
+/// \return The underlying data converted into a Scalar
+static Scalar DerefSizeExtractDataHelper(uint8_t *addr_bytes,
+ size_t size_addr_bytes,
+ ByteOrder byte_order, size_t size) {
+  DataExtractor addr_data(addr_bytes, size_addr_bytes, byte_order, size);
+
+  lldb::offset_t addr_data_offset = 0;
+  switch (size) {
+  case 1:
+return addr_data.GetU8(&addr_data_offset);
+  case 2:
+return addr_data.GetU16(&addr_data_offset);
+  case 4:
+return addr_data.GetU32(&addr_data_offset);
+  case 8:
+return addr_data.GetU64(&addr_data_offset);
+  default:
+return addr_data.GetAddress(&addr_data_offset);
+  }
+}
+
 bool DWARFExpression::Evaluate(
 ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
 lldb::ModuleSP module_sp, const Da

[Lldb-commits] [PATCH] D121605: [lldb/test] Make category-skipping logic "platform"-independent

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

I think this PR broke green dragon incremental build: 
https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/42185/

It looks like the failures are all related to platforms in some ways.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121605

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

[Lldb-commits] [lldb] be09f83 - Revert "[lldb/test] Make category-skipping logic "platform"-independent"

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-03-15T16:23:43+01:00
New Revision: be09f83760ebe7cf698746d5504976ad82679815

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

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

This reverts commit dddf4ce034a8e06cc1351492dceece3fa2344c14. It breaks
a couple of tests on macos.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/packages/Python/lldbsuite/test/lldbplatformutil.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index 2d8bf5311a699..ce01146055b2c 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():
-from lldbsuite.test import lldbplatformutil
+import lldb
 
-platform = lldbplatformutil.getPlatform()
+platform = lldb.selected_platform.GetTriple().split('-')[2]
 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 configuration.lldb_platform_name:
+if lldb.remote_platform:
 # 
 configuration.skip_categories.append("debugserver")
 if configuration.verbose:
 print(skip_msg%"debugserver");
 else:
 configuration.skip_categories.append("debugserver")
-if configuration.lldb_platform_name and lldbplatformutil.getPlatform() 
== "windows":
+if lldb.remote_platform and lldbplatformutil.getPlatform() == 
"windows":
 configuration.skip_categories.append("llgs")
 if configuration.verbose:
 print(skip_msg%"lldb-server");
@@ -881,16 +881,7 @@ 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
@@ -901,7 +892,6 @@ def run_suite():
   (configuration.lldb_platform_name))
 lldb.remote_platform = lldb.SBPlatform(
 configuration.lldb_platform_name)
-lldb.selected_platform = lldb.remote_platform
 if not lldb.remote_platform.IsValid():
 print(
 "error: unable to create the LLDB platform named '%s'." %
@@ -948,6 +938,14 @@ 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 6b93dcf38bbe6..94b133589dcc5 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
@@ -57,7 +57,12 @@ def _run_adb_command(cmd, device_id):
 
 
 def target_is_android():
-return configuration.lldb_platform_name == "remote-android"
+if not hasattr(target_is_android, 'result'):
+triple = lldb.selected_platform.GetTriple()
+match = re.match(".*-.*-.*-android", triple)
+target_is_android.result = match is not None
+return target_is_android.result
+
 
 def android_device_api():
 if not hasattr(android_device_api, 'result'):
@@ -134,16 +139,18 @@ def getPlatform():
 platform = 'ios'
 return platform
 
-platform = configuration.lldb_platform_name
-if platform is None:
-platform = "host"
-if platform == "qemu-user":
-platform = "host"
-if platform == "host":
-return getHostPlatform()
-if platform.startswith("remote-"):
-return platform[7:]
-return platform
+# Use the triple to determine the platform if set.
+triple = lldb.selected_platform.GetTriple()
+if triple:
+platform = tri

[Lldb-commits] [PATCH] D121605: [lldb/test] Make category-skipping logic "platform"-independent

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Yeah, I'm pretty sure its due to this patch. Though the failures don't look 
near as bad as I feared. :)

I've reverted the patch while I work on a fix. Do let me know if you see some 
other failures related to this, particularly in some more exotic 
(remote/on-device/on-simulator) modes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121605

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

[Lldb-commits] [PATCH] D121711: [trace][intelpt] Rename IntelPTManager and change TraceCursor::GetTimestampCounter API to general trace counter API

[Jakob Johnson via Phabricator via lldb-commits]

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

Minor refactor and renaming:

- Rename `IntelPTManager` class and files to `IntelPTCollector`
- Change `TraceCursor::GetTimestampCounter` API to general trace counter API, 
`GetCounter`

First of a series of smaller diffs that split https://reviews.llvm.org/D120595 
up.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121711

Files:
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Plugins/Process/Linux/CMakeLists.txt
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/IntelPTCollector.h
  lldb/source/Plugins/Process/Linux/IntelPTManager.cpp
  lldb/source/Plugins/Process/Linux/IntelPTManager.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Target/TraceInstructionDumper.cpp
  lldb/unittests/Process/Linux/CMakeLists.txt
  lldb/unittests/Process/Linux/IntelPTCollectorTests.cpp
  lldb/unittests/Process/Linux/IntelPTManagerTests.cpp

Index: lldb/unittests/Process/Linux/IntelPTCollectorTests.cpp
===
--- lldb/unittests/Process/Linux/IntelPTCollectorTests.cpp
+++ lldb/unittests/Process/Linux/IntelPTCollectorTests.cpp
@@ -1,4 +1,4 @@
-//===-- IntelPTManagerTests.cpp ---===//
+//===-- IntelPTCollectorTests.cpp ---===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -8,7 +8,7 @@
 
 #include "gtest/gtest.h"
 
-#include "IntelPTManager.h"
+#include "IntelPTCollector.h"
 #include "llvm/ADT/ArrayRef.h"
 
 
Index: lldb/unittests/Process/Linux/CMakeLists.txt
===
--- lldb/unittests/Process/Linux/CMakeLists.txt
+++ lldb/unittests/Process/Linux/CMakeLists.txt
@@ -1,5 +1,5 @@
 add_lldb_unittest(TraceIntelPTTests
-  IntelPTManagerTests.cpp
+  IntelPTCollectorTests.cpp
 
   LINK_LIBS
 lldbPluginProcessLinux
Index: lldb/source/Target/TraceInstructionDumper.cpp
===
--- lldb/source/Target/TraceInstructionDumper.cpp
+++ lldb/source/Target/TraceInstructionDumper.cpp
@@ -183,7 +183,7 @@
 if (m_show_tsc) {
   s.Printf("[tsc=");
 
-  if (Optional timestamp = m_cursor_up->GetTimestampCounter())
+  if (Optional timestamp = m_cursor_up->GetCounter(lldb::eTraceCounterTSC))
 s.Printf("0x%016" PRIx64, *timestamp);
   else
 s.Printf("unavailable");
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -28,7 +28,7 @@
 
   lldb::addr_t GetLoadAddress() override;
 
-  llvm::Optional GetTimestampCounter() override;
+  llvm::Optional GetCounter(lldb::TraceCounter counter_type) override;
 
   lldb::TraceInstructionControlFlowType
   GetInstructionControlFlowType() override;
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -85,8 +85,11 @@
   return m_decoded_thread_sp->GetInstructions()[m_pos].GetLoadAddress();
 }
 
-Optional TraceCursorIntelPT::GetTimestampCounter() {
-  return m_decoded_thread_sp->GetInstructions()[m_pos].GetTimestampCounter();
+Optional TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+  switch (counter_type) {
+case lldb::eTraceCounterTSC:
+  return m_decoded_thread_sp->GetInstructions()[m_pos].GetTimestampCounter();
+  }
 }
 
 TraceInstructionControlFlowType
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -20,7 +20,7 @@
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/lldb-types.h"
 
-#include "IntelPTManager.h"
+#include "IntelPTCollector.h"
 #include "NativeThreadLinux.h"
 #include "Plugins/Process/POSIX/NativeProcessELF.h"
 #include "Plugins/Process/Utility/NativeProcessSoftwareSingleStep.h"
@@ -241,7 +241,7 @@
   Status PopulateMemoryRegionCache();
 
   /// Manages Intel PT process and thread traces.
-  IntelPTManager m_intel_pt_manager;
+ 

[Lldb-commits] [lldb] c22c7a6 - [lldb] Fix platform selection on Apple Silicon (again)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-03-15T09:06:29-07:00
New Revision: c22c7a61b6d9c90d5d4292205c63cd576f4fd05b

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

LOG: [lldb] Fix platform selection on Apple Silicon (again)

This patch is another attempt to fix platform selection on Apple
Silicon. It partially undoes D117340 which tried to fix the issue by
always instantiating a remote-ios platform for "iPhone and iPad Apps on
Apple Silicon Macs".

While the previous patch worked for attaching, it broke launching and
everything else that expects the remote platform to be connected. I made
an attempt to work around that, but quickly found out that there were
just too may places that had this assumption baked in.

This patch takes a different approach and reverts back to marking the
host platform compatible with iOS triples. This brings us back to the
original situation where platform selection was broken for remote iOS
debugging on Apple Silicon. To fix that, we now look at the process'
host architecture to differentiate between iOS binaries running remotely
and iOS binaries running locally.

I tested the following scenarios, which now all uses the desired
platform:

  - Launching an iOS binary on macOS: uses the host platform
  - Attaching to an iOS binary on macOS: uses the host platform
  - Attaching to a remote iOS binary: uses the remote-ios platform

rdar://89840215

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

Added: 
lldb/lldb/test/API/functionalities/gdb_remote_client/TestPlatformMacOSX.py
lldb/lldb/unittests/Platform/PlatformMacOSXTest.cpp
lldb/test/API/functionalities/gdb_remote_client/TestPlatformMacOSX.py
lldb/unittests/Platform/PlatformMacOSXTest.cpp

Modified: 
lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
lldb/unittests/Platform/CMakeLists.txt

Removed: 




diff  --git 
a/lldb/lldb/test/API/functionalities/gdb_remote_client/TestPlatformMacOSX.py 
b/lldb/lldb/test/API/functionalities/gdb_remote_client/TestPlatformMacOSX.py
new file mode 100644
index 0..3d3de0a9707c0
--- /dev/null
+++ b/lldb/lldb/test/API/functionalities/gdb_remote_client/TestPlatformMacOSX.py
@@ -0,0 +1,60 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+
+class TestPlatformMacOSX(GDBRemoteTestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+class MyResponder(MockGDBServerResponder):
+
+def __init__(self, host):
+self.host_ostype = host
+MockGDBServerResponder.__init__(self)
+
+def respond(self, packet):
+if packet == "qProcessInfo":
+return self.qProcessInfo()
+return MockGDBServerResponder.respond(self, packet)
+
+def qHostInfo(self):
+return 
"cputype:16777223;cpusubtype:2;ostype:%s;vendor:apple;os_version:10.15.4;maccatalyst_version:13.4;endian:little;ptrsize:8;"
 % self.host_ostype
+
+def qProcessInfo(self):
+return 
"pid:a860;parent-pid:d2a0;real-uid:1f5;real-gid:14;effective-uid:1f5;effective-gid:14;cputype:10c;cpusubtype:2;ptrsize:8;ostype:ios;vendor:apple;endian:little;"
+
+def vCont(self):
+return "vCont;"
+
+def platform_test(self, host, expected_triple, expected_platform):
+self.server.responder = self.MyResponder(host)
+if self.TraceOn():
+self.runCmd("log enable gdb-remote packets")
+self.addTearDownHook(
+lambda: self.runCmd("log disable gdb-remote packets"))
+
+target = self.dbg.CreateTargetWithFileAndArch(None, None)
+process = self.connect(target)
+
+triple = target.GetTriple()
+self.assertEqual(triple, expected_triple)
+
+platform = target.GetPlatform()
+self.assertEqual(platform.GetName(), expected_platform)
+
+@skipIfRemote
+def test_ios(self):
+self.platform_test(host="ios",
+   expected_triple="arm64e-apple-ios-",
+   expected_platform="remote-ios")
+
+@skipIfRemote
+@skipUnlessDarwin
+@skipUnlessArch("arm64")
+def test_macos(self):
+self.platform_test(host="macosx",
+   expected_triple="arm64e-apple-ios-",
+   expected_platform="host")

diff  --git a/lldb/lldb/unittests/Platform/PlatformMacOSXTest.cpp 
b/lldb/lldb/unittests/Platform/PlatformMacOSXTest.cpp
new file mode 100644
index 0..e35489a47d87e
--- /dev/null
+++ b/lldb/lldb/unittests/Platform/PlatformMacOSXTest.cpp
@@ -0,0 +1,52 @@
+//===-- PlatformMacOSXTest.cpp ===//
+//
+// Pa

[Lldb-commits] [PATCH] D121444: [lldb] Fix platform selection on Apple Silicon (again)

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc22c7a61b6d9: [lldb] Fix platform selection on Apple Silicon 
(again) (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D121444?vs=415211&id=415463#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121444

Files:
  lldb/lldb/test/API/functionalities/gdb_remote_client/TestPlatformMacOSX.py
  lldb/lldb/unittests/Platform/PlatformMacOSXTest.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestPlatformMacOSX.py
  lldb/unittests/Platform/CMakeLists.txt
  lldb/unittests/Platform/PlatformMacOSXTest.cpp

Index: lldb/unittests/Platform/PlatformMacOSXTest.cpp
===
--- /dev/null
+++ lldb/unittests/Platform/PlatformMacOSXTest.cpp
@@ -0,0 +1,52 @@
+//===-- PlatformMacOSXTest.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/MacOSX/PlatformMacOSX.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/Platform.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class PlatformMacOSXTest : public ::testing::Test {
+  SubsystemRAII subsystems;
+};
+
+static bool containsArch(const std::vector &archs,
+ const ArchSpec &arch) {
+  return std::find_if(archs.begin(), archs.end(), [&](const ArchSpec &other) {
+   return arch.IsExactMatch(other);
+ }) != archs.end();
+}
+
+TEST_F(PlatformMacOSXTest, TestGetSupportedArchitectures) {
+  PlatformMacOSX platform;
+
+  const ArchSpec x86_macosx_arch("x86_64-apple-macosx");
+
+  EXPECT_TRUE(containsArch(platform.GetSupportedArchitectures(x86_macosx_arch),
+   x86_macosx_arch));
+  EXPECT_TRUE(
+  containsArch(platform.GetSupportedArchitectures({}), x86_macosx_arch));
+
+#if defined(__arm__) || defined(__arm64__) || defined(__aarch64__)
+  const ArchSpec arm64_macosx_arch("arm64-apple-macosx");
+  const ArchSpec arm64_ios_arch("arm64-apple-ios");
+
+  EXPECT_TRUE(containsArch(
+  platform.GetSupportedArchitectures(arm64_macosx_arch), arm64_ios_arch));
+  EXPECT_TRUE(
+  containsArch(platform.GetSupportedArchitectures({}), arm64_ios_arch));
+  EXPECT_FALSE(containsArch(platform.GetSupportedArchitectures(arm64_ios_arch),
+arm64_ios_arch));
+#endif
+}
Index: lldb/unittests/Platform/CMakeLists.txt
===
--- lldb/unittests/Platform/CMakeLists.txt
+++ lldb/unittests/Platform/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_lldb_unittest(LLDBPlatformTests
   PlatformAppleSimulatorTest.cpp
   PlatformDarwinTest.cpp
+  PlatformMacOSXTest.cpp
   PlatformSiginfoTest.cpp
 
   LINK_LIBS
Index: lldb/test/API/functionalities/gdb_remote_client/TestPlatformMacOSX.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestPlatformMacOSX.py
@@ -0,0 +1,60 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+
+class TestPlatformMacOSX(GDBRemoteTestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+class MyResponder(MockGDBServerResponder):
+
+def __init__(self, host):
+self.host_ostype = host
+MockGDBServerResponder.__init__(self)
+
+def respond(self, packet):
+if packet == "qProcessInfo":
+return self.qProcessInfo()
+return MockGDBServerResponder.respond(self, packet)
+
+def qHostInfo(self):
+return "cputype:16777223;cpusubtype:2;ostype:%s;vendor:apple;os_version:10.15.4;maccatalyst_version:13.4;endian:little;ptrsize:8;" % self.host_ostype
+
+def qProcessInfo(self):
+return "pid:a860;parent-pid:d2a0;real-uid:1f5;real-gid:14;effective-uid:1f5;effective-gid:14;cputype:10c;cpusubtype:2;ptrsize:8;ostype:ios;vendor:apple;endian:little;"
+
+def vCont(self):
+return "vCont;"
+
+def platform_test(self, host, expected_triple, expected_platform):
+self.server.responder = self.MyResponder(host)
+if self.TraceOn():
+self.runCmd("log enable gdb-remote packets")
+self.addTearDownHook(
+   

[Lldb-commits] [PATCH] D121408: Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 415473.
shafik added a comment.

Fixing up diff


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

https://reviews.llvm.org/D121408

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s
@@ -0,0 +1,316 @@
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
+# RUN: %lldb %t -o "target variable ug" -b | FileCheck %s
+
+# CHECK: (lldb) target variable ug
+# CHECK: (U) ug = {
+# CHECK:   raw = 0
+# CHECK:= (a = 0, b = 0, c = 0, d = 0, e = 0, f = 0)
+# CHECK: }
+
+# We are testing how DWARFExpression::Evaluate(...) in the case of
+# DW_OP_deref_size deals with static variable.
+#
+# The address will be a ValueType::FileAddress type which we will not be able
+# to turn into a load address but it is a section offset. We should be able to
+# use Target::ReadMemory(...) to read the data in this case.
+# Compiling at -O1 allows us to capture this case and test it.
+#
+# typedef union {
+#   unsigned raw;
+#   struct {
+#  unsigned a : 8;
+#  unsigned b : 8;
+#  unsigned c : 6;
+#  unsigned d : 2;
+#  unsigned e : 6;
+#  unsigned f : 2;
+#   };
+# } U;
+#
+# // This appears first in the debug info and pulls the type definition in...
+# static U __attribute__((used)) _type_anchor;
+# // ... then our useful variable appears last in the debug info and we can
+# // tweak the assembly without needing to edit a lot of offsets by hand.
+# static U ug;
+#
+# extern void f(U);
+#
+# // Omit debug info for main.
+# __attribute__((nodebug))
+# int main() {
+#   ug.raw = 0x64A40101;
+#   f(ug);
+#   f((U)ug.raw);
+# }
+#
+# Compiled as follows:
+#
+#   clang -O1 -gdwarf-4 dw_op_deref_size_test.c -S -o dw_op_deref_size_test.s
+#
+# Hand modified .s file to remoce various section that are not needed for this
+# test
+
+.zerofill __DATA,__bss,_ug.0,1,2## @ug.0
+.zerofill __DATA,__bss,__type_anchor,4,2 ## @_type_anchor
+	.no_dead_strip	__type_anchor
+	.section	__DWARF,__debug_abbrev,regular,debug
+Lsection_abbrev:
+	.byte	1   ## Abbreviation Code
+	.byte	17  ## DW_TAG_compile_unit
+	.byte	1   ## DW_CHILDREN_yes
+	.byte	37  ## DW_AT_producer
+	.byte	14  ## DW_FORM_strp
+	.byte	19  ## DW_AT_language
+	.byte	5   ## DW_FORM_data2
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\202|" ## DW_AT_LLVM_sysroot
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\357\177"  ## DW_AT_APPLE_sdk
+	.byte	14  ## DW_FORM_strp
+	.byte	16  ## DW_AT_stmt_list
+	.byte	23  ## DW_FORM_sec_offset
+	.byte	27  ## DW_AT_comp_dir
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\341\177"  ## DW_AT_APPLE_optimized
+	.byte	25  ## DW_FORM_flag_present
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	2   ## Abbreviation Code
+	.byte	52  ## DW_TAG_variable
+	.byte	0   ## DW_CHILDREN_no
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	73  ## DW_AT_type
+	.byte	19  ## DW_FORM_ref4
+	.byte	58  ## DW_AT_decl_file
+	.byte	11  ## DW_FORM_data1
+	.byte	59  ## DW_AT_decl_line
+	.byte	11  ## DW_FORM_data1
+	.byte	2   ## DW_AT_location
+	.byte	24  ## DW_FORM_exprloc
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	3   ## Abbreviation Code
+	.byte	22  ## DW_TAG_typedef
+	.byte	0   ## DW_CHILDREN_no
+	.byte	73  ## DW_AT_type
+	.byte	19  ## DW_FORM_ref4
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	58  ## DW_AT_decl_file
+	.byte	11  ## DW_FORM_data1
+	.byte	59  ## DW_AT_decl_line
+	.byte	11   

[Lldb-commits] [lldb] 242c574 - [lldb] Synchronize output through the IOHandler

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-03-15T09:32:56-07:00
New Revision: 242c574dc03e4b90e992cc8d07436efc3954727f

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

LOG: [lldb] Synchronize output through the IOHandler

Add synchronization to the IOHandler to prevent multiple threads from
writing concurrently to the output or error stream.

A scenario where this could happen is when a thread (the default event
thread for example) is using the debugger's asynchronous stream. We
would delegate this operation to the IOHandler which might be running on
another thread. Until this patch there was nothing to synchronize the
two at the IOHandler level.

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

Added: 


Modified: 
lldb/include/lldb/Core/IOHandler.h
lldb/include/lldb/Host/Editline.h
lldb/include/lldb/Interpreter/CommandInterpreter.h
lldb/source/Core/IOHandler.cpp
lldb/source/Host/common/Editline.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/unittests/Editline/EditlineTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/IOHandler.h 
b/lldb/include/lldb/Core/IOHandler.h
index d58ad68a099c6..0f558f164fd51 100644
--- a/lldb/include/lldb/Core/IOHandler.h
+++ b/lldb/include/lldb/Core/IOHandler.h
@@ -165,11 +165,14 @@ class IOHandler {
 
   virtual void PrintAsync(const char *s, size_t len, bool is_stdout);
 
+  std::mutex &GetOutputMutex() { return m_output_mutex; }
+
 protected:
   Debugger &m_debugger;
   lldb::FileSP m_input_sp;
   lldb::StreamFileSP m_output_sp;
   lldb::StreamFileSP m_error_sp;
+  std::mutex m_output_mutex;
   repro::DataRecorder *m_data_recorder;
   Predicate m_popped;
   Flags m_flags;

diff  --git a/lldb/include/lldb/Host/Editline.h 
b/lldb/include/lldb/Host/Editline.h
index 14b6778d20764..987dd03e74a8b 100644
--- a/lldb/include/lldb/Host/Editline.h
+++ b/lldb/include/lldb/Host/Editline.h
@@ -154,7 +154,7 @@ using namespace line_editor;
 class Editline {
 public:
   Editline(const char *editor_name, FILE *input_file, FILE *output_file,
-   FILE *error_file, bool color_prompts);
+   FILE *error_file, std::mutex &output_mutex, bool color_prompts);
 
   ~Editline();
 
@@ -402,7 +402,7 @@ class Editline {
   std::string m_suggestion_ansi_suffix;
 
   std::size_t m_previous_autosuggestion_size = 0;
-  std::mutex m_output_mutex;
+  std::mutex &m_output_mutex;
 };
 }
 

diff  --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h 
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 938d36ba0f3fc..787dfdbcb21f5 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -655,7 +655,8 @@ class CommandInterpreter : public Broadcaster,
   const CommandObject::CommandMap &command_map);
 
   // An interruptible wrapper around the stream output
-  void PrintCommandOutput(Stream &stream, llvm::StringRef str);
+  void PrintCommandOutput(IOHandler &io_handler, llvm::StringRef str,
+  bool is_stdout);
 
   bool EchoCommandNonInteractive(llvm::StringRef line,
  const Flags &io_handler_flags) const;

diff  --git a/lldb/source/Core/IOHandler.cpp b/lldb/source/Core/IOHandler.cpp
index 8ed2cfb22873a..0761e1fdbe4de 100644
--- a/lldb/source/Core/IOHandler.cpp
+++ b/lldb/source/Core/IOHandler.cpp
@@ -123,6 +123,7 @@ void IOHandler::SetPopped(bool b) { m_popped.SetValue(b, 
eBroadcastOnChange); }
 void IOHandler::WaitForPop() { m_popped.WaitForValueEqualTo(true); }
 
 void IOHandler::PrintAsync(const char *s, size_t len, bool is_stdout) {
+  std::lock_guard guard(m_output_mutex);
   lldb::StreamFileSP stream = is_stdout ? m_output_sp : m_error_sp;
   stream->Write(s, len);
   stream->Flush();
@@ -266,9 +267,9 @@ IOHandlerEditline::IOHandlerEditline(
  m_input_sp && m_input_sp->GetIsRealTerminal();
 
   if (use_editline) {
-m_editline_up = std::make_unique(editline_name, GetInputFILE(),
-   GetOutputFILE(), GetErrorFILE(),
-   m_color_prompts);
+m_editline_up = std::make_unique(
+editline_name, GetInputFILE(), GetOutputFILE(), GetErrorFILE(),
+GetOutputMutex(), m_color_prompts);
 m_editline_up->SetIsInputCompleteCallback(
 [this](Editline *editline, StringList &lines) {
   return this->IsInputCompleteCallback(editline, lines);
@@ -619,6 +620,7 @@ void IOHandlerEditline::GotEOF() {
 void IOHandlerEditline::PrintAsync(const char *s, size_t len, bool is_stdout) {
 #if LLDB_ENABLE_LIBEDIT
   if (m_editline_up) {
+std::lock_guard guard(m_output_mutex);
 lldb::StreamFileSP stream = is_stdout ? m_output_sp : m_error_sp;
   

[Lldb-commits] [lldb] b783e5c - [lldb] Make the PlatformMacOSX unit test Apple specific

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-03-15T09:32:56-07:00
New Revision: b783e5c203c1959e04fc351a2e96f0700decc32c

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

LOG: [lldb] Make the PlatformMacOSX unit test Apple specific

I thought that x86GetSupportedArchitectures would always return
x86_64-apple-macosx as a compatible architecture, regardless of the host
achitecture, but the Debian bot disagrees with that.

Added: 


Modified: 
lldb/unittests/Platform/PlatformMacOSXTest.cpp

Removed: 




diff  --git a/lldb/unittests/Platform/PlatformMacOSXTest.cpp 
b/lldb/unittests/Platform/PlatformMacOSXTest.cpp
index e35489a47d87e..123fdbc39d672 100644
--- a/lldb/unittests/Platform/PlatformMacOSXTest.cpp
+++ b/lldb/unittests/Platform/PlatformMacOSXTest.cpp
@@ -21,6 +21,7 @@ class PlatformMacOSXTest : public ::testing::Test {
   SubsystemRAII subsystems;
 };
 
+#ifdef __APPLE__
 static bool containsArch(const std::vector &archs,
  const ArchSpec &arch) {
   return std::find_if(archs.begin(), archs.end(), [&](const ArchSpec &other) {
@@ -50,3 +51,4 @@ TEST_F(PlatformMacOSXTest, TestGetSupportedArchitectures) {
 arm64_ios_arch));
 #endif
 }
+#endif



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

[Lldb-commits] [PATCH] D121500: [lldb] Protect the debugger's output and error stream

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG242c574dc03e: [lldb] Synchronize output through the 
IOHandler (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D121500?vs=414775&id=415474#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121500

Files:
  lldb/include/lldb/Core/IOHandler.h
  lldb/include/lldb/Host/Editline.h
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Core/IOHandler.cpp
  lldb/source/Host/common/Editline.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/unittests/Editline/EditlineTest.cpp

Index: lldb/unittests/Editline/EditlineTest.cpp
===
--- lldb/unittests/Editline/EditlineTest.cpp
+++ lldb/unittests/Editline/EditlineTest.cpp
@@ -84,6 +84,7 @@
   bool IsInputComplete(lldb_private::Editline *editline,
lldb_private::StringList &lines);
 
+  std::mutex output_mutex;
   std::unique_ptr _editline_sp;
 
   PseudoTerminal _pty;
@@ -117,7 +118,7 @@
   // Create an Editline instance.
   _editline_sp.reset(new lldb_private::Editline(
   "gtest editor", *_el_secondary_file, *_el_secondary_file,
-  *_el_secondary_file, false));
+  *_el_secondary_file, output_mutex, false));
   _editline_sp->SetPrompt("> ");
 
   // Hookup our input complete callback.
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2975,8 +2975,12 @@
   return was_interrupted;
 }
 
-void CommandInterpreter::PrintCommandOutput(Stream &stream,
-llvm::StringRef str) {
+void CommandInterpreter::PrintCommandOutput(IOHandler &io_handler,
+llvm::StringRef str,
+bool is_stdout) {
+
+  lldb::StreamFileSP stream = is_stdout ? io_handler.GetOutputStreamFileSP()
+: io_handler.GetErrorStreamFileSP();
   // Split the output into lines and poll for interrupt requests
   const char *data = str.data();
   size_t size = str.size();
@@ -2989,15 +2993,19 @@
 break;
   }
 }
-chunk_size = stream.Write(data, chunk_size);
+{
+  std::lock_guard guard(io_handler.GetOutputMutex());
+  chunk_size = stream->Write(data, chunk_size);
+}
 lldbassert(size >= chunk_size);
 data += chunk_size;
 size -= chunk_size;
   }
-  if (size > 0) {
-stream.Printf("\n... Interrupted.\n");
-  }
-  stream.Flush();
+
+  std::lock_guard guard(io_handler.GetOutputMutex());
+  if (size > 0)
+stream->Printf("\n... Interrupted.\n");
+  stream->Flush();
 }
 
 bool CommandInterpreter::EchoCommandNonInteractive(
@@ -3033,9 +3041,11 @@
 // When using a non-interactive file handle (like when sourcing commands
 // from a file) we need to echo the command out so we don't just see the
 // command output and no command...
-if (EchoCommandNonInteractive(line, io_handler.GetFlags()))
+if (EchoCommandNonInteractive(line, io_handler.GetFlags())) {
+  std::lock_guard guard(io_handler.GetOutputMutex());
   io_handler.GetOutputStreamFileSP()->Printf(
   "%s%s\n", io_handler.GetPrompt(), line.c_str());
+}
   }
 
   StartHandlingCommand();
@@ -3057,13 +3067,13 @@
 
 if (!result.GetImmediateOutputStream()) {
   llvm::StringRef output = result.GetOutputData();
-  PrintCommandOutput(*io_handler.GetOutputStreamFileSP(), output);
+  PrintCommandOutput(io_handler, output, true);
 }
 
 // Now emit the command error text from the command we just executed
 if (!result.GetImmediateErrorStream()) {
   llvm::StringRef error = result.GetErrorData();
-  PrintCommandOutput(*io_handler.GetErrorStreamFileSP(), error);
+  PrintCommandOutput(io_handler, error, false);
 }
   }
 
Index: lldb/source/Host/common/Editline.cpp
===
--- lldb/source/Host/common/Editline.cpp
+++ lldb/source/Host/common/Editline.cpp
@@ -1376,10 +1376,12 @@
 }
 
 Editline::Editline(const char *editline_name, FILE *input_file,
-   FILE *output_file, FILE *error_file, bool color_prompts)
+   FILE *output_file, FILE *error_file,
+   std::mutex &output_mutex, bool color_prompts)
 : m_editor_status(EditorStatus::Complete), m_color_prompts(color_prompts),
   m_input_file(input_file), m_output_file(output_file),
-  m_error_file(error_file), m_input_connection(fileno(input_file), false) {
+  m_error_file(error_file), m_input_connection(fileno(input_file), false),
+  m_output_mutex(output_mutex) {
   // Get a shared history inst

[Lldb-commits] [lldb] 6583f01 - [LLDB] Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

[Shafik Yaghmour via lldb-commits]

Author: Shafik Yaghmour
Date: 2022-03-15T09:36:20-07:00
New Revision: 6583f01707211ed14d8e35f2183a8805a301b6f9

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

LOG: [LLDB] Fixing DWARFExpression handling of ValueType::FileAddress case for 
DW_OP_deref_size

Currently DW_OP_deref_size just drops the ValueType::FileAddress case and does
not attempt to handle it. This adds support for this case and a test that
verifies this support.

I did a little refactoring since DW_OP_deref and DW_OP_deref_size have some
overlap in code.

Also see: rdar://66870821

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

Added: 
lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s

Modified: 
lldb/source/Expression/DWARFExpression.cpp

Removed: 




diff  --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 8fa8248d796d1..0f6c380fe0357 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -943,6 +943,80 @@ void UpdateValueTypeFromLocationDescription(Log *log, 
const DWARFUnit *dwarf_cu,
 }
 } // namespace
 
+/// Helper function to move common code used to resolve a file address and turn
+/// into a load address.
+///
+/// \param exe_ctx Pointer to the execution context
+/// \param module_sp shared_ptr contains the module if we have one
+/// \param error_ptr pointer to Status object if we have one
+/// \param dw_op_type C-style string used to vary the error output
+/// \param file_addr the file address we are trying to resolve and turn into a
+///  load address
+/// \param so_addr out parameter, will be set to load addresss or section 
offset
+/// \param check_sectionoffset bool which determines if having a section offset
+///but not a load address is considerd a success
+/// \returns llvm::Optional containing the load address if resolving and 
getting
+///  the load address succeed or an empty Optinal otherwise. If
+///  check_sectionoffset is true we consider LLDB_INVALID_ADDRESS a
+///  success if so_addr.IsSectionOffset() is true.
+static llvm::Optional
+ResolveAndLoadFileAddress(ExecutionContext *exe_ctx, lldb::ModuleSP module_sp,
+  Status *error_ptr, const char *dw_op_type,
+  lldb::addr_t file_addr, Address &so_addr,
+  bool check_sectionoffset = false) {
+  if (!module_sp) {
+if (error_ptr)
+  error_ptr->SetErrorStringWithFormat(
+  "need module to resolve file address for %s", dw_op_type);
+return {};
+  }
+
+  if (!module_sp->ResolveFileAddress(file_addr, so_addr)) {
+if (error_ptr)
+  error_ptr->SetErrorString("failed to resolve file address in module");
+return {};
+  }
+
+  addr_t load_addr = so_addr.GetLoadAddress(exe_ctx->GetTargetPtr());
+
+  if (load_addr == LLDB_INVALID_ADDRESS &&
+  (check_sectionoffset && !so_addr.IsSectionOffset())) {
+if (error_ptr)
+  error_ptr->SetErrorString("failed to resolve load address");
+return {};
+  }
+
+  return load_addr;
+}
+
+/// Helper function to move common code used to load sized data from a uint8_t
+/// buffer.
+///
+/// \param addr_bytes uint8_t buffer containg raw data
+/// \param size_addr_bytes how large is the underlying raw data
+/// \param byte_order what is the byter order of the underlyig data
+/// \param size How much of the underlying data we want to use
+/// \return The underlying data converted into a Scalar
+static Scalar DerefSizeExtractDataHelper(uint8_t *addr_bytes,
+ size_t size_addr_bytes,
+ ByteOrder byte_order, size_t size) {
+  DataExtractor addr_data(addr_bytes, size_addr_bytes, byte_order, size);
+
+  lldb::offset_t addr_data_offset = 0;
+  switch (size) {
+  case 1:
+return addr_data.GetU8(&addr_data_offset);
+  case 2:
+return addr_data.GetU16(&addr_data_offset);
+  case 4:
+return addr_data.GetU32(&addr_data_offset);
+  case 8:
+return addr_data.GetU64(&addr_data_offset);
+  default:
+return addr_data.GetAddress(&addr_data_offset);
+  }
+}
+
 bool DWARFExpression::Evaluate(
 ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
 lldb::ModuleSP module_sp, const DataExtractor &opcodes,
@@ -1025,6 +1099,7 @@ bool DWARFExpression::Evaluate(
   if (frame)
 stack.back().ConvertToLoadAddress(module_sp.get(),
   frame->CalculateTarget().get());
+
   break;
 
 // The DW_OP_addr_sect_offset4 is used for any location expressions in
@@ -1088,26 +1163,15 @@ bool DWARFExpression::Evaluate(
   case Value::ValueType::FileAddress: {
 auto file_addr 

[Lldb-commits] [PATCH] D121408: Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

[Shafik Yaghmour via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6583f0170721: [LLDB] Fixing DWARFExpression handling of 
ValueType::FileAddress case for… (authored by shafik).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121408

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_deref_size_static_var.s
@@ -0,0 +1,316 @@
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
+# RUN: %lldb %t -o "target variable ug" -b | FileCheck %s
+
+# CHECK: (lldb) target variable ug
+# CHECK: (U) ug = {
+# CHECK:   raw = 0
+# CHECK:= (a = 0, b = 0, c = 0, d = 0, e = 0, f = 0)
+# CHECK: }
+
+# We are testing how DWARFExpression::Evaluate(...) in the case of
+# DW_OP_deref_size deals with static variable.
+#
+# The address will be a ValueType::FileAddress type which we will not be able
+# to turn into a load address but it is a section offset. We should be able to
+# use Target::ReadMemory(...) to read the data in this case.
+# Compiling at -O1 allows us to capture this case and test it.
+#
+# typedef union {
+#   unsigned raw;
+#   struct {
+#  unsigned a : 8;
+#  unsigned b : 8;
+#  unsigned c : 6;
+#  unsigned d : 2;
+#  unsigned e : 6;
+#  unsigned f : 2;
+#   };
+# } U;
+#
+# // This appears first in the debug info and pulls the type definition in...
+# static U __attribute__((used)) _type_anchor;
+# // ... then our useful variable appears last in the debug info and we can
+# // tweak the assembly without needing to edit a lot of offsets by hand.
+# static U ug;
+#
+# extern void f(U);
+#
+# // Omit debug info for main.
+# __attribute__((nodebug))
+# int main() {
+#   ug.raw = 0x64A40101;
+#   f(ug);
+#   f((U)ug.raw);
+# }
+#
+# Compiled as follows:
+#
+#   clang -O1 -gdwarf-4 dw_op_deref_size_test.c -S -o dw_op_deref_size_test.s
+#
+# Hand modified .s file to remoce various section that are not needed for this
+# test
+
+.zerofill __DATA,__bss,_ug.0,1,2## @ug.0
+.zerofill __DATA,__bss,__type_anchor,4,2 ## @_type_anchor
+	.no_dead_strip	__type_anchor
+	.section	__DWARF,__debug_abbrev,regular,debug
+Lsection_abbrev:
+	.byte	1   ## Abbreviation Code
+	.byte	17  ## DW_TAG_compile_unit
+	.byte	1   ## DW_CHILDREN_yes
+	.byte	37  ## DW_AT_producer
+	.byte	14  ## DW_FORM_strp
+	.byte	19  ## DW_AT_language
+	.byte	5   ## DW_FORM_data2
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\202|" ## DW_AT_LLVM_sysroot
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\357\177"  ## DW_AT_APPLE_sdk
+	.byte	14  ## DW_FORM_strp
+	.byte	16  ## DW_AT_stmt_list
+	.byte	23  ## DW_FORM_sec_offset
+	.byte	27  ## DW_AT_comp_dir
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\341\177"  ## DW_AT_APPLE_optimized
+	.byte	25  ## DW_FORM_flag_present
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	2   ## Abbreviation Code
+	.byte	52  ## DW_TAG_variable
+	.byte	0   ## DW_CHILDREN_no
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	73  ## DW_AT_type
+	.byte	19  ## DW_FORM_ref4
+	.byte	58  ## DW_AT_decl_file
+	.byte	11  ## DW_FORM_data1
+	.byte	59  ## DW_AT_decl_line
+	.byte	11  ## DW_FORM_data1
+	.byte	2   ## DW_AT_location
+	.byte	24  ## DW_FORM_exprloc
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	3   ## Abbreviation Code
+	.byte	22  ## DW_TAG_typedef
+	.byte	0   ## DW_CHILDREN_no
+	.byte	73  ## DW_AT_type
+	.byte	19  ## DW_FORM_ref4
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	58  

[Lldb-commits] [lldb] 9a5f04e - Revert "[lldb] Synchronize output through the IOHandler"

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-03-15T09:55:30-07:00
New Revision: 9a5f04e01dadf7db89ef2d6ef24b1cc51af07337

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

LOG: Revert "[lldb] Synchronize output through the IOHandler"

This reverts commit 242c574dc03e4b90e992cc8d07436efc3954727f because it
breaks the following tests on the bots:

 - TestGuiExpandThreadsTree.py
 - TestBreakpointCallbackCommandSource.py

Added: 


Modified: 
lldb/include/lldb/Core/IOHandler.h
lldb/include/lldb/Host/Editline.h
lldb/include/lldb/Interpreter/CommandInterpreter.h
lldb/source/Core/IOHandler.cpp
lldb/source/Host/common/Editline.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/unittests/Editline/EditlineTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/IOHandler.h 
b/lldb/include/lldb/Core/IOHandler.h
index 0f558f164fd51..d58ad68a099c6 100644
--- a/lldb/include/lldb/Core/IOHandler.h
+++ b/lldb/include/lldb/Core/IOHandler.h
@@ -165,14 +165,11 @@ class IOHandler {
 
   virtual void PrintAsync(const char *s, size_t len, bool is_stdout);
 
-  std::mutex &GetOutputMutex() { return m_output_mutex; }
-
 protected:
   Debugger &m_debugger;
   lldb::FileSP m_input_sp;
   lldb::StreamFileSP m_output_sp;
   lldb::StreamFileSP m_error_sp;
-  std::mutex m_output_mutex;
   repro::DataRecorder *m_data_recorder;
   Predicate m_popped;
   Flags m_flags;

diff  --git a/lldb/include/lldb/Host/Editline.h 
b/lldb/include/lldb/Host/Editline.h
index 987dd03e74a8b..14b6778d20764 100644
--- a/lldb/include/lldb/Host/Editline.h
+++ b/lldb/include/lldb/Host/Editline.h
@@ -154,7 +154,7 @@ using namespace line_editor;
 class Editline {
 public:
   Editline(const char *editor_name, FILE *input_file, FILE *output_file,
-   FILE *error_file, std::mutex &output_mutex, bool color_prompts);
+   FILE *error_file, bool color_prompts);
 
   ~Editline();
 
@@ -402,7 +402,7 @@ class Editline {
   std::string m_suggestion_ansi_suffix;
 
   std::size_t m_previous_autosuggestion_size = 0;
-  std::mutex &m_output_mutex;
+  std::mutex m_output_mutex;
 };
 }
 

diff  --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h 
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 787dfdbcb21f5..938d36ba0f3fc 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -655,8 +655,7 @@ class CommandInterpreter : public Broadcaster,
   const CommandObject::CommandMap &command_map);
 
   // An interruptible wrapper around the stream output
-  void PrintCommandOutput(IOHandler &io_handler, llvm::StringRef str,
-  bool is_stdout);
+  void PrintCommandOutput(Stream &stream, llvm::StringRef str);
 
   bool EchoCommandNonInteractive(llvm::StringRef line,
  const Flags &io_handler_flags) const;

diff  --git a/lldb/source/Core/IOHandler.cpp b/lldb/source/Core/IOHandler.cpp
index 0761e1fdbe4de..8ed2cfb22873a 100644
--- a/lldb/source/Core/IOHandler.cpp
+++ b/lldb/source/Core/IOHandler.cpp
@@ -123,7 +123,6 @@ void IOHandler::SetPopped(bool b) { m_popped.SetValue(b, 
eBroadcastOnChange); }
 void IOHandler::WaitForPop() { m_popped.WaitForValueEqualTo(true); }
 
 void IOHandler::PrintAsync(const char *s, size_t len, bool is_stdout) {
-  std::lock_guard guard(m_output_mutex);
   lldb::StreamFileSP stream = is_stdout ? m_output_sp : m_error_sp;
   stream->Write(s, len);
   stream->Flush();
@@ -267,9 +266,9 @@ IOHandlerEditline::IOHandlerEditline(
  m_input_sp && m_input_sp->GetIsRealTerminal();
 
   if (use_editline) {
-m_editline_up = std::make_unique(
-editline_name, GetInputFILE(), GetOutputFILE(), GetErrorFILE(),
-GetOutputMutex(), m_color_prompts);
+m_editline_up = std::make_unique(editline_name, GetInputFILE(),
+   GetOutputFILE(), GetErrorFILE(),
+   m_color_prompts);
 m_editline_up->SetIsInputCompleteCallback(
 [this](Editline *editline, StringList &lines) {
   return this->IsInputCompleteCallback(editline, lines);
@@ -620,7 +619,6 @@ void IOHandlerEditline::GotEOF() {
 void IOHandlerEditline::PrintAsync(const char *s, size_t len, bool is_stdout) {
 #if LLDB_ENABLE_LIBEDIT
   if (m_editline_up) {
-std::lock_guard guard(m_output_mutex);
 lldb::StreamFileSP stream = is_stdout ? m_output_sp : m_error_sp;
 m_editline_up->PrintAsync(stream.get(), s, len);
   } else

diff  --git a/lldb/source/Host/common/Editline.cpp 
b/lldb/source/Host/common/Editline.cpp
index d632c9cd69bbc..826db61f19a6b 100644
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -1376,12 +1

[Lldb-commits] [PATCH] D121030: [LLDB][NativePDB] Don't complete static members' types when completing a record type.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu updated this revision to Diff 415501.
zequanwu marked an inline comment as done.
zequanwu added a comment.

Remove `REQUIRES: system-windows`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121030

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/test/Shell/SymbolFile/NativePDB/Inputs/lookup-by-types.lldbinit
  lldb/test/Shell/SymbolFile/NativePDB/lookup-by-types.cpp


Index: lldb/test/Shell/SymbolFile/NativePDB/lookup-by-types.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/lookup-by-types.cpp
@@ -0,0 +1,46 @@
+// clang-format off
+
+// RUN: %build -o %t.exe -- %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
+// RUN: %p/Inputs/lookup-by-types.lldbinit 2>&1 | FileCheck %s
+
+class B;
+class A {
+public:
+static const A constA;
+static A a;
+static B b;
+int val = 1;
+};
+class B {
+public:
+static A a;
+int val = 2;
+};
+A varA;
+B varB;
+const A A::constA = varA;
+A A::a = varA;
+B A::b = varB;
+A B::a = varA;
+
+int main(int argc, char **argv) {
+  return varA.val + varB.val;
+}
+
+// CHECK:  image lookup -type A
+// CHECK-NEXT: 1 match found in {{.*}}.exe
+// CHECK-NEXT: compiler_type = "class A {
+// CHECK-NEXT: static const A constA;
+// CHECK-NEXT: static A a;
+// CHECK-NEXT: static B b;
+// CHECK-NEXT: public:
+// CHECK-NEXT: int val;
+// CHECK-NEXT: }"
+// CHECK:  image lookup -type B
+// CHECK-NEXT: 1 match found in {{.*}}.exe
+// CHECK-NEXT:  compiler_type = "class B {
+// CHECK-NEXT: static A a;
+// CHECK-NEXT: public:
+// CHECK-NEXT: int val;
+// CHECK-NEXT: }"
Index: lldb/test/Shell/SymbolFile/NativePDB/Inputs/lookup-by-types.lldbinit
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/Inputs/lookup-by-types.lldbinit
@@ -0,0 +1,4 @@
+image lookup -type A
+image lookup -type B
+
+quit
\ No newline at end of file
Index: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -138,8 +138,6 @@
   clang::QualType member_type =
   m_ast_builder.GetOrCreateType(PdbTypeSymId(static_data_member.Type));
 
-  m_ast_builder.CompleteType(member_type);
-
   CompilerType member_ct = m_ast_builder.ToCompilerType(member_type);
 
   lldb::AccessType access =
@@ -149,7 +147,7 @@
 
   // Static constant members may be a const[expr] declaration.
   // Query the symbol's value as the variable initializer if valid.
-  if (member_ct.IsConst()) {
+  if (member_ct.IsConst() && member_ct.IsCompleteType()) {
 std::string qual_name = decl->getQualifiedNameAsString();
 
 auto results =
Index: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -1231,8 +1231,10 @@
 clang::QualType PdbAstBuilder::CreateArrayType(const ArrayRecord &ar) {
   clang::QualType element_type = GetOrCreateType(ar.ElementType);
 
-  uint64_t element_count =
-  ar.Size / GetSizeOfType({ar.ElementType}, m_index.tpi());
+  uint64_t element_size = GetSizeOfType({ar.ElementType}, m_index.tpi());
+  if (element_size == 0)
+return {};
+  uint64_t element_count = ar.Size / element_size;
 
   CompilerType array_ct = m_clang.CreateArrayType(ToCompilerType(element_type),
   element_count, false);


Index: lldb/test/Shell/SymbolFile/NativePDB/lookup-by-types.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/lookup-by-types.cpp
@@ -0,0 +1,46 @@
+// clang-format off
+
+// RUN: %build -o %t.exe -- %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
+// RUN: %p/Inputs/lookup-by-types.lldbinit 2>&1 | FileCheck %s
+
+class B;
+class A {
+public:
+static const A constA;
+static A a;
+static B b;
+int val = 1;
+};
+class B {
+public:
+static A a;
+int val = 2;
+};
+A varA;
+B varB;
+const A A::constA = varA;
+A A::a = varA;
+B A::b = varB;
+A B::a = varA;
+
+int main(int argc, char **argv) {
+  return varA.val + varB.val;
+}
+
+// CHECK:  image lookup -type A
+// CHECK-NEXT: 1 match found in {{.*}}.exe
+// CHECK-NEXT: compiler_type = "class A {
+// CHECK-NEXT: static const A constA;
+// CHECK-NEXT: static A a;
+// CHECK-NEXT: static B b;
+// CHECK-NEXT: public:
+// CHECK-NEXT: int val;
+// CHECK-NEXT: }"
+// CHECK:  image lookup -type B
+// CHE

[Lldb-commits] [PATCH] D121030: [LLDB][NativePDB] Don't complete static members' types when completing a record type.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu added inline comments.



Comment at: lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp:8-26
+class B;
+class A {
+public:
+static const A constA;
+static A a;
+static B b;
+int val = 1;

labath wrote:
> zequanwu wrote:
> > labath wrote:
> > > Would it be possible to put this into a new test case, one that does not 
> > > require running the binary? If we're able to read global variables 
> > > without a running process (we can do it with elf+dwarf, but I don't know 
> > > what's the state with coff+pdb), then we could run this test on 
> > > non-windows hosts as well (which is great for test coverage).
> > Printing doesn't work with any expression involves `::` in NativePDB. So, I 
> > changed the test to `image lookup -type` which also tries to complete given 
> > type.
> That's cool, but I see the new test still has a `REQUIRES: system-windows`. 
> Would it be possible to drop that requirement (if not, then why)?
This is not necessary. Removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121030

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

[Lldb-commits] [PATCH] D121030: [LLDB][NativePDB] Don't complete static members' types when completing a record type.

[Pavel Labath via Phabricator via lldb-commits]

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

awesome


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121030

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

[Lldb-commits] [PATCH] D121408: Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

[Sterling Augustine via Phabricator via lldb-commits]

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

This change uses a variable-sized array, which is c99 only:

  
third_party/llvm/llvm-project/lldb/source/Expression/DWARFExpression.cpp:1300:30:
 error: variable length arrays are a C99 feature [-Werror,-Wvla-extension]
uint8_t addr_bytes[size];
   ^~~~
  
third_party/llvm/llvm-project/lldb/source/Expression/DWARFExpression.cpp:1300:30:
 note: read of non-const variable 'size' is not allowed in a constant expression
  
third_party/llvm/llvm-project/lldb/source/Expression/DWARFExpression.cpp:1242:15:
 note: declared here
uint8_t size = opcodes.GetU8(&offset);

But I the size of the array is limited to only to the address size of the 
target machine, so converting that to a simple eight-byte array and then using 
size instead of sizeof(addr_bytes) should be fine. I have a change in testing 
to do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121408

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

[Lldb-commits] [lldb] 8e776bb - Re-land "[lldb] Synchronize output through the IOHandler"

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-03-15T12:53:46-07:00
New Revision: 8e776bb660dda6c51ce7ca6cea641db1f47aa9cf

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

LOG: Re-land "[lldb] Synchronize output through the IOHandler"

Add synchronization to the IOHandler to prevent multiple threads from
writing concurrently to the output or error stream.

A scenario where this could happen is when a thread (the default event
thread for example) is using the debugger's asynchronous stream. We
would delegate this operation to the IOHandler which might be running on
another thread. Until this patch there was nothing to synchronize the
two at the IOHandler level.

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

Added: 


Modified: 
lldb/include/lldb/Core/IOHandler.h
lldb/include/lldb/Host/Editline.h
lldb/include/lldb/Interpreter/CommandInterpreter.h
lldb/source/Core/IOHandler.cpp
lldb/source/Host/common/Editline.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/unittests/Editline/EditlineTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/IOHandler.h 
b/lldb/include/lldb/Core/IOHandler.h
index d58ad68a099c6..2ba421f7ba506 100644
--- a/lldb/include/lldb/Core/IOHandler.h
+++ b/lldb/include/lldb/Core/IOHandler.h
@@ -34,7 +34,7 @@ class Debugger;
 namespace repro {
 class DataRecorder;
 }
-}
+} // namespace lldb_private
 
 namespace curses {
 class Application;
@@ -165,11 +165,14 @@ class IOHandler {
 
   virtual void PrintAsync(const char *s, size_t len, bool is_stdout);
 
+  std::recursive_mutex &GetOutputMutex() { return m_output_mutex; }
+
 protected:
   Debugger &m_debugger;
   lldb::FileSP m_input_sp;
   lldb::StreamFileSP m_output_sp;
   lldb::StreamFileSP m_error_sp;
+  std::recursive_mutex m_output_mutex;
   repro::DataRecorder *m_data_recorder;
   Predicate m_popped;
   Flags m_flags;

diff  --git a/lldb/include/lldb/Host/Editline.h 
b/lldb/include/lldb/Host/Editline.h
index 14b6778d20764..ffe2e3500aaa2 100644
--- a/lldb/include/lldb/Host/Editline.h
+++ b/lldb/include/lldb/Host/Editline.h
@@ -154,7 +154,8 @@ using namespace line_editor;
 class Editline {
 public:
   Editline(const char *editor_name, FILE *input_file, FILE *output_file,
-   FILE *error_file, bool color_prompts);
+   FILE *error_file, std::recursive_mutex &output_mutex,
+   bool color_prompts);
 
   ~Editline();
 
@@ -402,7 +403,7 @@ class Editline {
   std::string m_suggestion_ansi_suffix;
 
   std::size_t m_previous_autosuggestion_size = 0;
-  std::mutex m_output_mutex;
+  std::recursive_mutex &m_output_mutex;
 };
 }
 

diff  --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h 
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 938d36ba0f3fc..787dfdbcb21f5 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -655,7 +655,8 @@ class CommandInterpreter : public Broadcaster,
   const CommandObject::CommandMap &command_map);
 
   // An interruptible wrapper around the stream output
-  void PrintCommandOutput(Stream &stream, llvm::StringRef str);
+  void PrintCommandOutput(IOHandler &io_handler, llvm::StringRef str,
+  bool is_stdout);
 
   bool EchoCommandNonInteractive(llvm::StringRef line,
  const Flags &io_handler_flags) const;

diff  --git a/lldb/source/Core/IOHandler.cpp b/lldb/source/Core/IOHandler.cpp
index 8ed2cfb22873a..44b5a7e9c8a40 100644
--- a/lldb/source/Core/IOHandler.cpp
+++ b/lldb/source/Core/IOHandler.cpp
@@ -123,6 +123,7 @@ void IOHandler::SetPopped(bool b) { m_popped.SetValue(b, 
eBroadcastOnChange); }
 void IOHandler::WaitForPop() { m_popped.WaitForValueEqualTo(true); }
 
 void IOHandler::PrintAsync(const char *s, size_t len, bool is_stdout) {
+  std::lock_guard guard(m_output_mutex);
   lldb::StreamFileSP stream = is_stdout ? m_output_sp : m_error_sp;
   stream->Write(s, len);
   stream->Flush();
@@ -266,9 +267,9 @@ IOHandlerEditline::IOHandlerEditline(
  m_input_sp && m_input_sp->GetIsRealTerminal();
 
   if (use_editline) {
-m_editline_up = std::make_unique(editline_name, GetInputFILE(),
-   GetOutputFILE(), GetErrorFILE(),
-   m_color_prompts);
+m_editline_up = std::make_unique(
+editline_name, GetInputFILE(), GetOutputFILE(), GetErrorFILE(),
+GetOutputMutex(), m_color_prompts);
 m_editline_up->SetIsInputCompleteCallback(
 [this](Editline *editline, StringList &lines) {
   return this->IsInputCompleteCallback(editline, lines);
@@ -619,6 +620,7 @@ void IOHandlerEditline::GotEOF() {
 void IOHandlerEditline::PrintAsync

[Lldb-commits] [PATCH] D121732: Avoid using a variable-sized array for a tiny allocation.

[Sterling Augustine via Phabricator via lldb-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121732

Files:
  lldb/source/Expression/DWARFExpression.cpp


Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -1297,7 +1297,7 @@
 addr_t load_addr = *maybe_load_addr;
 
 if (load_addr == LLDB_INVALID_ADDRESS && so_addr.IsSectionOffset()) {
-  uint8_t addr_bytes[size];
+  uint8_t addr_bytes[8];
   Status error;
 
   if (exe_ctx->GetTargetRef().ReadMemory(
@@ -1306,7 +1306,7 @@
 ObjectFile *objfile = module_sp->GetObjectFile();
 
 stack.back().GetScalar() = DerefSizeExtractDataHelper(
-addr_bytes, sizeof(addr_bytes), objfile->GetByteOrder(), size);
+addr_bytes, size, objfile->GetByteOrder(), size);
 stack.back().ClearContext();
 break;
   } else {


Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -1297,7 +1297,7 @@
 addr_t load_addr = *maybe_load_addr;
 
 if (load_addr == LLDB_INVALID_ADDRESS && so_addr.IsSectionOffset()) {
-  uint8_t addr_bytes[size];
+  uint8_t addr_bytes[8];
   Status error;
 
   if (exe_ctx->GetTargetRef().ReadMemory(
@@ -1306,7 +1306,7 @@
 ObjectFile *objfile = module_sp->GetObjectFile();
 
 stack.back().GetScalar() = DerefSizeExtractDataHelper(
-addr_bytes, sizeof(addr_bytes), objfile->GetByteOrder(), size);
+addr_bytes, size, objfile->GetByteOrder(), size);
 stack.back().ClearContext();
 break;
   } else {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 7518e0f - Avoid using a variable-sized array for a tiny allocation.

[Sterling Augustine via lldb-commits]

Author: Sterling Augustine
Date: 2022-03-15T13:00:14-07:00
New Revision: 7518e0ff63cdee4b4082e918e1b5603115777d9b

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

LOG: Avoid using a variable-sized array for a tiny allocation.

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

Added: 


Modified: 
lldb/source/Expression/DWARFExpression.cpp

Removed: 




diff  --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 0f6c380fe0357..5ee62fb4376b8 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -1297,7 +1297,7 @@ bool DWARFExpression::Evaluate(
 addr_t load_addr = *maybe_load_addr;
 
 if (load_addr == LLDB_INVALID_ADDRESS && so_addr.IsSectionOffset()) {
-  uint8_t addr_bytes[size];
+  uint8_t addr_bytes[8];
   Status error;
 
   if (exe_ctx->GetTargetRef().ReadMemory(
@@ -1306,7 +1306,7 @@ bool DWARFExpression::Evaluate(
 ObjectFile *objfile = module_sp->GetObjectFile();
 
 stack.back().GetScalar() = DerefSizeExtractDataHelper(
-addr_bytes, sizeof(addr_bytes), objfile->GetByteOrder(), size);
+addr_bytes, size, objfile->GetByteOrder(), size);
 stack.back().ClearContext();
 break;
   } else {



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

[Lldb-commits] [PATCH] D121732: Avoid using a variable-sized array for a tiny allocation.

[Sterling Augustine via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7518e0ff63cd: Avoid using a variable-sized array for a tiny 
allocation. (authored by saugustine).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121732

Files:
  lldb/source/Expression/DWARFExpression.cpp


Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -1297,7 +1297,7 @@
 addr_t load_addr = *maybe_load_addr;
 
 if (load_addr == LLDB_INVALID_ADDRESS && so_addr.IsSectionOffset()) {
-  uint8_t addr_bytes[size];
+  uint8_t addr_bytes[8];
   Status error;
 
   if (exe_ctx->GetTargetRef().ReadMemory(
@@ -1306,7 +1306,7 @@
 ObjectFile *objfile = module_sp->GetObjectFile();
 
 stack.back().GetScalar() = DerefSizeExtractDataHelper(
-addr_bytes, sizeof(addr_bytes), objfile->GetByteOrder(), size);
+addr_bytes, size, objfile->GetByteOrder(), size);
 stack.back().ClearContext();
 break;
   } else {


Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -1297,7 +1297,7 @@
 addr_t load_addr = *maybe_load_addr;
 
 if (load_addr == LLDB_INVALID_ADDRESS && so_addr.IsSectionOffset()) {
-  uint8_t addr_bytes[size];
+  uint8_t addr_bytes[8];
   Status error;
 
   if (exe_ctx->GetTargetRef().ReadMemory(
@@ -1306,7 +1306,7 @@
 ObjectFile *objfile = module_sp->GetObjectFile();
 
 stack.back().GetScalar() = DerefSizeExtractDataHelper(
-addr_bytes, sizeof(addr_bytes), objfile->GetByteOrder(), size);
+addr_bytes, size, objfile->GetByteOrder(), size);
 stack.back().ClearContext();
 break;
   } else {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D121408: Fixing DWARFExpression handling of ValueType::FileAddress case for DW_OP_deref_size

[Sterling Augustine via Phabricator via lldb-commits]

saugustine added a comment.

Fix for the variable-sized-array issue in rG7518e0ff63cd 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121408

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

[Lldb-commits] [PATCH] D121732: Avoid using a variable-sized array for a tiny allocation.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Expression/DWARFExpression.cpp:1309
 stack.back().GetScalar() = DerefSizeExtractDataHelper(
-addr_bytes, sizeof(addr_bytes), objfile->GetByteOrder(), size);
+addr_bytes, size, objfile->GetByteOrder(), size);
 stack.back().ClearContext();

Why size and not `8`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121732

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

[Lldb-commits] [PATCH] D121732: Avoid using a variable-sized array for a tiny allocation.

[Sterling Augustine via Phabricator via lldb-commits]

saugustine added inline comments.



Comment at: lldb/source/Expression/DWARFExpression.cpp:1309
 stack.back().GetScalar() = DerefSizeExtractDataHelper(
-addr_bytes, sizeof(addr_bytes), objfile->GetByteOrder(), size);
+addr_bytes, size, objfile->GetByteOrder(), size);
 stack.back().ClearContext();

JDevlieghere wrote:
> Why size and not `8`?
Because that preserves the semantics of the original code.  The original code 
allocates anywhere from 1 to 8 bytes, based on size ```uint8_t 
addr_bytes[size];```. Then reads that amount from memory (line 1303 - 1304), 
and then passes the size of the allocation on this line (via 
```sizeof(addr_bytes)```).

Always passing eight here would be wrong, because the size of the pointer could 
have been anywhere from 1 to 8.

Not also how line 1309 read size-bytes read into addr_bytes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121732

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

[Lldb-commits] [PATCH] D121030: [LLDB][NativePDB] Don't complete static members' types when completing a record type.

[Zequan Wu via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG583223cd5ec4: [LLDB][NativePDB] Don't complete static 
members' types when completing a record… (authored by zequanwu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121030

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/test/Shell/SymbolFile/NativePDB/Inputs/lookup-by-types.lldbinit
  lldb/test/Shell/SymbolFile/NativePDB/lookup-by-types.cpp


Index: lldb/test/Shell/SymbolFile/NativePDB/lookup-by-types.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/lookup-by-types.cpp
@@ -0,0 +1,46 @@
+// clang-format off
+
+// RUN: %build -o %t.exe -- %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
+// RUN: %p/Inputs/lookup-by-types.lldbinit 2>&1 | FileCheck %s
+
+class B;
+class A {
+public:
+static const A constA;
+static A a;
+static B b;
+int val = 1;
+};
+class B {
+public:
+static A a;
+int val = 2;
+};
+A varA;
+B varB;
+const A A::constA = varA;
+A A::a = varA;
+B A::b = varB;
+A B::a = varA;
+
+int main(int argc, char **argv) {
+  return varA.val + varB.val;
+}
+
+// CHECK:  image lookup -type A
+// CHECK-NEXT: 1 match found in {{.*}}.exe
+// CHECK-NEXT: compiler_type = "class A {
+// CHECK-NEXT: static const A constA;
+// CHECK-NEXT: static A a;
+// CHECK-NEXT: static B b;
+// CHECK-NEXT: public:
+// CHECK-NEXT: int val;
+// CHECK-NEXT: }"
+// CHECK:  image lookup -type B
+// CHECK-NEXT: 1 match found in {{.*}}.exe
+// CHECK-NEXT:  compiler_type = "class B {
+// CHECK-NEXT: static A a;
+// CHECK-NEXT: public:
+// CHECK-NEXT: int val;
+// CHECK-NEXT: }"
Index: lldb/test/Shell/SymbolFile/NativePDB/Inputs/lookup-by-types.lldbinit
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/Inputs/lookup-by-types.lldbinit
@@ -0,0 +1,4 @@
+image lookup -type A
+image lookup -type B
+
+quit
\ No newline at end of file
Index: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -138,8 +138,6 @@
   clang::QualType member_type =
   m_ast_builder.GetOrCreateType(PdbTypeSymId(static_data_member.Type));
 
-  m_ast_builder.CompleteType(member_type);
-
   CompilerType member_ct = m_ast_builder.ToCompilerType(member_type);
 
   lldb::AccessType access =
@@ -149,7 +147,7 @@
 
   // Static constant members may be a const[expr] declaration.
   // Query the symbol's value as the variable initializer if valid.
-  if (member_ct.IsConst()) {
+  if (member_ct.IsConst() && member_ct.IsCompleteType()) {
 std::string qual_name = decl->getQualifiedNameAsString();
 
 auto results =
Index: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -1231,8 +1231,10 @@
 clang::QualType PdbAstBuilder::CreateArrayType(const ArrayRecord &ar) {
   clang::QualType element_type = GetOrCreateType(ar.ElementType);
 
-  uint64_t element_count =
-  ar.Size / GetSizeOfType({ar.ElementType}, m_index.tpi());
+  uint64_t element_size = GetSizeOfType({ar.ElementType}, m_index.tpi());
+  if (element_size == 0)
+return {};
+  uint64_t element_count = ar.Size / element_size;
 
   CompilerType array_ct = m_clang.CreateArrayType(ToCompilerType(element_type),
   element_count, false);


Index: lldb/test/Shell/SymbolFile/NativePDB/lookup-by-types.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/lookup-by-types.cpp
@@ -0,0 +1,46 @@
+// clang-format off
+
+// RUN: %build -o %t.exe -- %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
+// RUN: %p/Inputs/lookup-by-types.lldbinit 2>&1 | FileCheck %s
+
+class B;
+class A {
+public:
+static const A constA;
+static A a;
+static B b;
+int val = 1;
+};
+class B {
+public:
+static A a;
+int val = 2;
+};
+A varA;
+B varB;
+const A A::constA = varA;
+A A::a = varA;
+B A::b = varB;
+A B::a = varA;
+
+int main(int argc, char **argv) {
+  return varA.val + varB.val;
+}
+
+// CHECK:  image lookup -type A
+// CHECK-NEXT: 1 match found in {{.*}}.exe
+// CHECK-NEXT: compiler_type = "class A {
+// CHECK-NEXT: static const A constA;
+// CHECK-NEXT: static A a;
+// CHECK-NEXT: static B b;
+// CHECK-NEXT: public:
+// CHECK-NEXT: int val;
+//

[Lldb-commits] [lldb] 583223c - [LLDB][NativePDB] Don't complete static members' types when completing a record type.

[Zequan Wu via lldb-commits]

Author: Zequan Wu
Date: 2022-03-15T14:06:54-07:00
New Revision: 583223cd5ec42f369702a146eaac1bf20bdfbd46

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

LOG: [LLDB][NativePDB] Don't complete static members' types when completing a 
record type.

`UdtRecordCompleter` shouldn't complete static members' types. static members' 
types are going to be completed when the types are called in 
`SymbolFile::CompleteType`.

Reviewed By: labath

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

Added: 
lldb/test/Shell/SymbolFile/NativePDB/Inputs/lookup-by-types.lldbinit
lldb/test/Shell/SymbolFile/NativePDB/lookup-by-types.cpp

Modified: 
lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
index c2c89bdf4ce83..c6f9ef66a4819 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -1231,8 +1231,10 @@ clang::QualType 
PdbAstBuilder::CreateEnumType(PdbTypeSymId id,
 clang::QualType PdbAstBuilder::CreateArrayType(const ArrayRecord &ar) {
   clang::QualType element_type = GetOrCreateType(ar.ElementType);
 
-  uint64_t element_count =
-  ar.Size / GetSizeOfType({ar.ElementType}, m_index.tpi());
+  uint64_t element_size = GetSizeOfType({ar.ElementType}, m_index.tpi());
+  if (element_size == 0)
+return {};
+  uint64_t element_count = ar.Size / element_size;
 
   CompilerType array_ct = m_clang.CreateArrayType(ToCompilerType(element_type),
   element_count, false);

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
index 582c6d2827231..856ba1cb296d8 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -138,8 +138,6 @@ Error UdtRecordCompleter::visitKnownMember(
   clang::QualType member_type =
   m_ast_builder.GetOrCreateType(PdbTypeSymId(static_data_member.Type));
 
-  m_ast_builder.CompleteType(member_type);
-
   CompilerType member_ct = m_ast_builder.ToCompilerType(member_type);
 
   lldb::AccessType access =
@@ -149,7 +147,7 @@ Error UdtRecordCompleter::visitKnownMember(
 
   // Static constant members may be a const[expr] declaration.
   // Query the symbol's value as the variable initializer if valid.
-  if (member_ct.IsConst()) {
+  if (member_ct.IsConst() && member_ct.IsCompleteType()) {
 std::string qual_name = decl->getQualifiedNameAsString();
 
 auto results =

diff  --git 
a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/lookup-by-types.lldbinit 
b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/lookup-by-types.lldbinit
new file mode 100644
index 0..afe3f2c8b943e
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/lookup-by-types.lldbinit
@@ -0,0 +1,4 @@
+image lookup -type A
+image lookup -type B
+
+quit
\ No newline at end of file

diff  --git a/lldb/test/Shell/SymbolFile/NativePDB/lookup-by-types.cpp 
b/lldb/test/Shell/SymbolFile/NativePDB/lookup-by-types.cpp
new file mode 100644
index 0..f3aea8115f385
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/NativePDB/lookup-by-types.cpp
@@ -0,0 +1,46 @@
+// clang-format off
+
+// RUN: %build -o %t.exe -- %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
+// RUN: %p/Inputs/lookup-by-types.lldbinit 2>&1 | FileCheck %s
+
+class B;
+class A {
+public:
+static const A constA;
+static A a;
+static B b;
+int val = 1;
+};
+class B {
+public:
+static A a;
+int val = 2;
+};
+A varA;
+B varB;
+const A A::constA = varA;
+A A::a = varA;
+B A::b = varB;
+A B::a = varA;
+
+int main(int argc, char **argv) {
+  return varA.val + varB.val;
+}
+
+// CHECK:  image lookup -type A
+// CHECK-NEXT: 1 match found in {{.*}}.exe
+// CHECK-NEXT: compiler_type = "class A {
+// CHECK-NEXT: static const A constA;
+// CHECK-NEXT: static A a;
+// CHECK-NEXT: static B b;
+// CHECK-NEXT: public:
+// CHECK-NEXT: int val;
+// CHECK-NEXT: }"
+// CHECK:  image lookup -type B
+// CHECK-NEXT: 1 match found in {{.*}}.exe
+// CHECK-NEXT:  compiler_type = "class B {
+// CHECK-NEXT: static A a;
+// CHECK-NEXT: public:
+// CHECK-NEXT: int val;
+// CHECK-NEXT: }"



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

[Lldb-commits] [lldb] 527d2c5 - [lldb] Fix AppleObjCRuntime log channels

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-03-15T14:47:33-07:00
New Revision: 527d2c5867ce93bbd4e28384ea9006a5032162d4

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

LOG: [lldb] Fix AppleObjCRuntime log channels

The log channel was changed from Types to Commands in
a007a6d84471bb956abe10974cac3066799f583f:

-Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS | 
LIBLLDB_LOG_TYPES));
+Log *log = GetLog(LLDBLog::Process | LLDBLog::Commands);

Added: 


Modified: 

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

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

Removed: 




diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
index d991624e1386d..5e83d43526efa 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
@@ -543,7 +543,7 @@ void 
ClassDescriptorV2::iVarsStorage::fill(AppleObjCRuntimeV2 &runtime,
   if (m_filled)
 return;
   std::lock_guard guard(m_mutex);
-  Log *log = GetLog(LLDBLog::Commands);
+  Log *log = GetLog(LLDBLog::Types);
   LLDB_LOGV(log, "class_name = {0}", descriptor.GetClassName());
   m_filled = true;
   ObjCLanguageRuntime::EncodingToTypeSP encoding_to_type_sp(

diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index 853da6a0c3f6a..a3c3e0b4900e9 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -1467,7 +1467,7 @@ AppleObjCRuntimeV2::GetClassDescriptor(ValueObject 
&valobj) {
 
   objc_class_sp = GetClassDescriptorFromISA(isa);
   if (isa && !objc_class_sp) {
-Log *log = GetLog(LLDBLog::Process | LLDBLog::Commands);
+Log *log = GetLog(LLDBLog::Process | LLDBLog::Types);
 LLDB_LOGF(log,
   "0x%" PRIx64 ": AppleObjCRuntimeV2::GetClassDescriptor() ISA was 
"
   "not in class descriptor cache 0x%" PRIx64,
@@ -1539,7 +1539,7 @@ lldb::addr_t AppleObjCRuntimeV2::GetISAHashTablePointer() 
{
 std::unique_ptr
 AppleObjCRuntimeV2::DynamicClassInfoExtractor::GetClassInfoUtilityFunctionImpl(
 ExecutionContext &exe_ctx, std::string code, std::string name) {
-  Log *log = GetLog(LLDBLog::Process | LLDBLog::Commands);
+  Log *log = GetLog(LLDBLog::Process | LLDBLog::Types);
 
   LLDB_LOG(log, "Creating utility function {0}", name);
 
@@ -1644,7 +1644,7 @@ 
AppleObjCRuntimeV2::DynamicClassInfoExtractor::ComputeHelper() const {
 std::unique_ptr
 AppleObjCRuntimeV2::SharedCacheClassInfoExtractor::
 GetClassInfoUtilityFunctionImpl(ExecutionContext &exe_ctx) {
-  Log *log = GetLog(LLDBLog::Process | LLDBLog::Commands);
+  Log *log = GetLog(LLDBLog::Process | LLDBLog::Types);
 
   LLDB_LOG(log, "Creating utility function {0}",
g_get_shared_cache_class_info_name);
@@ -1747,7 +1747,7 @@ 
AppleObjCRuntimeV2::DynamicClassInfoExtractor::UpdateISAToDescriptorMap(
 
   uint32_t num_class_infos = 0;
 
-  Log *log = GetLog(LLDBLog::Process | LLDBLog::Commands);
+  Log *log = GetLog(LLDBLog::Process | LLDBLog::Types);
 
   ExecutionContext exe_ctx;
 
@@ -1823,7 +1823,7 @@ 
AppleObjCRuntimeV2::DynamicClassInfoExtractor::UpdateISAToDescriptorMap(
 
   // Only dump the runtime classes from the expression evaluation if the log is
   // verbose:
-  Log *type_log = GetLog(LLDBLog::Commands);
+  Log *type_log = GetLog(LLDBLog::Types);
   bool dump_log = type_log && type_log->GetVerbose();
 
   arguments.GetValueAtIndex(3)->GetScalar() = dump_log ? 1 : 0;
@@ -1903,7 +1903,7 @@ uint32_t AppleObjCRuntimeV2::ParseClassInfoArray(const 
DataExtractor &data,
   //uint32_t hash;
   //} __attribute__((__packed__));
 
-  Log *log = GetLog(LLDBLog::Commands);
+  Log *log = GetLog(LLDBLog::Types);
   bool should_log = log && log->GetVerbose();
 
   uint32_t num_parsed = 0;
@@ -1976,7 +1976,7 @@ 
AppleObjCRuntimeV2::SharedCacheClassInfoExtractor::UpdateISAToDescriptorMap() {
   if (process == nullptr)
 return DescriptorMapUpdateResult::Fail();
 
-  Log *log = GetLog(LLDBLog::Process | LLDBLog::Commands);
+  Log *log = GetLog(LLDBLog::Process | LLDBLog::Types);
 
   ExecutionContext exe_ctx;
 
@@ -2057,7 +2057,7 @@ 
AppleObjCRuntimeV2::SharedCacheClassInfoExtractor::UpdateISAToDescriptorMap() {
   arguments.GetValueAtIndex(4)->GetScalar() = class_infos_byte_size;
   // Only dump the runtime classes from

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

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan marked 2 inline comments as done.
yinghuitan added a comment.

Jim, Pavel, thanks for the comment!

I am happy to start a strategy discussion for this feature. @clayborg will 
chrime in later to suggest the approach how we can discuss.

I would like to answer some of the questions here to share some context.

> speed for visibility tradeoff

Right, that is the basic principle of this feature. We are designing this 
feature primarily targeting IDE use cases which, we found, majority of the 
users are focusing on several modules authored/depended by their own code. 
Also they primarily use basic subset features of lldb.

> They (and we) also need to know what operations might trigger a full 
> "hydration" of a symbol table

We would like to make the feature work *by default* as much as possible so we 
decided to use breakpoint match, callstack and symbol table match to guide a 
module's debug info hydration. To direct answer this question: I haven't find a 
common case that can hydrate all process modules debug info yet unless there is 
callstack covering all modules or a function name matching in all module's 
symbol table while setting function breakpoint.

> whenever the user tells about a shared library, we should "hydrate" it

I agree. Since this feature is designed for IDE scenario so we haven't focused 
on covering all lldb commands when a module is specified. In "break set -s 
someLib.dylib -f foo.c -l 12" example, if there is a matching foo.c in 
someLib.dylib, it will be hydrated; otherwise not.

> It also seems like there should be a way to say "These libraries I'm unlikely 
> to care about, but these I always want to have fully expanded".

I agree. It is in my plan to add a "symbols.on-demand-modules" setting which 
users can explicitly white list some modules for hydration.

> unify with `target.preload-symbols` feature

I do not have strong opinion on this. One concern is that 
`target.preload-symbols` provides full experience but `symbols.load-on-demand` 
provides trade-off experience (for example some global expression evaluation 
may fail because we won't automatically hydrate based on type query).

> I don't think that "run everything with the setting turned on" is a good 
> testing strategy

I do not have strong opinion either. I understand the concern is extra test 
time. I do want to share that there are many tests that do not set breakpoints 
which exercised symbol ondemand code path and helped to catch real bugs in the 
implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121631

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

[Lldb-commits] [PATCH] D121511: [lldb] Report debugger diagnostics as events

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 415643.
JDevlieghere marked 2 inline comments as done.
JDevlieghere added a comment.
Herald added a subscriber: mgorny.

Address code review feedback


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

https://reviews.llvm.org/D121511

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/DebuggerEvents.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/DebuggerEvents.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/DiagnosticEventTest.cpp

Index: lldb/unittests/Core/DiagnosticEventTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/DiagnosticEventTest.cpp
@@ -0,0 +1,144 @@
+//===-- DiagnosticEventTest.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/MacOSX/PlatformMacOSX.h"
+#include "Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/DebuggerEvents.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Utility/Broadcaster.h"
+#include "lldb/Utility/Event.h"
+#include "lldb/Utility/Listener.h"
+#include "lldb/Utility/Reproducer.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::repro;
+
+static const constexpr std::chrono::seconds TIMEOUT(0);
+static const constexpr size_t DEBUGGERS = 3;
+
+static std::once_flag debugger_initialize_flag;
+
+namespace {
+class DiagnosticEventTest : public ::testing::Test {
+public:
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+FileSystem::Initialize();
+HostInfo::Initialize();
+PlatformMacOSX::Initialize();
+std::call_once(debugger_initialize_flag,
+   []() { Debugger::Initialize(nullptr); });
+  }
+  void TearDown() override {
+PlatformMacOSX::Terminate();
+HostInfo::Terminate();
+FileSystem::Terminate();
+Reproducer::Terminate();
+  }
+};
+} // namespace
+
+TEST_F(DiagnosticEventTest, Warning) {
+  ArchSpec arch("x86_64-apple-macosx-");
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+
+  Broadcaster &broadcaster = debugger_sp->GetBroadcaster();
+  ListenerSP listener_sp = Listener::MakeListener("test-listener");
+
+  listener_sp->StartListeningForEvents(&broadcaster,
+   Debugger::eBroadcastBitWarning);
+  EXPECT_TRUE(
+  broadcaster.EventTypeHasListeners(Debugger::eBroadcastBitWarning));
+
+  Debugger::ReportWarning("foo", debugger_sp->GetID());
+
+  EventSP event_sp;
+  EXPECT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+  ASSERT_TRUE(event_sp);
+
+  const DiagnosticEventData *data =
+  DiagnosticEventData::GetEventDataFromEvent(event_sp.get());
+  ASSERT_NE(data, nullptr);
+  EXPECT_EQ(data->GetPrefix(), "warning");
+  EXPECT_EQ(data->GetMessage(), "foo");
+
+  Debugger::Destroy(debugger_sp);
+}
+
+TEST_F(DiagnosticEventTest, Error) {
+  ArchSpec arch("x86_64-apple-macosx-");
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+
+  Broadcaster &broadcaster = debugger_sp->GetBroadcaster();
+  ListenerSP listener_sp = Listener::MakeListener("test-listener");
+
+  listener_sp->StartListeningForEvents(&broadcaster,
+   Debugger::eBroadcastBitError);
+  EXPECT_TRUE(broadcaster.EventTypeHasListeners(Debugger::eBroadcastBitError));
+
+  Debugger::ReportError("bar", debugger_sp->GetID());
+
+  EventSP event_sp;
+  EXPECT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+  ASSERT_TRUE(event_sp);
+
+  const DiagnosticEventData *data =
+  DiagnosticEventData::GetEventDataFromEvent(event_sp.get());
+  ASSERT_NE(data, nullptr);
+  EXPECT_EQ(data->GetPrefix(), "error");
+  EXPECT_EQ(data->GetMessage(), "bar");
+
+  Debugger::Destroy(debugger_sp);
+}
+
+TEST_F(DiagnosticEventTest, MultipleDebuggers) {
+  ArchSpec arch("x86_64-apple-macosx-");
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+
+  std::vector debuggers;
+  std::vector listeners;
+
+  for (size_t i = 0; i < DEBUGGERS; ++i) {
+DebuggerSP debugger = Debugger::CreateInstance();
+ListenerSP listener = Listener::MakeListener("listener");
+
+debuggers.push_back(debugger);
+listeners.push_

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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D121631#3380824 , @jingham wrote:

> This sort of change needs to do a lot more work to help the user understand 
> what they can and can't see if they accept the speed for visibility tradeoff.
>
> They (and we) also need to know what operations might trigger a full 
> "hydration" of a symbol table.  It isn't useful to have all this 
> infrastructure in place if fairly common operations trigger all the work 
> anyway.

Symbol tables are always on and are used to help us find things that will 
possibly be in the debug info from a function name (look for Code symbols) and 
global variable (Data symbols) perspective. So a few things here:

- symbol tables are always parsed for everything and help us enable debug info 
if we find a match for FindFunctions and FindGlobals calls on the symbol file 
even if these SymbolFile calls specify to not check the symbol table. If we 
find a symbol that matches, we turn on debug info and repeat the call by 
calling into the symbol file if debug info wasn't previously enabled.
- any Address -> symbol context where the user asks for debug info enables 
debug info: which usually means any PC from any stack frame, but can also mean 
if a user queries an address
- line tables are always left enabled and if any match occurs we enable debug 
info

> It also seems to me that whenever the user tells about a shared library, we 
> should "hydrate" it.  So for instance, if I do:
>
> (lldb) break set -s someLib.dylib -f foo.c -l 12
>
> they you should first hydrate someLib.dylib and then set the breakpoint.

that already works. Line tables are relatively cheap and they are always 
enabled through SymbolFileOnDemand and if. "foo.c:12" kicks up any matches, 
regardless of wether the shared library was specified, will enable debug info 
for any libraries that match.

> In general, there should be a strategy discussion up-front before just making 
> this change, with use cases of what won't work (not just some disabled tests) 
> and how ordinary users might figure out something didn't work because of the 
> on-demand loading.  There needs to also be a way for lldb to inform users 
> what to expect or the experience will just end up being frustrating.

Jim: I met with you to discuss all of these strategies a month ago and I left 
with the impression the ideas I told you about could work pretty well and that 
is what Jeffrey implemented here.

> It also seems like there should be a way to say "These libraries I'm unlikely 
> to care about, but these I always want to have fully expanded".

We were hoping to see if we can automagically make this stuff work well enough 
that it requires no settings, but we are happy to add these settings in 
subsequent patches. The patch is already large as it is. Let us know if these 
should be added with this initial patch. We might also think about asking the 
Platform layer if they have any libraries that they know should be always on.

> Normally, it would be fine to say "we can add those details as we go along" 
> but you're designing a new mode of interaction with the debugger, and I think 
> we need to sketch out what we think are acceptable tradeoffs and where we 
> need to give people the ability to manually intervene, etc.  So it seems like 
> we should first be having a strategy discussion, before getting down to the 
> details of this patch.

We would love to setup a Video Chat with anyone interested if that would work. 
Talking back and forth on email or lists takes a while and I think we can 
easily hash things out if any interested parties are willing to attend. We are 
on Pacific time zone, anyone else that wants to participate, please chime in 
and we will see if we can set something up?!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121631

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

[Lldb-commits] [PATCH] D121511: [lldb] Report debugger diagnostics as events

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 415644.

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

https://reviews.llvm.org/D121511

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/DebuggerEvents.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/DebuggerEvents.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/DiagnosticEventTest.cpp

Index: lldb/unittests/Core/DiagnosticEventTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/DiagnosticEventTest.cpp
@@ -0,0 +1,185 @@
+//===-- DiagnosticEventTest.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/MacOSX/PlatformMacOSX.h"
+#include "Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/DebuggerEvents.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Utility/Broadcaster.h"
+#include "lldb/Utility/Event.h"
+#include "lldb/Utility/Listener.h"
+#include "lldb/Utility/Reproducer.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::repro;
+
+static const constexpr std::chrono::seconds TIMEOUT(0);
+static const constexpr size_t DEBUGGERS = 3;
+
+static std::once_flag debugger_initialize_flag;
+
+namespace {
+class DiagnosticEventTest : public ::testing::Test {
+public:
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+FileSystem::Initialize();
+HostInfo::Initialize();
+PlatformMacOSX::Initialize();
+std::call_once(debugger_initialize_flag,
+   []() { Debugger::Initialize(nullptr); });
+  }
+  void TearDown() override {
+PlatformMacOSX::Terminate();
+HostInfo::Terminate();
+FileSystem::Terminate();
+Reproducer::Terminate();
+  }
+};
+} // namespace
+
+TEST_F(DiagnosticEventTest, Warning) {
+  ArchSpec arch("x86_64-apple-macosx-");
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+
+  Broadcaster &broadcaster = debugger_sp->GetBroadcaster();
+  ListenerSP listener_sp = Listener::MakeListener("test-listener");
+
+  listener_sp->StartListeningForEvents(&broadcaster,
+   Debugger::eBroadcastBitWarning);
+  EXPECT_TRUE(
+  broadcaster.EventTypeHasListeners(Debugger::eBroadcastBitWarning));
+
+  Debugger::ReportWarning("foo", debugger_sp->GetID());
+
+  EventSP event_sp;
+  EXPECT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+  ASSERT_TRUE(event_sp);
+
+  const DiagnosticEventData *data =
+  DiagnosticEventData::GetEventDataFromEvent(event_sp.get());
+  ASSERT_NE(data, nullptr);
+  EXPECT_EQ(data->GetPrefix(), "warning");
+  EXPECT_EQ(data->GetMessage(), "foo");
+
+  Debugger::Destroy(debugger_sp);
+}
+
+TEST_F(DiagnosticEventTest, Error) {
+  ArchSpec arch("x86_64-apple-macosx-");
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+
+  Broadcaster &broadcaster = debugger_sp->GetBroadcaster();
+  ListenerSP listener_sp = Listener::MakeListener("test-listener");
+
+  listener_sp->StartListeningForEvents(&broadcaster,
+   Debugger::eBroadcastBitError);
+  EXPECT_TRUE(broadcaster.EventTypeHasListeners(Debugger::eBroadcastBitError));
+
+  Debugger::ReportError("bar", debugger_sp->GetID());
+
+  EventSP event_sp;
+  EXPECT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+  ASSERT_TRUE(event_sp);
+
+  const DiagnosticEventData *data =
+  DiagnosticEventData::GetEventDataFromEvent(event_sp.get());
+  ASSERT_NE(data, nullptr);
+  EXPECT_EQ(data->GetPrefix(), "error");
+  EXPECT_EQ(data->GetMessage(), "bar");
+
+  Debugger::Destroy(debugger_sp);
+}
+
+TEST_F(DiagnosticEventTest, MultipleDebuggers) {
+  ArchSpec arch("x86_64-apple-macosx-");
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+
+  std::vector debuggers;
+  std::vector listeners;
+
+  for (size_t i = 0; i < DEBUGGERS; ++i) {
+DebuggerSP debugger = Debugger::CreateInstance();
+ListenerSP listener = Listener::MakeListener("listener");
+
+debuggers.push_back(debugger);
+listeners.push_back(listener);
+
+listener->StartListeningForEvents(&debugger->GetBroadcaster(),
+  Debugger::eBroadca

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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I am personally not a fan of the preloading of symbols settings being on by 
default as there are large delays in debug session startup on linux due to the 
lack of any kind of viable acceleration tables. I understand that delays can 
occur later in the debug session as a result, but those are usually tied to 
global lookups from expressions and many people using IDEs will set file and 
line breakpoints and just use the variable display which doesn't need 
everything preloaded many times. We are also looking into trying to track who 
is doing these global lookups and trying to ensure that the global lookup is 
needed. One call to fine all "iterator" types is an example of a really bad 
global lookup that can cause tons of types to be pulled in.

I do think that possibly enabling this feature by changing the preload seeting 
to allow "preload", "on-demand" or "lazily" could easily work here.

I also agree that we should provide documentation for this feature, or possibly 
print out a blurb of text to the console that explains the rules if this 
feature gets enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121631

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

[Lldb-commits] [PATCH] D121426: [VFS] Update uses of getVFSFromYAML to use the new getVFSFromYAMLs API

[Ben Barham via Phabricator via lldb-commits]

bnbarham marked an inline comment as done.
bnbarham added a comment.

Merged into D121425  instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121426

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

[Lldb-commits] [PATCH] D121746: Use lit_config.substitute instead of foo % lit_config.params everywhere

[Sam McCall via Phabricator via lldb-commits]

sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added a reviewer: bollu.
Herald added subscribers: ayermolo, sdasgup3, wenzhicui, wrengr, Chia-hungDuan, 
dcaballe, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, 
grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, antiagainst, 
shauheen, rriddle, mehdi_amini, delcypher.
Herald added a reviewer: sscalpone.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added projects: Flang, All.
sammccall requested review of this revision.
Herald added subscribers: llvm-commits, lldb-commits, Sanitizers, cfe-commits, 
yota9, stephenneuendorffer, nicolasvasilache, jdoerfert.
Herald added projects: clang, Sanitizers, LLDB, MLIR, LLVM.

This mechanically applies the same changes from D121427 
 everywhere.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121746

Files:
  bolt/test/Unit/lit.site.cfg.py.in
  bolt/test/lit.site.cfg.py.in
  clang/test/Unit/lit.site.cfg.py.in
  clang/test/lit.site.cfg.py.in
  clang/utils/perf-training/lit.site.cfg.in
  clang/utils/perf-training/order-files.lit.site.cfg.in
  compiler-rt/test/lit.common.configured.in
  compiler-rt/unittests/lit.common.unit.configured.in
  cross-project-tests/lit.site.cfg.py.in
  flang/test/NonGtestUnit/lit.site.cfg.py.in
  flang/test/Unit/lit.site.cfg.py.in
  flang/test/lit.site.cfg.py.in
  lld/test/Unit/lit.site.cfg.py.in
  lld/test/lit.site.cfg.py.in
  lldb/test/API/lit.site.cfg.py.in
  lldb/test/Shell/lit.site.cfg.py.in
  lldb/test/Unit/lit.site.cfg.py.in
  lldb/test/lit.site.cfg.py.in
  llvm/test/Unit/lit.site.cfg.py.in
  llvm/test/lit.site.cfg.py.in
  llvm/utils/lit/tests/lit.site.cfg.in
  mlir/examples/standalone/test/lit.site.cfg.py.in
  mlir/test/Unit/lit.site.cfg.py.in
  mlir/test/lit.site.cfg.py.in
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in

Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -2,8 +2,8 @@
 
 config.llvm_src_root = "@LLVM_SOURCE_DIR@"
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
-config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
-config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_tools_dir = lit_config.substitute("@LLVM_TOOLS_DIR@")
+config.llvm_libs_dir = lit_config.substitute("@LLVM_LIBS_DIR@")
 config.polly_obj_root = "@POLLY_BINARY_DIR@"
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@LLVM_TARGET_TRIPLE@"
@@ -21,16 +21,6 @@
 for arch in config.targets_to_build.split():
 config.available_features.add(arch.lower() + '-registered-target')
 
-# Support substitution of the tools and libs dirs with user parameters. This is
-# used when we can't determine the tool dir at configuration time.
-try:
-config.llvm_tools_dir = config.llvm_tools_dir % lit_config.params
-config.llvm_libs_dir = config.llvm_libs_dir % lit_config.params
-except KeyError:
-e = sys.exc_info()[1]
-key, = e.args
-lit_config.fatal("unable to find %r parameter, use '--param=%s=VALUE'" % (key,key))
-
 # excludes: A list of directories to exclude from the testsuite. The 'Inputs'
 # subdirectories contain auxiliary inputs for various tests in their parent
 # directories.
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -4,9 +4,9 @@
 
 config.llvm_src_root = "@LLVM_SOURCE_DIR@"
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
-config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
-config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
-config.llvm_build_mode = "@LLVM_BUILD_MODE@"
+config.llvm_tools_dir = lit_config.substitute("@LLVM_TOOLS_DIR@")
+config.llvm_libs_dir = lit_config.substitute("@LLVM_LIBS_DIR@")
+config.llvm_build_mode = lit_config.substitute("@LLVM_BUILD_MODE@")
 config.polly_obj_root = "@POLLY_BINARY_DIR@"
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.enable_shared = @ENABLE_SHARED@
@@ -16,17 +16,5 @@
 config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.has_unittests = @POLLY_GTEST_AVAIL@
 
-# Support substitution of the tools_dir, libs_dirs, and build_mode with user
-# parameters. This is used when we can't determine the tool dir at
-# configuration time.
-try:
-config.llvm_tools_dir = config.llvm_tools_dir % lit_config.params
-config.llvm_libs_dir = config.llvm_libs_dir % lit_config.params
-config.llvm_build_mode = config.llvm_build_mode % lit_config.params
-except KeyError:
-e = sys.exc_info()[1]
-key, = e.args
-lit_config.fatal("unable to find %r parameter, use '--param=%s=VALUE'" % (key,key))
-
 # Let the main config do the real work.
 lit_config.load_config(config, "@POLLY_SOURCE_DIR@/test/Unit/lit.cfg")
Index: mlir/test/lit.site.cfg.py.in
===
--- mlir/test/li

[Lldb-commits] [PATCH] D121425: [VFS] Add a new getVFSFromYAMLs API

[Ben Barham via Phabricator via lldb-commits]

bnbarham updated this revision to Diff 415615.
bnbarham edited the summary of this revision.
bnbarham added a comment.
Herald added subscribers: cfe-commits, lldb-commits, carlosgalvezp.
Herald added projects: clang, LLDB, clang-tools-extra.

Re-order to be before D121424  and merge with 
D121426 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121425

Files:
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/VFS/Inputs/vfsroot.yaml
  clang/test/VFS/directory.c
  clang/test/VFS/multiple-overlays.c
  clang/test/VFS/vfsroot-with-overlay.c
  lldb/source/Commands/CommandObjectReproducer.cpp
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/tools/dsymutil/Reproducer.cpp
  llvm/tools/dsymutil/Reproducer.h
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -14,6 +14,7 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Testing/Support/Error.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -2571,7 +2572,7 @@
 }
 
 TEST_F(VFSFromYAMLTest, WorkingDirectory) {
-  IntrusiveRefCntPtr Lower(new DummyFileSystem());
+  IntrusiveRefCntPtr Lower(new ErrorDummyFileSystem());
   Lower->addDirectory("//root/");
   Lower->addDirectory("//root/foo");
   Lower->addRegularFile("//root/foo/a");
@@ -2593,6 +2594,7 @@
   "}",
   Lower);
   ASSERT_NE(FS.get(), nullptr);
+
   std::error_code EC = FS->setCurrentWorkingDirectory("//root/bar");
   ASSERT_FALSE(EC);
 
@@ -2621,6 +2623,14 @@
   ASSERT_TRUE(WorkingDir);
   EXPECT_EQ(*WorkingDir, "//root/");
 
+  Status = FS->status("bar/a");
+  ASSERT_FALSE(Status.getError());
+  EXPECT_TRUE(Status->exists());
+
+  Status = FS->status("foo/a");
+  ASSERT_FALSE(Status.getError());
+  EXPECT_TRUE(Status->exists());
+
   EC = FS->setCurrentWorkingDirectory("bar");
   ASSERT_FALSE(EC);
   WorkingDir = FS->getCurrentWorkingDirectory();
@@ -2710,43 +2720,6 @@
   EXPECT_TRUE(Status->exists());
 }
 
-TEST_F(VFSFromYAMLTest, WorkingDirectoryFallthroughInvalid) {
-  IntrusiveRefCntPtr Lower(new ErrorDummyFileSystem());
-  Lower->addDirectory("//root/");
-  Lower->addDirectory("//root/foo");
-  Lower->addRegularFile("//root/foo/a");
-  Lower->addRegularFile("//root/foo/b");
-  Lower->addRegularFile("//root/c");
-  IntrusiveRefCntPtr FS = getFromYAMLString(
-  "{ 'use-external-names': false,\n"
-  "  'roots': [\n"
-  "{\n"
-  "  'type': 'directory',\n"
-  "  'name': '//root/bar',\n"
-  "  'contents': [ {\n"
-  "  'type': 'file',\n"
-  "  'name': 'a',\n"
-  "  'external-contents': '//root/foo/a'\n"
-  "}\n"
-  "  ]\n"
-  "}\n"
-  "]\n"
-  "}",
-  Lower);
-  ASSERT_NE(FS.get(), nullptr);
-  std::error_code EC = FS->setCurrentWorkingDirectory("//root/");
-  ASSERT_FALSE(EC);
-  ASSERT_NE(FS.get(), nullptr);
-
-  llvm::ErrorOr Status = FS->status("bar/a");
-  ASSERT_FALSE(Status.getError());
-  EXPECT_TRUE(Status->exists());
-
-  Status = FS->status("foo/a");
-  ASSERT_FALSE(Status.getError());
-  EXPECT_TRUE(Status->exists());
-}
-
 TEST_F(VFSFromYAMLTest, VirtualWorkingDirectory) {
   IntrusiveRefCntPtr Lower(new ErrorDummyFileSystem());
   Lower->addDirectory("//root/");
@@ -3207,3 +3180,168 @@
 "  DummyFileSystem (RecursiveContents)\n",
 Output);
 }
+
+static std::unique_ptr
+getVFSOrNull(ArrayRef YAMLOverlays,
+ IntrusiveRefCntPtr ExternalFS) {
+  SmallVector OverlayRefs;
+  for (const auto &Overlay : YAMLOverlays) {
+OverlayRefs.emplace_back(Overlay, "");
+  }
+
+  auto ExpectedFS = vfs::getVFSFromYAMLs(OverlayRefs, ExternalFS);
+  if (auto Err = ExpectedFS.takeError()) {
+consumeError(std::move(Err));
+return nullptr;
+  }
+  return std::move(*ExpectedFS);
+}
+
+static std::string createSimpleOverlay(StringRef RedirectKind, StringRef From,
+   StringRef To) {
+  return ("{\n"
+  "  'version': 0,\n"
+  "  'redirecting-with': '" +
+  RedirectKind +
+  "'\n"
+  "  'roots': [\n"
+  " {\n"
+  "   'type': 'directory-remap',\n"
+  "   'name': '" +
+  From +
+  "',\n"
+  "   'external-contents': '" +
+  To +
+  "',\n"
+  "   }]\n"
+  " }"
+  "  ]")
+  .str();
+}
+
+// Make sure that overlays are not transitive. Giv

[Lldb-commits] [PATCH] D121425: [VFS] Add a new getVFSFromYAMLs API

[Ben Barham via Phabricator via lldb-commits]

bnbarham added a comment.

There's two failing tests with this change:

- VFSFromYAMLTest.ReturnsExternalPathVFSHit
- VFSFromYAMLTest.ReturnsInternalPathVFSHit

Apparently we allow relative paths in external-contents *without* specifying 
`overlay-relative: true`. In this case the relative paths are resolved based on 
the CWD at the time of the operation. Do we really want that? I believe this 
wouldn't have worked prior to https://reviews.llvm.org/D109128 as there was no 
canonicalising before then as well. @keith was that change intentional? If we 
want it then it will *require* setting CWD on each FS in OverlayFS, which I was 
really hoping to avoid.

To give the concrete case:

  OverlayFileSystem
RedirectingFileSystem
  /a/foo -> bar
RealFileSystem
  /a/bar
  /b/bar
  
  cd /a
  cat foo # /a/bar
  
  cd /b # would fail previously since it doesn't exist in RedirectingFS but 
would still setCWD for RealFS
  cat foo # and thus return /a/bar since RedirectingFS is still in /a

This now fails completely because OverlayFS doesn't setCWD on the underlying 
filesystems. So RedirectingFS has the CWD of whatever ExternalFS was at 
creation. In D121424  it will fail because 
there's no longer any canonicalise after mapping the path in RedirectingFS (I 
assumed this was unintentional and missed the test case).

If we want to allow this then I think we'll need to go with the previous plan 
for OverlayFS, ie. keep track of filesystems that setCWD failed on and don't 
call them at all until they succeed again. That then doesn't allow certain 
cases that the current method allows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121425

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

[Lldb-commits] [PATCH] D121425: [VFS] Add a new getVFSFromYAMLs API

[Ben Barham via Phabricator via lldb-commits]

bnbarham added a comment.

In D121425#3384188 , @bnbarham wrote:

> There's two failing tests with this change:
>
> - VFSFromYAMLTest.ReturnsExternalPathVFSHit
> - VFSFromYAMLTest.ReturnsInternalPathVFSHit
>
> Apparently we allow relative paths in external-contents *without* specifying 
> `overlay-relative: true`. In this case the relative paths are resolved based 
> on the CWD at the time of the operation. Do we really want that? I believe 
> this wouldn't have worked prior to https://reviews.llvm.org/D109128 as there 
> was no canonicalising before then as well. @keith was that change 
> intentional? If we want it then it will *require* setting CWD on each FS in 
> OverlayFS, which I was really hoping to avoid.

I spoke to Keith offline. This has always worked - it previously worked by 
`RedirectingFileSystem` setting CWD on `ExternalFS` when `setCWD` was called on 
it. It's also important to keep supporting as it's used in Bazel 
(https://github.com/bazelbuild/rules_swift/blob/c1d7d1df6969c2675c7826ecf1202d78016b1753/swift/internal/vfsoverlay.bzl#L41-L55).

I'm hoping I can fix this by resolving the paths when the overlay is created. 
I'll see if that works (it'll depend on when -working-directory is actually 
used by the frontend).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121425

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

[Lldb-commits] [lldb] 384e890 - [LLDB][NativePDB] Remove REQUIRES: system-windows for local-variables-regsiters.s

[Zequan Wu via lldb-commits]

Author: Zequan Wu
Date: 2022-03-15T17:49:46-07:00
New Revision: 384e890dd3a3feeaa482dd973f5cadcf465fe7ec

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

LOG: [LLDB][NativePDB] Remove REQUIRES: system-windows for 
local-variables-regsiters.s

Added: 


Modified: 
lldb/test/Shell/SymbolFile/NativePDB/local-variables-registers.s

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/NativePDB/local-variables-registers.s 
b/lldb/test/Shell/SymbolFile/NativePDB/local-variables-registers.s
index 3be80e63a95dc..e2b5da4a4c17d 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/local-variables-registers.s
+++ b/lldb/test/Shell/SymbolFile/NativePDB/local-variables-registers.s
@@ -1,8 +1,8 @@
 # clang-format off
-# REQUIRES: lld, system-windows
+# REQUIRES: lld
 
-# RUN: %clang_cl --target=x86_64-windows-msvc /Fo%t.obj %s
-# RUN: lld-link /debug %t.obj /out:%t.exe /base:0x14000
+# RUN: llvm-mc -triple=x86_64-windows-msvc --filetype=obj %s > %t.obj
+# RUN: lld-link /debug:full /nodefaultlib /entry:main %t.obj /out:%t.exe 
/base:0x14000
 # RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
 # RUN: %p/Inputs/local-variables-registers.lldbinit 2>&1 | FileCheck %s
 



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

[Lldb-commits] [lldb] 37400dd - [LLDB][NFC] Remove dead code from Section.cpp

[Shafik Yaghmour via lldb-commits]

Author: Shafik Yaghmour
Date: 2022-03-15T18:11:30-07:00
New Revision: 37400dd3e817ea43ed9c32d545211dd93b5e7c59

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

LOG: [LLDB][NFC] Remove dead code from Section.cpp

Removing comment out code, looks like debugging code left over from a while ago.

Added: 


Modified: 
lldb/source/Core/Section.cpp

Removed: 




diff  --git a/lldb/source/Core/Section.cpp b/lldb/source/Core/Section.cpp
index cb9d41f19b507..acad3a2b8cf3d 100644
--- a/lldb/source/Core/Section.cpp
+++ b/lldb/source/Core/Section.cpp
@@ -164,12 +164,6 @@ Section::Section(const ModuleSP &module_sp, ObjectFile 
*obj_file,
   m_log2align(log2align), m_children(), m_fake(false), m_encrypted(false),
   m_thread_specific(false), m_readable(false), m_writable(false),
   m_executable(false), m_relocated(false), 
m_target_byte_size(target_byte_size) {
-  //printf ("Section::Section(%p): module=%p, sect_id = 0x%16.16" PRIx64 ",
-  //addr=[0x%16.16" PRIx64 " - 0x%16.16" PRIx64 "), file [0x%16.16" PRIx64 
"
-  //- 0x%16.16" PRIx64 "), flags = 0x%8.8x, name = %s\n",
-  //this, module_sp.get(), sect_id, file_addr, file_addr +
-  //byte_size, file_offset, file_offset + file_size, flags,
-  //name.GetCString());
 }
 
 Section::Section(const lldb::SectionSP &parent_section_sp,
@@ -186,19 +180,11 @@ Section::Section(const lldb::SectionSP &parent_section_sp,
   m_log2align(log2align), m_children(), m_fake(false), m_encrypted(false),
   m_thread_specific(false), m_readable(false), m_writable(false),
   m_executable(false), m_relocated(false), 
m_target_byte_size(target_byte_size) {
-  //printf ("Section::Section(%p): module=%p, sect_id = 0x%16.16" PRIx64 ",
-  //addr=[0x%16.16" PRIx64 " - 0x%16.16" PRIx64 "), file [0x%16.16" PRIx64 
"
-  //- 0x%16.16" PRIx64 "), flags = 0x%8.8x, name = %s.%s\n",
-  //this, module_sp.get(), sect_id, file_addr, file_addr +
-  //byte_size, file_offset, file_offset + file_size, flags,
-  //parent_section_sp->GetName().GetCString(), name.GetCString());
   if (parent_section_sp)
 m_parent_wp = parent_section_sp;
 }
 
-Section::~Section() {
-  //printf ("Section::~Section(%p)\n", this);
-}
+Section::~Section() {}
 
 addr_t Section::GetFileAddress() const {
   SectionSP parent_sp(GetParent());



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

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

[Jim Ingham via lldb-commits]

> On Mar 15, 2022, at 5:03 PM, Greg Clayton via Phabricator 
>  wrote:
> 
> clayborg added a comment.
> 
> In D121631#3380824 , @jingham wrote:
> 
>> This sort of change needs to do a lot more work to help the user understand 
>> what they can and can't see if they accept the speed for visibility tradeoff.
>> 
>> They (and we) also need to know what operations might trigger a full 
>> "hydration" of a symbol table.  It isn't useful to have all this 
>> infrastructure in place if fairly common operations trigger all the work 
>> anyway.
> 
> Symbol tables are always on and are used to help us find things that will 
> possibly be in the debug info from a function name (look for Code symbols) 
> and global variable (Data symbols) perspective. So a few things here:
> 
> - symbol tables are always parsed for everything and help us enable debug 
> info if we find a match for FindFunctions and FindGlobals calls on the symbol 
> file even if these SymbolFile calls specify to not check the symbol table. If 
> we find a symbol that matches, we turn on debug info and repeat the call by 
> calling into the symbol file if debug info wasn't previously enabled.
> - any Address -> symbol context where the user asks for debug info enables 
> debug info: which usually means any PC from any stack frame, but can also 
> mean if a user queries an address
> - line tables are always left enabled and if any match occurs we enable debug 
> info
> 
>> It also seems to me that whenever the user tells about a shared library, we 
>> should "hydrate" it.  So for instance, if I do:
>> 
>> (lldb) break set -s someLib.dylib -f foo.c -l 12
>> 
>> they you should first hydrate someLib.dylib and then set the breakpoint.
> 
> that already works. Line tables are relatively cheap and they are always 
> enabled through SymbolFileOnDemand and if. "foo.c:12" kicks up any matches, 
> regardless of wether the shared library was specified, will enable debug info 
> for any libraries that match.
> 
>> In general, there should be a strategy discussion up-front before just 
>> making this change, with use cases of what won't work (not just some 
>> disabled tests) and how ordinary users might figure out something didn't 
>> work because of the on-demand loading.  There needs to also be a way for 
>> lldb to inform users what to expect or the experience will just end up being 
>> frustrating.
> 
> Jim: I met with you to discuss all of these strategies a month ago and I left 
> with the impression the ideas I told you about could work pretty well and 
> that is what Jeffrey implemented here.

I'm not so much questioning which strategies this acceleration should use.  
Based on our discussion and previous experience with this feature, it seemed 
likely that this will "mostly work".  But an inchoate "mostly working" is very 
frustrating in the context of a debugger.  So going along with this feature 
should be some sketch of the things that it doesn't support (do inline 
breakpoints always work?, what kinds of symbols are you likely not to find, 
etc, what trouble is this likely to cause to the expression parser and how 
would a user work around this).  This isn't an explanation so much for me as 
for users of the feature.  I don't think "This is mostly for users of GUI 
debuggers and they probably won't do anything that might get into trouble" is a 
good support strategy.  

> 
>> It also seems like there should be a way to say "These libraries I'm 
>> unlikely to care about, but these I always want to have fully expanded".
> 
> We were hoping to see if we can automagically make this stuff work well 
> enough that it requires no settings, but we are happy to add these settings 
> in subsequent patches. The patch is already large as it is. Let us know if 
> these should be added with this initial patch. We might also think about 
> asking the Platform layer if they have any libraries that they know should be 
> always on.
> 
>> Normally, it would be fine to say "we can add those details as we go along" 
>> but you're designing a new mode of interaction with the debugger, and I 
>> think we need to sketch out what we think are acceptable tradeoffs and where 
>> we need to give people the ability to manually intervene, etc.  So it seems 
>> like we should first be having a strategy discussion, before getting down to 
>> the details of this patch.
> 
> We would love to setup a Video Chat with anyone interested if that would 
> work. Talking back and forth on email or lists takes a while and I think we 
> can easily hash things out if any interested parties are willing to attend. 
> We are on Pacific time zone, anyone else that wants to participate, please 
> chime in and we will see if we can set something up?!

This sounds fine, but I think as a starting point it would be good to have a 
list of what doesn't work with this approach.  Obviously getting hold of types 
is going to be a problem.  How far will that affec

[Lldb-commits] [PATCH] D121511: [lldb] Report debugger diagnostics as events

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Print diagnostics directly to the debugger's error stream if nobody is 
listening for them.


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

https://reviews.llvm.org/D121511

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/DebuggerEvents.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/DebuggerEvents.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/DiagnosticEventTest.cpp

Index: lldb/unittests/Core/DiagnosticEventTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/DiagnosticEventTest.cpp
@@ -0,0 +1,185 @@
+//===-- DiagnosticEventTest.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/MacOSX/PlatformMacOSX.h"
+#include "Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/DebuggerEvents.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Utility/Broadcaster.h"
+#include "lldb/Utility/Event.h"
+#include "lldb/Utility/Listener.h"
+#include "lldb/Utility/Reproducer.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::repro;
+
+static const constexpr std::chrono::seconds TIMEOUT(0);
+static const constexpr size_t DEBUGGERS = 3;
+
+static std::once_flag debugger_initialize_flag;
+
+namespace {
+class DiagnosticEventTest : public ::testing::Test {
+public:
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+FileSystem::Initialize();
+HostInfo::Initialize();
+PlatformMacOSX::Initialize();
+std::call_once(debugger_initialize_flag,
+   []() { Debugger::Initialize(nullptr); });
+  }
+  void TearDown() override {
+PlatformMacOSX::Terminate();
+HostInfo::Terminate();
+FileSystem::Terminate();
+Reproducer::Terminate();
+  }
+};
+} // namespace
+
+TEST_F(DiagnosticEventTest, Warning) {
+  ArchSpec arch("x86_64-apple-macosx-");
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+
+  Broadcaster &broadcaster = debugger_sp->GetBroadcaster();
+  ListenerSP listener_sp = Listener::MakeListener("test-listener");
+
+  listener_sp->StartListeningForEvents(&broadcaster,
+   Debugger::eBroadcastBitWarning);
+  EXPECT_TRUE(
+  broadcaster.EventTypeHasListeners(Debugger::eBroadcastBitWarning));
+
+  Debugger::ReportWarning("foo", debugger_sp->GetID());
+
+  EventSP event_sp;
+  EXPECT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+  ASSERT_TRUE(event_sp);
+
+  const DiagnosticEventData *data =
+  DiagnosticEventData::GetEventDataFromEvent(event_sp.get());
+  ASSERT_NE(data, nullptr);
+  EXPECT_EQ(data->GetPrefix(), "warning");
+  EXPECT_EQ(data->GetMessage(), "foo");
+
+  Debugger::Destroy(debugger_sp);
+}
+
+TEST_F(DiagnosticEventTest, Error) {
+  ArchSpec arch("x86_64-apple-macosx-");
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+
+  Broadcaster &broadcaster = debugger_sp->GetBroadcaster();
+  ListenerSP listener_sp = Listener::MakeListener("test-listener");
+
+  listener_sp->StartListeningForEvents(&broadcaster,
+   Debugger::eBroadcastBitError);
+  EXPECT_TRUE(broadcaster.EventTypeHasListeners(Debugger::eBroadcastBitError));
+
+  Debugger::ReportError("bar", debugger_sp->GetID());
+
+  EventSP event_sp;
+  EXPECT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+  ASSERT_TRUE(event_sp);
+
+  const DiagnosticEventData *data =
+  DiagnosticEventData::GetEventDataFromEvent(event_sp.get());
+  ASSERT_NE(data, nullptr);
+  EXPECT_EQ(data->GetPrefix(), "error");
+  EXPECT_EQ(data->GetMessage(), "bar");
+
+  Debugger::Destroy(debugger_sp);
+}
+
+TEST_F(DiagnosticEventTest, MultipleDebuggers) {
+  ArchSpec arch("x86_64-apple-macosx-");
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+
+  std::vector debuggers;
+  std::vector listeners;
+
+  for (size_t i = 0; i < DEBUGGERS; ++i) {
+DebuggerSP debugger = Debugger::CreateInstance();
+ListenerSP listener = Listener::MakeListener("listener");
+
+debuggers.push_back(debugger);
+listeners.push_back(listener);
+
+

[Lldb-commits] [PATCH] D121511: [lldb] Report debugger diagnostics as events

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Migrate a few more call sites to `Debugger::Report{Error,Warning}`


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

https://reviews.llvm.org/D121511

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/DebuggerEvents.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/DebuggerEvents.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/DiagnosticEventTest.cpp

Index: lldb/unittests/Core/DiagnosticEventTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/DiagnosticEventTest.cpp
@@ -0,0 +1,185 @@
+//===-- DiagnosticEventTest.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/MacOSX/PlatformMacOSX.h"
+#include "Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/DebuggerEvents.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Utility/Broadcaster.h"
+#include "lldb/Utility/Event.h"
+#include "lldb/Utility/Listener.h"
+#include "lldb/Utility/Reproducer.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::repro;
+
+static const constexpr std::chrono::seconds TIMEOUT(0);
+static const constexpr size_t DEBUGGERS = 3;
+
+static std::once_flag debugger_initialize_flag;
+
+namespace {
+class DiagnosticEventTest : public ::testing::Test {
+public:
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+FileSystem::Initialize();
+HostInfo::Initialize();
+PlatformMacOSX::Initialize();
+std::call_once(debugger_initialize_flag,
+   []() { Debugger::Initialize(nullptr); });
+  }
+  void TearDown() override {
+PlatformMacOSX::Terminate();
+HostInfo::Terminate();
+FileSystem::Terminate();
+Reproducer::Terminate();
+  }
+};
+} // namespace
+
+TEST_F(DiagnosticEventTest, Warning) {
+  ArchSpec arch("x86_64-apple-macosx-");
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+
+  Broadcaster &broadcaster = debugger_sp->GetBroadcaster();
+  ListenerSP listener_sp = Listener::MakeListener("test-listener");
+
+  listener_sp->StartListeningForEvents(&broadcaster,
+   Debugger::eBroadcastBitWarning);
+  EXPECT_TRUE(
+  broadcaster.EventTypeHasListeners(Debugger::eBroadcastBitWarning));
+
+  Debugger::ReportWarning("foo", debugger_sp->GetID());
+
+  EventSP event_sp;
+  EXPECT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+  ASSERT_TRUE(event_sp);
+
+  const DiagnosticEventData *data =
+  DiagnosticEventData::GetEventDataFromEvent(event_sp.get());
+  ASSERT_NE(data, nullptr);
+  EXPECT_EQ(data->GetPrefix(), "warning");
+  EXPECT_EQ(data->GetMessage(), "foo");
+
+  Debugger::Destroy(debugger_sp);
+}
+
+TEST_F(DiagnosticEventTest, Error) {
+  ArchSpec arch("x86_64-apple-macosx-");
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+
+  Broadcaster &broadcaster = debugger_sp->GetBroadcaster();
+  ListenerSP listener_sp = Listener::MakeListener("test-listener");
+
+  listener_sp->StartListeningForEvents(&broadcaster,
+   Debugger::eBroadcastBitError);
+  EXPECT_TRUE(broadcaster.EventTypeHasListeners(Debugger::eBroadcastBitError));
+
+  Debugger::ReportError("bar", debugger_sp->GetID());
+
+  EventSP event_sp;
+  EXPECT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+  ASSERT_TRUE(event_sp);
+
+  const DiagnosticEventData *data =
+  DiagnosticEventData::GetEventDataFromEvent(event_sp.get());
+  ASSERT_NE(data, nullptr);
+  EXPECT_EQ(data->GetPrefix(), "error");
+  EXPECT_EQ(data->GetMessage(), "bar");
+
+  Debugger::Destroy(debugger_sp);
+}
+
+TEST_F(DiagnosticEventTest, MultipleDebuggers) {
+  ArchSpec arch("x86_64-apple-macosx-");
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+
+  

[Lldb-commits] [PATCH] D121511: [lldb] Report debugger diagnostics as events

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 415699.

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

https://reviews.llvm.org/D121511

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/DebuggerEvents.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/DebuggerEvents.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/DiagnosticEventTest.cpp

Index: lldb/unittests/Core/DiagnosticEventTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/DiagnosticEventTest.cpp
@@ -0,0 +1,185 @@
+//===-- DiagnosticEventTest.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/MacOSX/PlatformMacOSX.h"
+#include "Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/DebuggerEvents.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Utility/Broadcaster.h"
+#include "lldb/Utility/Event.h"
+#include "lldb/Utility/Listener.h"
+#include "lldb/Utility/Reproducer.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::repro;
+
+static const constexpr std::chrono::seconds TIMEOUT(0);
+static const constexpr size_t DEBUGGERS = 3;
+
+static std::once_flag debugger_initialize_flag;
+
+namespace {
+class DiagnosticEventTest : public ::testing::Test {
+public:
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+FileSystem::Initialize();
+HostInfo::Initialize();
+PlatformMacOSX::Initialize();
+std::call_once(debugger_initialize_flag,
+   []() { Debugger::Initialize(nullptr); });
+  }
+  void TearDown() override {
+PlatformMacOSX::Terminate();
+HostInfo::Terminate();
+FileSystem::Terminate();
+Reproducer::Terminate();
+  }
+};
+} // namespace
+
+TEST_F(DiagnosticEventTest, Warning) {
+  ArchSpec arch("x86_64-apple-macosx-");
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+
+  Broadcaster &broadcaster = debugger_sp->GetBroadcaster();
+  ListenerSP listener_sp = Listener::MakeListener("test-listener");
+
+  listener_sp->StartListeningForEvents(&broadcaster,
+   Debugger::eBroadcastBitWarning);
+  EXPECT_TRUE(
+  broadcaster.EventTypeHasListeners(Debugger::eBroadcastBitWarning));
+
+  Debugger::ReportWarning("foo", debugger_sp->GetID());
+
+  EventSP event_sp;
+  EXPECT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+  ASSERT_TRUE(event_sp);
+
+  const DiagnosticEventData *data =
+  DiagnosticEventData::GetEventDataFromEvent(event_sp.get());
+  ASSERT_NE(data, nullptr);
+  EXPECT_EQ(data->GetPrefix(), "warning");
+  EXPECT_EQ(data->GetMessage(), "foo");
+
+  Debugger::Destroy(debugger_sp);
+}
+
+TEST_F(DiagnosticEventTest, Error) {
+  ArchSpec arch("x86_64-apple-macosx-");
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+
+  Broadcaster &broadcaster = debugger_sp->GetBroadcaster();
+  ListenerSP listener_sp = Listener::MakeListener("test-listener");
+
+  listener_sp->StartListeningForEvents(&broadcaster,
+   Debugger::eBroadcastBitError);
+  EXPECT_TRUE(broadcaster.EventTypeHasListeners(Debugger::eBroadcastBitError));
+
+  Debugger::ReportError("bar", debugger_sp->GetID());
+
+  EventSP event_sp;
+  EXPECT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+  ASSERT_TRUE(event_sp);
+
+  const DiagnosticEventData *data =
+  DiagnosticEventData::GetEventDataFromEvent(event_sp.get());
+  ASSERT_NE(data, nullptr);
+  EXPECT_EQ(data->GetPrefix(), "error");
+  EXPECT_EQ(data->GetMessage(), "bar");
+
+  Debugger::Destroy(debugger_sp);
+}
+
+TEST_F(DiagnosticEventTest, MultipleDebuggers) {
+  ArchSpec arch("x86_64-apple-macosx-");
+  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+
+  std::vector debuggers;
+  std::vector listeners;
+
+  for (size_t i = 0; i < DEBUGGERS; ++i) {
+