[Lldb-commits] [PATCH] D127895: [lldb] Don't overwrite quit and exit builtins in the interactive python interpreter

JDevlieghere created this revision. JDevlieghere added reviewers: labath, mib. Herald added a project: All. JDevlieghere requested review of this revision. The interactive interpreter is overwriting the `exit` and `quit` builtins with an instance of `LLDBQuitter` in order to make `exit` and `quit

[Lldb-commits] [PATCH] D127895: [lldb] Don't overwrite quit and exit builtins in the interactive python interpreter

JDevlieghere marked 3 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/test/Shell/ScriptInterpreter/Python/exit.test:1-5 +# RUN: %lldb -o 'script quit' | FileCheck %s --check-prefix SILENT +# RUN: %lldb -o 'script quit()' | FileCheck %s --check-prefix

[Lldb-commits] [PATCH] D127895: [lldb] Don't overwrite quit and exit builtins in the interactive python interpreter

JDevlieghere updated this revision to Diff 437328. JDevlieghere marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127895/new/ https://reviews.llvm.org/D127895 Files: lldb/source/Interpreter/embedded_interpreter.py lldb/test/Shell/ScriptInterpreter/Python

[Lldb-commits] [PATCH] D127882: [lldb/Fuzzer] Create ninja target for target fuzzer

JDevlieghere added a comment. This needs to be moved into the `if(TARGET lldb-target-fuzzer)` clause. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127882/new/ https://reviews.llvm.org/D127882 ___ lldb-c

[Lldb-commits] [PATCH] D127895: [lldb] Don't overwrite quit and exit builtins in the interactive python interpreter

This revision was automatically updated to reflect the committed changes. Closed by commit rG6cde6ac03c2c: [lldb] Don't overwrite quit and exit builtins in the Python interpreter (authored by JDevlieghere). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[Lldb-commits] [PATCH] D127586: [lldb][Interpreter] Add newline terminal to one-line script calls

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Herald added a subscriber: Michael137. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127586/new/ https://reviews.llvm.org/D1275

[Lldb-commits] [PATCH] D127922: [lldb] Introduce the concept of a log handler (NFC)

JDevlieghere created this revision. JDevlieghere added reviewers: labath, clayborg, mib, kastiglione. Herald added a subscriber: mgorny. Herald added a project: All. JDevlieghere requested review of this revision. This patch introduces the concept of a log handlers. Log handlers allow customizing

[Lldb-commits] [PATCH] D127922: [lldb] Introduce the concept of a log handler (NFC)

JDevlieghere updated this revision to Diff 437409. JDevlieghere added a comment. Update unit tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127922/new/ https://reviews.llvm.org/D127922 Files: lldb/include/lldb/Core/Debugger.h lldb/include/lldb/Utility/Log.h lldb/include/lldb

[Lldb-commits] [PATCH] D127937: [lldb] Add RotatingLogHandler

JDevlieghere created this revision. JDevlieghere added reviewers: labath, kastiglione, mib. Herald added a project: All. JDevlieghere requested review of this revision. Add a log handler that maintains a circular buffer. https://reviews.llvm.org/D127937 Files: lldb/include/lldb/Utility/Log.h

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

JDevlieghere created this revision. JDevlieghere added reviewers: labath, clayborg, jingham, kastiglione. Herald added a project: All. JDevlieghere requested review of this revision. This patch adds a buffer logging mode to lldb. It can be enabled with `log buffered enable`, which enables logging

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

JDevlieghere added a comment. Adding a test is trivial, but I'm holding off on spending time on that until there's consensus about the approach. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127986/new/ https://reviews.llvm.org/D127986 ___ l

[Lldb-commits] [PATCH] D127922: [lldb] Introduce the concept of a log handler (NFC)

JDevlieghere marked 3 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/source/Utility/Log.cpp:323-324 if (options.Test(LLDB_LOG_OPTION_THREADSAFE)) { static std::recursive_mutex g_LogThreadedMutex; std::lock_guard guard(g_LogThreadedMutex

[Lldb-commits] [PATCH] D127922: [lldb] Introduce the concept of a log handler (NFC)

JDevlieghere updated this revision to Diff 437618. JDevlieghere marked 2 inline comments as done. JDevlieghere added a comment. - Make Emit take a StringRef - Move mutex in the log handler CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127922/new/ https://reviews.llvm.org/D127922 Files:

[Lldb-commits] [PATCH] D127997: Remember whether all owners of the site we hit were internal in StopInfoBreakpoint

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Neat! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127997/new/ https://reviews.llvm.org/D127997 __

[Lldb-commits] [PATCH] D127922: [lldb] Introduce the concept of a log handler (NFC)

JDevlieghere updated this revision to Diff 437651. JDevlieghere marked 4 inline comments as done. JDevlieghere added a comment. - Add `EmitThreadSafe` - Remove `Flush` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127922/new/ https://reviews.llvm.org/D127922 Files: lldb/include/lldb/

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

JDevlieghere updated this revision to Diff 437679. JDevlieghere added a comment. Implement Greg's suggestion of integrating this with `log enable`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127986/new/ https://reviews.llvm.org/D127986 Files: lldb/include/lldb/Core/Debugger.h ll

[Lldb-commits] [PATCH] D127922: [lldb] Introduce the concept of a log handler (NFC)

This revision was automatically updated to reflect the committed changes. Closed by commit rG6ff49af33d09: [lldb] Introduce the concept of a log handler (NFC) (authored by JDevlieghere). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D127922?vs=437651&id=437686#

[Lldb-commits] [PATCH] D127937: [lldb] Add RotatingLogHandler

JDevlieghere updated this revision to Diff 437695. JDevlieghere added a comment. - Use dynamic array instead of vector. - Rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127937/new/ https://reviews.llvm.org/D127937 Files: lldb/include/lldb/Utility/Log.h lldb/source/Utility/Log

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

JDevlieghere added a comment. In D127986#3590157 , @clayborg wrote: > The main questions is if we want --buffered for any log channel (file, > callback, or circular). If we add a --circular flag, then we just let things > accumulate in the LogHandler cl

[Lldb-commits] [PATCH] D127937: [lldb] Add RotatingLogHandler

JDevlieghere marked an inline comment as done. JDevlieghere added inline comments. Comment at: lldb/unittests/Utility/LogTest.cpp:112 + handler.Dump(stream); + return stream.str(); +} kastiglione wrote: > minor: `return buffer;` I did that on purpose to avoid t

[Lldb-commits] [PATCH] D127937: [lldb] Add RotatingLogHandler

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. JDevlieghere marked an inline comment as done. Closed by commit rG6ac608b3d897: [lldb] Add RotatingLogHandler (authored by JDevlieghere). Herald added a project: LLDB.

[Lldb-commits] [PATCH] D128026: [lldb] Add a BufferedLogHandler

JDevlieghere created this revision. JDevlieghere added a reviewer: clayborg. Herald added a project: All. JDevlieghere requested review of this revision. Add a BufferedLogHandler as requested by Greg in D127986 . https://reviews.llvm.org/D128026 Files: lldb/

[Lldb-commits] [PATCH] D128026: [lldb] Add a BufferedLogHandler

JDevlieghere updated this revision to Diff 437784. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128026/new/ https://reviews.llvm.org/D128026 Files: lldb/include/lldb/Utility/Log.h lldb/source/Utility/Log.cpp lldb/unittests/Utility/LogTest.cpp Index: lldb/unittests/Utility/LogTes

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

JDevlieghere updated this revision to Diff 437786. JDevlieghere added a comment. Generalize support for buffering. You can see it in action below: (lldb) log enable -b 10 lldb default (lldb) target create ./bin/count Current executable set to '/Users/jonas/llvm/build-ra/bin/count' (arm64).

[Lldb-commits] [PATCH] D128026: [lldb] Add a BufferedLogHandler

JDevlieghere updated this revision to Diff 437788. JDevlieghere added a comment. Flush on destruction CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128026/new/ https://reviews.llvm.org/D128026 Files: lldb/include/lldb/Utility/Log.h lldb/source/Utility/Log.cpp lldb/unittests/Utili

[Lldb-commits] [PATCH] D128063: [LLDB][ExpressionParser] Fix indices inside format-strings passed to LLDB_LOG

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128063/new/ https://reviews.llvm.org/D128063 ___

[Lldb-commits] [PATCH] D128069: [lldb] add SBSection.alignment to python bindings

JDevlieghere added a comment. Is this something we could add a test for? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128069/new/ https://reviews.llvm.org/D128069 ___ lldb-commits mailing list lldb-comm

[Lldb-commits] [PATCH] D128077: [lldb][tests] Automatically call compute_mydir (NFC)

JDevlieghere accepted this revision. JDevlieghere added a comment. Neat :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128077/new/ https://reviews.llvm.org/D128077 ___ lldb-commits mailing list lldb-c

[Lldb-commits] [PATCH] D128069: [lldb] add SBSection.alignment to python bindings

JDevlieghere added a comment. In D128069#3592733 , @dmlary wrote: > I went through the existing tests for SBSection, and there is only one test > case for all the getters & props, and that is for `target_byte_size`. Based > on that lack, and the simpli

[Lldb-commits] [PATCH] D128107: [trace] Add LoadTraceFromFile to SBDebugger and SBTrace

JDevlieghere added inline comments. Comment at: lldb/include/lldb/API/SBDebugger.h:403 + /// trace session. + SBTrace LoadTraceFromFile(SBError &error, const char *trace_file_path); + Can this take a filespec? Repository: rG LLVM Github Monorepo CHANG

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

JDevlieghere added inline comments. Comment at: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt:3 + Support + ObjectYAML + ) I assume we don't need this anymore if we're using the dummy target? Comment at: lldb/tools/l

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

JDevlieghere added inline comments. Comment at: lldb/tools/lldb-fuzzer/CMakeLists.txt:1-2 add_subdirectory(lldb-target-fuzzer) +add_subdirectory(lldb-commandinterpreter-fuzzer) add_subdirectory(utils) nit: I'd sort these alphabetically Commen

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

JDevlieghere updated this revision to Diff 438831. JDevlieghere edited the summary of this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127986/new/ https://reviews.llvm.org/D127986 Files: lldb/include/lldb/Core/Debugger.h lldb/include/lldb/Utility/Log.h lldb/source/API/S

[Lldb-commits] [PATCH] D128026: [lldb] Add a BufferedLogHandler

JDevlieghere added a comment. In D128026#3595457 , @labath wrote: > I feel I should note that the llvm streams already have a buffered mode, and > given the way things are implemented now (our log class writes the entire log > message in one call, and t

[Lldb-commits] [PATCH] D128306: [lldb] Instantiate lazily named classes on macOS Ventura.

JDevlieghere created this revision. JDevlieghere added a reviewer: aprantl. Herald added a project: All. JDevlieghere requested review of this revision. Recent revisions of the Objective-C runtime changed objc_debug_class_getNameRaw() in a way that no longer triggers lazy names to be instantiated.

[Lldb-commits] [PATCH] D128312: [lldb] Add a setting to specify the preferred dynamic class info extractor function

JDevlieghere created this revision. JDevlieghere added a reviewer: aprantl. Herald added a project: All. JDevlieghere requested review of this revision. Add a setting to specify the preferred dynamic class info extractor function. https://reviews.llvm.org/D128312 Files: lldb/include/lldb/Targ

[Lldb-commits] [PATCH] D128026: [lldb] Add a BufferedLogHandler

JDevlieghere abandoned this revision. JDevlieghere added a comment. With D127986 I don't think we need this anymore. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128026/new/ https://reviews.llvm.org/D128026 __

[Lldb-commits] [PATCH] D128312: [lldb] Add a setting to specify the preferred dynamic class info extractor function

JDevlieghere updated this revision to Diff 438876. JDevlieghere marked 2 inline comments as done. JDevlieghere added a comment. Adress Adrian's code review feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128312/new/ https://reviews.llvm.org/D128312 Files: lldb/include/lldb/Tar

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

JDevlieghere added a comment. @labath let me know if this is what you had in mind CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127986/new/ https://reviews.llvm.org/D127986 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lis

[Lldb-commits] [PATCH] D128306: [lldb] Instantiate lazily named classes on macOS Ventura.

This revision was automatically updated to reflect the committed changes. Closed by commit rGc08f61b45e3b: [lldb] Instantiate lazily named classes on macOS Ventura. (authored by JDevlieghere). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[Lldb-commits] [PATCH] D128312: [lldb] Add a setting to specify the preferred dynamic class info extractor function

This revision was automatically updated to reflect the committed changes. Closed by commit rGc866f8544c92: [lldb] Add a setting to specify the preferred dynamic class info extractor o (authored by JDevlieghere). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

JDevlieghere created this revision. JDevlieghere added reviewers: aprantl, labath, clayborg. Herald added a project: All. JDevlieghere requested review of this revision. Add an OSLog log handler on macOS. These log messages end up in Console.app and will be part of a sysdiagnose. https://review

[Lldb-commits] [PATCH] D128323: [lldb] Add support for specifying a log handler

JDevlieghere created this revision. JDevlieghere added reviewers: labath, clayborg, kastiglione, mib. Herald added a project: All. JDevlieghere requested review of this revision. This patch adds a new flag to `log enable` allowing the user to specify a log handler. This makes it possible to enabl

[Lldb-commits] [PATCH] D127922: [lldb] Introduce the concept of a log handler (NFC)

JDevlieghere added a comment. In D127922#3595530 , @labath wrote: > I think we should talk about the threadsafe flag. The reason that the > non-threadsafe mode is safe now is because we're using unbuffered streams, > where each stream operation will map

[Lldb-commits] [PATCH] D128377: [lldb] Fix off-by-one error in the AppleObjCRuntimeV2 utility function

JDevlieghere created this revision. JDevlieghere added a reviewer: aprantl. Herald added a project: All. JDevlieghere requested review of this revision. Fix an off-by-one error in the utility function used to extract the dynamic class info. This resulted in a buffer overflow in the inferior which

[Lldb-commits] [PATCH] D128378: [lldb] Return the correct utility function in AppleObjCRuntimeV2

JDevlieghere created this revision. JDevlieghere added a reviewer: aprantl. Herald added a project: All. JDevlieghere requested review of this revision. A copy/paste error in GetClassInfoUtilityFunction resulted in the wrong utility function being returned: copyRealizedClassList instead of getReal

[Lldb-commits] [PATCH] D128377: [lldb] Fix off-by-one error in the AppleObjCRuntimeV2 utility function

JDevlieghere added a comment. In D128377#3602856 , @aprantl wrote: > I'm curious now why there is both `count` and `max_class_infos` and if the > second is workaround for this bug? Anyway, this *looks* plausible! `count` is the actual value reported by

[Lldb-commits] [PATCH] D128378: [lldb] Return the correct utility function in AppleObjCRuntimeV2

JDevlieghere added a comment. In D128378#3602888 , @aprantl wrote: > Oh boy, these are scary. Should LLDB fail harder when the utility expression > fails, so we can detect these earlier? Fortunately we print an error, but we're also pretty resilient aga

[Lldb-commits] [PATCH] D128378: [lldb] Return the correct utility function in AppleObjCRuntimeV2

This revision was automatically updated to reflect the committed changes. Closed by commit rGf0d87dfe30e8: [lldb] Return the correct utility function in AppleObjCRuntimeV2 (authored by JDevlieghere). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D128377: [lldb] Fix off-by-one error in the AppleObjCRuntimeV2 utility function

This revision was automatically updated to reflect the committed changes. Closed by commit rGd95c406c20ef: [lldb] Fix off-by-one error in the AppleObjCRuntimeV2 utility function (authored by JDevlieghere). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

JDevlieghere added a comment. In D128321#3602129 , @clayborg wrote: > I think we should allow this class to always exist and not conditionally > compile it in or out. Then we add a new Host.h method to emit a log message > in "lldb/Host/Host.h" and allo

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

JDevlieghere updated this revision to Diff 439164. JDevlieghere marked 5 inline comments as done. JDevlieghere added a comment. Address remaining code review feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128321/new/ https://reviews.llvm.org/D128321 Files: lldb/include/lldb/U

[Lldb-commits] [PATCH] D128323: [lldb] Add support for specifying a log handler

JDevlieghere marked 11 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/source/Commands/Options.td:436-437 Desc<"Set the log to be buffered, using the specified buffer size.">; + def log_handler : Option<"handler", "h">, Group<1>, +EnumArg<"

[Lldb-commits] [PATCH] D128323: [lldb] Add support for specifying a log handler

JDevlieghere updated this revision to Diff 439170. JDevlieghere marked 2 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128323/new/ https://reviews.llvm.org/D128323 Files: lldb/include/lldb/Core/Debugger.h lldb/include/lldb/lldb-private-enumerations.h lldb/s

[Lldb-commits] [PATCH] D128386: [lldb] Make thread safety the responsibility of the log handlers

JDevlieghere created this revision. JDevlieghere added reviewers: labath, clayborg, mib. Herald added a project: All. JDevlieghere requested review of this revision. As per Pavel's suggestion in D127922 , make thread safety the responsibility of the log handlers.

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

JDevlieghere marked 2 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/source/Utility/Log.cpp:406 + if (__builtin_available(macos 10.12, iOS 10, tvOS 10, watchOS 3, *)) { +os_log(g_os_log, "%{public}s", str.c_str()); + } else {

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

JDevlieghere marked an inline comment as done. JDevlieghere added inline comments. Comment at: lldb/source/Utility/Log.cpp:406 + if (__builtin_available(macos 10.12, iOS 10, tvOS 10, watchOS 3, *)) { +os_log(g_os_log, "%{public}s", str.c_str()); + } else {

[Lldb-commits] [PATCH] D128450: [lldb/Fuzzer] Have target fuzzer write artifacts to specific directory

JDevlieghere requested changes to this revision. JDevlieghere added a comment. This revision now requires changes to proceed. Instead of bundling together multiple shell commands into a single one, you should break them down so CMake can have the build system (e.g. ninja) schedule the different

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG09dea546692f: [lldb] Support a buffered logging mode (authored by JDevlieghere). Herald added a project: LLDB. Changed prior to commit: https://re

[Lldb-commits] [PATCH] D128386: [lldb] Make thread safety the responsibility of the log handlers

JDevlieghere closed this revision. JDevlieghere added a comment. Landed as 70841b97eb2e5c68d46b70579877e378511a47b4 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128386/new/ https://reviews.llvm.org/D128386

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

JDevlieghere added a comment. In D128321#3603947 , @labath wrote: > In D128321#3603007 , @JDevlieghere > wrote: > >> In D128321#3602129 , @clayborg >> wrote: >> >>> I th

[Lldb-commits] [PATCH] D128453: Automate checking for "command that takes no arguments" being passed arguments in CommandObjectParsed

JDevlieghere accepted this revision. JDevlieghere added a subscriber: labath. JDevlieghere added a comment. This revision is now accepted and ready to land. This is great, it both guarantees consistently and enforces command objects registering their arguments. LGTM. For the RenderScript plugin,

[Lldb-commits] [PATCH] D128480: [lldb] Replace Host::SystemLog with Debugger::Report{Error, Warning}

JDevlieghere created this revision. JDevlieghere added reviewers: labath, clayborg, aprantl, mib. Herald added a project: All. JDevlieghere requested review of this revision. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. As it exists today, Host::SystemLog is used exclus

[Lldb-commits] [PATCH] D139247: [lldb/Interpreter] Make ScriptedProcessInfo more generic

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139247/new/ https://reviews.llvm.org/D139247 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D139054: Delay calling ObjC class list read utility functions very early in process startup

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Comment at: lldb/tools/debugserver/source/RNBRemote.h:111-139 +prefix_reg_packets_with_tid,// 'QPrefixRegisterPacketsWithThreadID +set_logging_mo

[Lldb-commits] [PATCH] D139684: Switch the "command script add" interactive editor to use the exe_ctx interface

JDevlieghere added a comment. Just to make sure I understand: this works because we already supported both variants, and Python picks up the correct one based on the function signature? If so then this LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[Lldb-commits] [PATCH] D139453: Switch to a different library-loaded notification function breakpoint in Darwin dyld

JDevlieghere added inline comments. Comment at: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp:307-308 + +int addr_size = +process->GetTarget().GetArchitecture().GetAddressByteSize(); for (uint64_t i = 0; i < image_i

[Lldb-commits] [PATCH] D139853: [lldb/Process] Populate queues in Scripted Process

JDevlieghere added inline comments. Comment at: lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp:757 + bool is_scripted_process = m_process->GetPluginName() == "ScriptedProcess"; for (ThreadSP thread_sp : m_process->Threads()) { Comparing th

[Lldb-commits] [PATCH] D139252: [lldb/Plugins] Introduce Scripted Platform Plugin

JDevlieghere added inline comments. Comment at: lldb/source/Plugins/Platform/CMakeLists.txt:9 add_subdirectory(POSIX) +add_subdirectory(scripted) add_subdirectory(QemuUser) Comment at: lldb/source/Plugins/Platform/scripted/ScriptedPlatform.c

[Lldb-commits] [PATCH] D139463: Fix breakpoint-command.test when no script interpreter is compiled in.

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139463/new/ https://reviews.llvm.org/D139463 __

[Lldb-commits] [PATCH] D139955: [LLDB] Change formatting to use llvm::formatv

JDevlieghere requested changes to this revision. JDevlieghere added a comment. This revision now requires changes to proceed. I don't really understand the motivation. Can you elaborate on why "enabling 64 bit support" requires this change? I definitely prefer the `formatv` approach, but wouldn'

[Lldb-commits] [PATCH] D139951: [lldb/crashlog] Refactor CrashLogParser into a Factory patern

JDevlieghere added inline comments. Comment at: lldb/examples/python/crashlog.py:419-426 +def CrashLogParserFactory(debugger, path, verbose): +data = JSONCrashLogParser.is_valid_json(path) +if data: +parser = JSONCrashLogParser(debugger, path, verbose) +pa

[Lldb-commits] [PATCH] D139484: [lldb/test] Fix data racing issue in TestStackCoreScriptedProcess

JDevlieghere added inline comments. Comment at: lldb/test/API/functionalities/scripted_process/main.cpp:25-26 void call_and_wait(int &n) { - std::cout << "waiting for computation!" << std::endl; - while (baz(n) != 42) + while (baz(n, mutex, cv) != 42) { ; + } --

[Lldb-commits] [PATCH] D139853: [lldb/Process] Populate queues in Scripted Process

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. This looks like what I had in mind. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139853/new/ https://reviews.llvm.org/D139853 _

[Lldb-commits] [PATCH] D139250: [lldb] Add ScriptedPlatform python implementation

JDevlieghere added inline comments. Comment at: lldb/bindings/python/python-wrapper.swig:317 +PythonObject lldb_private::LLDBSwigPythonCreateScriptedPlatform( +const char *python_class_name, const char *session_dictionary_name, This looks pretty similar to

[Lldb-commits] [PATCH] D139252: [lldb/Plugins] Introduce Scripted Platform Plugin

JDevlieghere added inline comments. Comment at: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp:25-28 void ScriptedThread::CheckInterpreterAndScriptObject() const { - lldbassert(m_script_object_sp && "Invalid Script Object."); - lldbassert(GetInterface() && "Invalid Sc

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

JDevlieghere added a comment. Last week I debugged this further. Here's what I believe is going on. Consider the following simplified example with two CUs and a type `Foo`. 0x01: DW_TAG_compile_unit 0x02: DW_TAG_class_type DW_AT_containing_type (0x11) DW_AT_name

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

JDevlieghere updated this revision to Diff 483018. JDevlieghere added a comment. Revert to old naming CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138176/new/ https://reviews.llvm.org/D138176 Files: llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h llvm/lib/DWARFLinker/DWARFL

[Lldb-commits] [PATCH] D140067: Fix an ASAN bug I introduced in debugserver, accessing off the end of an array intentionally

JDevlieghere added inline comments. Comment at: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp:2026-2037 +if (reg == gpr_pc) + value->value.uint64 = clear_pac_bits( + reinterpret_cast(m_state.context.gpr.__opaque_pc)); +else

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

JDevlieghere added a comment. In D138176#3997559 , @avl wrote: > In D138176#3995675 , @JDevlieghere > wrote: > >> Last week I debugged this further. Here's what I believe is going on. >> Consider the following s

[Lldb-commits] [PATCH] D140067: Fix an ASAN bug I introduced in debugserver, accessing off the end of an array intentionally

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. I think a switch case is easier to read (and spot things like missing cases) but this is code you own so go with the approach you like best. LGTM. Repository: rG LLVM Github Mon

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

JDevlieghere added a comment. In D138176#4001739 , @avl wrote: > Looks like I have a test case for this problem: That's awesome, thank you for spending time on that! I'll add the assert you suggested to this patch. Do you want to commit the test yourse

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

This revision was automatically updated to reflect the committed changes. Closed by commit rGc9cbe6937b1f: [dsymutil] Track uncloned references in the DIEInfo. (authored by JDevlieghere). Changed prior to commit: https://reviews.llvm.org/D138176?vs=483018&id=484357#toc Repository: rG LLVM Gi

[Lldb-commits] [PATCH] D140385: [lldb] Add lldb-server targets to build docs

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140385/new/ https://reviews.llvm.org/D140385

[Lldb-commits] [PATCH] D139250: [lldb] Add ScriptedPlatform python implementation

JDevlieghere added a comment. I'm happy with the launching/attaching discussion resolved. Similar to the deduplication I think it makes more sense to tackle that first instead of landing this "as is" and then fixing it after the fact. Comment at: lldb/bindings/python/python-w

[Lldb-commits] [PATCH] D139945: [lldb] Add scripted process launch/attach option to platform commands

JDevlieghere added a comment. In D139945#3999351 , @labath wrote: > For a "plugin", the scripted process is sure getting a lot of special > handling in generic code. (I know this isn't being introduced here, but I > wasn't involved in the previous revie

[Lldb-commits] [PATCH] D139955: [LLDB] Change formatting to use llvm::formatv

JDevlieghere added a comment. This looks good to me if we drop the explicit `+` for right alignment: it's the default and other places in LLDB (and LLVM, at least as far as I'm aware) don't include this unless there's ambiguity. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D141021: [lldb] Add lldb-framework-cleanup target

JDevlieghere created this revision. JDevlieghere added reviewers: bulbazord, mib. Herald added a project: All. JDevlieghere requested review of this revision. CMake supports building Framework bundles for Apple platforms. We rely on this functionality to create LLDB.framework. From CMake's perspec

[Lldb-commits] [PATCH] D141042: [lldb] Allow configuring on Windows with python interpreter within a junction

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Looks reasonable to me. @lawrence_danna do you happen to remember why you wrote: exe = os.path.join(os.path.realpath(os.path.dirname(exe)), os.readlink(exe)) instead of exe =

[Lldb-commits] [PATCH] D140066: Add a final fallback technique to the old Darwin dynamic loader plugin to find the dyld_all_image_infos structure

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140066/new/ https://reviews.llvm.org/D140066 ___

[Lldb-commits] [PATCH] D141165: [lldb] Fix symbol table use after free

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Comment at: lldb/source/Symbol/SymbolFile.cpp:188-190 + auto *symtab = GetSymtab(); + if (symtab) +symtab->SectionFileAddressesChanged(); -

[Lldb-commits] [PATCH] D139955: [LLDB] Change formatting to use llvm::formatv

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM too Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139955/new/ https://reviews.llvm.org/D139955 ___

[Lldb-commits] [PATCH] D68655: Trust the arange accelerator tables in dSYMs

JDevlieghere updated this revision to Diff 487509. JDevlieghere added a comment. > Apparently for very small / gapless programs, clang emits single-value > DW_AT_low_pc/high_pc in the compile unit, and dsymutil does not rewrite that > into DW_AT_ranges. Although this statement is correct, it sh

[Lldb-commits] [PATCH] D141318: [lldb] Store shared pointers in DieToTypePtr map instead of raw pointers

JDevlieghere added a comment. In D141318#4037541 , @augusto2112 wrote: > I changed the value of the map to a shared pointer, but could change it to a > weak pointer instead, not sure which is more appropriate in this case. I was about to ask the same t

[Lldb-commits] [PATCH] D141318: [lldb] Store shared pointers in DieToTypePtr map instead of raw pointers

JDevlieghere added a comment. In D141318#4037640 , @augusto2112 wrote: > In D141318#4037557 , @JDevlieghere > wrote: > >> In D141318#4037541 , @augusto2112 >> wrote: >>

[Lldb-commits] [PATCH] D68655: Trust the arange accelerator tables in dSYMs

This revision was automatically updated to reflect the committed changes. Closed by commit rG8b259fe573e1: [lldb] Trust the arange accelerator tables in dSYMs (authored by JDevlieghere). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68655/new/ https

[Lldb-commits] [PATCH] D141330: [lldb] Limit 8b259fe573e1 to dSYMs

This revision was automatically updated to reflect the committed changes. Closed by commit rG9b737f148d88: [lldb] Limit trusting aranges to dSYMs only. (authored by JDevlieghere). Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Changed prior to commit: https://reviews.llv

[Lldb-commits] [PATCH] D141021: [lldb] Add lldb-framework-cleanup target

JDevlieghere added a comment. In D141021#4037840 , @bulbazord wrote: > I think this idea will work but I have a few comments and questions: > > Based on my understanding of this change, we're supposed to manually run the > build system with the target `l

[Lldb-commits] [PATCH] D141021: [lldb] Add lldb-framework-cleanup target

JDevlieghere updated this revision to Diff 487613. JDevlieghere marked an inline comment as done. JDevlieghere added a comment. Make `lldb-framework-cleanup` a dependency of `install-liblldb`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141021/new/ https://reviews.llvm.org/D141021 Fi

[Lldb-commits] [PATCH] D141021: [lldb] Remove tools copied into LLDB.framework before install

JDevlieghere updated this revision to Diff 487901. JDevlieghere added a comment. Add dependency to `install-liblldb-stripped`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141021/new/ https://reviews.llvm.org/D141021 Files: lldb/cmake/modules/AddLLDB.cmake lldb/cmake/modules/LLDBC

<    11   12   13   14   15   16   17   18   19   20   >