[Lldb-commits] [PATCH] D146263: [lldb] Move UnixSignals creation into Platform plugins

This revision was automatically updated to reflect the committed changes. Closed by commit rGee232506b870: [lldb] Move UnixSignals creation into Platform plugins (authored by bulbazord). Changed prior to commit: https://reviews.llvm.org/D146263?vs=505934&id=506757#toc Repository: rG LLVM Git

[Lldb-commits] [PATCH] D146473: [lldb][CMake] Enforce not letting lldbUtility link against any other lldb libs

bulbazord created this revision. bulbazord added a reviewer: JDevlieghere. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. lldbUtility is not supposed to depend on anything else in lldb. Let's enforce

[Lldb-commits] [PATCH] D146473: [lldb][CMake] Enforce not letting lldbUtility link against any other lldb libs

bulbazord added a comment. Note that this won't stop people from performing `#include` with non-Utility files, but this marks the intent that Utility is not supposed to be using anything else in lldb. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D146541: [lldb][expr] Propagate ClangDynamicCheckerFunctions::Install() errors to caller

bulbazord accepted this revision. bulbazord added a comment. Thanks for improving the error message here! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146541/new/ https://reviews.llvm.org/D146541 ___ ll

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

bulbazord added a subscriber: aprantl. bulbazord added a comment. In D145580#4209621 , @DavidSpickett wrote: > On the subject of not using `TypeSystemClang`, looking around it seems like > `TypeSystem` is purely used to look up basic types, never to mak

[Lldb-commits] [PATCH] D146473: [lldb][CMake] Enforce not letting lldbUtility link against any other lldb libs

bulbazord added a comment. > Can we do something similar for plugins? Yep, I'll tackle that in a follow-up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146473/new/ https://reviews.llvm.org/D146473 ___

[Lldb-commits] [PATCH] D146473: [lldb][CMake] Enforce not letting lldbUtility link against any other lldb libs

This revision was automatically updated to reflect the committed changes. Closed by commit rG2356bf27f722: [lldb][CMake] Enforce not letting lldbUtility link against any other lldb libs (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[Lldb-commits] [PATCH] D146553: [lldb][CMake] Enforce not linking against plugin libs in core libs

bulbazord created this revision. bulbazord added a reviewer: JDevlieghere. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Non-plugin lldb libraries should generally not be linking against lldb plugin

[Lldb-commits] [PATCH] D146473: [lldb][CMake] Enforce not letting lldbUtility link against any other lldb libs

bulbazord added a comment. In D146473#4210440 , @bulbazord wrote: >> Can we do something similar for plugins? > > Yep, I'll tackle that in a follow-up. For those following along at home: D146553 Repository: rG LLVM

[Lldb-commits] [PATCH] D146553: [lldb][CMake] Enforce not linking against plugin libs in core libs

This revision was automatically updated to reflect the committed changes. Closed by commit rG5499b026d218: [lldb][CMake] Enforce not linking against plugin libs in core libs (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14655

[Lldb-commits] [PATCH] D146590: [lldb] Update some uses of Python2 API in typemaps.

bulbazord added a comment. > `lldb -o "script data=lldb.SBData(); data.SetDataFromUInt64Array([2**63])"` Just tried this, didn't crash LLDB but it does give me the OverflowError you mentioned. Either way, this is probably okay? What do you think @mib? Repository: rG LLVM Github Monorepo CH

[Lldb-commits] [PATCH] D146553: [lldb][CMake] Enforce not linking against plugin libs in core libs

bulbazord added a comment. In D146553#4212417 , @DavidSpickett wrote: > Is this policy documented anywhere? Perhaps you can update one of the design > pages if it isn't already there. > > Because I am a bit confused about it. Some parts of lldb use plug

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

bulbazord added inline comments. Comment at: lldb/source/Commands/CommandObjectRegister.cpp:229 +if (!DumpRegister(m_exe_ctx, strm, reg_ctx, reg_info, + /*print_flags=*/true, type_system)) strm.Printf("%-12s = error: unavail

[Lldb-commits] [PATCH] D146263: [lldb] Move UnixSignals creation into Platform plugins

bulbazord added a comment. In D146263#4214336 , @rupprecht wrote: > This seems to cause a regression on Linux where we no longer get the signal > details. > > Given an intentionally-crashy test binary, such as: > > void crash() { > // Allocate vali

[Lldb-commits] [PATCH] D146263: [lldb] Move UnixSignals creation into Platform plugins

bulbazord added inline comments. Comment at: lldb/source/Target/UnixSignals.cpp:39 + // If we have no host platform, be resilient and use default UnixSignals. + if (!host_platform_sp) +s_unix_signals_sp = std::make_shared(); rupprecht wrote: > I think the d

[Lldb-commits] [PATCH] D146263: [lldb] Move UnixSignals creation into Platform plugins

bulbazord added inline comments. Comment at: lldb/source/Target/UnixSignals.cpp:39 + // If we have no host platform, be resilient and use default UnixSignals. + if (!host_platform_sp) +s_unix_signals_sp = std::make_shared(); rupprecht wrote: > bulbazord wro

[Lldb-commits] [PATCH] D146668: [lldb-server] Use Platform plugin corresponding to the host

bulbazord created this revision. bulbazord added a reviewer: rupprecht. Herald added a subscriber: krytarowski. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. In ee232506b870ce5282cc4da5ca493d41d361fe

[Lldb-commits] [PATCH] D146263: [lldb] Move UnixSignals creation into Platform plugins

bulbazord added inline comments. Comment at: lldb/source/Target/UnixSignals.cpp:39 + // If we have no host platform, be resilient and use default UnixSignals. + if (!host_platform_sp) +s_unix_signals_sp = std::make_shared(); bulbazord wrote: > rupprecht wro

[Lldb-commits] [PATCH] D146668: [lldb-server] Use Platform plugin corresponding to the host

bulbazord added a comment. Herald added a subscriber: JDevlieghere. With this patch: alex@ubuntu:~/llvm-project/build$ ./bin/lldb foo (lldb) target create "foo" Current executable set to '/home/alex/llvm-project/build/foo' (aarch64). (lldb) r Process 19623 launched: '/home/alex/llvm-pro

[Lldb-commits] [PATCH] D146668: [lldb-server] Use Platform plugin corresponding to the host

bulbazord updated this revision to Diff 507546. bulbazord added a comment. Adding a test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146668/new/ https://reviews.llvm.org/D146668 Files: lldb/test/API/functionalities/inferior-crashing/TestInferi

[Lldb-commits] [PATCH] D146679: [lldb] Add support for the DW_AT_trampoline attribute with mangled names

bulbazord added inline comments. Comment at: lldb/include/lldb/Symbol/Function.h:439 /// The section offset based address for this function. + /// \param[in] range + /// The symbol name this function should trampoline to. Comment

[Lldb-commits] [PATCH] D146668: [lldb-server] Use Platform plugin corresponding to the host

This revision was automatically updated to reflect the committed changes. Closed by commit rG0c5cee779929: [lldb-server] Use Platform plugin corresponding to the host (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146668/new/

[Lldb-commits] [PATCH] D146765: [lldb/crashlog] Load inlined symbol into interactive crashlog

bulbazord added inline comments. Comment at: lldb/examples/python/crashlog.py:540 +image = self.get_used_image(image_id) frame_offset = int(json_frame['imageOffset']) is `image` not the same as `json_image` from a few lines above this?

[Lldb-commits] [PATCH] D146152: Add __lldb_init_module_with_target for use when sourcing modules for a target

bulbazord added inline comments. Comment at: lldb/bindings/python/python-wrapper.swig:1031-1048 + // First call the target initializer: + if (target) { +python_function_name_string += ".__lldb_init_module_with_target"; +python_function_name = python_function_name_string

[Lldb-commits] [PATCH] D146668: [lldb-server] Use Platform plugin corresponding to the host

bulbazord added a comment. In D146668#4221223 , @thakis wrote: > Hello, > https://github.com/llvm/llvm-project/commit/5fb4147f74ad90095f08f243e5f19877d09d11b9 > broke the windows build (http://45.33.8.238/win/76494/step_4.txt) because > it's now includ

[Lldb-commits] [PATCH] D146668: [lldb-server] Use Platform plugin corresponding to the host

bulbazord added a comment. In D146668#4223456 , @labath wrote: > I'm sorry, but I don't think this is a good change (which I guess means the > previous one is not good either, but this one drives the point). I really > don't want lldb-server to depend o

[Lldb-commits] [PATCH] D146965: [lldb] Add support for MSP430 in LLDB.

bulbazord added a comment. I'm not too familiar with MSP430 but the general idea looks fine. Others may have comments about areas I'm less familiar with. One concern I have is that there are no tests. I can understand that it may be difficult to get automated tests running testing the debugging

[Lldb-commits] [PATCH] D146590: [lldb] Update some uses of Python2 API in typemaps.

bulbazord added a comment. In D146590#4220388 , @jgorbe wrote: > I found the actual reason for the crash I was talking about. This patch only > addressed the incorrect `OverflowError`, but the crash comes from a double > free (as @rupprecht mentioned) i

[Lldb-commits] [PATCH] D147001: [lldb] TestInferiorCrashing.py should check for crash reason

bulbazord created this revision. bulbazord added reviewers: labath, rupprecht. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. In a now-reverted series of patches, I inadvertently broke the ability for

[Lldb-commits] [PATCH] D147007: [lldb] Fix double free in python bindings error handling.

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. Thanks for taking care of that! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147007/new/ https://reviews.llvm.org/D147007 __

[Lldb-commits] [PATCH] D147009: [lldb] Remove lldbExpression's dependency on ObjectFileJIT

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, labath, clayborg, mib, jingham. Herald added subscribers: pmatos, asb, sbc100, emaste. Herald added a project: All. bulbazord requested review of this revision. Herald added subscribers: lldb-commits, MaskRay, aheejin. Herald

[Lldb-commits] [PATCH] D147009: [lldb] Remove lldbExpression's dependency on ObjectFileJIT

bulbazord updated this revision to Diff 508817. bulbazord added a comment. Herald added a subscriber: Michael137. Minor fix to ObjectFileWasm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147009/new/ https://reviews.llvm.org/D147009 Files: lldb/

[Lldb-commits] [PATCH] D147001: [lldb] TestInferiorCrashing.py should check for crash reason

This revision was automatically updated to reflect the committed changes. Closed by commit rG9efb4db29dcc: [lldb] TestInferiorCrashing.py should check for crash reason (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147001/new/

[Lldb-commits] [PATCH] D147009: [lldb] Remove lldbExpression's dependency on ObjectFileJIT

bulbazord planned changes to this revision. bulbazord added a comment. In D147009#4227163 , @labath wrote: > I already regret getting involved in this, but here it goes... Your effort and participation is greatly appreciated. I'm sorry if this is frustr

[Lldb-commits] [PATCH] D147084: [lldb] Move ObjectFileJIT to lldbExpression

bulbazord created this revision. bulbazord added reviewers: labath, JDevlieghere, mib, jingham, clayborg. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. In the spirit of not having lldbExpression rely

[Lldb-commits] [PATCH] D147009: [lldb] Remove lldbExpression's dependency on ObjectFileJIT

bulbazord abandoned this revision. bulbazord added a comment. I'm abandoning this approach in favor of what Pavel suggested: https://reviews.llvm.org/D147084 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147009/new/ https://reviews.llvm.org/D14700

[Lldb-commits] [PATCH] D147084: [lldb] Move ObjectFileJIT to lldbExpression

This revision was automatically updated to reflect the committed changes. Closed by commit rGa52054cfa29d: [lldb] Move ObjectFileJIT to lldbExpression (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147084/new/ https://reviews

[Lldb-commits] [PATCH] D147587: Fix the check in StopInfoBreakpoint for "are we running an expression"

bulbazord accepted this revision. bulbazord added a comment. LGTM. It'd be nice if we could have a test that wasn't specific to Darwin or ObjC though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147587/new/ https://reviews.llvm.org/D147587

[Lldb-commits] [PATCH] D146965: [lldb] Add support for MSP430 in LLDB.

bulbazord added inline comments. Comment at: lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp:127 llvm::Triple::hexagon, llvm::Triple::mips, llvm::Triple::mips64el, - llvm::Triple::mipsel, llvm::Triple::systemz}, + llvm::Triple::mipsel, llvm::Triple:

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

bulbazord added a comment. In D145580#4245911 , @DavidSpickett wrote: > @bulbazord Please take a look and see if I've done the plugin/core code split > correctly. This looks fine. Thanks for being flexible and working with us to find something that ma

[Lldb-commits] [PATCH] D147669: PECOFF: enforce move semantics and consume errors properly

bulbazord added inline comments. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:869-873 LLDB_LOG(log, "ObjectFilePECOFF::AppendFromExportTable - failed to get export " "table entry name: {0}", llvm::f

[Lldb-commits] [PATCH] D147669: PECOFF: enforce move semantics and consume errors properly

bulbazord added inline comments. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:869-873 LLDB_LOG(log, "ObjectFilePECOFF::AppendFromExportTable - failed to get export " "table entry name: {0}", llvm::f

[Lldb-commits] [PATCH] D147736: [lldb] Add an LLDB_DEPRECATED macro similar to LLVM_DEPRECATED

bulbazord accepted this revision. bulbazord added inline comments. This revision is now accepted and ready to land. Comment at: lldb/include/lldb/API/SBDefines.h:33 +// supports the attribute. +#if defined(SWIG) or _cplusplus < 201300 +#undef LLDB_DEPRECATED nit:

[Lldb-commits] [PATCH] D147746: [lldb][NFC] Delete unused function Breakpoint::GetEventIdentifier

bulbazord created this revision. bulbazord added reviewers: mib, JDevlieghere, jingham. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This is unused Repository: rG LLVM Github Monorepo https://r

[Lldb-commits] [PATCH] D147746: [lldb][NFC] Delete unused function Breakpoint::GetEventIdentifier

This revision was automatically updated to reflect the committed changes. Closed by commit rGb04bc87c9edc: [lldb][NFC] Delete unused function Breakpoint::GetEventIdentifier (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147746

[Lldb-commits] [PATCH] D147801: [lldb] Add unittests for a few FileSpec methods

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, jingham, mib, clayborg. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This adds tests for: - FileSpec::TestFileNameExtension

[Lldb-commits] [PATCH] D147801: [lldb] Add unittests for a few FileSpec methods

This revision was automatically updated to reflect the committed changes. Closed by commit rGde5f96e99aed: [lldb] Add unittests for a few FileSpec methods (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147801/new/ https://rev

[Lldb-commits] [PATCH] D147833: [lldb] Change return type of EventData::GetFlavor

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, labath, jingham, mib, jasonmolenda. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. There's no reason these strings need to be

[Lldb-commits] [PATCH] D147833: [lldb] Change return type of EventData::GetFlavor

bulbazord added a comment. In D147833#4252651 , @jasonmolenda wrote: > Am I missing something, how does this work when we have uses like > `event_data->GetFlavor() == TargetEventData::GetFlavorString()` - is this > changing from a one-time construction

[Lldb-commits] [PATCH] D147841: [lldb][NFC] Update syntax description for language cplusplus demangle

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, Michael137, aprantl. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Also added some tests because this is completely untested.

[Lldb-commits] [PATCH] D147833: [lldb] Change return type of EventData::GetFlavor

bulbazord added a comment. In D147833#4253416 , @jasonmolenda wrote: > idk maybe I'm over-thinking it, but this does make me a little uncomfortable. > We have at least one instance where someone did > `platform_sp->GetPluginName() == "ios-simulator"`

[Lldb-commits] [PATCH] D147833: [lldb] Change return type of EventData::GetFlavor

bulbazord updated this revision to Diff 511954. bulbazord added a comment. Change return type to llvm::StringRef Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147833/new/ https://reviews.llvm.org/D147833 Files: lldb/include/lldb/Breakpoint/Break

[Lldb-commits] [PATCH] D147841: [lldb][NFC] Update syntax description for language cplusplus demangle

This revision was automatically updated to reflect the committed changes. Closed by commit rG469bdbd62ce2: [lldb][NFC] Update syntax description for language cplusplus demangle (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14

[Lldb-commits] [PATCH] D147833: [lldb] Change return type of EventData::GetFlavor

This revision was automatically updated to reflect the committed changes. Closed by commit rG6ebf1bc66b89: [lldb] Change return type of EventData::GetFlavor (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147833/new/ https://r

[Lldb-commits] [PATCH] D148050: [lldb] Change formatter helper function parameter list to remove ConstString

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, jingham, mib, jasonmolenda. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. All of these functions take a ConstString for the t

[Lldb-commits] [PATCH] D148050: [lldb] Change formatter helper function parameter list to remove ConstString

This revision was automatically updated to reflect the committed changes. Closed by commit rG0e22b8cba786: [lldb] Change formatter helper function parameter list to remove ConstString (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[Lldb-commits] [PATCH] D148282: Fix the help for "type X delete" to make the -a & -w behaviors clear

bulbazord added inline comments. Comment at: lldb/source/Commands/CommandObjectType.cpp:112-113 +return "format"; + } +}; + maybe add an `llvm_unreachable` after the switch statement? Comment at: lldb/source/Commands/CommandObjectType.cpp

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This class is not safe in multithreaded code. It's

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

bulbazord added a comment. In D148380#4269862 , @JDevlieghere wrote: > Does this actually have to be a //recursive// mutex? Good point, I don't think it does. I'll update this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

bulbazord added a comment. In D148380#4269876 , @bulbazord wrote: > In D148380#4269862 , @JDevlieghere > wrote: > >> Does this actually have to be a //recursive// mutex? > > Good point, I don't think it does. I'l

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

bulbazord added a comment. In D148380#4270090 , @JDevlieghere wrote: > In D148380#4270085 , @bulbazord > wrote: > >> In D148380#4269876 , @bulbazord >> wrote: >> >>> In

[Lldb-commits] [PATCH] D148402: [lldb] Remove use of ConstString from Args::GetShellSafeArgument

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Having the names of various shells in ConstString's

[Lldb-commits] [PATCH] D148395: [lldb] Unify default/hijack listener between Process{Attach, Launch}Info (NFC)

bulbazord added a comment. Creating a ProcessAttachInfo from a ProcessLaunchInfo with this change means they'll have different listeners. Is that ProcessLaunchInfo kept around for any other reason? Or is it just made to create the ProcessAttachInfo? This seems like a reasonable move to me, but

[Lldb-commits] [PATCH] D146965: [lldb] Add support for MSP430 in LLDB.

bulbazord accepted this revision. bulbazord added a comment. In D146965#4274274 , @DavidSpickett wrote: > I'm sure there's something missing but LGTM. > > @bulbazord ? I'm satisfied with this since there are tests. I'm not sure what might be missing bu

[Lldb-commits] [PATCH] D148541: [lldb] fix build issue on MSVC because of missing byte-swap builtins

bulbazord requested changes to this revision. bulbazord added a comment. This revision now requires changes to proceed. Instead of not using `__builtin_bswap{32,64}` entirely, we should check if they're available. You can use the clang feature-checking macro `__has_builtin` (and if it doesn't e

[Lldb-commits] [PATCH] D148541: [lldb] fix build issue on MSVC because of missing byte-swap builtins

bulbazord accepted this revision. bulbazord 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/D148541/new/ https://reviews.llvm.org/D148541 _

[Lldb-commits] [PATCH] D148397: [lldb/Utility] Add opt-in passthrough mode to event listeners

bulbazord added inline comments. Comment at: lldb/include/lldb/Utility/Broadcaster.h:520-522 +/// A optional listener that all private events get also broadcasted to, +/// on top the hijacked / default listeners. +lldb::ListenerSP m_passthrough_listener = nullptr; ---

[Lldb-commits] [PATCH] D148399: [lldb] Improve logging for process state change (NFC)

bulbazord added a comment. +1 to what Jonas said. `LLDB_LOG` would greatly simplify this since it puts `__FILE__` and `__func__` in each message, which is what these are doing manually. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148399/new/ ht

[Lldb-commits] [PATCH] D148400: [lldb] Fix bug to update process public run lock with process state

bulbazord requested changes to this revision. bulbazord added a comment. This revision now requires changes to proceed. Idea is good, few concerns about the implementation. Comment at: lldb/source/Target/Process.cpp:404-419 +llvm::StringRef Process::GetAttachSynchronousHijackLi

[Lldb-commits] [PATCH] D145296: [lldb/Plugin] Add breakpoint setting support to ScriptedProcesses.

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. Seems ok to me, one small thought. Shouldn't hold this patch up though. Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:270-271 +Status ScriptedP

[Lldb-commits] [PATCH] D148401: [lldb/API] Add convenience constructor for SBError (NFC)

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. I like this! Reading through the call stack, it looks like we shouldn't hit any issues if `message` is `nullptr`. Could you add a small test to `TestSBError.py` constructing an error wit

[Lldb-commits] [PATCH] D148401: [lldb/API] Add convenience constructor for SBError (NFC)

bulbazord added inline comments. Comment at: lldb/include/lldb/API/SBError.h:26 + SBError(const char *message); + JDevlieghere wrote: > Why not a StringRef? I'm not sure we have `StringRef` type map in swig? We would have to create one for swig to pick up on

[Lldb-commits] [PATCH] D145297: [lldb] Add an example of interactive scripted process debugging

bulbazord added a comment. A few high-level comments: - You're doing a lot of type-annotations in python which is good, but you're not being very consistent about it. It would be tremendously helpful if you could add type annotations everywhere. - I would recommend using f-strings over `%` and

[Lldb-commits] [PATCH] D148548: [lldb] Improve breakpoint management for interactive scripted process

bulbazord added a comment. Seems okay to me, but it's a little messy that we're having to manage breakpoints like this. Comment at: lldb/bindings/interface/SBTargetExtensions.i:144-171 +class watchpoints_access(object): +'''A helper object that will lazily

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

This revision was automatically updated to reflect the committed changes. Closed by commit rG43ac269bdd00: [lldb] Lock accesses to PathMappingLists's internals (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148380/new/ https:

[Lldb-commits] [PATCH] D148402: [lldb] Remove use of ConstString from Args::GetShellSafeArgument

This revision was automatically updated to reflect the committed changes. Closed by commit rGc8bb7c234c68: [lldb] Remove use of ConstString from Args::GetShellSafeArgument (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148402/

[Lldb-commits] [PATCH] D148579: [lldb] Change parameter type of StructuredData::ParseJSON

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Instead of taking a `const std::string &` we can take an `llvm::S

[Lldb-commits] [PATCH] D148579: [lldb] Change parameter type of StructuredData::ParseJSON

This revision was automatically updated to reflect the committed changes. Closed by commit rG8bddb13c2470: [lldb] Change parameter type of StructuredData::ParseJSON (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148579/new/ h

[Lldb-commits] [PATCH] D148676: [lldb][NFCI] Stop creating additional temporary string in Log::VAPrintf

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, clayborg. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Instead of creating a std::string from the `SmallString

[Lldb-commits] [PATCH] D148400: [lldb] Fix bug to update process public run lock with process state

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

[Lldb-commits] [PATCH] D148679: [lldb] Change setting descriptions to use StringRef instead of ConstString

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. These probably do not need to be in the ConstString StringPool as

[Lldb-commits] [PATCH] D148676: [lldb][NFCI] Stop creating additional temporary string in Log::VAPrintf

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG77e3914be7c9: [lldb][NFCI] Stop creating additional temporary string in Log::VAPrintf (authored by bulbazord). Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D148679: [lldb] Change setting descriptions to use StringRef instead of ConstString

This revision was automatically updated to reflect the committed changes. Closed by commit rG96a800c07f02: [lldb] Change setting descriptions to use StringRef instead of ConstString (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[Lldb-commits] [PATCH] D148399: [lldb] Improve logging for process state change (NFC)

bulbazord added a comment. We'd go from: LLDB_LOGF(log, "Process::SetPrivateState (plugin = %s, state = %s) state didn't " "change. Ignoring...", GetPluginName().data(), StateAsCString(new_state)); to LLDB_LOG(log, "(plugin = %s, state = %s) state didn't

[Lldb-commits] [PATCH] D148399: [lldb] Improve logging for process state change (NFC)

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

[Lldb-commits] [PATCH] D148395: [lldb] Unify default/hijack listener between Process{Attach, Launch}Info (NFC)

bulbazord added a comment. In D148395#4285017 , @mib wrote: > In D148395#4270508 , @bulbazord > wrote: > >> Creating a ProcessAttachInfo from a ProcessLaunchInfo with this change means >> they'll have different

[Lldb-commits] [PATCH] D148548: [lldb] Improve breakpoint management for interactive scripted process

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. one nit but otherwise LGTM. Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp:124 + if (!CheckStructuredDataObject(LLVM_PRET

[Lldb-commits] [PATCH] D145297: [lldb] Add an example of interactive scripted process debugging

bulbazord added a comment. Looks good to me. Might want to let @jingham or @JDevlieghere give it a look-over though. Comment at: lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py:9 + +import os,json,struct,signal +import time -

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

bulbazord added inline comments. Comment at: lldb/source/Target/PathMappingList.cpp:72 void PathMappingList::Append(const PathMappingList &rhs, bool notify) { + std::scoped_lock locks(m_mutex, rhs.m_mutex); ++m_mod_id; nickdesaulniers wrote: > ``` > /android

[Lldb-commits] [PATCH] D148395: [lldb] Unify default/hijack listener between Process{Attach, Launch}Info (NFC)

bulbazord added inline comments. Comment at: lldb/include/lldb/Target/Process.h:122 m_async(false) { ProcessInfo::operator=(launch_info); SetProcessPluginName(launch_info.GetProcessPluginName()); mib wrote: > Because we moved `m_listener_sp` an

[Lldb-commits] [PATCH] D148395: [lldb] Unify default/hijack listener between Process{Attach, Launch}Info (NFC)

bulbazord added inline comments. Comment at: lldb/include/lldb/Target/Process.h:122 m_async(false) { ProcessInfo::operator=(launch_info); SetProcessPluginName(launch_info.GetProcessPluginName()); bulbazord wrote: > mib wrote: > > Because we mov

[Lldb-commits] [PATCH] D148395: [lldb] Unify default/hijack listener between Process{Attach, Launch}Info (NFC)

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. I don't have an issue here then. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148395/new/ https://reviews.llvm.org/D148395

[Lldb-commits] [PATCH] D149096: [lldb] Speed up looking for dSYM next to executable

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda, clayborg. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. The goal of this patch is to speed up the

[Lldb-commits] [PATCH] D149096: [lldb] Speed up looking for dSYM next to executable

bulbazord added a comment. Perhaps the right thing to do would be to try to do the larger FileSpec changes instead? The changes in `Symbols::LocateExecutableSymbolFile` can probably go in regardless though. We call `Resolve` on all the file specs in `debug_file_search_paths` on line 307, so no

[Lldb-commits] [PATCH] D149111: [lldb] Add support for specifying language when setting watchpoint by expression

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda, kastiglione, Michael137. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This is useful in context

[Lldb-commits] [PATCH] D149175: [lldb/test] Update lldbutil.fetch_next_event to match broadcaster class

bulbazord added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/lldbutil.py:1206-1207 +def fetch_next_event(test, listener, broadcaster, match_class=False, timeout=10): """Fetch one event from the listener and return it if it matches the provided broadcast

[Lldb-commits] [PATCH] D149175: [lldb/test] Update lldbutil.fetch_next_event to match broadcaster class

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

[Lldb-commits] [PATCH] D149111: [lldb] Add support for specifying language when setting watchpoint by expression

bulbazord added inline comments. Comment at: lldb/source/Interpreter/OptionGroupWatchpoint.cpp:67 + "Number of bytes to use to watch a region."}, +{LLDB_OPT_SET_2, + false, jasonmolenda wrote: > I don't care much, but the formatting of this entry is p

[Lldb-commits] [PATCH] D149111: [lldb] Add support for specifying language when setting watchpoint by expression

bulbazord requested review of this revision. bulbazord added a comment. In D149111#4296670 , @mib wrote: > I think we should still be able to use this new flag with the previous ones > (i.e. `watchpoint set -l c++ -w read -- &my_var`) You can do that st

[Lldb-commits] [PATCH] D149179: [lldb/test] Consolidate interactive scripted process debugging test

bulbazord added a comment. Seems ok to me. You might want to update the dependencies of this diff so that the pre-merge checks don't fail. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149179/new/ https://reviews.llvm.org/D149179

<    1   2   3   4   5   6   7   8   9   10   >