date:20240308

[Lldb-commits] [lldb] 0f8cb68 - [lldb][Docs] Update the build guide

2024-03-08 Thread David Spickett via lldb-commits


Author: David Spickett
Date: 2024-03-08T10:43:17Z
New Revision: 0f8cb6818d788c34d70d586f701469a5adabfce1

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

LOG: [lldb][Docs] Update the build guide

* gmake is needed on FreeBSD
* pkg can install libxml2 on FreeBSD
* Make the swig note into an RST note box.

Added: 


Modified: 
lldb/docs/resources/build.rst

Removed: 




diff  --git a/lldb/docs/resources/build.rst b/lldb/docs/resources/build.rst
index 55fe73c52f61aa..fe8e293db642e2 100644
--- a/lldb/docs/resources/build.rst
+++ b/lldb/docs/resources/build.rst
@@ -33,6 +33,9 @@ scripting support.
 * `Python `_
 * `SWIG `_ 4 or later.
 
+If you are on FreeBSD or NetBSD, you will need to install ``gmake`` for 
building
+the test programs. On other platforms ``make`` is used.
+
 .. _Optional Dependencies:
 
 Optional Dependencies
@@ -71,16 +74,16 @@ commands below.
 
   $ yum install libedit-devel libxml2-devel ncurses-devel python-devel swig
   $ sudo apt-get install build-essential swig python3-dev libedit-dev 
libncurses5-dev
-  $ pkg install swig python
+  $ pkg install swig python libxml2
   $ pkgin install swig python36 cmake ninja-build
   $ brew install swig cmake ninja
 
-Note that there's an `incompatibility
-`_ between Python version 3.7 and 
later
-and swig versions older than 4.0.0 which makes builds of LLDB using debug
-versions of python unusable. This primarily affects Windows, as debug builds of
-LLDB must use debug python as well.
-
+.. note::
+   There is an `incompatibility
+   `_ between Python version 3.7 and 
later
+   and swig versions older than 4.0.0 which makes builds of LLDB using debug
+   versions of python unusable. This primarily affects Windows, as debug 
builds of
+   LLDB must use debug python as well.
 
 Windows
 ***



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

[Lldb-commits] [lldb] 2353321 - [lldb][Docs] Add Curses version note to build page

2024-03-08 Thread David Spickett via lldb-commits


Author: David Spickett
Date: 2024-03-08T11:33:17Z
New Revision: 235332150d52d11b340a10be1bb88432d2cf4179

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

LOG: [lldb][Docs] Add Curses version note to build page

This explains a thing that hit me on FreeBSD because the base system
has an ncursesw at one version and I installed from pkg another
version that was simply ncurses (no wide char support).

For whatever reason, when we pass -lcurses to the linker it ends up
picking bits of both installs. This led to lldb crashing immediately
if you tried to use the `gui` command.

In a way that gave little information but I stumbled onto
https://github.com/vifm/vifm/issues/325 which is very similar.

```
ec2-user@freebsd:~/build-llvm $ ldd ./bin/lldb | grep curses
libncursesw.so.9 => /lib/libncursesw.so.9 (0x6a515206e000)
libncurses.so.6 => /usr/local/lib/libncurses.so.6 (0x6a5158e86000)
```

We should only see one version, and it and libpanel etc should
all have "w" or not have "w". This was not the case for my build.

What I can see from the CMake side seemed fine, it found the pkg
installed ncurses in /usr/local. Something else must decide that
-lcurses should pull in the other one.

Regardless, I don't know how to fix that but the solution for most
people is just not to add another ncurses if they already have one.
So I've added a note saying so, and how to check what your lldb
is using.

Added: 


Modified: 
lldb/docs/resources/build.rst

Removed: 




diff  --git a/lldb/docs/resources/build.rst b/lldb/docs/resources/build.rst
index fe8e293db642e2..5f4d35ced6236c 100644
--- a/lldb/docs/resources/build.rst
+++ b/lldb/docs/resources/build.rst
@@ -85,6 +85,13 @@ commands below.
versions of python unusable. This primarily affects Windows, as debug 
builds of
LLDB must use debug python as well.
 
+.. note::
+  Installing multiple versions of Curses, particularly when only one is built 
with
+  wide character support, can cause lldb to be linked with an incorrect set of
+  libraries. If your system already has Curses, we recommend you use that 
version.
+  If you do install another one, use a tool like ``ldd`` to ensure only one 
version
+  of Curses is being used in the final ``lldb`` executable.
+
 Windows
 ***
 



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

[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)

2024-03-08 Thread Felipe de Azevedo Piovezan via lldb-commits



@@ -339,6 +339,15 @@ class Language : public PluginInterface {
 
   virtual llvm::StringRef GetInstanceVariableName() { return {}; }
 
+  /// Returns true if this SymbolContext should be ignored when setting
+  /// breakpoints by line (number or regex). This is useful for languages that
+  /// create artificial functions without any meaningful user code associated
+  /// with them (e.g. code that gets expanded in late compilation stages, like
+  /// by CoroSplitter).
+  virtual bool IgnoreForLineBreakpoints(const SymbolContext &) const {

felipepiovezan wrote:

brevity: 
This is useful for -> Helpful for
any meaningful -> meaningful

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

[Lldb-commits] [lldb] 73b2d67 - [lldb][FreeBSD] Fix crash when execve fails with asserts enabled

2024-03-08 Thread David Spickett via lldb-commits


Author: David Spickett
Date: 2024-03-08T17:15:11Z
New Revision: 73b2d672c1c8318cd16a02812c39ae3997b9dbcd

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

LOG: [lldb][FreeBSD] Fix crash when execve fails with asserts enabled

535da10842c7309e9eeaf9828cf6bb034fecaf16 introduced a check, when execve fails,
to see if we are allowed to trace programs at all.

Unfortunately because we like to call Status vars "error" and "status"
in various combinations, one got misnamed. This lead to lldb-server trying
to make an error value out of a success value when you did the following:

```
$ ./bin/lldb-server gdbserver 127.0.0.1:1234 -- is_not_a_file
Assertion failed: (Err && "Cannot create Expected from Error success 
value."), function Expected...
```

This happened because the execve fails, but the check whether we can
trace says yes we can trace, but then we use the Status from the check
to create the return value. That Status is in fact a success value not
the failed Status we got from the execve attempt.

With the name corrected you now get:
```
$ ./bin/lldb-server gdbserver 127.0.0.1:1234 -- is_not_a_file
error: failed to launch 'is_not_a_file': execve failed: No such file or 
directory
```

Which is what we expect to see. This also fixes the test 
`TestGDBRemoteLaunch.py`
when asserts are enabled.

Added: 


Modified: 
lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp 
b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
index 9c620e4807e344..064bddd3705205 100644
--- a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -76,9 +76,9 @@ NativeProcessFreeBSD::Manager::Launch(ProcessLaunchInfo 
&launch_info,
 .GetProcessId();
   LLDB_LOG(log, "pid = {0:x}", pid);
   if (status.Fail()) {
-auto error = CanTrace();
 LLDB_LOG(log, "failed to launch process: {0}", status);
-if (status.Fail())
+auto error = CanTrace();
+if (error.Fail())
   return error.ToError();
 return status.ToError();
   }



___
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)

2024-03-08 Thread David Spickett via lldb-commits



@@ -48,20 +48,38 @@ static Status EnsureFDFlags(int fd, int flags) {
   return error;
 }
 
+static Status CanTrace() {
+  int proc_debug, ret;
+  size_t len = sizeof(proc_debug);
+  ret = ::sysctlbyname("security.bsd.unprivileged_proc_debug", &proc_debug,
+   &len, nullptr, 0);
+  if (ret != 0)
+return Status("sysctlbyname() security.bsd.unprivileged_proc_debug 
failed");
+
+  if (proc_debug < 1)
+return Status(
+"process debug disabled by security.bsd.unprivileged_proc_debug oid");
+
+  return {};
+}
+
 // Public Static Methods
 
 llvm::Expected>
 NativeProcessFreeBSD::Manager::Launch(ProcessLaunchInfo &launch_info,
   NativeDelegate &native_delegate) {
   Log *log = GetLog(POSIXLog::Process);
-
   Status status;
+
   ::pid_t pid = ProcessLauncherPosixFork()
 .LaunchProcess(launch_info, status)
 .GetProcessId();
   LLDB_LOG(log, "pid = {0:x}", pid);
   if (status.Fail()) {
+auto error = CanTrace();
 LLDB_LOG(log, "failed to launch process: {0}", status);
+if (status.Fail())
+  return error.ToError();

DavidSpickett wrote:

FYI I fixed a mistake here - 
https://github.com/llvm/llvm-project/commit/73b2d672c1c8318cd16a02812c39ae3997b9dbcd

The rest look correct to me.

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] Change GetNumChildren()/CalculateNumChildren() methods return llvm::Expected (PR #84219)

2024-03-08 Thread Adrian Prantl via lldb-commits


https://github.com/adrian-prantl updated 
https://github.com/llvm/llvm-project/pull/84219

>From 97a80ffa93d9fd648f0aa995b51981f0ad949530 Mon Sep 17 00:00:00 2001
From: Adrian Prantl 
Date: Thu, 7 Mar 2024 13:03:14 -0800
Subject: [PATCH] Let GetNumChildren()/CalculateNumChildren() return
 llvm::Expected

This is an NFC change that does not yet add any error handling or
change any code to return any errors. Some select APIs may
legitimately not care about errors, so this patch also adds a
GetNumChildrenIgnoringErrors() variant to the API.

As we convert more APIs to returning llvm::Expected, we expect the
number of calls to GetNumChildrenIgnoringErrors() to dwindle.
---
 lldb/include/lldb/Core/ValueObject.h  | 11 +++-
 lldb/include/lldb/Core/ValueObjectCast.h  |  2 +-
 lldb/include/lldb/Core/ValueObjectChild.h |  2 +-
 .../lldb/Core/ValueObjectConstResult.h|  2 +-
 .../lldb/Core/ValueObjectDynamicValue.h   |  2 +-
 lldb/include/lldb/Core/ValueObjectMemory.h|  2 +-
 lldb/include/lldb/Core/ValueObjectRegister.h  |  4 +-
 .../lldb/Core/ValueObjectSyntheticFilter.h|  2 +-
 lldb/include/lldb/Core/ValueObjectVTable.h|  2 +-
 lldb/include/lldb/Core/ValueObjectVariable.h  |  2 +-
 .../lldb/DataFormatters/TypeSynthetic.h   | 20 --
 .../lldb/DataFormatters/VectorIterator.h  |  2 +-
 lldb/include/lldb/Symbol/CompilerType.h   |  5 +-
 lldb/include/lldb/Symbol/Type.h   |  2 +-
 lldb/include/lldb/Symbol/TypeSystem.h |  7 +-
 .../lldb/Target/StackFrameRecognizer.h|  3 +-
 lldb/include/lldb/Utility/Log.h   | 14 
 lldb/source/API/SBValue.cpp   |  2 +-
 lldb/source/Core/FormatEntity.cpp |  2 +-
 lldb/source/Core/IOHandlerCursesGUI.cpp   |  2 +-
 lldb/source/Core/ValueObject.cpp  | 31 ++---
 lldb/source/Core/ValueObjectCast.cpp  |  6 +-
 lldb/source/Core/ValueObjectChild.cpp |  6 +-
 lldb/source/Core/ValueObjectConstResult.cpp   |  7 +-
 lldb/source/Core/ValueObjectDynamicValue.cpp  |  7 +-
 lldb/source/Core/ValueObjectMemory.cpp| 10 ++-
 lldb/source/Core/ValueObjectRegister.cpp  | 12 ++--
 .../Core/ValueObjectSyntheticFilter.cpp   | 24 ---
 lldb/source/Core/ValueObjectVTable.cpp|  6 +-
 lldb/source/Core/ValueObjectVariable.cpp  |  7 +-
 lldb/source/DataFormatters/FormatManager.cpp  | 13 ++--
 lldb/source/DataFormatters/TypeSynthetic.cpp  | 17 -
 .../DataFormatters/ValueObjectPrinter.cpp |  2 +-
 lldb/source/DataFormatters/VectorType.cpp | 16 +++--
 .../Clang/ClangExpressionSourceCode.cpp   |  2 +-
 .../TSan/InstrumentationRuntimeTSan.cpp   |  4 +-
 .../Language/CPlusPlus/BlockPointer.cpp   |  4 +-
 .../Plugins/Language/CPlusPlus/Coroutines.cpp |  8 ++-
 .../Plugins/Language/CPlusPlus/Coroutines.h   |  2 +-
 .../Language/CPlusPlus/GenericBitset.cpp  |  4 +-
 .../Language/CPlusPlus/GenericOptional.cpp|  6 +-
 .../Plugins/Language/CPlusPlus/LibCxx.cpp | 18 +++---
 .../Plugins/Language/CPlusPlus/LibCxx.h   |  8 +--
 .../Language/CPlusPlus/LibCxxAtomic.cpp   |  6 +-
 .../CPlusPlus/LibCxxInitializerList.cpp   |  6 +-
 .../Plugins/Language/CPlusPlus/LibCxxList.cpp | 12 ++--
 .../Plugins/Language/CPlusPlus/LibCxxMap.cpp  | 14 ++--
 .../Language/CPlusPlus/LibCxxQueue.cpp|  2 +-
 .../CPlusPlus/LibCxxRangesRefView.cpp |  2 +-
 .../Plugins/Language/CPlusPlus/LibCxxSpan.cpp |  6 +-
 .../Language/CPlusPlus/LibCxxTuple.cpp|  4 +-
 .../Language/CPlusPlus/LibCxxUnorderedMap.cpp |  8 +--
 .../Language/CPlusPlus/LibCxxValarray.cpp |  6 +-
 .../Language/CPlusPlus/LibCxxVariant.cpp  |  2 +-
 .../Language/CPlusPlus/LibCxxVector.cpp   | 14 ++--
 .../Plugins/Language/CPlusPlus/LibStdcpp.cpp  | 17 +++--
 .../Language/CPlusPlus/LibStdcppTuple.cpp |  7 +-
 .../CPlusPlus/LibStdcppUniquePointer.cpp  |  5 +-
 lldb/source/Plugins/Language/ObjC/Cocoa.cpp   |  2 +-
 lldb/source/Plugins/Language/ObjC/NSArray.cpp | 30 -
 .../Plugins/Language/ObjC/NSDictionary.cpp| 64 +--
 lldb/source/Plugins/Language/ObjC/NSError.cpp |  2 +-
 .../Plugins/Language/ObjC/NSException.cpp |  4 +-
 .../Plugins/Language/ObjC/NSIndexPath.cpp |  6 +-
 lldb/source/Plugins/Language/ObjC/NSSet.cpp   | 32 +-
 .../AppleObjCRuntime/AppleObjCRuntime.cpp |  3 +-
 .../TypeSystem/Clang/TypeSystemClang.cpp  | 40 
 .../TypeSystem/Clang/TypeSystemClang.h|  7 +-
 lldb/source/Symbol/CompilerType.cpp   |  5 +-
 lldb/source/Symbol/Type.cpp   |  2 +-
 lldb/source/Symbol/Variable.cpp   |  6 +-
 lldb/source/Target/StackFrame.cpp | 19 +++---
 72 files changed, 395 insertions(+), 248 deletions(-)

diff --git a/lldb/include/lldb/Core/ValueObject.h 
b/lldb/include/lldb/Core/ValueObject.h
index b4d2c8098edc71..e7e35e2b2bffc0 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/Value

[Lldb-commits] [lldb] Change GetNumChildren()/CalculateNumChildren() methods return llvm::Expected (PR #84219)

2024-03-08 Thread Adrian Prantl via lldb-commits


adrian-prantl wrote:

@walter-erquinigo @jimingham I updated the patch to implement Jim's suggestion 
of adding an explicit `GetNumChilrdrenIgnoringErrors()` API that also ticks off 
the "big red warning flag" @JDevlieghere was pushing for.

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

[Lldb-commits] [lldb] Report back errors in GetNumChildren() (PR #84265)

2024-03-08 Thread Adrian Prantl via lldb-commits


adrian-prantl wrote:

> From looking at the code, it seems like there are two clear classes of users, 
> those that need to check the error, and those that, in the error case, 0 is 
> just as good as the error. The latter appears not infrequently as:
> 
> `llvm::expectedToStdOptional(value_sp->GetNumChildren(max)).value_or(0)`
> 
> which is a bit ugly, but also by embedding this everywhere inline it isn't 
> easy to see who is checking errors and who doesn't need to.
> 
> Might be useful to have a GetNumChildrenUnchecked that returns 0 for the 
> error, and use that in place of sprinkling this construct about.

See the latest update in https://github.com/llvm/llvm-project/pull/84219

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

[Lldb-commits] [lldb] Report back errors in GetNumChildren() (PR #84265)

2024-03-08 Thread Adrian Prantl via lldb-commits


adrian-prantl wrote:

Rebased

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

[Lldb-commits] [lldb] Change GetNumChildren()/CalculateNumChildren() methods return llvm::Expected (PR #84219)

2024-03-08 Thread via lldb-commits


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

LGTM.  The "IgnoringErrors" makes this construct easier to spot, and also 
should give anyone a bad feeling if they are tempted to use it...

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

[Lldb-commits] [lldb] Change GetNumChildren()/CalculateNumChildren() methods return llvm::Expected (PR #84219)

2024-03-08 Thread Adrian Prantl via lldb-commits


https://github.com/adrian-prantl updated 
https://github.com/llvm/llvm-project/pull/84219

>From 0504cd6b50df6fafe36bade310fe1b729a3d9cea Mon Sep 17 00:00:00 2001
From: Adrian Prantl 
Date: Thu, 7 Mar 2024 13:03:14 -0800
Subject: [PATCH] Change GetNumChildren()/CalculateNumChildren() to return
 llvm::Expected

This is an NFC change that does not yet add any error handling or
change any code to return any errors. Some select APIs may
legitimately not care about errors, so this patch also adds a
GetNumChildrenIgnoringErrors() variant to the API.

As we convert more APIs to returning llvm::Expected, we expect the
number of calls to GetNumChildrenIgnoringErrors() to dwindle.
---
 lldb/include/lldb/Core/ValueObject.h  | 11 +++-
 lldb/include/lldb/Core/ValueObjectCast.h  |  2 +-
 lldb/include/lldb/Core/ValueObjectChild.h |  2 +-
 .../lldb/Core/ValueObjectConstResult.h|  2 +-
 .../lldb/Core/ValueObjectDynamicValue.h   |  2 +-
 lldb/include/lldb/Core/ValueObjectMemory.h|  2 +-
 lldb/include/lldb/Core/ValueObjectRegister.h  |  4 +-
 .../lldb/Core/ValueObjectSyntheticFilter.h|  2 +-
 lldb/include/lldb/Core/ValueObjectVTable.h|  2 +-
 lldb/include/lldb/Core/ValueObjectVariable.h  |  2 +-
 .../lldb/DataFormatters/TypeSynthetic.h   | 20 --
 .../lldb/DataFormatters/VectorIterator.h  |  2 +-
 lldb/include/lldb/Symbol/CompilerType.h   |  5 +-
 lldb/include/lldb/Symbol/Type.h   |  2 +-
 lldb/include/lldb/Symbol/TypeSystem.h |  7 +-
 .../lldb/Target/StackFrameRecognizer.h|  3 +-
 lldb/include/lldb/Utility/Log.h   | 14 
 lldb/source/API/SBValue.cpp   |  2 +-
 lldb/source/Core/FormatEntity.cpp |  2 +-
 lldb/source/Core/IOHandlerCursesGUI.cpp   |  2 +-
 lldb/source/Core/ValueObject.cpp  | 31 ++---
 lldb/source/Core/ValueObjectCast.cpp  |  6 +-
 lldb/source/Core/ValueObjectChild.cpp |  6 +-
 lldb/source/Core/ValueObjectConstResult.cpp   |  7 +-
 lldb/source/Core/ValueObjectDynamicValue.cpp  |  7 +-
 lldb/source/Core/ValueObjectMemory.cpp| 10 ++-
 lldb/source/Core/ValueObjectRegister.cpp  | 12 ++--
 .../Core/ValueObjectSyntheticFilter.cpp   | 24 ---
 lldb/source/Core/ValueObjectVTable.cpp|  6 +-
 lldb/source/Core/ValueObjectVariable.cpp  |  7 +-
 lldb/source/DataFormatters/FormatManager.cpp  | 11 +++-
 lldb/source/DataFormatters/TypeSynthetic.cpp  | 17 -
 .../DataFormatters/ValueObjectPrinter.cpp |  2 +-
 lldb/source/DataFormatters/VectorType.cpp | 16 +++--
 .../Clang/ClangExpressionSourceCode.cpp   |  2 +-
 .../TSan/InstrumentationRuntimeTSan.cpp   |  4 +-
 .../Language/CPlusPlus/BlockPointer.cpp   |  4 +-
 .../Plugins/Language/CPlusPlus/Coroutines.cpp |  8 ++-
 .../Plugins/Language/CPlusPlus/Coroutines.h   |  2 +-
 .../Language/CPlusPlus/GenericBitset.cpp  |  4 +-
 .../Language/CPlusPlus/GenericOptional.cpp|  6 +-
 .../Plugins/Language/CPlusPlus/LibCxx.cpp | 18 +++---
 .../Plugins/Language/CPlusPlus/LibCxx.h   |  8 +--
 .../Language/CPlusPlus/LibCxxAtomic.cpp   |  6 +-
 .../CPlusPlus/LibCxxInitializerList.cpp   |  6 +-
 .../Plugins/Language/CPlusPlus/LibCxxList.cpp | 12 ++--
 .../Plugins/Language/CPlusPlus/LibCxxMap.cpp  | 14 ++--
 .../Language/CPlusPlus/LibCxxQueue.cpp|  2 +-
 .../CPlusPlus/LibCxxRangesRefView.cpp |  2 +-
 .../Plugins/Language/CPlusPlus/LibCxxSpan.cpp |  6 +-
 .../Language/CPlusPlus/LibCxxTuple.cpp|  4 +-
 .../Language/CPlusPlus/LibCxxUnorderedMap.cpp |  8 +--
 .../Language/CPlusPlus/LibCxxValarray.cpp |  6 +-
 .../Language/CPlusPlus/LibCxxVariant.cpp  |  2 +-
 .../Language/CPlusPlus/LibCxxVector.cpp   | 14 ++--
 .../Plugins/Language/CPlusPlus/LibStdcpp.cpp  | 17 +++--
 .../Language/CPlusPlus/LibStdcppTuple.cpp |  7 +-
 .../CPlusPlus/LibStdcppUniquePointer.cpp  |  5 +-
 lldb/source/Plugins/Language/ObjC/Cocoa.cpp   |  2 +-
 lldb/source/Plugins/Language/ObjC/NSArray.cpp | 30 -
 .../Plugins/Language/ObjC/NSDictionary.cpp| 64 +--
 lldb/source/Plugins/Language/ObjC/NSError.cpp |  2 +-
 .../Plugins/Language/ObjC/NSException.cpp |  4 +-
 .../Plugins/Language/ObjC/NSIndexPath.cpp |  6 +-
 lldb/source/Plugins/Language/ObjC/NSSet.cpp   | 32 +-
 .../AppleObjCRuntime/AppleObjCRuntime.cpp |  3 +-
 .../TypeSystem/Clang/TypeSystemClang.cpp  | 40 
 .../TypeSystem/Clang/TypeSystemClang.h|  7 +-
 lldb/source/Symbol/CompilerType.cpp   |  5 +-
 lldb/source/Symbol/Type.cpp   |  2 +-
 lldb/source/Symbol/Variable.cpp   |  6 +-
 lldb/source/Target/StackFrame.cpp | 19 +++---
 72 files changed, 394 insertions(+), 247 deletions(-)

diff --git a/lldb/include/lldb/Core/ValueObject.h 
b/lldb/include/lldb/Core/ValueObject.h
index b4d2c8098edc71..e7e35e2b2bffc0 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core

[Lldb-commits] [lldb] Change GetNumChildren()/CalculateNumChildren() methods return llvm::Expected (PR #84219)

2024-03-08 Thread Adrian Prantl via lldb-commits


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

[Lldb-commits] [lldb] 99118c8 - Change GetNumChildren()/CalculateNumChildren() methods return llvm::Expected (#84219)

2024-03-08 Thread via lldb-commits


Author: Adrian Prantl
Date: 2024-03-08T10:39:34-08:00
New Revision: 99118c809367d518ffe4de60c16da953744b68b9

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

LOG: Change GetNumChildren()/CalculateNumChildren() methods return 
llvm::Expected (#84219)

Change GetNumChildren()/CalculateNumChildren() methods return
llvm::Expected

This is an NFC change that does not yet add any error handling or change
any code to return any errors.

This is the second big change in the patch series started with
https://github.com/llvm/llvm-project/pull/83501

A follow-up PR will wire up error handling.

Added: 


Modified: 
lldb/include/lldb/Core/ValueObject.h
lldb/include/lldb/Core/ValueObjectCast.h
lldb/include/lldb/Core/ValueObjectChild.h
lldb/include/lldb/Core/ValueObjectConstResult.h
lldb/include/lldb/Core/ValueObjectDynamicValue.h
lldb/include/lldb/Core/ValueObjectMemory.h
lldb/include/lldb/Core/ValueObjectRegister.h
lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
lldb/include/lldb/Core/ValueObjectVTable.h
lldb/include/lldb/Core/ValueObjectVariable.h
lldb/include/lldb/DataFormatters/TypeSynthetic.h
lldb/include/lldb/DataFormatters/VectorIterator.h
lldb/include/lldb/Symbol/CompilerType.h
lldb/include/lldb/Symbol/Type.h
lldb/include/lldb/Symbol/TypeSystem.h
lldb/include/lldb/Target/StackFrameRecognizer.h
lldb/include/lldb/Utility/Log.h
lldb/source/API/SBValue.cpp
lldb/source/Core/FormatEntity.cpp
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Core/ValueObject.cpp
lldb/source/Core/ValueObjectCast.cpp
lldb/source/Core/ValueObjectChild.cpp
lldb/source/Core/ValueObjectConstResult.cpp
lldb/source/Core/ValueObjectDynamicValue.cpp
lldb/source/Core/ValueObjectMemory.cpp
lldb/source/Core/ValueObjectRegister.cpp
lldb/source/Core/ValueObjectSyntheticFilter.cpp
lldb/source/Core/ValueObjectVTable.cpp
lldb/source/Core/ValueObjectVariable.cpp
lldb/source/DataFormatters/FormatManager.cpp
lldb/source/DataFormatters/TypeSynthetic.cpp
lldb/source/DataFormatters/ValueObjectPrinter.cpp
lldb/source/DataFormatters/VectorType.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp

lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxRangesRefView.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxValarray.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
lldb/source/Plugins/Language/ObjC/Cocoa.cpp
lldb/source/Plugins/Language/ObjC/NSArray.cpp
lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
lldb/source/Plugins/Language/ObjC/NSError.cpp
lldb/source/Plugins/Language/ObjC/NSException.cpp
lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
lldb/source/Plugins/Language/ObjC/NSSet.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
lldb/source/Symbol/CompilerType.cpp
lldb/source/Symbol/Type.cpp
lldb/source/Symbol/Variable.cpp
lldb/source/Target/StackFrame.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/ValueObject.h 
b/lldb/include/lldb/Core/ValueObject.h
index b4d2c8098edc71..e7e35e2b2bffc0 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -476,7 +476,13 @@ class ValueObject {
 
   virtual size_t GetIndexOfChildWithName(llvm::StringRef name);
 
-  uint32_t GetNumChildren(uint32_t max = UINT32_MAX)

[Lldb-commits] [lldb] Report back errors in GetNumChildren() (PR #84265)

2024-03-08 Thread Adrian Prantl via lldb-commits


https://github.com/adrian-prantl updated 
https://github.com/llvm/llvm-project/pull/84265

>From 76d65446227d18f4f3c4675b1aa12678037dff8c Mon Sep 17 00:00:00 2001
From: Adrian Prantl 
Date: Wed, 6 Mar 2024 16:21:14 -0800
Subject: [PATCH] Report back errors in GetNumChildren()

This is a proof-of-concept patch that illustrates how to use the
Expected return values to surface rich error messages all the way up
to the ValueObjectPrinter.
---
 .../lldb/DataFormatters/ValueObjectPrinter.h  |  2 +-
 lldb/source/Core/ValueObjectVariable.cpp  |  3 ++-
 .../DataFormatters/ValueObjectPrinter.cpp | 22 +++
 .../TypeSystem/Clang/TypeSystemClang.cpp  |  8 ---
 lldb/source/Symbol/CompilerType.cpp   |  3 ++-
 .../functionalities/valobj_errors/Makefile|  9 
 .../valobj_errors/TestValueObjectErrors.py| 15 +
 .../functionalities/valobj_errors/hidden.c|  4 
 .../API/functionalities/valobj_errors/main.c  |  9 
 9 files changed, 65 insertions(+), 10 deletions(-)
 create mode 100644 lldb/test/API/functionalities/valobj_errors/Makefile
 create mode 100644 
lldb/test/API/functionalities/valobj_errors/TestValueObjectErrors.py
 create mode 100644 lldb/test/API/functionalities/valobj_errors/hidden.c
 create mode 100644 lldb/test/API/functionalities/valobj_errors/main.c

diff --git a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h 
b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
index fe46321c3186cf..32b101a2f9843c 100644
--- a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
+++ b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
@@ -127,7 +127,7 @@ class ValueObjectPrinter {
   void PrintChild(lldb::ValueObjectSP child_sp,
   const DumpValueObjectOptions::PointerDepth &curr_ptr_depth);
 
-  uint32_t GetMaxNumChildrenToPrint(bool &print_dotdotdot);
+  llvm::Expected GetMaxNumChildrenToPrint(bool &print_dotdotdot);
 
   void
   PrintChildren(bool value_printed, bool summary_printed,
diff --git a/lldb/source/Core/ValueObjectVariable.cpp 
b/lldb/source/Core/ValueObjectVariable.cpp
index fb29c22c0ab5af..67d71c90a959d5 100644
--- a/lldb/source/Core/ValueObjectVariable.cpp
+++ b/lldb/source/Core/ValueObjectVariable.cpp
@@ -99,7 +99,8 @@ ValueObjectVariable::CalculateNumChildren(uint32_t max) {
   CompilerType type(GetCompilerType());
 
   if (!type.IsValid())
-return 0;
+return llvm::make_error("invalid type",
+   llvm::inconvertibleErrorCode());
 
   ExecutionContext exe_ctx(GetExecutionContextRef());
   const bool omit_empty_base_classes = true;
diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp 
b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index b853199e878c95..bbdc2a99815706 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -621,13 +621,17 @@ void ValueObjectPrinter::PrintChild(
   }
 }
 
-uint32_t ValueObjectPrinter::GetMaxNumChildrenToPrint(bool &print_dotdotdot) {
+llvm::Expected
+ValueObjectPrinter::GetMaxNumChildrenToPrint(bool &print_dotdotdot) {
   ValueObject &synth_valobj = GetValueObjectForChildrenGeneration();
 
   if (m_options.m_pointer_as_array)
 return m_options.m_pointer_as_array.m_element_count;
 
-  uint32_t num_children = synth_valobj.GetNumChildrenIgnoringErrors();
+  auto num_children_or_err = synth_valobj.GetNumChildren();
+  if (!num_children_or_err)
+return num_children_or_err;
+  uint32_t num_children = *num_children_or_err;
   print_dotdotdot = false;
   if (num_children) {
 const size_t max_num_children = GetMostSpecializedValue()
@@ -704,7 +708,12 @@ void ValueObjectPrinter::PrintChildren(
   ValueObject &synth_valobj = GetValueObjectForChildrenGeneration();
 
   bool print_dotdotdot = false;
-  size_t num_children = GetMaxNumChildrenToPrint(print_dotdotdot);
+  auto num_children_or_err = GetMaxNumChildrenToPrint(print_dotdotdot);
+  if (!num_children_or_err) {
+*m_stream << " <" << llvm::toString(num_children_or_err.takeError()) << 
'>';
+return;
+  }
+  uint32_t num_children = *num_children_or_err;
   if (num_children) {
 bool any_children_printed = false;
 
@@ -753,7 +762,12 @@ bool ValueObjectPrinter::PrintChildrenOneLiner(bool 
hide_names) {
   ValueObject &synth_valobj = GetValueObjectForChildrenGeneration();
 
   bool print_dotdotdot = false;
-  size_t num_children = GetMaxNumChildrenToPrint(print_dotdotdot);
+  auto num_children_or_err = GetMaxNumChildrenToPrint(print_dotdotdot);
+  if (!num_children_or_err) {
+*m_stream << '<' << llvm::toString(num_children_or_err.takeError()) << '>';
+return true;
+  }
+  uint32_t num_children = *num_children_or_err;
 
   if (num_children) {
 m_stream->PutChar('(');
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index c02b08cb478280..16974224f3e2ca 100644
--- a/lldb/source/Plugins/TypeS

[Lldb-commits] [lldb] Change GetNumChildren()/CalculateNumChildren() methods return llvm::Expected (PR #84219)

2024-03-08 Thread Florian Mayer via lldb-commits


fmayer wrote:

I think this broke the build completely?

```
/usr/local/google/home/fmayer/large/llvm-project/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp:771:14:
 error: no viable conversion from 'llvm::Expected' (aka 
'Expected') to 'uint32_t' (aka 'unsigned int')
uint32_t n = m_type.GetNumChildren(omit_empty_base_classes, nullptr);
 ^   ~~~
```

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

[Lldb-commits] [lldb] 300a39b - Revert "Change GetNumChildren()/CalculateNumChildren() methods return llvm::Expected (#84219)"

2024-03-08 Thread Florian Mayer via lldb-commits


Author: Florian Mayer
Date: 2024-03-08T12:14:22-08:00
New Revision: 300a39bdad4593fdc2618eb28f6e838df735619a

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

LOG: Revert "Change GetNumChildren()/CalculateNumChildren() methods return 
llvm::Expected (#84219)"

This reverts commit 99118c809367d518ffe4de60c16da953744b68b9.

Added: 


Modified: 
lldb/include/lldb/Core/ValueObject.h
lldb/include/lldb/Core/ValueObjectCast.h
lldb/include/lldb/Core/ValueObjectChild.h
lldb/include/lldb/Core/ValueObjectConstResult.h
lldb/include/lldb/Core/ValueObjectDynamicValue.h
lldb/include/lldb/Core/ValueObjectMemory.h
lldb/include/lldb/Core/ValueObjectRegister.h
lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
lldb/include/lldb/Core/ValueObjectVTable.h
lldb/include/lldb/Core/ValueObjectVariable.h
lldb/include/lldb/DataFormatters/TypeSynthetic.h
lldb/include/lldb/DataFormatters/VectorIterator.h
lldb/include/lldb/Symbol/CompilerType.h
lldb/include/lldb/Symbol/Type.h
lldb/include/lldb/Symbol/TypeSystem.h
lldb/include/lldb/Target/StackFrameRecognizer.h
lldb/include/lldb/Utility/Log.h
lldb/source/API/SBValue.cpp
lldb/source/Core/FormatEntity.cpp
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Core/ValueObject.cpp
lldb/source/Core/ValueObjectCast.cpp
lldb/source/Core/ValueObjectChild.cpp
lldb/source/Core/ValueObjectConstResult.cpp
lldb/source/Core/ValueObjectDynamicValue.cpp
lldb/source/Core/ValueObjectMemory.cpp
lldb/source/Core/ValueObjectRegister.cpp
lldb/source/Core/ValueObjectSyntheticFilter.cpp
lldb/source/Core/ValueObjectVTable.cpp
lldb/source/Core/ValueObjectVariable.cpp
lldb/source/DataFormatters/FormatManager.cpp
lldb/source/DataFormatters/TypeSynthetic.cpp
lldb/source/DataFormatters/ValueObjectPrinter.cpp
lldb/source/DataFormatters/VectorType.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp

lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxRangesRefView.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxValarray.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
lldb/source/Plugins/Language/ObjC/Cocoa.cpp
lldb/source/Plugins/Language/ObjC/NSArray.cpp
lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
lldb/source/Plugins/Language/ObjC/NSError.cpp
lldb/source/Plugins/Language/ObjC/NSException.cpp
lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
lldb/source/Plugins/Language/ObjC/NSSet.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
lldb/source/Symbol/CompilerType.cpp
lldb/source/Symbol/Type.cpp
lldb/source/Symbol/Variable.cpp
lldb/source/Target/StackFrame.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/ValueObject.h 
b/lldb/include/lldb/Core/ValueObject.h
index e7e35e2b2bffc0..b4d2c8098edc71 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -476,13 +476,7 @@ class ValueObject {
 
   virtual size_t GetIndexOfChildWithName(llvm::StringRef name);
 
-  llvm::Expected GetNumChildren(uint32_t max = UINT32_MAX);
-  /// Like \c GetNumChildren but returns 0 on error.  You probably
-  /// shouldn't be using this function. It exists primarily to ease the
-  /// transition to more pervasive error handling while not all APIs
-  /// have been updated.
-  uint32_t GetNumChild

[Lldb-commits] [lldb] Report back errors in GetNumChildren() (PR #84265)

2024-03-08 Thread Adrian Prantl via lldb-commits


https://github.com/adrian-prantl updated 
https://github.com/llvm/llvm-project/pull/84265

>From 934ad65f660a43917ec66861127230c68c697743 Mon Sep 17 00:00:00 2001
From: Adrian Prantl 
Date: Fri, 8 Mar 2024 14:46:13 -0800
Subject: [PATCH] Report back errors in GetNumChildren()

This is a proof-of-concept patch that illustrates how to use the
Expected return values to surface rich error messages all the way up
to the ValueObjectPrinter.
---
 .../lldb/DataFormatters/ValueObjectPrinter.h  |  2 +-
 lldb/source/Core/ValueObjectVariable.cpp  |  3 ++-
 .../DataFormatters/ValueObjectPrinter.cpp | 22 +++
 .../TypeSystem/Clang/TypeSystemClang.cpp  |  8 ---
 lldb/source/Symbol/CompilerType.cpp   |  3 ++-
 .../functionalities/valobj_errors/Makefile|  9 
 .../valobj_errors/TestValueObjectErrors.py| 15 +
 .../functionalities/valobj_errors/hidden.c|  4 
 .../API/functionalities/valobj_errors/main.c  |  9 
 9 files changed, 65 insertions(+), 10 deletions(-)
 create mode 100644 lldb/test/API/functionalities/valobj_errors/Makefile
 create mode 100644 
lldb/test/API/functionalities/valobj_errors/TestValueObjectErrors.py
 create mode 100644 lldb/test/API/functionalities/valobj_errors/hidden.c
 create mode 100644 lldb/test/API/functionalities/valobj_errors/main.c

diff --git a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h 
b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
index fe46321c3186cf..32b101a2f9843c 100644
--- a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
+++ b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
@@ -127,7 +127,7 @@ class ValueObjectPrinter {
   void PrintChild(lldb::ValueObjectSP child_sp,
   const DumpValueObjectOptions::PointerDepth &curr_ptr_depth);
 
-  uint32_t GetMaxNumChildrenToPrint(bool &print_dotdotdot);
+  llvm::Expected GetMaxNumChildrenToPrint(bool &print_dotdotdot);
 
   void
   PrintChildren(bool value_printed, bool summary_printed,
diff --git a/lldb/source/Core/ValueObjectVariable.cpp 
b/lldb/source/Core/ValueObjectVariable.cpp
index dc62bb6358dc97..76680b30896833 100644
--- a/lldb/source/Core/ValueObjectVariable.cpp
+++ b/lldb/source/Core/ValueObjectVariable.cpp
@@ -98,7 +98,8 @@ uint32_t ValueObjectVariable::CalculateNumChildren(uint32_t 
max) {
   CompilerType type(GetCompilerType());
 
   if (!type.IsValid())
-return 0;
+return llvm::make_error("invalid type",
+   llvm::inconvertibleErrorCode());
 
   ExecutionContext exe_ctx(GetExecutionContextRef());
   const bool omit_empty_base_classes = true;
diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp 
b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index 46e50a8d421a71..bbdc2a99815706 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -621,13 +621,17 @@ void ValueObjectPrinter::PrintChild(
   }
 }
 
-uint32_t ValueObjectPrinter::GetMaxNumChildrenToPrint(bool &print_dotdotdot) {
+llvm::Expected
+ValueObjectPrinter::GetMaxNumChildrenToPrint(bool &print_dotdotdot) {
   ValueObject &synth_valobj = GetValueObjectForChildrenGeneration();
 
   if (m_options.m_pointer_as_array)
 return m_options.m_pointer_as_array.m_element_count;
 
-  size_t num_children = synth_valobj.GetNumChildren();
+  auto num_children_or_err = synth_valobj.GetNumChildren();
+  if (!num_children_or_err)
+return num_children_or_err;
+  uint32_t num_children = *num_children_or_err;
   print_dotdotdot = false;
   if (num_children) {
 const size_t max_num_children = GetMostSpecializedValue()
@@ -704,7 +708,12 @@ void ValueObjectPrinter::PrintChildren(
   ValueObject &synth_valobj = GetValueObjectForChildrenGeneration();
 
   bool print_dotdotdot = false;
-  size_t num_children = GetMaxNumChildrenToPrint(print_dotdotdot);
+  auto num_children_or_err = GetMaxNumChildrenToPrint(print_dotdotdot);
+  if (!num_children_or_err) {
+*m_stream << " <" << llvm::toString(num_children_or_err.takeError()) << 
'>';
+return;
+  }
+  uint32_t num_children = *num_children_or_err;
   if (num_children) {
 bool any_children_printed = false;
 
@@ -753,7 +762,12 @@ bool ValueObjectPrinter::PrintChildrenOneLiner(bool 
hide_names) {
   ValueObject &synth_valobj = GetValueObjectForChildrenGeneration();
 
   bool print_dotdotdot = false;
-  size_t num_children = GetMaxNumChildrenToPrint(print_dotdotdot);
+  auto num_children_or_err = GetMaxNumChildrenToPrint(print_dotdotdot);
+  if (!num_children_or_err) {
+*m_stream << '<' << llvm::toString(num_children_or_err.takeError()) << '>';
+return true;
+  }
+  uint32_t num_children = *num_children_or_err;
 
   if (num_children) {
 m_stream->PutChar('(');
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 51ab13108feb3a..041011254a85eb 100644
--- a/lldb/source/Plugins/TypeSystem/

[Lldb-commits] [lldb] Change GetNumChildren()/CalculateNumChildren() methods return llvm::Expected (PR #84219)

2024-03-08 Thread Adrian Prantl via lldb-commits


adrian-prantl wrote:

Apologies, I made a last-minute rebase without rebuilding. Thanks for reverting!

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

[Lldb-commits] [lldb] Change GetNumChildren()/CalculateNumChildren() methods return llvm::Expected (PR #84219)

2024-03-08 Thread Adrian Prantl via lldb-commits


adrian-prantl wrote:

Or rather, I didn't build with all LLVM targets.

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

[Lldb-commits] [lldb] [LLDB] ASanLibsanitizers Use `sanitizers_address_on_report` breakpoint (PR #84583)

2024-03-08 Thread Usama Hameed via lldb-commits


https://github.com/usama54321 created 
https://github.com/llvm/llvm-project/pull/84583

symbol

This patch puts the default breakpoint on the
sanitizers_address_on_report symbol, and uses the old symbol as a backup if the 
default case is not found

rdar://123911522

>From aa8f732f4d6d2cdae5b32462b7be8f3afda512fc Mon Sep 17 00:00:00 2001
From: usama 
Date: Fri, 8 Mar 2024 15:30:46 -0800
Subject: [PATCH] [LLDB] ASanLibsanitizers Use `sanitizers_address_on_report`
 breakpoint symbol

This patch puts the default breakpoint on the
sanitizers_address_on_report symbol, and uses the old symbol as a backup
if the default case is not found

rdar://123911522
---
 .../InstrumentationRuntimeASanLibsanitizers.cpp  | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git 
a/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
 
b/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
index d84cd36d7ce17b..e095d3e6b2e339 100644
--- 
a/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
+++ 
b/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
@@ -90,9 +90,17 @@ void InstrumentationRuntimeASanLibsanitizers::Activate() {
   if (!process_sp)
 return;
 
+  lldb::ModuleSP module_sp = GetRuntimeModuleSP();
+
   Breakpoint *breakpoint = ReportRetriever::SetupBreakpoint(
-  GetRuntimeModuleSP(), process_sp,
-  ConstString("_Z22raise_sanitizers_error23sanitizer_error_context"));
+  module_sp, process_sp,
+  ConstString("_sanitizers_address_on_report"));
+
+  if (!breakpoint) {
+breakpoint = ReportRetriever::SetupBreakpoint(
+module_sp, process_sp,
+ConstString("_Z22raise_sanitizers_error23sanitizer_error_context"));
+  }
 
   if (!breakpoint)
 return;

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

[Lldb-commits] [lldb] [LLDB] ASanLibsanitizers Use `sanitizers_address_on_report` breakpoint (PR #84583)

2024-03-08 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Usama Hameed (usama54321)


Changes

symbol

This patch puts the default breakpoint on the
sanitizers_address_on_report symbol, and uses the old symbol as a backup if the 
default case is not found

rdar://123911522

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


1 Files Affected:

- (modified) 
lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
 (+10-2) 


``diff
diff --git 
a/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
 
b/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
index d84cd36d7ce17b..e095d3e6b2e339 100644
--- 
a/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
+++ 
b/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
@@ -90,9 +90,17 @@ void InstrumentationRuntimeASanLibsanitizers::Activate() {
   if (!process_sp)
 return;
 
+  lldb::ModuleSP module_sp = GetRuntimeModuleSP();
+
   Breakpoint *breakpoint = ReportRetriever::SetupBreakpoint(
-  GetRuntimeModuleSP(), process_sp,
-  ConstString("_Z22raise_sanitizers_error23sanitizer_error_context"));
+  module_sp, process_sp,
+  ConstString("_sanitizers_address_on_report"));
+
+  if (!breakpoint) {
+breakpoint = ReportRetriever::SetupBreakpoint(
+module_sp, process_sp,
+ConstString("_Z22raise_sanitizers_error23sanitizer_error_context"));
+  }
 
   if (!breakpoint)
 return;

``




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

[Lldb-commits] [lldb] [LLDB] ASanLibsanitizers Use `sanitizers_address_on_report` breakpoint (PR #84583)

2024-03-08 Thread via lldb-commits


github-actions[bot] wrote:




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



You can test this locally with the following command:


``bash
git-clang-format --diff cb6ff746e0c7b9218b6f5c11db44162cacd623a4 
aa8f732f4d6d2cdae5b32462b7be8f3afda512fc -- 
lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
``





View the diff from clang-format here.


``diff
diff --git 
a/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
 
b/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
index e095d3e6b2..dd936fc07f 100644
--- 
a/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
+++ 
b/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
@@ -93,8 +93,7 @@ void InstrumentationRuntimeASanLibsanitizers::Activate() {
   lldb::ModuleSP module_sp = GetRuntimeModuleSP();
 
   Breakpoint *breakpoint = ReportRetriever::SetupBreakpoint(
-  module_sp, process_sp,
-  ConstString("_sanitizers_address_on_report"));
+  module_sp, process_sp, ConstString("_sanitizers_address_on_report"));
 
   if (!breakpoint) {
 breakpoint = ReportRetriever::SetupBreakpoint(

``




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

[Lldb-commits] [lldb] [LLDB] ASanLibsanitizers Use `sanitizers_address_on_report` breakpoint (PR #84583)

2024-03-08 Thread Usama Hameed via lldb-commits


https://github.com/usama54321 updated 
https://github.com/llvm/llvm-project/pull/84583

>From fbe08446991972f1fde90206d0281c506d668d6b Mon Sep 17 00:00:00 2001
From: usama 
Date: Fri, 8 Mar 2024 15:55:25 -0800
Subject: [PATCH] [LLDB] ASanLibsanitizers Use `sanitizers_address_on_report`
 breakpoint symbol

This patch puts the default breakpoint on the
sanitizers_address_on_report symbol, and uses the old symbol as a backup
if the default case is not found

rdar://123911522
---
 .../InstrumentationRuntimeASanLibsanitizers.cpp   | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git 
a/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
 
b/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
index d84cd36d7ce17b..dd936fc07fc3c4 100644
--- 
a/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
+++ 
b/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
@@ -90,9 +90,16 @@ void InstrumentationRuntimeASanLibsanitizers::Activate() {
   if (!process_sp)
 return;
 
+  lldb::ModuleSP module_sp = GetRuntimeModuleSP();
+
   Breakpoint *breakpoint = ReportRetriever::SetupBreakpoint(
-  GetRuntimeModuleSP(), process_sp,
-  ConstString("_Z22raise_sanitizers_error23sanitizer_error_context"));
+  module_sp, process_sp, ConstString("_sanitizers_address_on_report"));
+
+  if (!breakpoint) {
+breakpoint = ReportRetriever::SetupBreakpoint(
+module_sp, process_sp,
+ConstString("_Z22raise_sanitizers_error23sanitizer_error_context"));
+  }
 
   if (!breakpoint)
 return;

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

[Lldb-commits] [lldb] 624ea68 - Change GetNumChildren()/CalculateNumChildren() methods return llvm::Expected (#84219)

2024-03-08 Thread Adrian Prantl via lldb-commits


Author: Adrian Prantl
Date: 2024-03-08T16:03:04-08:00
New Revision: 624ea68cbc3ce422b3ee110c0c0af839eec2e278

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

LOG: Change GetNumChildren()/CalculateNumChildren() methods return 
llvm::Expected (#84219)

Change GetNumChildren()/CalculateNumChildren() methods return
llvm::Expected

This is an NFC change that does not yet add any error handling or change
any code to return any errors.

This is the second big change in the patch series started with
https://github.com/llvm/llvm-project/pull/83501

A follow-up PR will wire up error handling.

Added: 


Modified: 
lldb/include/lldb/Core/ValueObject.h
lldb/include/lldb/Core/ValueObjectCast.h
lldb/include/lldb/Core/ValueObjectChild.h
lldb/include/lldb/Core/ValueObjectConstResult.h
lldb/include/lldb/Core/ValueObjectDynamicValue.h
lldb/include/lldb/Core/ValueObjectMemory.h
lldb/include/lldb/Core/ValueObjectRegister.h
lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
lldb/include/lldb/Core/ValueObjectVTable.h
lldb/include/lldb/Core/ValueObjectVariable.h
lldb/include/lldb/DataFormatters/TypeSynthetic.h
lldb/include/lldb/DataFormatters/VectorIterator.h
lldb/include/lldb/Symbol/CompilerType.h
lldb/include/lldb/Symbol/Type.h
lldb/include/lldb/Symbol/TypeSystem.h
lldb/include/lldb/Target/StackFrameRecognizer.h
lldb/include/lldb/Utility/Log.h
lldb/source/API/SBValue.cpp
lldb/source/Core/FormatEntity.cpp
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Core/ValueObject.cpp
lldb/source/Core/ValueObjectCast.cpp
lldb/source/Core/ValueObjectChild.cpp
lldb/source/Core/ValueObjectConstResult.cpp
lldb/source/Core/ValueObjectDynamicValue.cpp
lldb/source/Core/ValueObjectMemory.cpp
lldb/source/Core/ValueObjectRegister.cpp
lldb/source/Core/ValueObjectSyntheticFilter.cpp
lldb/source/Core/ValueObjectVTable.cpp
lldb/source/Core/ValueObjectVariable.cpp
lldb/source/DataFormatters/FormatManager.cpp
lldb/source/DataFormatters/TypeSynthetic.cpp
lldb/source/DataFormatters/ValueObjectPrinter.cpp
lldb/source/DataFormatters/VectorType.cpp
lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp

lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxRangesRefView.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxValarray.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
lldb/source/Plugins/Language/ObjC/Cocoa.cpp
lldb/source/Plugins/Language/ObjC/NSArray.cpp
lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
lldb/source/Plugins/Language/ObjC/NSError.cpp
lldb/source/Plugins/Language/ObjC/NSException.cpp
lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp
lldb/source/Plugins/Language/ObjC/NSSet.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
lldb/source/Symbol/CompilerType.cpp
lldb/source/Symbol/Type.cpp
lldb/source/Symbol/Variable.cpp
lldb/source/Target/StackFrame.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/ValueObject.h 
b/lldb/include/lldb/Core/ValueObject.h
index b4d2c8098edc71..e7e35e2b2bffc0 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -476,7 +476,13 @@ class ValueObject {
 
   virtual size_t GetIndexOfChildWithName(llvm::StringRef name);

[Lldb-commits] [lldb] [LLDB] ASanLibsanitizers Use `sanitizers_address_on_report` breakpoint (PR #84583)

2024-03-08 Thread Julian Lettner via lldb-commits


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

LGTM, thanks!

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

[Lldb-commits] [lldb] [LLDB] ASanLibsanitizers Use `sanitizers_address_on_report` breakpoint (PR #84583)

2024-03-08 Thread via lldb-commits


jimingham wrote:

ReportRetriever::SetupBreakpoint is a weird way to set a breakpoint.  This is 
somewhat off topic, but do you know why it was done this way?  You could set it 
by symbol & module and then you'd have something that survives rebuilds of the 
module, or you could set it by Address, in which case it would survive 
re-running, but instead it converts the Address to an addr_t - the most fragile 
way to set a breakpoint - and uses that...

This came up because you could (if this weren't done so oddly) handle this by 
having a list of candidate functions, and make a single breakpoint with all 
those names...

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

[Lldb-commits] [lldb] Report back errors in GetNumChildren() (PR #84265)

2024-03-08 Thread Adrian Prantl via lldb-commits


https://github.com/adrian-prantl updated 
https://github.com/llvm/llvm-project/pull/84265

>From 9b02d004a3aec681047d47cb481a269637bdd9fc Mon Sep 17 00:00:00 2001
From: Adrian Prantl 
Date: Fri, 8 Mar 2024 14:46:13 -0800
Subject: [PATCH] Report back errors in GetNumChildren()

This is a proof-of-concept patch that illustrates how to use the
Expected return values to surface rich error messages all the way up
to the ValueObjectPrinter.
---
 .../lldb/DataFormatters/ValueObjectPrinter.h  |  2 +-
 lldb/source/Core/ValueObjectVariable.cpp  |  3 ++-
 .../DataFormatters/ValueObjectPrinter.cpp | 22 +++
 .../TypeSystem/Clang/TypeSystemClang.cpp  |  8 ---
 lldb/source/Symbol/CompilerType.cpp   |  3 ++-
 .../functionalities/valobj_errors/Makefile|  9 
 .../valobj_errors/TestValueObjectErrors.py| 15 +
 .../functionalities/valobj_errors/hidden.c|  4 
 .../API/functionalities/valobj_errors/main.c  |  9 
 9 files changed, 65 insertions(+), 10 deletions(-)
 create mode 100644 lldb/test/API/functionalities/valobj_errors/Makefile
 create mode 100644 
lldb/test/API/functionalities/valobj_errors/TestValueObjectErrors.py
 create mode 100644 lldb/test/API/functionalities/valobj_errors/hidden.c
 create mode 100644 lldb/test/API/functionalities/valobj_errors/main.c

diff --git a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h 
b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
index fe46321c3186cf..32b101a2f9843c 100644
--- a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
+++ b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
@@ -127,7 +127,7 @@ class ValueObjectPrinter {
   void PrintChild(lldb::ValueObjectSP child_sp,
   const DumpValueObjectOptions::PointerDepth &curr_ptr_depth);
 
-  uint32_t GetMaxNumChildrenToPrint(bool &print_dotdotdot);
+  llvm::Expected GetMaxNumChildrenToPrint(bool &print_dotdotdot);
 
   void
   PrintChildren(bool value_printed, bool summary_printed,
diff --git a/lldb/source/Core/ValueObjectVariable.cpp 
b/lldb/source/Core/ValueObjectVariable.cpp
index fb29c22c0ab5af..67d71c90a959d5 100644
--- a/lldb/source/Core/ValueObjectVariable.cpp
+++ b/lldb/source/Core/ValueObjectVariable.cpp
@@ -99,7 +99,8 @@ ValueObjectVariable::CalculateNumChildren(uint32_t max) {
   CompilerType type(GetCompilerType());
 
   if (!type.IsValid())
-return 0;
+return llvm::make_error("invalid type",
+   llvm::inconvertibleErrorCode());
 
   ExecutionContext exe_ctx(GetExecutionContextRef());
   const bool omit_empty_base_classes = true;
diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp 
b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index b853199e878c95..bbdc2a99815706 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -621,13 +621,17 @@ void ValueObjectPrinter::PrintChild(
   }
 }
 
-uint32_t ValueObjectPrinter::GetMaxNumChildrenToPrint(bool &print_dotdotdot) {
+llvm::Expected
+ValueObjectPrinter::GetMaxNumChildrenToPrint(bool &print_dotdotdot) {
   ValueObject &synth_valobj = GetValueObjectForChildrenGeneration();
 
   if (m_options.m_pointer_as_array)
 return m_options.m_pointer_as_array.m_element_count;
 
-  uint32_t num_children = synth_valobj.GetNumChildrenIgnoringErrors();
+  auto num_children_or_err = synth_valobj.GetNumChildren();
+  if (!num_children_or_err)
+return num_children_or_err;
+  uint32_t num_children = *num_children_or_err;
   print_dotdotdot = false;
   if (num_children) {
 const size_t max_num_children = GetMostSpecializedValue()
@@ -704,7 +708,12 @@ void ValueObjectPrinter::PrintChildren(
   ValueObject &synth_valobj = GetValueObjectForChildrenGeneration();
 
   bool print_dotdotdot = false;
-  size_t num_children = GetMaxNumChildrenToPrint(print_dotdotdot);
+  auto num_children_or_err = GetMaxNumChildrenToPrint(print_dotdotdot);
+  if (!num_children_or_err) {
+*m_stream << " <" << llvm::toString(num_children_or_err.takeError()) << 
'>';
+return;
+  }
+  uint32_t num_children = *num_children_or_err;
   if (num_children) {
 bool any_children_printed = false;
 
@@ -753,7 +762,12 @@ bool ValueObjectPrinter::PrintChildrenOneLiner(bool 
hide_names) {
   ValueObject &synth_valobj = GetValueObjectForChildrenGeneration();
 
   bool print_dotdotdot = false;
-  size_t num_children = GetMaxNumChildrenToPrint(print_dotdotdot);
+  auto num_children_or_err = GetMaxNumChildrenToPrint(print_dotdotdot);
+  if (!num_children_or_err) {
+*m_stream << '<' << llvm::toString(num_children_or_err.takeError()) << '>';
+return true;
+  }
+  uint32_t num_children = *num_children_or_err;
 
   if (num_children) {
 m_stream->PutChar('(');
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index c02b08cb478280..16974224f3e2ca 100644
--- a/lldb/source/Plugins/TypeS

[Lldb-commits] [lldb] [LLDB] ASanLibsanitizers Use `sanitizers_address_on_report` breakpoint (PR #84583)

2024-03-08 Thread Julian Lettner via lldb-commits

yln wrote:

@jimingham, thanks for the review and feedback on how to improve the code.  Are 
you okay with us addressing this in a follow-up?

> do you know why it was done this way?

I don't think we do.  I think we just copied what was done in the old ASan 
plugin.

> survives rebuilds

I think we are fine for rebuilds.  The module we are setting the breakpoint in 
is the ASan runtime, which doesn't get rebuilt.

> This came up because you could (if this weren't done so oddly) handle this by 
> having a list of candidate functions, and make a single breakpoint with all 
> those names...

@usama54321 please add a FIXME/TODO in the code and file a radar to improve 
this (you can assign it o me)

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

[Lldb-commits] [lldb] Make ValueObject::Cast work for casts from smaller to larger structs in the cases where this currently can work. (PR #84588)

2024-03-08 Thread via lldb-commits


https://github.com/jimingham created 
https://github.com/llvm/llvm-project/pull/84588

The ValueObjectConstResult classes that back expression result variables play a 
complicated game with where the data for their values is stored.  They try to 
make it appear as though they are still tied to the memory in the target into 
which their value was written when the expression is run, but they also keep a 
copy in the Host which they use after the value is made (expression results are 
"history values" so that's how we make sure they have "the value at the time of 
the expression".)

However, that means that if you ask them to cast themselves to a value bigger 
than their original size, they don't have a way to get more memory for that 
purpose.  The same thing is true of ValueObjects backed by DataExtractors, the 
data extractors don't know how to get more data than they were made with in 
general.

The only place where we actually ask ValueObjects to sample outside their 
captured bounds is when you do ValueObject::Cast from one structure type to a 
bigger structure type.  In https://reviews.llvm.org/D153657 I handled this by 
just disallowing casts from one structure value to a larger one.  My reasoning 
at the time was that the use case for this was to support discriminator based C 
inheritance schemes, and you can't directly cast values in C, only pointers, so 
this was not a natural way to handle those types. It seemed logical that since 
you would have had to start with pointers in the implementation, that's how you 
would write your lldb introspection code as well.

Famous last words...

Turns out there are some heavy users of the SB API's who were relying on this 
working, and this is a behavior change, so this patch makes this work in the 
cases where it used to work before, while still disallowing the cases we don't 
know how to support.

Note that if you had done this Cast operation before with either expression 
results or value objects from data extractors, lldb would not have returned the 
correct results, so the cases this patch outlaws are ones that actually produce 
invalid results.  So nobody should be using Cast in these cases, or if they 
were, this patch will point out the bug they hadn't yet noticed.

>From e85c0108f3508537b4d2d9fe9120dd07ebd00361 Mon Sep 17 00:00:00 2001
From: Jim Ingham 
Date: Fri, 8 Mar 2024 15:39:50 -0800
Subject: [PATCH] Make ValueObject::Cast work for casts from smaller to larger
 structs in the cases where this currently can work.

---
 lldb/source/Core/ValueObject.cpp  | 20 --
 .../test/API/python_api/value/TestValueAPI.py | 68 ---
 lldb/test/API/python_api/value/main.c | 15 +++-
 3 files changed, 89 insertions(+), 14 deletions(-)

diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index d813044d02ff5f..f39bd07a255366 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -2744,8 +2744,19 @@ ValueObjectSP ValueObject::DoCast(const CompilerType 
&compiler_type) {
 
 ValueObjectSP ValueObject::Cast(const CompilerType &compiler_type) {
   // Only allow casts if the original type is equal or larger than the cast
-  // type.  We don't know how to fetch more data for all the ConstResult types,
-  // so we can't guarantee this will work:
+  // type, unless we know this is a load address.  Getting the size wrong for
+  // a host side storage could leak lldb memory, so we absolutely want to 
+  // prevent that.  We may not always get the right value, for instance if we
+  // have an expression result value that's copied into a storage location in
+  // the target may not have copied enough memory.  I'm not trying to fix that
+  // here, I'm just making Cast from a smaller to a larger possible in all the
+  // cases where that doesn't risk making a Value out of random lldb memory.
+  // You have to check the ValueObject's Value for the address types, since
+  // ValueObjects that use live addresses will tell you they fetch data from 
the
+  // live address, but once they are made, they actually don't.
+  // FIXME: Can we make ValueObject's with a live address fetch "more data" 
from
+  // the live address if it is still valid?
+
   Status error;
   CompilerType my_type = GetCompilerType();
 
@@ -2753,9 +2764,10 @@ ValueObjectSP ValueObject::Cast(const CompilerType 
&compiler_type) {
   = ExecutionContext(GetExecutionContextRef())
   .GetBestExecutionContextScope();
   if (compiler_type.GetByteSize(exe_scope)
-  <= GetCompilerType().GetByteSize(exe_scope)) {
+  <= GetCompilerType().GetByteSize(exe_scope) 
+  || m_value.GetValueType() == Value::ValueType::LoadAddress)
 return DoCast(compiler_type);
-  }
+
   error.SetErrorString("Can only cast to a type that is equal to or smaller "
"than the orignal type.");
 
diff --git a/lldb/test/API/python_api/value/TestValueAPI.py 
b/lldb/test/API/python_api/value/TestValueAPI.p

[Lldb-commits] [lldb] Make ValueObject::Cast work for casts from smaller to larger structs in the cases where this currently can work. (PR #84588)

2024-03-08 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: None (jimingham)


Changes

The ValueObjectConstResult classes that back expression result variables play a 
complicated game with where the data for their values is stored.  They try to 
make it appear as though they are still tied to the memory in the target into 
which their value was written when the expression is run, but they also keep a 
copy in the Host which they use after the value is made (expression results are 
"history values" so that's how we make sure they have "the value at the time of 
the expression".)

However, that means that if you ask them to cast themselves to a value bigger 
than their original size, they don't have a way to get more memory for that 
purpose.  The same thing is true of ValueObjects backed by DataExtractors, the 
data extractors don't know how to get more data than they were made with in 
general.

The only place where we actually ask ValueObjects to sample outside their 
captured bounds is when you do ValueObject::Cast from one structure type to a 
bigger structure type.  In https://reviews.llvm.org/D153657 I handled this by 
just disallowing casts from one structure value to a larger one.  My reasoning 
at the time was that the use case for this was to support discriminator based C 
inheritance schemes, and you can't directly cast values in C, only pointers, so 
this was not a natural way to handle those types. It seemed logical that since 
you would have had to start with pointers in the implementation, that's how you 
would write your lldb introspection code as well.

Famous last words...

Turns out there are some heavy users of the SB API's who were relying on this 
working, and this is a behavior change, so this patch makes this work in the 
cases where it used to work before, while still disallowing the cases we don't 
know how to support.

Note that if you had done this Cast operation before with either expression 
results or value objects from data extractors, lldb would not have returned the 
correct results, so the cases this patch outlaws are ones that actually produce 
invalid results.  So nobody should be using Cast in these cases, or if they 
were, this patch will point out the bug they hadn't yet noticed.

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


3 Files Affected:

- (modified) lldb/source/Core/ValueObject.cpp (+16-4) 
- (modified) lldb/test/API/python_api/value/TestValueAPI.py (+60-8) 
- (modified) lldb/test/API/python_api/value/main.c (+13-2) 


``diff
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index d813044d02ff5f..f39bd07a255366 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -2744,8 +2744,19 @@ ValueObjectSP ValueObject::DoCast(const CompilerType 
&compiler_type) {
 
 ValueObjectSP ValueObject::Cast(const CompilerType &compiler_type) {
   // Only allow casts if the original type is equal or larger than the cast
-  // type.  We don't know how to fetch more data for all the ConstResult types,
-  // so we can't guarantee this will work:
+  // type, unless we know this is a load address.  Getting the size wrong for
+  // a host side storage could leak lldb memory, so we absolutely want to 
+  // prevent that.  We may not always get the right value, for instance if we
+  // have an expression result value that's copied into a storage location in
+  // the target may not have copied enough memory.  I'm not trying to fix that
+  // here, I'm just making Cast from a smaller to a larger possible in all the
+  // cases where that doesn't risk making a Value out of random lldb memory.
+  // You have to check the ValueObject's Value for the address types, since
+  // ValueObjects that use live addresses will tell you they fetch data from 
the
+  // live address, but once they are made, they actually don't.
+  // FIXME: Can we make ValueObject's with a live address fetch "more data" 
from
+  // the live address if it is still valid?
+
   Status error;
   CompilerType my_type = GetCompilerType();
 
@@ -2753,9 +2764,10 @@ ValueObjectSP ValueObject::Cast(const CompilerType 
&compiler_type) {
   = ExecutionContext(GetExecutionContextRef())
   .GetBestExecutionContextScope();
   if (compiler_type.GetByteSize(exe_scope)
-  <= GetCompilerType().GetByteSize(exe_scope)) {
+  <= GetCompilerType().GetByteSize(exe_scope) 
+  || m_value.GetValueType() == Value::ValueType::LoadAddress)
 return DoCast(compiler_type);
-  }
+
   error.SetErrorString("Can only cast to a type that is equal to or smaller "
"than the orignal type.");
 
diff --git a/lldb/test/API/python_api/value/TestValueAPI.py 
b/lldb/test/API/python_api/value/TestValueAPI.py
index 18376f76e3c850..512100912d6fe7 100644
--- a/lldb/test/API/python_api/value/TestValueAPI.py
+++ b/lldb/test/API/python_api/value/TestValueAPI.py
@@ -148,14 +148,66 @@ def test(self):
 
 # Test some other

[Lldb-commits] [lldb] Make ValueObject::Cast work for casts from smaller to larger structs in the cases where this currently can work. (PR #84588)

2024-03-08 Thread via lldb-commits


github-actions[bot] wrote:




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



You can test this locally with the following command:


``bash
git-clang-format --diff 9405d5af65853ac548cce2656497195010db1d86 
e85c0108f3508537b4d2d9fe9120dd07ebd00361 -- lldb/source/Core/ValueObject.cpp 
lldb/test/API/python_api/value/main.c
``





View the diff from clang-format here.


``diff
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index f39bd07a25..81c1f7fc20 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -2745,7 +2745,7 @@ ValueObjectSP ValueObject::DoCast(const CompilerType 
&compiler_type) {
 ValueObjectSP ValueObject::Cast(const CompilerType &compiler_type) {
   // Only allow casts if the original type is equal or larger than the cast
   // type, unless we know this is a load address.  Getting the size wrong for
-  // a host side storage could leak lldb memory, so we absolutely want to 
+  // a host side storage could leak lldb memory, so we absolutely want to
   // prevent that.  We may not always get the right value, for instance if we
   // have an expression result value that's copied into a storage location in
   // the target may not have copied enough memory.  I'm not trying to fix that
@@ -2763,10 +2763,10 @@ ValueObjectSP ValueObject::Cast(const CompilerType 
&compiler_type) {
   ExecutionContextScope *exe_scope
   = ExecutionContext(GetExecutionContextRef())
   .GetBestExecutionContextScope();
-  if (compiler_type.GetByteSize(exe_scope)
-  <= GetCompilerType().GetByteSize(exe_scope) 
-  || m_value.GetValueType() == Value::ValueType::LoadAddress)
-return DoCast(compiler_type);
+  if (compiler_type.GetByteSize(exe_scope) <=
+  GetCompilerType().GetByteSize(exe_scope) ||
+  m_value.GetValueType() == Value::ValueType::LoadAddress)
+return DoCast(compiler_type);
 
   error.SetErrorString("Can only cast to a type that is equal to or smaller "
"than the orignal type.");
diff --git a/lldb/test/API/python_api/value/main.c 
b/lldb/test/API/python_api/value/main.c
index cdb2aa2f61..78a7de6210 100644
--- a/lldb/test/API/python_api/value/main.c
+++ b/lldb/test/API/python_api/value/main.c
@@ -22,7 +22,7 @@ const char *weekdays[5] = { "Monday",
 const char **g_table[2] = { days_of_week, weekdays };
 
 typedef int MyInt;
-  
+
 struct MyStruct
 {
   int a;
@@ -36,15 +36,14 @@ struct MyBiggerStruct
   int c;
 };
 
-struct Container
-{
+struct Container {
   int discriminator;
   union Data {
 struct MyStruct small;
 struct MyBiggerStruct big;
   } data;
 };
-  
+
 int main (int argc, char const *argv[])
 {
 uint32_t uinthex = 0xE0A35F10;

``




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

[Lldb-commits] [lldb] Report back errors in GetNumChildren() (PR #84265)

2024-03-08 Thread Adrian Prantl via lldb-commits


https://github.com/adrian-prantl updated 
https://github.com/llvm/llvm-project/pull/84265

>From 78866b8977b655109d85bd1c231b1659c5c90c8a Mon Sep 17 00:00:00 2001
From: Adrian Prantl 
Date: Fri, 8 Mar 2024 14:46:13 -0800
Subject: [PATCH] Report back errors in GetNumChildren()

This is a proof-of-concept patch that illustrates how to use the
Expected return values to surface rich error messages all the way up
to the ValueObjectPrinter.
---
 .../lldb/DataFormatters/ValueObjectPrinter.h  |  2 +-
 lldb/source/Core/ValueObjectVariable.cpp  |  3 ++-
 .../DataFormatters/ValueObjectPrinter.cpp | 22 +++
 .../TypeSystem/Clang/TypeSystemClang.cpp  |  9 +---
 lldb/source/Symbol/CompilerType.cpp   |  3 ++-
 .../functionalities/valobj_errors/Makefile|  9 
 .../valobj_errors/TestValueObjectErrors.py| 15 +
 .../functionalities/valobj_errors/hidden.c|  4 
 .../API/functionalities/valobj_errors/main.c  |  9 
 .../x86/DW_AT_declaration-with-children.s |  2 +-
 .../x86/debug-types-missing-signature.test|  2 +-
 11 files changed, 68 insertions(+), 12 deletions(-)
 create mode 100644 lldb/test/API/functionalities/valobj_errors/Makefile
 create mode 100644 
lldb/test/API/functionalities/valobj_errors/TestValueObjectErrors.py
 create mode 100644 lldb/test/API/functionalities/valobj_errors/hidden.c
 create mode 100644 lldb/test/API/functionalities/valobj_errors/main.c

diff --git a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h 
b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
index fe46321c3186cf..32b101a2f9843c 100644
--- a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
+++ b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
@@ -127,7 +127,7 @@ class ValueObjectPrinter {
   void PrintChild(lldb::ValueObjectSP child_sp,
   const DumpValueObjectOptions::PointerDepth &curr_ptr_depth);
 
-  uint32_t GetMaxNumChildrenToPrint(bool &print_dotdotdot);
+  llvm::Expected GetMaxNumChildrenToPrint(bool &print_dotdotdot);
 
   void
   PrintChildren(bool value_printed, bool summary_printed,
diff --git a/lldb/source/Core/ValueObjectVariable.cpp 
b/lldb/source/Core/ValueObjectVariable.cpp
index fb29c22c0ab5af..67d71c90a959d5 100644
--- a/lldb/source/Core/ValueObjectVariable.cpp
+++ b/lldb/source/Core/ValueObjectVariable.cpp
@@ -99,7 +99,8 @@ ValueObjectVariable::CalculateNumChildren(uint32_t max) {
   CompilerType type(GetCompilerType());
 
   if (!type.IsValid())
-return 0;
+return llvm::make_error("invalid type",
+   llvm::inconvertibleErrorCode());
 
   ExecutionContext exe_ctx(GetExecutionContextRef());
   const bool omit_empty_base_classes = true;
diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp 
b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index b853199e878c95..bbdc2a99815706 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -621,13 +621,17 @@ void ValueObjectPrinter::PrintChild(
   }
 }
 
-uint32_t ValueObjectPrinter::GetMaxNumChildrenToPrint(bool &print_dotdotdot) {
+llvm::Expected
+ValueObjectPrinter::GetMaxNumChildrenToPrint(bool &print_dotdotdot) {
   ValueObject &synth_valobj = GetValueObjectForChildrenGeneration();
 
   if (m_options.m_pointer_as_array)
 return m_options.m_pointer_as_array.m_element_count;
 
-  uint32_t num_children = synth_valobj.GetNumChildrenIgnoringErrors();
+  auto num_children_or_err = synth_valobj.GetNumChildren();
+  if (!num_children_or_err)
+return num_children_or_err;
+  uint32_t num_children = *num_children_or_err;
   print_dotdotdot = false;
   if (num_children) {
 const size_t max_num_children = GetMostSpecializedValue()
@@ -704,7 +708,12 @@ void ValueObjectPrinter::PrintChildren(
   ValueObject &synth_valobj = GetValueObjectForChildrenGeneration();
 
   bool print_dotdotdot = false;
-  size_t num_children = GetMaxNumChildrenToPrint(print_dotdotdot);
+  auto num_children_or_err = GetMaxNumChildrenToPrint(print_dotdotdot);
+  if (!num_children_or_err) {
+*m_stream << " <" << llvm::toString(num_children_or_err.takeError()) << 
'>';
+return;
+  }
+  uint32_t num_children = *num_children_or_err;
   if (num_children) {
 bool any_children_printed = false;
 
@@ -753,7 +762,12 @@ bool ValueObjectPrinter::PrintChildrenOneLiner(bool 
hide_names) {
   ValueObject &synth_valobj = GetValueObjectForChildrenGeneration();
 
   bool print_dotdotdot = false;
-  size_t num_children = GetMaxNumChildrenToPrint(print_dotdotdot);
+  auto num_children_or_err = GetMaxNumChildrenToPrint(print_dotdotdot);
+  if (!num_children_or_err) {
+*m_stream << '<' << llvm::toString(num_children_or_err.takeError()) << '>';
+return true;
+  }
+  uint32_t num_children = *num_children_or_err;
 
   if (num_children) {
 m_stream->PutChar('(');
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/

[Lldb-commits] [lldb] Report back errors in GetNumChildren() (PR #84265)

2024-03-08 Thread Adrian Prantl via lldb-commits


adrian-prantl wrote:

Found two more tests that were (positively) affected by this change!

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

[Lldb-commits] [lldb] [LLDB] ASanLibsanitizers Use `sanitizers_address_on_report` breakpoint (PR #84583)

2024-03-08 Thread via lldb-commits


jimingham wrote:

I have no problem with doing this in stages, what needs doing right now and 
cleanups to follow.

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

[Lldb-commits] [lldb] [LLDB] ASanLibsanitizers Use `sanitizers_address_on_report` breakpoint (PR #84583)

2024-03-08 Thread via lldb-commits


jimingham wrote:

> @jimingham, thanks for the review and feedback on how to improve the code. 
> Are you okay with us addressing this in a follow-up?
> 
> > do you know why it was done this way?
> 
> I don't think we do. I think we just copied what was done in the old ASan 
> plugin. My guess for the original code is that there was no specific reason 
> other than "it worked". I definitely want to follow your guidance and improve 
> this if it has benefits (and it seems easy to do).
> 
> > survives rebuilds
> 
> I think we are fine for rebuilds. The module we are setting the breakpoint in 
> is the ASan runtime, which doesn't get rebuilt.

Unless you happen to be working on the asan runtime...

> 
> > This came up because you could (if this weren't done so oddly) handle this 
> > by having a list of candidate functions, and make a single breakpoint with 
> > all those names...
> 
> @usama54321 please add a FIXME/TODO in the code and file a radar to improve 
> this (you can assign it to me)



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

[Lldb-commits] [lldb] [LLDB] ASanLibsanitizers Use `sanitizers_address_on_report` breakpoint (PR #84583)

2024-03-08 Thread via lldb-commits


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

LGTM

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

[Lldb-commits] [lldb] Turn off instruction flow control annotations by default (PR #84607)

2024-03-08 Thread Jason Molenda via lldb-commits


https://github.com/jasonmolenda created 
https://github.com/llvm/llvm-project/pull/84607

Walter Erquinigo added optional instruction annotations for x86 instructions in 
2022 for the `thread trace dump instruction` command, and code to 
DisassemblerLLVMC to add annotations for instructions that change flow control, 
v.  https://reviews.llvm.org/D128477

This was added as an option to `disassemble`, and the trace dump command 
enables it by default, but several other instruction dumpers were changed to 
display them by default as well.  These are only implemented for Intel 
instructions, so our disassembly on other targets ends up looking like

```
(lldb) x/5i 0x186e4
0x186e4: 0xa9be6ffc   unknown stpx28, x27, [sp, #-0x20]!
0x186e8: 0xa9017bfd   unknown stpx29, x30, [sp, #0x10]
0x186ec: 0x910043fd   unknown addx29, sp, #0x10
0x186f0: 0xd11843ff   unknown subsp, sp, #0x610
0x186f4: 0x910c63e8   unknown addx8, sp, #0x318
```

instead of `disassemble`'s output style of

```
lldb`main:
lldb[0x186e4] <+0>:  stpx28, x27, [sp, #-0x20]!
lldb[0x186e8] <+4>:  stpx29, x30, [sp, #0x10]
lldb[0x186ec] <+8>:  addx29, sp, #0x10
lldb[0x186f0] <+12>: subsp, sp, #0x610
lldb[0x186f4] <+16>: addx8, sp, #0x318
```

Adding symbolic annotations for assembly instructions is something I'm 
interested in too, because we may have users investigating a crash or 
apparent-incorrect behavior who must debug optimized assembly and they may not 
be familiar with the ISA they're using, so short of flipping through a 
many-thousand-page PDF to understand each instruction, they're lost.  They 
don't write assembly or work at that level, but to understand a bug, they have 
to understand what the instructions are actually doing.

But the annotations that exist today don't move us forward much on that front - 
I'd argue that the flow control instructions on Intel are not hard to 
understand from their names, but that might just be my personal bias.  Much 
trickier instructions exist in any event.

Displaying this information by default for all targets when we only have one 
class of instructions on one target is not a good default.

Also, in 2011 when Greg implemented the `memory read -f i` (aka `x/i`) command
```
commit 5009f9d5010a7e34ae15f962dac8505ea11a8716
Author: Greg Clayton 
Date:   Thu Oct 27 17:55:14 2011 +
[...]
eFormatInstruction will print out disassembly with bytes and it will use the
current target's architecture. The format character for this is "i" (which
used to be being used for the integer format, but the integer format also 
has
"d", so we gave the "i" format to disassembly), the long format is
"instruction".
```

he had DumpDataExtractor's DumpInstructions print the bytes of the instruction 
-- that's the first field we see above for the `x/5i` after the address -- and 
this is only useful for people who are debugging the disassembler itself, I 
would argue.  I don't want this displayed by default either.

tl;dr this patch removes both fields from `memory read -f -i` and I think this 
is the right call today.  While I'm really interested in instruction 
annotation, I don't think `x/i` is the right place to have it enabled by 
default unless it's really compelling on at least some of our major targets.

>From 3969aaf70b22e2551e5d30a87cc80fa45ca83080 Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Fri, 8 Mar 2024 21:50:28 -0800
Subject: [PATCH] Turn off instruction flow control annotations by default

Walter Erquinigo added optional instruction annotations for x86
instructions in 2022 for the `thread trace dump instruction` command,
and code to DisassemblerLLVMC to add annotations for instructions
that change flow control, v.  https://reviews.llvm.org/D128477

This was added as an option to `disassemble`, and the trace dump
command enables it by default, but several other instruction dumpers
were changed to display them by default as well.  These are only
implemented for Intel instructions, so our disassembly on other
targets ends up looking like

```
(lldb) x/5i 0x186e4
0x186e4: 0xa9be6ffc   unknown stpx28, x27, [sp, #-0x20]!
0x186e8: 0xa9017bfd   unknown stpx29, x30, [sp, #0x10]
0x186ec: 0x910043fd   unknown addx29, sp, #0x10
0x186f0: 0xd11843ff   unknown subsp, sp, #0x610
0x186f4: 0x910c63e8   unknown addx8, sp, #0x318
```

instead of `disassemble`'s output style of

```
lldb`main:
lldb[0x186e4] <+0>:  stpx28, x27, [sp, #-0x20]!
lldb[0x186e8] <+4>:  stpx29, x30, [sp, #0x10]
lldb[0x186ec] <+8>:  addx29, sp, #0x10
lldb[0x186f0] <+12>: subsp, sp, #0x610
lldb[0x186f4] <+16>: addx8, sp, #0x318
```

Adding symbolic annotations for assembly instructions is something
I'm interested in too, because we may have users investigating a
crash or apparent-incorrect behavior who must debug optimized
assemb

[Lldb-commits] [lldb] Turn off instruction flow control annotations by default (PR #84607)

2024-03-08 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jason Molenda (jasonmolenda)


Changes

Walter Erquinigo added optional instruction annotations for x86 instructions in 
2022 for the `thread trace dump instruction` command, and code to 
DisassemblerLLVMC to add annotations for instructions that change flow control, 
v.  https://reviews.llvm.org/D128477

This was added as an option to `disassemble`, and the trace dump command 
enables it by default, but several other instruction dumpers were changed to 
display them by default as well.  These are only implemented for Intel 
instructions, so our disassembly on other targets ends up looking like

```
(lldb) x/5i 0x186e4
0x186e4: 0xa9be6ffc   unknown stpx28, x27, [sp, #-0x20]!
0x186e8: 0xa9017bfd   unknown stpx29, x30, [sp, #0x10]
0x186ec: 0x910043fd   unknown addx29, sp, #0x10
0x186f0: 0xd11843ff   unknown subsp, sp, #0x610
0x186f4: 0x910c63e8   unknown addx8, sp, #0x318
```

instead of `disassemble`'s output style of

```
lldb`main:
lldb[0x186e4] <+0>:  stpx28, x27, [sp, #-0x20]!
lldb[0x186e8] <+4>:  stpx29, x30, [sp, #0x10]
lldb[0x186ec] <+8>:  addx29, sp, #0x10
lldb[0x186f0] <+12>: subsp, sp, #0x610
lldb[0x186f4] <+16>: addx8, sp, #0x318
```

Adding symbolic annotations for assembly instructions is something I'm 
interested in too, because we may have users investigating a crash or 
apparent-incorrect behavior who must debug optimized assembly and they may not 
be familiar with the ISA they're using, so short of flipping through a 
many-thousand-page PDF to understand each instruction, they're lost.  They 
don't write assembly or work at that level, but to understand a bug, they have 
to understand what the instructions are actually doing.

But the annotations that exist today don't move us forward much on that front - 
I'd argue that the flow control instructions on Intel are not hard to 
understand from their names, but that might just be my personal bias.  Much 
trickier instructions exist in any event.

Displaying this information by default for all targets when we only have one 
class of instructions on one target is not a good default.

Also, in 2011 when Greg implemented the `memory read -f i` (aka `x/i`) command
```
commit 5009f9d5010a7e34ae15f962dac8505ea11a8716
Author: Greg Clayton 
Date:   Thu Oct 27 17:55:14 2011 +
[...]
eFormatInstruction will print out disassembly with bytes and it will use the
current target's architecture. The format character for this is "i" (which
used to be being used for the integer format, but the integer format also 
has
"d", so we gave the "i" format to disassembly), the long format is
"instruction".
```

he had DumpDataExtractor's DumpInstructions print the bytes of the instruction 
-- that's the first field we see above for the `x/5i` after the address -- and 
this is only useful for people who are debugging the disassembler itself, I 
would argue.  I don't want this displayed by default either.

tl;dr this patch removes both fields from `memory read -f -i` and I think this 
is the right call today.  While I'm really interested in instruction 
annotation, I don't think `x/i` is the right place to have it enabled by 
default unless it's really compelling on at least some of our major targets.

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


3 Files Affected:

- (modified) lldb/source/Core/DumpDataExtractor.cpp (+2-2) 
- (modified) lldb/source/Expression/IRExecutionUnit.cpp (+1-1) 
- (modified) 
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
 (+1-1) 


``diff
diff --git a/lldb/source/Core/DumpDataExtractor.cpp 
b/lldb/source/Core/DumpDataExtractor.cpp
index 986c9a181919ee..826edd7bab046e 100644
--- a/lldb/source/Core/DumpDataExtractor.cpp
+++ b/lldb/source/Core/DumpDataExtractor.cpp
@@ -150,8 +150,8 @@ static lldb::offset_t DumpInstructions(const DataExtractor 
&DE, Stream *s,
   if (bytes_consumed) {
 offset += bytes_consumed;
 const bool show_address = base_addr != LLDB_INVALID_ADDRESS;
-const bool show_bytes = true;
-const bool show_control_flow_kind = true;
+const bool show_bytes = false;
+const bool show_control_flow_kind = false;
 ExecutionContext exe_ctx;
 exe_scope->CalculateExecutionContext(exe_ctx);
 disassembler_sp->GetInstructionList().Dump(
diff --git a/lldb/source/Expression/IRExecutionUnit.cpp 
b/lldb/source/Expression/IRExecutionUnit.cpp
index 0682746e448e30..e4e131d70d4319 100644
--- a/lldb/source/Expression/IRExecutionUnit.cpp
+++ b/lldb/source/Expression/IRExecutionUnit.cpp
@@ -201,7 +201,7 @@ Status IRExecutionUnit::DisassembleFunction(Stream &stream,
   UINT32_MAX, false, false);
 
   InstructionList &instruction_list = disassembler_sp->GetInstructionList();
-  instruction

38 matches

Mail list logo