[Lldb-commits] [lldb] [lldb] Add QSupported key to report watchpoint types supported (PR #80376)
bulbazord wrote: > @bulbazord in the most recent commit I moved this internal-only enum from > lldb-enumerations.h to lldb-private-enumerations.h, but I need to use the > constexpr templatey thing that LLDB_MARK_AS_BITMASK_ENUM() defines for the > enum so I can use bit-wise | & operations without casting everywhere; that's > defined in lldb-enumerations.h so I included the public enums in the > lldb-private-enumerations.h. It seems like it's probably not a great choice, > but the other one is breaking out this and FLAGS_ENUM etc into a little > lldb-common-enumerations.h or something. What do you think? Not the greatest thing in the world but it's not terrible either. We use the lldb public enumerations everywhere in private code, so as long as we're not going the other way this is ok to do. I will say, by moving the definitions from `lldb-enumerations.h` to `lldb-private-enumerations.h` you have removed these values from the python bindings. Technically that's an API break, but I'm not sure where anyone could have used these values otherwise. LGTM. https://github.com/llvm/llvm-project/pull/80376 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add QSupported key to report watchpoint types supported (PR #80376)
https://github.com/bulbazord approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/80376 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
https://github.com/bulbazord edited https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
https://github.com/bulbazord requested changes to this pull request. Haven't looked over everything yet but this has an ABI-breaking change. Please revert the existing changes to SBTarget. https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -86,9 +86,13 @@ class LLDB_API SBTarget {
/// Returns a dump of the collected statistics.
///
+ /// \param[in] summary_only
+ /// If true, only report high level summary statistics without
+ /// targets/modules/breakpoints etc.. details.
+ ///
/// \return
/// A SBStructuredData with the statistics collected.
- lldb::SBStructuredData GetStatistics();
bulbazord wrote:
This is an ABI-breaking change. You'll want to add a new method entirely.
https://github.com/llvm/llvm-project/pull/80745
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -186,6 +186,10 @@ class SymbolFileDWARF : public SymbolFileCommon {
GetMangledNamesForFunction(const std::string &scope_qualified_name,
std::vector &mangled_names) override;
+ // Return total currently loaded debug info.
+ // For cases like .dwo files, the debug info = skeleton debug info + all dwo
+ // debug info where .dwo files might not be loaded yet. Calling this function
+ // will not force the loading of any .dwo files.
bulbazord wrote:
Could you make this into a proper doxygen comment? With `///` instead of `//`
https://github.com/llvm/llvm-project/pull/80745
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -241,7 +241,7 @@ class DWARFUnit : public UserID {
FileSpec GetFile(size_t file_idx);
FileSpec::Style GetPathStyle();
- SymbolFileDWARFDwo *GetDwoSymbolFile();
+ SymbolFileDWARFDwo *GetDwoSymbolFile(bool load_if_needed = true);
bulbazord wrote:
Instead of having a bool here, you could make this an enum with 2 values.
Something like this:
```
enum LoadMode : bool { eDoNotForceLoad, eForceLoad };
```
https://github.com/llvm/llvm-project/pull/80745
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
bulbazord wrote: I'm not sure I understand. Why do you need to get the path to the core file through the SBAPI? Didn't you also load it through the SBAPI too? https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Expand background symbol lookup (PR #80890)
@@ -5,10 +5,11 @@ let Definition = "modulelist" in {
Global,
DefaultTrue,
Desc<"Control the use of external tools and repositories to locate symbol
files. Directories listed in target.debug-file-search-paths and directory of
the executable are always checked first for separate debug info files. Then
depending on this setting: On macOS, Spotlight would be also used to locate a
matching .dSYM bundle based on the UUID of the executable. On NetBSD, directory
/usr/libdata/debug would be also searched. On platforms other than NetBSD
directory /usr/lib/debug would be also searched. If all other methods fail
there may be symbol-locator plugins that, if configured properly, will also
attempt to acquire symbols. The debuginfod plugin defaults to the
DEGUFINFOD_URLS environment variable which is configurable through the
'plugin.symbol-locator.debuginfod.server_urls' setting.">;
- def EnableBackgroundLookup: Property<"enable-background-lookup", "Boolean">,
+ def LazySymbolLookup: Property<"lazy-lookup", "Enum">,
bulbazord wrote:
I think the name should probably be different. "LazySymbolLookup" being set to
off makes me think it will look things up eagerly. Looking up the possible
values of "off", "background", and "foreground" also don't really illustrate
what they mean either.
I would propose the setting name "external-symbol-load-behavior" with the
enumeration values "off", "background", and "blocking" or something to this
effect.
https://github.com/llvm/llvm-project/pull/80890
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Expand background symbol lookup (PR #80890)
@@ -5,10 +5,11 @@ let Definition = "modulelist" in {
Global,
DefaultTrue,
Desc<"Control the use of external tools and repositories to locate symbol
files. Directories listed in target.debug-file-search-paths and directory of
the executable are always checked first for separate debug info files. Then
depending on this setting: On macOS, Spotlight would be also used to locate a
matching .dSYM bundle based on the UUID of the executable. On NetBSD, directory
/usr/libdata/debug would be also searched. On platforms other than NetBSD
directory /usr/lib/debug would be also searched. If all other methods fail
there may be symbol-locator plugins that, if configured properly, will also
attempt to acquire symbols. The debuginfod plugin defaults to the
DEGUFINFOD_URLS environment variable which is configurable through the
'plugin.symbol-locator.debuginfod.server_urls' setting.">;
- def EnableBackgroundLookup: Property<"enable-background-lookup", "Boolean">,
+ def LazySymbolLookup: Property<"lazy-lookup", "Enum">,
bulbazord wrote:
`symbol-loading-behavior`? `symbol-lookup-strategy`? 😄
https://github.com/llvm/llvm-project/pull/80890
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Expand background symbol lookup (PR #80890)
https://github.com/bulbazord approved this pull request. I'm happy w/ the name. Thanks! https://github.com/llvm/llvm-project/pull/80890 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Don't count all the frames just to skip the current inlined ones. (PR #80918)
@@ -608,11 +608,10 @@ static bool Evaluate_DW_OP_entry_value(std::vector
&stack,
StackFrameSP parent_frame = nullptr;
addr_t return_pc = LLDB_INVALID_ADDRESS;
uint32_t current_frame_idx = current_frame->GetFrameIndex();
- uint32_t num_frames = thread->GetStackFrameCount();
- for (uint32_t parent_frame_idx = current_frame_idx + 1;
- parent_frame_idx < num_frames; ++parent_frame_idx) {
+
+ for (uint32_t parent_frame_idx = current_frame_idx + 1;;parent_frame_idx++) {
bulbazord wrote:
Suggestion: If you initialize `parent_frame` to
`thread->GetStackFrameAtIndex(current_frame_idx + 1)` and move the
`parent_frame = ...` bit to the end of the loop, you can have the loop
condition be `parent_frame != nullptr` instead of relying on a break statement.
https://github.com/llvm/llvm-project/pull/80918
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add comment to ParseInternal in FormatEntity (NFC) (PR #81018)
@@ -2204,7 +2204,9 @@ static Status ParseInternal(llvm::StringRef &format,
Entry &parent_entry,
return error;
}
} else if (FormatManager::GetFormatFromCString(
- entry.printf_format.c_str(), true, entry.fmt)) {
+ entry.printf_format.c_str(), true,
+ entry.fmt)) { // Try GetFormatFromCString again,
bulbazord wrote:
Is the ordering important here? Why not try partial formatting first from the
start? The code paths are nearly identical except for one parameter... The
comment definitely helps disambiguate but it would be nicer if we could just
rewrite this entirely.
https://github.com/llvm/llvm-project/pull/81018
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Refactor GetFormatFromCString to always check for partial matches (NFC) (PR #81018)
https://github.com/bulbazord approved this pull request. Makes sense to me. If the test suite is happy then I think this is fine. https://github.com/llvm/llvm-project/pull/81018 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [NFC] Remove min pkt size for compression setting (PR #81075)
https://github.com/bulbazord approved this pull request. LGTM. I also noticed there's no tests for this functionality... https://github.com/llvm/llvm-project/pull/81075 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add the ability to define a Python based command that uses CommandObjectParsed (PR #70734)
@@ -0,0 +1,315 @@ +""" +This module implements a couple of utility classes to make writing +lldb parsed commands more Pythonic. +The way to use it is to make a class for you command that inherits from ParsedCommandBase. +That will make an LLDBOVParser which you will use for your +option definition, and to fetch option values for the current invocation bulbazord wrote: I don't remember why I suggested this, it looks pretty clear to me after another pass. https://github.com/llvm/llvm-project/pull/70734 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add the ability to define a Python based command that uses CommandObjectParsed (PR #70734)
https://github.com/bulbazord approved this pull request. Ok, I think this is good to go in. There are things that can be improved but it would be better to land this and change things in follow up PRs instead of continually adjusting this one. https://github.com/llvm/llvm-project/pull/70734 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] checks beforehand if lldb can trace/attach a process on FreeBSD. (PR #79662)
https://github.com/bulbazord approved this pull request. LGTM but I'm not really an expert on FreeBSD. @emaste ? https://github.com/llvm/llvm-project/pull/79662 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Docs] Replace LLDB_RELOCATABLE_PYTHON with LLDB_EMBED_PYTHON_HOME (PR #81310)
https://github.com/bulbazord approved this pull request. https://github.com/llvm/llvm-project/pull/81310 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Store proper integer bitwidth in Scalar Type (PR #81451)
https://github.com/bulbazord requested changes to this pull request. This uses `DataExtractor::GetMaxU64` which already does this under the hood. What does this do that isn't already being done? It may help if you add a test case to show what you are trying to fix. https://github.com/llvm/llvm-project/pull/81451 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFCI] Add header guard to PlatformRemoteAppleXR.h (PR #81565)
https://github.com/bulbazord created
https://github.com/llvm/llvm-project/pull/81565
None
>From a1bb825c34a951a0d99ecf03fe86545ed43bbe42 Mon Sep 17 00:00:00 2001
From: Alex Langford
Date: Mon, 12 Feb 2024 19:05:55 -0800
Subject: [PATCH] [lldb][NFCI] Add header guard to PlatformRemoteAppleXR.h
---
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h
b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h
index 4fed6e15eda31c..a3e83b217149a0 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h
@@ -6,6 +6,9 @@
//
//===--===//
+#ifdef LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMREMOTEAPPLEXR_H
+#define LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMREMOTEAPPLEXR_H
+
#include "PlatformRemoteDarwinDevice.h"
namespace lldb_private {
@@ -36,3 +39,5 @@ class PlatformRemoteAppleXR : public
PlatformRemoteDarwinDevice {
llvm::StringRef GetPlatformName() override;
};
} // namespace lldb_private
+
+#endif // LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMREMOTEAPPLEXR_H
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFCI] Add header guard to PlatformRemoteAppleXR.h (PR #81565)
https://github.com/bulbazord updated
https://github.com/llvm/llvm-project/pull/81565
>From a1bb825c34a951a0d99ecf03fe86545ed43bbe42 Mon Sep 17 00:00:00 2001
From: Alex Langford
Date: Mon, 12 Feb 2024 19:05:55 -0800
Subject: [PATCH 1/2] [lldb][NFCI] Add header guard to PlatformRemoteAppleXR.h
---
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h
b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h
index 4fed6e15eda31c..a3e83b217149a0 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h
@@ -6,6 +6,9 @@
//
//===--===//
+#ifdef LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMREMOTEAPPLEXR_H
+#define LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMREMOTEAPPLEXR_H
+
#include "PlatformRemoteDarwinDevice.h"
namespace lldb_private {
@@ -36,3 +39,5 @@ class PlatformRemoteAppleXR : public
PlatformRemoteDarwinDevice {
llvm::StringRef GetPlatformName() override;
};
} // namespace lldb_private
+
+#endif // LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMREMOTEAPPLEXR_H
>From 916014411b73cc3132f0d2cc64438ca080d67b3e Mon Sep 17 00:00:00 2001
From: Alex Langford
Date: Mon, 12 Feb 2024 19:14:07 -0800
Subject: [PATCH 2/2] ifdef -> ifndef
---
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h
b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h
index a3e83b217149a0..2fbb6caad8110f 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleXR.h
@@ -6,7 +6,7 @@
//
//===--===//
-#ifdef LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMREMOTEAPPLEXR_H
+#ifndef LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMREMOTEAPPLEXR_H
#define LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMREMOTEAPPLEXR_H
#include "PlatformRemoteDarwinDevice.h"
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFCI] Add header guard to PlatformRemoteAppleXR.h (PR #81565)
https://github.com/bulbazord closed https://github.com/llvm/llvm-project/pull/81565 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Detect a Darwin kernel issue and work around it (PR #81573)
@@ -825,6 +806,56 @@ StopInfoSP StopInfoMachException::CreateStopReasonWithMachException( break; } - return StopInfoSP(new StopInfoMachException(thread, exc_type, exc_data_count, - exc_code, exc_sub_code)); + return StopInfoSP(new StopInfoMachException( bulbazord wrote: Since you're already here: `make_shared`? 😄 https://github.com/llvm/llvm-project/pull/81573 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Detect a Darwin kernel issue and work around it (PR #81573)
https://github.com/bulbazord commented: Makes sense to me! https://github.com/llvm/llvm-project/pull/81573 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Detect a Darwin kernel issue and work around it (PR #81573)
@@ -79,6 +79,11 @@ class StopInfo : public
std::enable_shared_from_this {
virtual bool IsValidForOperatingSystemThread(Thread &thread) { return true; }
+ /// A Continue operation can result in a false stop event
+ /// before any execution has happened in certain cases on Darwin.
+ /// We need to silently continue again time.
bulbazord wrote:
"continue again time." -> "continue again one more time."
Is this what you meant?
https://github.com/llvm/llvm-project/pull/81573
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Detect a Darwin kernel issue and work around it (PR #81573)
https://github.com/bulbazord edited https://github.com/llvm/llvm-project/pull/81573 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Store proper integer bitwidth in Scalar Type (PR #81451)
bulbazord wrote: > > This uses `DataExtractor::GetMaxU64` which already does this under the > > hood. What does this do that isn't already being done? It may help if you > > add a test case to show what you are trying to fix. > > @clayborg @bulbazord The problem with GetMaxU64 is that it always returns > uint64_t even though actual type was uint32_t, so when byteswap is performed > it becomes invalid integer, to fixed this we need to store correct bitwidth > not higher. i.e. Suppose there is actual 32 bit integer i.e. 0x > `GetMaxU64` will return 0x (prmoted to uint64_t from > uint32_t) and when performing byteswap on this it becomes 0x > which is invalid. That makes sense to me, but how are you encountering this behavior? What specifically is this fixing? If you can write a test case that fails without your change but works with your change, that would help us understand what you're trying to fix. Correctness is important, and tests help us verify that we're not inadvertently introducing regressions. https://github.com/llvm/llvm-project/pull/81451 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFCI] Remove CommandObjectProcessHandle::VerifyCommandOptionValue (PR #79901)
bulbazord wrote: ping @clayborg How does this look now? https://github.com/llvm/llvm-project/pull/79901 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFCI] Remove CommandObjectProcessHandle::VerifyCommandOptionValue (PR #79901)
@@ -1666,33 +1646,52 @@ class CommandObjectProcessHandle : public
CommandObjectParsed {
// the user's options.
ProcessSP process_sp = target.GetProcessSP();
-int stop_action = -1; // -1 means leave the current setting alone
-int pass_action = -1; // -1 means leave the current setting alone
-int notify_action = -1; // -1 means leave the current setting alone
+std::optional stop_action = {};
+std::optional pass_action = {};
+std::optional notify_action = {};
-if (!m_options.stop.empty() &&
-!VerifyCommandOptionValue(m_options.stop, stop_action)) {
- result.AppendError("Invalid argument for command option --stop; must be "
- "true or false.\n");
- return;
+if (!m_options.stop.empty()) {
+ bool success = false;
+ bool value = OptionArgParser::ToBoolean(m_options.stop, false, &success);
bulbazord wrote:
My plan (that I wrote up 2 weeks ago) was to remove `VerifyCommandOptionValue`
entirely in this commit and then refactor `ToBoolean` the way you suggested as
a follow-up to this change. Does that sound good to you?
https://github.com/llvm/llvm-project/pull/79901
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFCI] Remove CommandObjectProcessHandle::VerifyCommandOptionValue (PR #79901)
https://github.com/bulbazord closed https://github.com/llvm/llvm-project/pull/79901 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Report only loaded debug info in statistics dump (PR #81706)
@@ -224,6 +224,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
const lldb_private::StatisticsOptions &options) {
const bool summary_only = options.summary_only;
+ const bool force_laoding = options.force_loading;
bulbazord wrote:
typo in this one
https://github.com/llvm/llvm-project/pull/81706
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Report only loaded debug info in statistics dump (PR #81706)
https://github.com/bulbazord commented: Idea looks ok, the naming could use a bit of work. Right now it feels a bit generic and I'm not sure what "loading" means. I like Greg's suggestion here. https://github.com/llvm/llvm-project/pull/81706 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Store proper integer bitwidth in Scalar Type (PR #81451)
bulbazord wrote: > @clayborg @bulbazord we have extension in lldb to support cobol code > debugging, we require byteswapping there. upstream version of lldb does not > do byteswapping on scalar so no problem seen. Maybe not, but this should still be testable with a unit test. If somebody modifies or refactors this code in the future without a test, this might break in a similar way and you would have to do all this work again. :) https://github.com/llvm/llvm-project/pull/81451 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Centralize the handling of completion for simple argument lists. (PR #82085)
@@ -305,6 +305,42 @@ void CommandObject::HandleCompletion(CompletionRequest
&request) {
}
}
+void
+CommandObject::HandleArgumentCompletion(CompletionRequest &request,
+ OptionElementVector &opt_element_vector) {
+ size_t num_arg_entries = GetNumArgumentEntries();
+ if (num_arg_entries != 1)
+return;
+
+ CommandArgumentEntry *entry_ptr = GetArgumentEntryAtIndex(0);
+ if (!entry_ptr)
+return; // Maybe this should be an assert, this shouldn't be possible.
bulbazord wrote:
If it shouldn't be possible, I think an assert is warranted here. I would say
add the assertion above the early return (and keep the early return since the
assertion will be compiled out in release builds).
https://github.com/llvm/llvm-project/pull/82085
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Centralize the handling of completion for simple argument lists. (PR #82085)
@@ -243,7 +243,7 @@ static constexpr CommandObject::ArgumentTableEntry
g_argument_table[] = {
{ lldb::eArgTypeLogCategory, "log-category",
lldb::CompletionType::eNoCompletion, {}, { nullptr, false }, "The name of a
category within a log channel, e.g. all (try \"log list\" to see a list of all
channels and their categories." },
{ lldb::eArgTypeLogChannel, "log-channel",
lldb::CompletionType::eNoCompletion, {}, { nullptr, false }, "The name of a log
channel, e.g. process.gdb-remote (try \"log list\" to see a list of all
channels and their categories)." },
{ lldb::eArgTypeMethod, "method", lldb::CompletionType::eNoCompletion, {},
{ nullptr, false }, "A C++ method name." },
-{ lldb::eArgTypeName, "name", lldb::eTypeCategoryNameCompletion, {}, {
nullptr, false }, "Help text goes here." },
bulbazord wrote:
Help text DOES go here! :)
https://github.com/llvm/llvm-project/pull/82085
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Centralize the handling of completion for simple argument lists. (PR #82085)
@@ -1139,6 +1097,15 @@ class CommandObjectPlatformProcessLaunch : public
CommandObjectParsed {
m_arguments.push_back({run_arg_arg});
}
+ void
+ HandleArgumentCompletion(CompletionRequest &request,
+ OptionElementVector &opt_element_vector) override {
bulbazord wrote:
Any way you could move this to the generic CommandObject implementation?
https://github.com/llvm/llvm-project/pull/82085
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Call Import_AppendInittab exactly once before Py_Initialize (PR #82095)
https://github.com/bulbazord approved this pull request. Makes sense to me. https://github.com/llvm/llvm-project/pull/82095 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Don't rely on ScriptInterpreterPythonImpl::Initialize in the unit tests (PR #82096)
bulbazord wrote: > The downside of doing the initialization manually is that we do lose a bit of > test coverage. For example, issue #70453 also manifested itself in the unit > tests. I think this is an acceptable tradeoff. The unit tests are for testing LLDB's python internals, not for testing LLDB's python interface and integration. https://github.com/llvm/llvm-project/pull/82096 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Don't rely on ScriptInterpreterPythonImpl::Initialize in the unit tests (PR #82096)
https://github.com/bulbazord approved this pull request. https://github.com/llvm/llvm-project/pull/82096 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Standardize command option parsing error messages (PR #82273)
https://github.com/bulbazord created
https://github.com/llvm/llvm-project/pull/82273
I have been looking to simplify parsing logic and improve the interfaces so
that they are both easier to use and harder to abuse. To be specific, I am
referring to functions such as `OptionArgParser::ToBoolean`: I would like to go
from its current interface to something more like `llvm::Error
ToBoolean(llvm::StringRef option_arg)`.
Through working on that, I encountered 2 inconveniences:
1. Option parsing code is not uniform. Every function writes a slightly
different error message, so incorporating an error message from the `ToBoolean`
implementation is going to be laborious as I figure out what exactly needs to
change or stay the same.
2. Changing the interface of `ToBoolean` would require a global atomic change
across all of the Command code. This would be quite frustrating to do because
of the non-uniformity of our existing code.
To address these frustrations, I think it would be easiest to first standardize
the error reporting mechanism when parsing options in commands. I do so by
introducing `CreateOptionParsingError` which will create an error message of
the shape:
Invalid valud ('${option_arg}') for -${short_value} ('${long_value}'):
${additional_context}
Concretely, it would look something like this:
(lldb) breakpoint set -n main -G yay
error: Invalid value ('yay') for -G (auto-continue): Failed to parse as boolean
After this, updating the interfaces for parsing the values themselves should
become simpler. Because this can be adopted incrementally, this should be able
to done over the course of time instead of all at once as a giant
difficult-to-review change. I've changed exactly one function where this
function would be used as an illustration of what I am proposing.
>From 062ec05006182838c8b0043051741ae6c26c2520 Mon Sep 17 00:00:00 2001
From: Alex Langford
Date: Thu, 15 Feb 2024 17:39:42 -0800
Subject: [PATCH] [lldb] Standardize command option parsing error messages
I have been looking to simplify parsing logic and improve the interfaces
so that they are both easier to use and harder to abuse. To be specific,
I am referring to functions such as `OptionArgParser::ToBoolean`: I
would like to go from its current interface to something more like
`llvm::Error ToBoolean(llvm::StringRef option_arg)`.
Through working on that, I encountered 2 inconveniences:
1. Option parsing code is not uniform. Every function writes a slightly
different error message, so incorporating an error message from the
`ToBoolean` implementation is going to be laborious as I figure out
what exactly needs to change or stay the same.
2. Changing the interface of `ToBoolean` would require a global atomic
change across all of the Command code. This would be quite
frustrating to do because of the non-uniformity of our existing code.
To address these frustrations, I think it would be easiest to first
standardize the error reporting mechanism when parsing options in
commands. I do so by introducing `CreateOptionParsingError` which will
create an error message of the shape:
Invalid valud ('${option_arg}') for -${short_value} ('${long_value}'):
${additional_context}
Concretely, it would look something like this:
(lldb) breakpoint set -n main -G yay
error: Invalid value ('yay') for -G (auto-continue): Failed to parse as boolean
After this, updating the interfaces for parsing the values themselves
should become simpler. Because this can be adopted incrementally, this
should be able to done over the course of time instead of all at once as
a giant difficult-to-review change. I've changed exactly one function
where this function would be used as an illustration of what I am
proposing.
---
lldb/include/lldb/Interpreter/Options.h | 10 +
.../Commands/CommandObjectBreakpoint.cpp | 37 ++-
lldb/source/Interpreter/Options.cpp | 13 +++
lldb/unittests/Interpreter/CMakeLists.txt | 1 +
lldb/unittests/Interpreter/TestOptions.cpp| 29 +++
5 files changed, 73 insertions(+), 17 deletions(-)
create mode 100644 lldb/unittests/Interpreter/TestOptions.cpp
diff --git a/lldb/include/lldb/Interpreter/Options.h
b/lldb/include/lldb/Interpreter/Options.h
index bf74927cf99db8..3351fb517d4df3 100644
--- a/lldb/include/lldb/Interpreter/Options.h
+++ b/lldb/include/lldb/Interpreter/Options.h
@@ -336,6 +336,16 @@ class OptionGroupOptions : public Options {
bool m_did_finalize = false;
};
+llvm::Error CreateOptionParsingError(llvm::StringRef option_arg,
+ const char short_option,
+ llvm::StringRef long_option = {},
+ llvm::StringRef additional_context = {});
+
+static constexpr llvm::StringLiteral g_bool_parsing_error_message =
+"Failed to parse as boolean";
+static constexpr llvm::StringLiteral g_int_parsing_error_message =
+"Failed to parse as in
[Lldb-commits] [lldb] Centralize the handling of completion for simple argument lists. (PR #82085)
https://github.com/bulbazord approved this pull request. LGTM, thanks! https://github.com/llvm/llvm-project/pull/82085 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Standardize command option parsing error messages (PR #82273)
bulbazord wrote: > This LGTM! > > I don't think I can see far enough ahead on what you are planning here, but I > was just wondering if the ultimate goal is to have the `option_arg.getAsT` > return an `Expected`. In this case, wouldn't all these arguments (short > option, long option, additional context) have to be part of the interface of > `getAsT`? I suspect this is not your goal, but I can't see a way around that The ultimate goal is to have option parsing be much more streamlined and automated. The majority of option parsing is just taking some primitive value and trying to grab its value from a string. There are places where we need additional context (e.g. in this PR, there's a place where we want to select a thread ID so there's more validation needed). I want to be able to write those pretty naturally too. The first step in my plan is to centralize how we create error messages. The majority of parsing errors will be "I couldn't turn this into a boolean/integer/address/other", so I'm going to create something that is better than what we have today. The step after that will be to revamp the parsing step. Ideally instead of writing `OptionArgParser::ToBoolean` the way we do today, there will be something like `llvm::Expected value_or_error = ParseAsBool(option);` where `option` is the actual OptionDefinition. I'm not tied to these abstractions, but I am committed to the path of making this easier to write and maintain. https://github.com/llvm/llvm-project/pull/82273 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Standardize command option parsing error messages (PR #82273)
bulbazord wrote: > > > This LGTM! > > > I don't think I can see far enough ahead on what you are planning here, > > > but I was just wondering if the ultimate goal is to have the > > > `option_arg.getAsT` return an `Expected`. In this case, wouldn't all > > > these arguments (short option, long option, additional context) have to > > > be part of the interface of `getAsT`? I suspect this is not your goal, > > > but I can't see a way around that > > > > > > The ultimate goal is to have option parsing be much more streamlined and > > automated. The majority of option parsing is just taking some primitive > > value and trying to grab its value from a string. There are places where we > > need additional context (e.g. in this PR, there's a place where we want to > > select a thread ID so there's more validation needed). I want to be able to > > write those pretty naturally too. > > The first step in my plan is to centralize how we create error messages. > > The majority of parsing errors will be "I couldn't turn this into a > > boolean/integer/address/other", so I'm going to create something that is > > better than what we have today. The step after that will be to revamp the > > parsing step. Ideally instead of writing `OptionArgParser::ToBoolean` the > > way we do today, there will be something like `llvm::Expected > > value_or_error = ParseAsBool(option);` where `option` is the actual > > OptionDefinition. I'm not tied to these abstractions, but I am committed to > > the path of making this easier to write and maintain. > > Ohhh I think I see it now! > > > where `option` is the actual OptionDefinition > > And the `option` itself would know how to report / construct the error > strings? I would like it to be that way. Let's see if it works out! 😄 https://github.com/llvm/llvm-project/pull/82273 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Modernize asserts (PR #82503)
https://github.com/bulbazord approved this pull request. I did a quick manual inspection of all of them, the transformation produced by Teyit looks correct to me. I did notice a few places where we were doing convoluted checks (I left a comment on one) but those should be addressed in follow-ups I think. https://github.com/llvm/llvm-project/pull/82503 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Modernize asserts (PR #82503)
https://github.com/bulbazord edited https://github.com/llvm/llvm-project/pull/82503 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Modernize asserts (PR #82503)
@@ -456,8 +457,9 @@ def queues_with_libBacktraceRecording(self): "doing_the_work_2", "queue 2's pending item #0 should be doing_the_work_2", ) -self.assertTrue( -queue_performer_2.GetPendingItemAtIndex().IsValid() == False, +self.assertEqual( +queue_performer_2.GetPendingItemAtIndex().IsValid(), bulbazord wrote: Might be useful to do an additional pass over these later. I think we could use `assertFalse` here, for example. https://github.com/llvm/llvm-project/pull/82503 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Standardize command option parsing error messages (PR #82273)
https://github.com/bulbazord updated
https://github.com/llvm/llvm-project/pull/82273
>From 790810f9318c7947fe2edd187f60425a85c949b5 Mon Sep 17 00:00:00 2001
From: Alex Langford
Date: Thu, 15 Feb 2024 17:39:42 -0800
Subject: [PATCH 1/2] [lldb] Standardize command option parsing error messages
I have been looking to simplify parsing logic and improve the interfaces
so that they are both easier to use and harder to abuse. To be specific,
I am referring to functions such as `OptionArgParser::ToBoolean`: I
would like to go from its current interface to something more like
`llvm::Error ToBoolean(llvm::StringRef option_arg)`.
Through working on that, I encountered 2 inconveniences:
1. Option parsing code is not uniform. Every function writes a slightly
different error message, so incorporating an error message from the
`ToBoolean` implementation is going to be laborious as I figure out
what exactly needs to change or stay the same.
2. Changing the interface of `ToBoolean` would require a global atomic
change across all of the Command code. This would be quite
frustrating to do because of the non-uniformity of our existing code.
To address these frustrations, I think it would be easiest to first
standardize the error reporting mechanism when parsing options in
commands. I do so by introducing `CreateOptionParsingError` which will
create an error message of the shape:
Invalid valud ('${option_arg}') for -${short_value} ('${long_value}'):
${additional_context}
Concretely, it would look something like this:
(lldb) breakpoint set -n main -G yay
error: Invalid value ('yay') for -G (auto-continue): Failed to parse as boolean
After this, updating the interfaces for parsing the values themselves
should become simpler. Because this can be adopted incrementally, this
should be able to done over the course of time instead of all at once as
a giant difficult-to-review change. I've changed exactly one function
where this function would be used as an illustration of what I am
proposing.
---
lldb/include/lldb/Interpreter/Options.h | 10 +
.../Commands/CommandObjectBreakpoint.cpp | 37 ++-
lldb/source/Interpreter/Options.cpp | 13 +++
lldb/unittests/Interpreter/CMakeLists.txt | 1 +
lldb/unittests/Interpreter/TestOptions.cpp| 29 +++
5 files changed, 73 insertions(+), 17 deletions(-)
create mode 100644 lldb/unittests/Interpreter/TestOptions.cpp
diff --git a/lldb/include/lldb/Interpreter/Options.h
b/lldb/include/lldb/Interpreter/Options.h
index bf74927cf99db8..3351fb517d4df3 100644
--- a/lldb/include/lldb/Interpreter/Options.h
+++ b/lldb/include/lldb/Interpreter/Options.h
@@ -336,6 +336,16 @@ class OptionGroupOptions : public Options {
bool m_did_finalize = false;
};
+llvm::Error CreateOptionParsingError(llvm::StringRef option_arg,
+ const char short_option,
+ llvm::StringRef long_option = {},
+ llvm::StringRef additional_context = {});
+
+static constexpr llvm::StringLiteral g_bool_parsing_error_message =
+"Failed to parse as boolean";
+static constexpr llvm::StringLiteral g_int_parsing_error_message =
+"Failed to parse as integer";
+
} // namespace lldb_private
#endif // LLDB_INTERPRETER_OPTIONS_H
diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp
b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index 3fdf5cd3cd43d2..fc2217608a0bb9 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -64,6 +64,8 @@ class lldb_private::BreakpointOptionGroup : public
OptionGroup {
Status error;
const int short_option =
g_breakpoint_modify_options[option_idx].short_option;
+const char *long_option =
+g_breakpoint_modify_options[option_idx].long_option;
switch (short_option) {
case 'c':
@@ -84,18 +86,17 @@ class lldb_private::BreakpointOptionGroup : public
OptionGroup {
case 'G': {
bool value, success;
value = OptionArgParser::ToBoolean(option_arg, false, &success);
- if (success) {
+ if (success)
m_bp_opts.SetAutoContinue(value);
- } else
-error.SetErrorStringWithFormat(
-"invalid boolean value '%s' passed for -G option",
-option_arg.str().c_str());
+ else
+error = CreateOptionParsingError(option_arg, short_option, long_option,
+ g_bool_parsing_error_message);
} break;
case 'i': {
uint32_t ignore_count;
if (option_arg.getAsInteger(0, ignore_count))
-error.SetErrorStringWithFormat("invalid ignore count '%s'",
- option_arg.str().c_str());
+error = CreateOptionParsingError(option_arg, short_option, long_option,
+ g_int_parsing_error_message);
else
m_bp_o
[Lldb-commits] [lldb] [lldb] Standardize command option parsing error messages (PR #82273)
bulbazord wrote: I've added a doxygen comment to the new function I introduced. I plan on landing this later today if there are no objections. https://github.com/llvm/llvm-project/pull/82273 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Modernize assertEqual(value, bool) (PR #82526)
https://github.com/bulbazord approved this pull request. Thanks! https://github.com/llvm/llvm-project/pull/82526 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Standardize command option parsing error messages (PR #82273)
https://github.com/bulbazord closed https://github.com/llvm/llvm-project/pull/82273 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)
@@ -3944,15 +3955,22 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
// The order of these checks is important.
if (process_does_not_exist (pid_attaching_to)) {
DNBLogError("Tried to attach to pid that doesn't exist");
- std::string return_message = "E96;";
- return_message += cstring_to_asciihex_string("no such process.");
+ std::string return_message = "E96";
bulbazord wrote:
I see this pattern in lots of places, is it worth abstracting out? Something
like:
```
std::string CreateAttachError(const char *additional_context = nullptr) {
std::string message = "E96";
if (additional_context)
message += ";" + cstring_to_asciihex_string(additional_context);
return message;
}
```
What do you think?
https://github.com/llvm/llvm-project/pull/82593
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)
https://github.com/bulbazord approved this pull request. LGTM, seems like a good fix. One question/suggestion. https://github.com/llvm/llvm-project/pull/82593 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)
https://github.com/bulbazord edited https://github.com/llvm/llvm-project/pull/82593 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Remove vendored packages `unittest2` and `progress` (PR #82670)
https://github.com/bulbazord approved this pull request. 🥳 Please wait a little bit for others to look. Thanks for taking care of this! https://github.com/llvm/llvm-project/pull/82670 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][docs] Remove/update docs pointing to unittest2 (PR #82672)
https://github.com/bulbazord approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/82672 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)
@@ -3944,15 +3955,22 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
// The order of these checks is important.
if (process_does_not_exist (pid_attaching_to)) {
DNBLogError("Tried to attach to pid that doesn't exist");
- std::string return_message = "E96;";
- return_message += cstring_to_asciihex_string("no such process.");
+ std::string return_message = "E96";
bulbazord wrote:
Sounds great to me 😄
https://github.com/llvm/llvm-project/pull/82593
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Use CreateOptionParsingError in CommandObjectBreakpoint (PR #83086)
https://github.com/bulbazord created
https://github.com/llvm/llvm-project/pull/83086
This updates the remaining SetOptionValue methods in CommandObjectBreakpoint to
use CreateOptionParsingError.
I found a few minor bugs that were fixed during this refactor (e.g. using the
wrong flag in an error message). That is one of the benefits of centralizing
error message creation.
I also found some option parsing code that is written incorrectly. I do not
make an attempt to update those here because this PR is primarily about
changing existing error handling code, not adding new error handling code.
>From feb4588cd12a5f513061dffa99f6370067f91463 Mon Sep 17 00:00:00 2001
From: Alex Langford
Date: Mon, 26 Feb 2024 16:01:24 -0800
Subject: [PATCH] [lldb] Use CreateOptionParsingError in
CommandObjectBreakpoint
This updates the remaining SetOptionValue methods in
CommandObjectBreakpoint to use CreateOptionParsingError.
I found a few minor bugs that were fixed during this refactor (e.g.
using the wrong flag in an error message). That is one of the benefits
of centralizing error message creation.
I also found some option parsing code that is written incorrectly. I do
not make an attempt to update those here because this PR is primarily
about changing existing error handling code, not adding new error
handling code.
---
lldb/include/lldb/Interpreter/Options.h | 2 +
.../Commands/CommandObjectBreakpoint.cpp | 101 +-
2 files changed, 54 insertions(+), 49 deletions(-)
diff --git a/lldb/include/lldb/Interpreter/Options.h
b/lldb/include/lldb/Interpreter/Options.h
index 18a87e49deee5c..9a6a17c2793fa4 100644
--- a/lldb/include/lldb/Interpreter/Options.h
+++ b/lldb/include/lldb/Interpreter/Options.h
@@ -368,6 +368,8 @@ static constexpr llvm::StringLiteral
g_bool_parsing_error_message =
"Failed to parse as boolean";
static constexpr llvm::StringLiteral g_int_parsing_error_message =
"Failed to parse as integer";
+static constexpr llvm::StringLiteral g_language_parsing_error_message =
+"Unknown language";
} // namespace lldb_private
diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp
b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index fc2217608a0bb9..cfb0b87a59e640 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -266,6 +266,8 @@ class CommandObjectBreakpointSet : public
CommandObjectParsed {
Status error;
const int short_option =
g_breakpoint_set_options[option_idx].short_option;
+ const char *long_option =
+ g_breakpoint_set_options[option_idx].long_option;
switch (short_option) {
case 'a': {
@@ -284,13 +286,15 @@ class CommandObjectBreakpointSet : public
CommandObjectParsed {
case 'u':
if (option_arg.getAsInteger(0, m_column))
- error.SetErrorStringWithFormat("invalid column number: %s",
- option_arg.str().c_str());
+ error =
+ CreateOptionParsingError(option_arg, short_option, long_option,
+ g_int_parsing_error_message);
break;
case 'E': {
LanguageType language =
Language::GetLanguageTypeFromString(option_arg);
+llvm::StringRef error_context;
switch (language) {
case eLanguageTypeC89:
case eLanguageTypeC:
@@ -308,19 +312,18 @@ class CommandObjectBreakpointSet : public
CommandObjectParsed {
m_exception_language = eLanguageTypeObjC;
break;
case eLanguageTypeObjC_plus_plus:
- error.SetErrorStringWithFormat(
- "Set exception breakpoints separately for c++ and objective-c");
+ error_context =
+ "Set exception breakpoints separately for c++ and objective-c";
break;
case eLanguageTypeUnknown:
- error.SetErrorStringWithFormat(
- "Unknown language type: '%s' for exception breakpoint",
- option_arg.str().c_str());
+ error_context = "Unknown language type for exception breakpoint";
break;
default:
- error.SetErrorStringWithFormat(
- "Unsupported language type: '%s' for exception breakpoint",
- option_arg.str().c_str());
+ error_context = "Unsupported language type for exception breakpoint";
}
+if (!error_context.empty())
+ error = CreateOptionParsingError(option_arg, short_option,
+ long_option, error_context);
} break;
case 'f':
@@ -336,9 +339,9 @@ class CommandObjectBreakpointSet : public
CommandObjectParsed {
bool success;
m_catch_bp = OptionArgParser::ToBoolean(option_arg, true, &success);
if (!success)
- error.SetErrorStringWithFormat(
- "Invalid boolean value for on-catch option: '%s'",
- optio
[Lldb-commits] [lldb] [lldb] Use CreateOptionParsingError in CommandObjectBreakpoint (PR #83086)
https://github.com/bulbazord closed https://github.com/llvm/llvm-project/pull/83086 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Exclude third_party packages by default (PR #83191)
https://github.com/bulbazord approved this pull request. LGTM @JDevlieghere @adrian-prantl we should consider adding this to the list of required python modules on our bots too and enforce it with `LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS`? Probably not in this PR, but after we can get our bots configured correctly. https://github.com/llvm/llvm-project/pull/83191 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Ignore swig warnings about shadowed overloads (PR #83317)
https://github.com/bulbazord created
https://github.com/llvm/llvm-project/pull/83317
This specifically addresses the warnings:
$LLVM/lldb/include/lldb/API/SBCommandReturnObject.h:119: Warning 509:
Overloaded method lldb::SBCommandReturnObject::PutCString(char const *)
effectively ignored, $LLVM/lldb/include/lldb/API/SBCommandReturnObject.h:119:
Warning 509: as it is shadowed by lldb::SBCommandReturnObject::PutCString(char
const *,int).
There is exactly one declaration of SBCommandReturnObject::PutCString. The
second parameter (of type `int`) has default value `-1`. Without investigating
why SWIG believes there are 2 method declarations, I believe it is safe to
ignore this warning. It does not appear to actually impact functionality in any
way.
rdar://117744660
>From 76a634a1ad00e90983391cdd04588f92b5f15432 Mon Sep 17 00:00:00 2001
From: Alex Langford
Date: Wed, 28 Feb 2024 10:58:33 -0800
Subject: [PATCH] [lldb] Ignore swig warnings about shadowed overloads
This specifically addresses the warnings:
$LLVM/lldb/include/lldb/API/SBCommandReturnObject.h:119: Warning 509:
Overloaded method lldb::SBCommandReturnObject::PutCString(char const *)
effectively ignored,
$LLVM/lldb/include/lldb/API/SBCommandReturnObject.h:119: Warning 509: as it is
shadowed by lldb::SBCommandReturnObject::PutCString(char const *,int).
There is exactly one declaration of SBCommandReturnObject::PutCString.
The second parameter (of type `int`) has default value `-1`. Without
investigating why SWIG believes there are 2 method declarations, I
believe it is safe to ignore this warning. It does not appear to
actually impact functionality in any way.
rdar://117744660
---
lldb/bindings/CMakeLists.txt | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lldb/bindings/CMakeLists.txt b/lldb/bindings/CMakeLists.txt
index b44ed59aa662b2..296eae1ae77f86 100644
--- a/lldb/bindings/CMakeLists.txt
+++ b/lldb/bindings/CMakeLists.txt
@@ -23,7 +23,11 @@ endif()
set(SWIG_COMMON_FLAGS
-c++
- -w361,362 # Ignore warnings about ignored operator overloads
+ # Ignored warnings:
+ # 361: operator! ignored.
+ # 362: operator= ignored.
+ # 509: Overloaded method declaration effectively ignored, shadowed by
previous declaration.
+ -w361,362,509
-features autodoc
-I${LLDB_SOURCE_DIR}/include
-I${CMAKE_CURRENT_SOURCE_DIR}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove -d(ebug) mode from the lldb driver (PR #83330)
https://github.com/bulbazord approved this pull request. :) https://github.com/llvm/llvm-project/pull/83330 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Change `image list -r` so that it actually shows the ref count and whether the image is in the shared cache. (PR #83341)
https://github.com/bulbazord approved this pull request. https://github.com/llvm/llvm-project/pull/83341 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix interactive use of "command script add". (PR #83350)
https://github.com/bulbazord approved this pull request. Thinkos are the most insidious bugs. Nice catch. https://github.com/llvm/llvm-project/pull/83350 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Ignore swig warnings about shadowed overloads (PR #83317)
https://github.com/bulbazord closed https://github.com/llvm/llvm-project/pull/83317 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test][NFC] Add option to exclude third_party packages (PR #83191)
bulbazord wrote: @DavidSpickett If you haven't already, might I suggest looking into a CMake setting I implemented last year? `LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS` -- this will cause your CMake configure step to fail if your bots are missing python packages so that you know your bots are running in a reasonable state. :) https://github.com/llvm/llvm-project/pull/83191 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress][NFC] Fix Doxygen information (PR #83502)
https://github.com/bulbazord approved this pull request. https://github.com/llvm/llvm-project/pull/83502 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)
https://github.com/bulbazord approved this pull request. Looks great!!! 😄 Thanks for fixing the bug and gardening! https://github.com/llvm/llvm-project/pull/82593 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Change the return type of CalculateNumChildren to uint32_t. (PR #83501)
bulbazord wrote: Why not increase TypeSystem::GetNumChildren to return a size_t instead? https://github.com/llvm/llvm-project/pull/83501 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Change the return type of CalculateNumChildren to uint32_t. (PR #83501)
bulbazord wrote: > I would almost vote to change everything to `uint64_t` except for the public > API since we can't change the API without breaking. Though I winder if we can > actually change this one: > > ``` > uint64_t SBValue::GetNumChildren(); > ``` > > Since the return value isn't mangled into the function name? > > The reason I mention the uint64_t is `lldb::SBValue` and > `lldb_private::ValueObject*` can represent _any_ object that can be expanded. > We could have a `lldb::SBValue` that represents all of memory in a process > where each object can represent an area in memory as a specific format and > size. So a 64 bit process _could_ have a `SBValue` with `UINT64_MAX` children > available if we wanted to have a `SBValue` that represented memory as > `uint8_t` objects. > > So if we are going to change stuff around, I would vote to use `uint64_t` > instead of `uint32_t` Changing the type of a return value if it changes the size/layout is ABI breaking. The mangled name wouldn't change but it may silently break something later without us realizing. https://github.com/llvm/llvm-project/pull/83501 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add ability to detect darwin host linker version to xfail tests (PR #83941)
https://github.com/bulbazord created
https://github.com/llvm/llvm-project/pull/83941
When Apple released its new linker, it had a subtle bug that caused LLDB's TLS
tests to fail. Unfortunately this means that TLS tests are not going to work on
machines that have affected versions of the linker, so we should annotate the
tests so that they only work when we are confident the linker has the required
fix.
I'm not completely satisfied with this implementation. That being said, I
believe that adding suport for linker versions in general is a non-trivial
change that would require far more thought. There are a few challenges involved:
- LLDB's testing infra takes an argument to change the compiler, but there's no
way to switch out the linker.
- There's no standard way to ask a compiler what linker it will use.
- There's no standard way to ask a linker what its version is. Many platforms
have the same name for their linker (ld).
- Some platforms automatically switch out the linker underneath you. We do this
for Windows tests (where we use LLD no matter what).
Given that this is affecting the tests on our CI, I think this is an acceptable
solution in the interim.
>From 480252e4b0eba8f631568b6da4e48f657c516576 Mon Sep 17 00:00:00 2001
From: Alex Langford
Date: Mon, 4 Mar 2024 14:17:20 -0800
Subject: [PATCH] [lldb] Add ability to detect darwin host linker version to
xfail tests
When Apple released its new linker, it had a subtle bug that caused
LLDB's TLS tests to fail. Unfortunately this means that TLS tests
are not going to work on machines that have affected versions of the
linker, so we should annotate the tests so that they only work when we
are confident the linker has the required fix.
I'm not completely satisfied with this implementation. That being said,
I believe that adding suport for linker versions in general is a
non-trivial change that would require far more thought. There are a few
challenges involved:
- LLDB's testing infra takes an argument to change the compiler, but
there's no way to switch out the linker.
- There's no standard way to ask a compiler what linker it will use.
- There's no standard way to ask a linker what its version is. Many
platforms have the same name for their linker (ld).
- Some platforms automatically switch out the linker underneath you. We
do this for Windows tests (where we use LLD no matter what).
Given that this is affecting the tests on our CI, I think this is an
acceptable solution in the interim.
---
.../Python/lldbsuite/test/lldbplatformutil.py | 40 +++
.../API/lang/c/tls_globals/TestTlsGlobals.py | 1 +
2 files changed, 41 insertions(+)
diff --git a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
index c4d063d3cc77ef..acb1a801e7f638 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
@@ -3,6 +3,7 @@
# System modules
import itertools
+import json
import re
import subprocess
import sys
@@ -16,6 +17,7 @@
from . import lldbtest_config
import lldbsuite.test.lldbplatform as lldbplatform
from lldbsuite.test.builders import get_builder
+from lldbsuite.test.lldbutil import is_exe
def check_first_register_readable(test_case):
@@ -333,3 +335,41 @@ def expectedCompiler(compilers):
return True
return False
+
+
+# This is a helper function to determine if a specific version of Xcode's
linker
+# contains a TLS bug. We want to skip TLS tests if they contain this bug, but
+# adding a linker/linker_version conditions to a decorator is challenging due
to
+# the number of ways linkers can enter the build process.
+def darwinLinkerHasTLSBug():
+"""Returns true iff a test is running on a darwin platform and the host
linker is between versions 1000 and 1109."""
+darwin_platforms = lldbplatform.translate(lldbplatform.darwin_all)
+if getPlatform() not in darwin_platforms:
+return False
+
+linker_path = (
+subprocess.check_output(["xcrun", "--find",
"ld"]).rstrip().decode("utf-8")
+)
+if not is_exe(linker_path):
+return False
+
+raw_linker_info = (
+subprocess.check_output([linker_path, "-version_details"])
+.rstrip()
+.decode("utf-8")
+)
+parsed_linker_info = json.loads(raw_linker_info)
+if "version" not in parsed_linker_info:
+return False
+
+raw_version = parsed_linker_info["version"]
+version = None
+try:
+version = int(raw_version)
+except:
+return False
+
+if version is None:
+return False
+
+return 1000 <= version and version <= 1109
diff --git a/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
b/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
index dfe29b451df0a6..526e49b68162f3 100644
--- a/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
+++ b/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
@@ -40,6 +4
[Lldb-commits] [lldb] Change the return type of CalculateNumChildren to uint32_t. (PR #83501)
https://github.com/bulbazord approved this pull request. I'm a little disappointed but I think this is the right step to take. Thanks! https://github.com/llvm/llvm-project/pull/83501 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove unused #includes in ClangModulesDeclVendor.cpp (PR #84262)
https://github.com/bulbazord approved this pull request. https://github.com/llvm/llvm-project/pull/84262 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Disable shell tests affected by ld64 bug (PR #84246)
https://github.com/bulbazord approved this pull request. Thanks for taking care of that! https://github.com/llvm/llvm-project/pull/84246 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add ability to detect darwin host linker version to xfail tests (PR #83941)
bulbazord wrote: > To start with option #​1, I created #84246 Just took a look, thanks for taking care of that. I'll address your feedback (or just look at what you did in the other PR) and update this tomorrow. https://github.com/llvm/llvm-project/pull/83941 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Disable shell tests affected by ld_new bug (PR #84246)
bulbazord wrote: > Called it `ld_new-bug`. If it's not the TLS bug, I wonder if the version > range is correct. The TLS bug in the new linker is explicitly from versions 1000 to 1109. If this isn't related to TLS, we'll need to confirm with the linker folks on our side. https://github.com/llvm/llvm-project/pull/84246 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add ability to detect darwin host linker version to xfail tests (PR #83941)
https://github.com/bulbazord updated
https://github.com/llvm/llvm-project/pull/83941
>From a72d9d259b441c338399340d630ed7a64c1e228a Mon Sep 17 00:00:00 2001
From: Alex Langford
Date: Mon, 4 Mar 2024 14:17:20 -0800
Subject: [PATCH] [lldb] Add ability to detect darwin host linker version to
xfail tests
When Apple released its new linker, it had a subtle bug that caused
LLDB's TLS tests to fail. Unfortunately this means that TLS tests
are not going to work on machines that have affected versions of the
linker, so we should annotate the tests so that they only work when we
are confident the linker has the required fix.
I'm not completely satisfied with this implementation. That being said,
I believe that adding suport for linker versions in general is a
non-trivial change that would require far more thought. There are a few
challenges involved:
- LLDB's testing infra takes an argument to change the compiler, but
there's no way to switch out the linker.
- There's no standard way to ask a compiler what linker it will use.
- There's no standard way to ask a linker what its version is. Many
platforms have the same name for their linker (ld).
- Some platforms automatically switch out the linker underneath you. We
do this for Windows tests (where we use LLD no matter what).
Given that this is affecting the tests on our CI, I think this is an
acceptable solution in the interim.
---
.../Python/lldbsuite/test/lldbplatformutil.py | 25 +++
.../API/lang/c/tls_globals/TestTlsGlobals.py | 1 +
2 files changed, 26 insertions(+)
diff --git a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
index c4d063d3cc77ef..03e28fe6ba7795 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
@@ -3,6 +3,7 @@
# System modules
import itertools
+import json
import re
import subprocess
import sys
@@ -16,6 +17,7 @@
from . import lldbtest_config
import lldbsuite.test.lldbplatform as lldbplatform
from lldbsuite.test.builders import get_builder
+from lldbsuite.test.lldbutil import is_exe
def check_first_register_readable(test_case):
@@ -333,3 +335,26 @@ def expectedCompiler(compilers):
return True
return False
+
+
+# This is a helper function to determine if a specific version of Xcode's
linker
+# contains a TLS bug. We want to skip TLS tests if they contain this bug, but
+# adding a linker/linker_version conditions to a decorator is challenging due
to
+# the number of ways linkers can enter the build process.
+def xcode15LinkerBug():
+"""Returns true iff a test is running on a darwin platform and the host
linker is between versions 1000 and 1109."""
+darwin_platforms = lldbplatform.translate(lldbplatform.darwin_all)
+if getPlatform() not in darwin_platforms:
+return False
+
+try:
+raw_version_details = subprocess.check_output(("xcrun", "ld",
"-version_details"))
+version_details = json.loads(raw_version_details)
+version = version_details.get("version", "0")
+version_tuple = tuple(int(x) for x in version.split("."))
+if (1000,) <= version_tuple <= (1109,):
+return True
+except:
+pass
+
+return False
diff --git a/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
b/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
index dfe29b451df0a6..2bffd2eea123a6 100644
--- a/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
+++ b/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
@@ -40,6 +40,7 @@ def setUp(self):
@skipIfWindows
@skipIf(oslist=["linux"], archs=["arm", "aarch64"])
@skipIf(oslist=no_match([lldbplatformutil.getDarwinOSTriples(), "linux"]))
+@expectedFailureIf(lldbplatformutil.xcode15LinkerBug())
def test(self):
"""Test thread-local storage."""
self.build()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix MSVC build issues (PR #84362)
@@ -168,8 +168,8 @@ class ConstString {
// Implicitly convert \class ConstString instances to \class StringRef.
operator llvm::StringRef() const { return GetStringRef(); }
- // Implicitly convert \class ConstString instances to \class
std::string_view.
- operator std::string_view() const {
+ // Explicitly convert \class ConstString instances to \class
std::string_view.
+ explicit operator std::string_view() const {
bulbazord wrote:
I think that would be nice to do. FWIW, I tried making the StringRef conversion
operator explicit a few months ago and found that it was a pretty large change.
We use the implicitness of the conversion in a lot of places.
https://github.com/llvm/llvm-project/pull/84362
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add ability to detect darwin host linker version to xfail tests (PR #83941)
https://github.com/bulbazord updated
https://github.com/llvm/llvm-project/pull/83941
>From 89d1c201636403bb26f12cf9681cbaf86b5c943b Mon Sep 17 00:00:00 2001
From: Alex Langford
Date: Mon, 4 Mar 2024 14:17:20 -0800
Subject: [PATCH] [lldb] Add ability to detect darwin host linker version to
xfail tests
When Apple released its new linker, it had a subtle bug that caused
LLDB's TLS tests to fail. Unfortunately this means that TLS tests
are not going to work on machines that have affected versions of the
linker, so we should annotate the tests so that they only work when we
are confident the linker has the required fix.
I'm not completely satisfied with this implementation. That being said,
I believe that adding suport for linker versions in general is a
non-trivial change that would require far more thought. There are a few
challenges involved:
- LLDB's testing infra takes an argument to change the compiler, but
there's no way to switch out the linker.
- There's no standard way to ask a compiler what linker it will use.
- There's no standard way to ask a linker what its version is. Many
platforms have the same name for their linker (ld).
- Some platforms automatically switch out the linker underneath you. We
do this for Windows tests (where we use LLD no matter what).
Given that this is affecting the tests on our CI, I think this is an
acceptable solution in the interim.
---
.../Python/lldbsuite/test/lldbplatformutil.py | 27 +++
.../API/lang/c/tls_globals/TestTlsGlobals.py | 1 +
2 files changed, 28 insertions(+)
diff --git a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
index c4d063d3cc77ef..187d16aa1baa68 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
@@ -3,6 +3,7 @@
# System modules
import itertools
+import json
import re
import subprocess
import sys
@@ -16,6 +17,7 @@
from . import lldbtest_config
import lldbsuite.test.lldbplatform as lldbplatform
from lldbsuite.test.builders import get_builder
+from lldbsuite.test.lldbutil import is_exe
def check_first_register_readable(test_case):
@@ -333,3 +335,28 @@ def expectedCompiler(compilers):
return True
return False
+
+
+# This is a helper function to determine if a specific version of Xcode's
linker
+# contains a TLS bug. We want to skip TLS tests if they contain this bug, but
+# adding a linker/linker_version conditions to a decorator is challenging due
to
+# the number of ways linkers can enter the build process.
+def xcode15LinkerBug():
+"""Returns true iff a test is running on a darwin platform and the host
linker is between versions 1000 and 1109."""
+darwin_platforms = lldbplatform.translate(lldbplatform.darwin_all)
+if getPlatform() not in darwin_platforms:
+return False
+
+try:
+raw_version_details = subprocess.check_output(
+("xcrun", "ld", "-version_details")
+)
+version_details = json.loads(raw_version_details)
+version = version_details.get("version", "0")
+version_tuple = tuple(int(x) for x in version.split("."))
+if (1000,) <= version_tuple <= (1109,):
+return True
+except:
+pass
+
+return False
diff --git a/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
b/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
index dfe29b451df0a6..2bffd2eea123a6 100644
--- a/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
+++ b/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
@@ -40,6 +40,7 @@ def setUp(self):
@skipIfWindows
@skipIf(oslist=["linux"], archs=["arm", "aarch64"])
@skipIf(oslist=no_match([lldbplatformutil.getDarwinOSTriples(), "linux"]))
+@expectedFailureIf(lldbplatformutil.xcode15LinkerBug())
def test(self):
"""Test thread-local storage."""
self.build()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add ability to detect darwin host linker version to xfail tests (PR #83941)
https://github.com/bulbazord closed https://github.com/llvm/llvm-project/pull/83941 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Show module name in progress update for downloading symbols (PR #85342)
https://github.com/bulbazord approved this pull request. +1 to Adrian's suggestion. LGTM since you've addressed that now https://github.com/llvm/llvm-project/pull/85342 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Omit --show-globals in `help target var` (PR #85855)
https://github.com/bulbazord commented: Big fan of centralizing the options here. I had one question about automating another portion of this, but otherwise LGTM. https://github.com/llvm/llvm-project/pull/85855 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Omit --show-globals in `help target var` (PR #85855)
@@ -50,6 +50,11 @@ static constexpr OptionDefinition g_variable_options[] = {
"Specify a summary string to use to format the variable output."},
};
+static constexpr auto g_num_frame_options = 4;
bulbazord wrote:
Is this number computable? Or does it need to be hand-maintained?
https://github.com/llvm/llvm-project/pull/85855
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Omit --show-globals in `help target var` (PR #85855)
https://github.com/bulbazord edited https://github.com/llvm/llvm-project/pull/85855 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Omit --show-globals in `help target var` (PR #85855)
https://github.com/bulbazord edited https://github.com/llvm/llvm-project/pull/85855 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Invert relationship between Process and AddressableBits (PR #85858)
https://github.com/bulbazord created
https://github.com/llvm/llvm-project/pull/85858
AddressableBits is in the Utility module of LLDB. It currently directly refers
to Process, which is from the Target LLDB module. This is a layering violation
which concretely means that it is impossible to link anything that uses Utility
without it also using Target as well. This is generally not an issue for LLDB
(since everything is built together) but it may make it difficult to write unit
tests for AddressableBits later on.
>From 476a9c40ce60008255a6f248b5ffd7c3f87b1215 Mon Sep 17 00:00:00 2001
From: Alex Langford
Date: Tue, 19 Mar 2024 12:59:16 -0700
Subject: [PATCH] [lldb] Invert relationship between Process and
AddressableBits
AddressableBits is in the Utility module of LLDB. It currently directly
refers to Process, which is from the Target LLDB module. This is a
layering violation which concretely means that it is impossible to link
anything that uses Utility without it also using Target as well. This is
generally not an issue for LLDB (since everything is built together)
but it may make it difficult to write unit tests for AddressableBits
later on.
---
lldb/include/lldb/Target/Process.h| 3 ++
lldb/include/lldb/Utility/AddressableBits.h | 6 ++--
.../Process/gdb-remote/ProcessGDBRemote.cpp | 4 +--
.../Process/mach-core/ProcessMachCore.cpp | 2 +-
lldb/source/Target/Process.cpp| 23 +++
lldb/source/Utility/AddressableBits.cpp | 28 +++
6 files changed, 43 insertions(+), 23 deletions(-)
diff --git a/lldb/include/lldb/Target/Process.h
b/lldb/include/lldb/Target/Process.h
index e260e1b4b797bc..2f3a3c22422efe 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -43,6 +43,7 @@
#include "lldb/Target/ThreadList.h"
#include "lldb/Target/ThreadPlanStack.h"
#include "lldb/Target/Trace.h"
+#include "lldb/Utility/AddressableBits.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/Broadcaster.h"
#include "lldb/Utility/Event.h"
@@ -3219,6 +3220,8 @@ void PruneThreadPlans();
void LoadOperatingSystemPlugin(bool flush);
+ void SetAddressableBitMasks(AddressableBits bit_masks);
+
private:
Status DestroyImpl(bool force_kill);
diff --git a/lldb/include/lldb/Utility/AddressableBits.h
b/lldb/include/lldb/Utility/AddressableBits.h
index 75752fcf840a44..0d27c3561ec272 100644
--- a/lldb/include/lldb/Utility/AddressableBits.h
+++ b/lldb/include/lldb/Utility/AddressableBits.h
@@ -32,11 +32,13 @@ class AddressableBits {
void SetLowmemAddressableBits(uint32_t lowmem_addressing_bits);
+ uint32_t GetLowmemAddressableBits() const;
+
void SetHighmemAddressableBits(uint32_t highmem_addressing_bits);
- static lldb::addr_t AddressableBitToMask(uint32_t addressable_bits);
+ uint32_t GetHighmemAddressableBits() const;
- void SetProcessMasks(lldb_private::Process &process);
+ static lldb::addr_t AddressableBitToMask(uint32_t addressable_bits);
private:
uint32_t m_low_memory_addr_bits;
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 5b9a9d71802f86..871683a605686f 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -900,7 +900,7 @@ void ProcessGDBRemote::DidLaunchOrAttach(ArchSpec
&process_arch) {
}
AddressableBits addressable_bits = m_gdb_comm.GetAddressableBits();
- addressable_bits.SetProcessMasks(*this);
+ SetAddressableBitMasks(addressable_bits);
if (process_arch.IsValid()) {
const ArchSpec &target_arch = GetTarget().GetArchitecture();
@@ -2337,7 +2337,7 @@ StateType
ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
}
}
-addressable_bits.SetProcessMasks(*this);
+SetAddressableBitMasks(addressable_bits);
ThreadSP thread_sp = SetThreadStopInfo(
tid, expedited_register_map, signo, thread_name, reason, description,
diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
index 7b9938d4f02020..1da7696c9a352a 100644
--- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -574,7 +574,7 @@ Status ProcessMachCore::DoLoadCore() {
CleanupMemoryRegionPermissions();
AddressableBits addressable_bits = core_objfile->GetAddressableBits();
- addressable_bits.SetProcessMasks(*this);
+ SetAddressableBitMasks(addressable_bits);
return error;
}
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 6d58873b54a3ad..f02ec37cb0f08f 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -63,6 +63,7 @@
#include "lldb/Target/ThreadPlanCallFunction.h"
#include "lldb/Target/ThreadPlanStack.h"
#include "lldb/Target/UnixS
[Lldb-commits] [lldb] [lldb] Remove process restart prompt from TestSourceManager (PR #85861)
https://github.com/bulbazord created
https://github.com/llvm/llvm-project/pull/85861
In TestSourceManager, test_artificial_source_location will give the process
restart prompt if you run the test individually. The reason is that we run the
process twice: first using a convenience function to run to a specific
breakpoint and then again to check for a specific message emitted when you hit
the breakpoint. Instead of running twice and making the test difficult to run
individually, we can just check for the specific messages using other commands.
>From c24630bbccd3d53079314f7dc6393ffa27ea192f Mon Sep 17 00:00:00 2001
From: Alex Langford
Date: Tue, 19 Mar 2024 13:18:23 -0700
Subject: [PATCH] [lldb] Remove process restart prompt from TestSourceManager
In TestSourceManager, test_artificial_source_location will give the
process restart prompt if you run the test individually. The reason is
that we run the process twice: first using a convenience function to run
to a specific breakpoint and then again to check for a specific message
emitted when you hit the breakpoint. Instead of running twice and making
the test difficult to run individually, we can just check for the
specific messages using other commands instead.
---
lldb/test/API/source-manager/TestSourceManager.py | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/lldb/test/API/source-manager/TestSourceManager.py
b/lldb/test/API/source-manager/TestSourceManager.py
index eab8924d108146..896fec24791cd2 100644
--- a/lldb/test/API/source-manager/TestSourceManager.py
+++ b/lldb/test/API/source-manager/TestSourceManager.py
@@ -323,13 +323,13 @@ def test_artificial_source_location(self):
)
self.expect(
-"run",
-RUN_SUCCEEDED,
+"thread info", substrs=[f"{src_file}:0", "stop reason =
breakpoint"]
+)
+self.expect(
+"frame select 0",
substrs=[
-"stop reason = breakpoint",
-"%s:%d" % (src_file, 0),
-"Note: this address is compiler-generated code in " "function",
-"that has no source code associated " "with it.",
+"Note: this address is compiler-generated code in function",
+"that has no source code associated with it.",
],
)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove process restart prompt from TestSourceManager (PR #85861)
@@ -323,13 +323,13 @@ def test_artificial_source_location(self):
)
self.expect(
-"run",
-RUN_SUCCEEDED,
+"thread info", substrs=[f"{src_file}:0", "stop reason =
breakpoint"]
+)
+self.expect(
+"frame select 0",
substrs=[
-"stop reason = breakpoint",
-"%s:%d" % (src_file, 0),
-"Note: this address is compiler-generated code in " "function",
-"that has no source code associated " "with it.",
+"Note: this address is compiler-generated code in function",
+"that has no source code associated with it.",
bulbazord wrote:
Just tried it out, it totally can be. Thanks for the tip!
https://github.com/llvm/llvm-project/pull/85861
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove process restart prompt from TestSourceManager (PR #85861)
https://github.com/bulbazord updated
https://github.com/llvm/llvm-project/pull/85861
>From c24630bbccd3d53079314f7dc6393ffa27ea192f Mon Sep 17 00:00:00 2001
From: Alex Langford
Date: Tue, 19 Mar 2024 13:18:23 -0700
Subject: [PATCH 1/2] [lldb] Remove process restart prompt from
TestSourceManager
In TestSourceManager, test_artificial_source_location will give the
process restart prompt if you run the test individually. The reason is
that we run the process twice: first using a convenience function to run
to a specific breakpoint and then again to check for a specific message
emitted when you hit the breakpoint. Instead of running twice and making
the test difficult to run individually, we can just check for the
specific messages using other commands instead.
---
lldb/test/API/source-manager/TestSourceManager.py | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/lldb/test/API/source-manager/TestSourceManager.py
b/lldb/test/API/source-manager/TestSourceManager.py
index eab8924d108146..896fec24791cd2 100644
--- a/lldb/test/API/source-manager/TestSourceManager.py
+++ b/lldb/test/API/source-manager/TestSourceManager.py
@@ -323,13 +323,13 @@ def test_artificial_source_location(self):
)
self.expect(
-"run",
-RUN_SUCCEEDED,
+"thread info", substrs=[f"{src_file}:0", "stop reason =
breakpoint"]
+)
+self.expect(
+"frame select 0",
substrs=[
-"stop reason = breakpoint",
-"%s:%d" % (src_file, 0),
-"Note: this address is compiler-generated code in " "function",
-"that has no source code associated " "with it.",
+"Note: this address is compiler-generated code in function",
+"that has no source code associated with it.",
],
)
>From ab0a0d026f129be17505f8b34406f7ca94332d2e Mon Sep 17 00:00:00 2001
From: Alex Langford
Date: Tue, 19 Mar 2024 16:12:29 -0700
Subject: [PATCH 2/2] Collapse into one command
---
lldb/test/API/source-manager/TestSourceManager.py | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/lldb/test/API/source-manager/TestSourceManager.py
b/lldb/test/API/source-manager/TestSourceManager.py
index 896fec24791cd2..ad7c85aac70eaf 100644
--- a/lldb/test/API/source-manager/TestSourceManager.py
+++ b/lldb/test/API/source-manager/TestSourceManager.py
@@ -323,11 +323,10 @@ def test_artificial_source_location(self):
)
self.expect(
-"thread info", substrs=[f"{src_file}:0", "stop reason =
breakpoint"]
-)
-self.expect(
-"frame select 0",
+"process status",
substrs=[
+"stop reason = breakpoint",
+f"{src_file}:0",
"Note: this address is compiler-generated code in function",
"that has no source code associated with it.",
],
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove process restart prompt from TestSourceManager (PR #85861)
https://github.com/bulbazord closed https://github.com/llvm/llvm-project/pull/85861 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Invert relationship between Process and AddressableBits (PR #85858)
https://github.com/bulbazord closed https://github.com/llvm/llvm-project/pull/85858 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystem] Enable colored AST dump (PR #86159)
bulbazord wrote: What happens if you have colors disabled in your terminal? Does this do nothing? Or does it start inserting ANSI escape codes in plain text? https://github.com/llvm/llvm-project/pull/86159 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Make the correct (5 argument) form of the command definition be the primary one suggested in the docs (PR #86593)
https://github.com/bulbazord edited https://github.com/llvm/llvm-project/pull/86593 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Make the correct (5 argument) form of the command definition be the primary one suggested in the docs (PR #86593)
https://github.com/bulbazord approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/86593 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Make the correct (5 argument) form of the command definition be the primary one suggested in the docs (PR #86593)
@@ -491,34 +491,42 @@ which will work like all the natively defined lldb commands. This provides a very flexible and easy way to extend LLDB to meet your debugging requirements. To write a python function that implements a new LLDB command define the -function to take four arguments as follows: +function to take five arguments as follows: :: - def command_function(debugger, command, result, internal_dict): + def command_function(debugger, command, exe_ctx, result, internal_dict): # Your code goes here -Optionally, you can also provide a Python docstring, and LLDB will use it when providing help for your command, as in: +The meaning of the arguments is given in the table below. + +If you provide a Python docstring in your command function LLDB will use it +when providing "long help" for your command, as in: :: def command_function(debugger, command, result, internal_dict): """This command takes a lot of options and does many fancy things""" # Your code goes here -Since lldb 3.5.2, LLDB Python commands can also take an SBExecutionContext as an -argument. This is useful in cases where the command's notion of where to act is -independent of the currently-selected entities in the debugger. +though providing help can also be done programmatically (see below). -This feature is enabled if the command-implementing function can be recognized -as taking 5 arguments, or a variable number of arguments, and it alters the -signature as such: +Prior to lldb 3.5.2, LLDB Python command definitions didn't take the SBExecutionContext +argument. So you may still see commands where the command definition is: :: - - def command_function(debugger, command, exe_ctx, result, internal_dict): + + def command_function(debugger, command, result, internal_dict): # Your code goes here +This form is deprecated because it can only operate on the "currently selected" +target, process, thread, frame. The command will behave as expected when run +directly on the command line. But if the command is used in a stop-hook, breakpoint +callback, etc. where the response to the callback determines whether we will select +this or that particular process/frame/thread, the global "currently selected" +entity is not necessarily the one the callback is meant to handle. In that case, this +command definition form can't do the right thing. bulbazord wrote: You wrote that it's deprecated at the beginning of the paragraph, but LLDB never removes functions nor removes working implementations of deprecated functions. I would recommend wording the conclusion more strongly, e.g. `In that case, this command definition form can't do the right thing and is therefore highly discouraged from use.` https://github.com/llvm/llvm-project/pull/86593 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Make the correct (5 argument) form of the command definition be the primary one suggested in the docs (PR #86593)
@@ -491,34 +491,42 @@ which will work like all the natively defined lldb commands. This provides a very flexible and easy way to extend LLDB to meet your debugging requirements. To write a python function that implements a new LLDB command define the -function to take four arguments as follows: +function to take five arguments as follows: :: - def command_function(debugger, command, result, internal_dict): + def command_function(debugger, command, exe_ctx, result, internal_dict): # Your code goes here -Optionally, you can also provide a Python docstring, and LLDB will use it when providing help for your command, as in: +The meaning of the arguments is given in the table below. + +If you provide a Python docstring in your command function LLDB will use it +when providing "long help" for your command, as in: :: def command_function(debugger, command, result, internal_dict): """This command takes a lot of options and does many fancy things""" # Your code goes here -Since lldb 3.5.2, LLDB Python commands can also take an SBExecutionContext as an -argument. This is useful in cases where the command's notion of where to act is -independent of the currently-selected entities in the debugger. +though providing help can also be done programmatically (see below). -This feature is enabled if the command-implementing function can be recognized -as taking 5 arguments, or a variable number of arguments, and it alters the -signature as such: +Prior to lldb 3.5.2, LLDB Python command definitions didn't take the SBExecutionContext +argument. So you may still see commands where the command definition is: :: - - def command_function(debugger, command, exe_ctx, result, internal_dict): + bulbazord wrote: nit: Spurious whitespace https://github.com/llvm/llvm-project/pull/86593 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [ObjC runtime] Don't cast to signed when left shifting (PR #86605)
@@ -3154,7 +3154,7 @@ AppleObjCRuntimeV2::TaggedPointerVendorExtended::GetClassDescriptor( << m_objc_debug_taggedpointer_ext_payload_lshift) >> m_objc_debug_taggedpointer_ext_payload_rshift); int64_t data_payload_signed = - ((int64_t)((int64_t)unobfuscated + ((int64_t)((uint64_t)unobfuscated << m_objc_debug_taggedpointer_ext_payload_lshift) >> bulbazord wrote: Since the goal is to extract some bits from the middle of this 64-bit value, is there a way we could produce a mask instead of shifting left and then shifting right? https://github.com/llvm/llvm-project/pull/86605 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][nfc] Delete unused variable (PR #86740)
https://github.com/bulbazord approved this pull request. https://github.com/llvm/llvm-project/pull/86740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [ObjC runtime] Don't cast to signed when left shifting (PR #86605)
@@ -3154,7 +3154,7 @@ AppleObjCRuntimeV2::TaggedPointerVendorExtended::GetClassDescriptor( << m_objc_debug_taggedpointer_ext_payload_lshift) >> m_objc_debug_taggedpointer_ext_payload_rshift); int64_t data_payload_signed = - ((int64_t)((int64_t)unobfuscated + ((int64_t)((uint64_t)unobfuscated << m_objc_debug_taggedpointer_ext_payload_lshift) >> bulbazord wrote: Makes sense to me. I think this is fine then. https://github.com/llvm/llvm-project/pull/86605 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [ObjC runtime] Don't cast to signed when left shifting (PR #86605)
https://github.com/bulbazord approved this pull request. https://github.com/llvm/llvm-project/pull/86605 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
