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

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. LGTM, small nit Comment at: lldb/test/API/functionalities/scripted_process/main.cpp:21 + // returns + lock.lock(); +} nit: no need t

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

bulbazord added inline comments. Comment at: lldb/examples/python/scripted_process/scripted_platform.py:33 + +processes = { +420: { JDevlieghere wrote: > Why is this method implemented and not a `pass` like the others? This method is not imple

[Lldb-commits] [PATCH] D140113: [lldb] Force override when adding crashlog command

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. Do we want to do this for all the modules in `examples/python` that do something to this effect? e.g. `memory.py` as well. It also might be worth changing some of those messages from err

[Lldb-commits] [PATCH] D140249: [lldb] Do not deallocate memory after exec

bulbazord created this revision. bulbazord added reviewers: jasonmolenda, jingham, JDevlieghere. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. After an exec has occured, resources used to manage the state of a Process are cleaned up. One su

[Lldb-commits] [PATCH] D140253: [debugserver] Clear memory allocations after exec

bulbazord created this revision. bulbazord added a reviewer: jasonmolenda. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. After an exec, the inferior is a new process and none of these memory regions are still allocated. Clear them out. Re

[Lldb-commits] [PATCH] D140249: [lldb] Do not deallocate memory after exec

bulbazord added inline comments. Comment at: lldb/source/Target/Memory.cpp:337 std::lock_guard guard(m_mutex); - if (m_process.IsAlive()) { + if (m_process.IsAlive() && !did_exec) { PermissionsToBlockMap::iterator pos, end = m_memory_map.end(); jasonmol

[Lldb-commits] [PATCH] D140249: [lldb] Do not deallocate memory after exec

bulbazord updated this revision to Diff 483692. bulbazord added a comment. Add a comment explaining why we do not dealloc after exec Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140249/new/ https://reviews.llvm.org/D140249 Files: lldb/include/l

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

bulbazord added a comment. 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 `lldb-framework-cleanup` before we perform an install. Is this the case? This seems most usef

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

bulbazord added inline comments. Comment at: lldb/source/API/CMakeLists.txt:215-216 + + add_dependencies(install-liblldb +lldb-framework-cleanup) endif() I think you need to add it as a dependency of `install-liblldb-stripped` as well? CHANGES SINCE LAST

[Lldb-commits] [PATCH] D140249: [lldb] Do not deallocate memory after exec

bulbazord updated this revision to Diff 488035. bulbazord added a comment. Address @jingham's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140249/new/ https://reviews.llvm.org/D140249 Files: lldb/include/lldb/Target/Memory.h lldb/sou

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

bulbazord added a comment. In D141021#4043411 , @labath wrote: > Does this mean that the in-tree lldb will cease to be functional once someone > "installs" it? (at least until the next rebuild) I believe so, but only in the case where you are building t

[Lldb-commits] [PATCH] D140249: [lldb] Do not deallocate memory after exec

This revision was automatically updated to reflect the committed changes. Closed by commit rGf2f3b1a87ad2: [lldb] Do not deallocate memory after exec (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140249/new/ https://reviews.

[Lldb-commits] [PATCH] D140253: [debugserver] Clear memory allocations after exec

This revision was automatically updated to reflect the committed changes. Closed by commit rG58def623ca46: [debugserver] Clear memory allocations after exec (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140253/new/ https://r

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

bulbazord added a comment. I'm somewhat unfamiliar with this code so I'll ask some clarifying questions. I think I understand the idea in general though. Comment at: lldb/examples/python/scripted_process/crashlog_scripted_process.py:34-37 +for section

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

bulbazord added a comment. Additionally, it looks like you're doing maybe 2-3 things in this patch (Progress reporting, adding a guard rail, changing the format of `loaded_images` in the scripted process example). You could probably break this up into multiple patches. Repository: rG LLVM G

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

bulbazord added a comment. Seems okay to me. @JDevlieghere How does this look to you? Comment at: lldb/examples/python/scripted_process/crashlog_scripted_process.py:34-37 +for section in image.section_infos: +if section.start

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

bulbazord requested changes to this revision. bulbazord added inline comments. Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:421 Target &target = GetTarget(); + bool force_lookup = m_scripted_metadata.GetClassName().contains("CrashLog"); -

[Lldb-commits] [PATCH] D142059: [lldb/Plugins] Add ScriptedProcess::GetCapabilities affordance (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/D142059/new/ https://reviews.llvm.org/D142059 ___ lldb-commits mailing list lldb-c

[Lldb-commits] [PATCH] D142926: [lldb][RFC] Replace SB swig interfaces with API headers

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, clayborg, jingham, labath, mib. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. This is a proof-of-concept change that shows what it would take to use the API headers a

[Lldb-commits] [PATCH] D142926: [lldb][RFC] Replace SB swig interfaces with API headers

bulbazord added inline comments. Comment at: lldb/cmake/modules/LLDBFramework.cmake:91 DEPENDS ${header} OUTPUT ${staged_header} -COMMAND ${CMAKE_COMMAND} -E copy ${header} ${staged_header} -COMMENT "LLDB.framework: collect framework header") +COMMAND unifdef -US

[Lldb-commits] [PATCH] D141658: [lldb/crashlog] Make interactive mode the new default

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. I like this. I have no opinion on having `-i/-b` versus `--mode/-m` to indicate which mode crashlog.py should use, though I think that using an enum makes it easier to add new modes down

[Lldb-commits] [PATCH] D143104: [lldb/Plugins] Add Attach capabilities to ScriptedProcess

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. Makes sense to me. It does seem a little weird that you can specify a name or a pid for attaching to a scripted process but I suppose that's just a part of the interface. Repository:

[Lldb-commits] [PATCH] D142926: [lldb][RFC] Replace SB swig interfaces with API headers

bulbazord added inline comments. Comment at: lldb/include/lldb/API/SBAddress.h:14-16 +#ifdef SWIG +%include "interface/SBAddressDocstrings.i" +#endif clayborg wrote: > bulbazord wrote: > > clayborg wrote: > > > Do we want two different .i files? Right now we have

[Lldb-commits] [PATCH] D143232: Return an error when the CFA resolves to no known register, instead of segfaulting

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. LGTM. I wonder if there's a good way to exercise this with a test? Like maybe we can create some bogus unwind information and see if LLDB falls over when consuming it? Repository: rG

[Lldb-commits] [PATCH] D143236: [lldb] Add a way to get a scripted process implementation from the SBAPI

bulbazord added a comment. This is an interesting idea. I suppose I find it a little strange that something from the SB API is passing a void * back to the caller instead of something more well defined (like another SB class or some primitive) but I suppose that's not the worst thing in the wo

[Lldb-commits] [PATCH] D143282: [lldb] Accept negative indexes in __getitem__

bulbazord added a comment. A simple test would be what you did in your commit summary, trying to get `thread.frame[-1]` and verifying that it's the correct frame. LGTM with a test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143282/new/ https://

[Lldb-commits] [PATCH] D142926: [lldb][RFC] Replace SB swig interfaces with API headers

bulbazord added a comment. In D142926#4102287 , @labath wrote: > I like this. > > I'm not sure what it would take, but I think it'd be nice if the > de-swig-ification was not specific to the framework build. Ideally, I'd make > it controlled by a separa

[Lldb-commits] [PATCH] D142926: [lldb][RFC] Replace SB swig interfaces with API headers

bulbazord updated this revision to Diff 495695. bulbazord added a comment. Herald added a reviewer: jdoerfert. Herald added subscribers: Michael137, sstefan1. First pass at a complete implementation. Actions taken: - Added swig flags to ignore swig warnings indicating that swig is ignoring certa

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

bulbazord added a comment. In addition to what I wrote above, I also fixed several documentation bugs (putting docstrings on the wrong method). I manually audited the generated code before and after this change. Here are my notes: Not too interesting: - Some parameter names changed - Fixed lo

[Lldb-commits] [PATCH] D143548: [lldb] Add the ability to remove diagnostic callbacks

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/D143548/new/ https://reviews.llvm.org/D143548 ___ lldb-commits mailing list lldb-c

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

bulbazord updated this revision to Diff 495978. bulbazord added a comment. - Removed a bunch of header guards effectively adding methods to the python bindings. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142926/new/ https://reviews.llvm.org/D14

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

bulbazord added a comment. In D142926#4113600 , @clayborg wrote: > In D142926#4111717 , @bulbazord > wrote: > >> Certainly interesting: >> >> - SBListener::StopListeningForEventClass return type conflicts (ABI br

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

bulbazord added a comment. Not sure how useful it would be but I recorded the full list of methods get added with this change. Take a look and let me know if there are any that you think shouldn't be added. SBAttachInfo::~SBAttachInfo() SBBroadcaster::operator<(const lldb::SBBroadcaster &)

[Lldb-commits] [PATCH] D143623: [lldb] Print an error for unsupported combinations of log options

bulbazord requested changes to this revision. bulbazord added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/Commands/CommandObjectLog.cpp:180-184 +if ((m_options.handler != eLogHandlerCircular && + m_options.handler != eLogHan

[Lldb-commits] [PATCH] D143623: [lldb] Print an error for unsupported combinations of log options

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/D143623/new/ https://reviews.llvm.org/D143623 ___ lldb-commits mailing list lldb-c

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

bulbazord updated this revision to Diff 496299. bulbazord added a comment. Stop exposing unsupported SBReproducer functions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142926/new/ https://reviews.llvm.org/D142926 Files: lldb/bindings/CMakeList

[Lldb-commits] [PATCH] D143690: [lldb] Add the ability to provide a message to a progress event update

bulbazord accepted this revision. bulbazord added a comment. Thanks for including the motivation in your summary, that made following this change a lot easier. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143690/new/ https://reviews.llvm.org/D143690

[Lldb-commits] [PATCH] D143792: [lldb] Fix image lookup crash

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. lldb may crash when performing `image lookup --verbose --address

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

bulbazord added a comment. Does anybody have any further concerns or objections? I'd like to get this change in early next week if possible. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142926/new/ https://reviews.llvm.org/D142926 __

[Lldb-commits] [PATCH] D143792: [lldb] Fix image lookup crash

This revision was automatically updated to reflect the committed changes. Closed by commit rGf841c4af5f5e: [lldb] Fix image lookup crash (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143792/new/ https://reviews.llvm.org/D143

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

bulbazord added a comment. In D142926#4123492 , @labath wrote: > Well... right now, there isn't anything else to do there. > > Anyway, I don't think this is particularly important. I don't know even if > the distros would like to do this or whether to th

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

bulbazord updated this revision to Diff 497117. bulbazord added a comment. Rebasing against top of tree Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142926/new/ https://reviews.llvm.org/D142926 Files: lldb/bindings/CMakeLists.txt lldb/binding

[Lldb-commits] [PATCH] D144142: [debugserver] Initialize logging earlier in the startup sequence

bulbazord created this revision. bulbazord added a reviewer: jasonmolenda. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Prior to setting up logging, we have uses of RNBLogSTDERR and RNBLogSTDOUT. Th

[Lldb-commits] [PATCH] D144142: [debugserver] Initialize logging earlier in the startup sequence

bulbazord added inline comments. Comment at: lldb/tools/debugserver/source/debugserver.cpp:1342 DNBLogSetLogMask(log_flags); } jasonmolenda wrote: > Should this be moved out of this `log_file != NULL` block so that the logging > flags are set based on

[Lldb-commits] [PATCH] D144142: [debugserver] Initialize logging earlier in the startup sequence

bulbazord updated this revision to Diff 497818. bulbazord added a comment. Moving second use of DNBSetLogMask outside of the log file block Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144142/new/ https://reviews.llvm.org/D144142 Files: lldb/to

[Lldb-commits] [PATCH] D144142: [debugserver] Initialize logging earlier in the startup sequence

This revision was automatically updated to reflect the committed changes. Closed by commit rGdcf18e4757b2: [debugserver] Initialize logging earlier in the startup sequence (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144142/

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

This revision was automatically updated to reflect the committed changes. Closed by commit rG662548c82683: [lldb] Replace SB swig interfaces with API headers (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142926/new/ https://

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

bulbazord added a comment. In D142926#4133356 , @saugustine wrote: > I don't think this necessarily matters or is a reason to revert, but we are > seeing issues with this patch and swig 3.0.2, that disappear with swig 4.0. > > The docs claim that versio

[Lldb-commits] [PATCH] D144228: [lldb] Stop generating swig bindings for SBLaunchInfo copy constructor

bulbazord created this revision. bulbazord added reviewers: saugustine, mib. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Given the line launch_info = lldb.SBLaunchInfo(None) We see different be

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

bulbazord added a comment. In D142926#4133440 , @bulbazord wrote: > I'll upload a patch to handle that. D144228 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142926/new/

[Lldb-commits] [PATCH] D144224: [lldb] Extend SWIG SBProcess interface with WriteCStringToMemory method

bulbazord added a reviewer: jingham. bulbazord requested changes to this revision. bulbazord added a comment. This revision now requires changes to proceed. I'm alright with the implementation, but I think you should add some documentation here. Also, what do you think of the name `WriteMemoryAsC

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

bulbazord added a comment. Just a personal preference but making those lambdas into static functions may make it easier to read. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141702/new/ https://reviews.llvm.org/D141702 ___ lldb-commits mail

[Lldb-commits] [PATCH] D144224: [lldb] Extend SWIG SBProcess interface with WriteMemoryAsCString method

bulbazord added a comment. LGTM. @jingham what do you think? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144224/new/ https://reviews.llvm.org/D144224 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/m

[Lldb-commits] [PATCH] D144237: [lldb/Plugins] Add memory writing capabilities to Scripted Process

bulbazord added inline comments. Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:310-311 +size_t ScriptedProcess::DoWriteMemory(lldb::addr_t vm_addr, const void *buf, + size_t size, Status &error) { + lldb::DataExtracto

[Lldb-commits] [PATCH] D144238: [lldb] StructuredData should not truncate uint64_t values

bulbazord created this revision. bulbazord added a reviewer: mib. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. In json::Value, getAsInteger returns an optional and getAsNumber returns an optional. I

[Lldb-commits] [PATCH] D144228: [lldb] Stop generating swig bindings for SBLaunchInfo copy constructor

This revision was automatically updated to reflect the committed changes. Closed by commit rGffe982f96bff: [lldb] Stop generating swig bindings for SBLaunchInfo copy constructor (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[Lldb-commits] [PATCH] D144238: [lldb] StructuredData should not truncate uint64_t values

bulbazord added a comment. In D144238#4133940 , @mib wrote: > Should we also add a negative integer to the dictionary to make sure this > didn't cause a regression ? Negative integers are not correctly handled currently. I plan on fixing that in a foll

[Lldb-commits] [PATCH] D144237: [lldb/Plugins] Add memory writing capabilities to Scripted Process

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/D144237/new/ https://reviews.llvm.org/D144237 ___ lldb-commits mailing list lldb-c

[Lldb-commits] [PATCH] D144238: [lldb] StructuredData should not truncate uint64_t values

bulbazord updated this revision to Diff 498479. bulbazord added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144238/new/ https://reviews.llvm.org/D144238 Files: lldb/source/Utility/StructuredData.cpp lldb/test/API/

[Lldb-commits] [PATCH] D144238: [lldb] StructuredData should not truncate uint64_t values

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2f88c07cf820: [lldb] StructuredData should not truncate uint64_t values (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINC

[Lldb-commits] [PATCH] D143104: [lldb/Plugins] Add Attach capabilities to ScriptedProcess

bulbazord added a comment. I'm not sure how much this matters but the fact that `ScriptedMetadata::IsValid` (and the bool operator overload) relies on the class name being empty is a little strange to me. I feel like you could get yourself into a strange situation where you set the ScriptedMeta

[Lldb-commits] [PATCH] D144311: [debugserver] Add one additional sleep before attaching after waiting

bulbazord created this revision. bulbazord added a reviewer: jasonmolenda. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. It's possible for debugserver to attach to a process during the handoff betwee

[Lldb-commits] [PATCH] D144528: [lldb] Warn when Mach-O files have overlapping segments

bulbazord created this revision. bulbazord added reviewers: jasonmolenda, jingham, mib. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. I recently came across a binary that for some reason had overlapp

[Lldb-commits] [PATCH] D144528: [lldb] Warn when Mach-O files have overlapping segments

bulbazord added a comment. In D144528#4145593 , @jasonmolenda wrote: > I'm OK with giving this a try. SectionLoadList::SetSectionLoadAddress > specifically notes that there are cases were sections overlap in the virtual > address space. All of the bi

[Lldb-commits] [PATCH] D144528: [lldb] Warn when Mach-O files have overlapping segments

This revision was automatically updated to reflect the committed changes. Closed by commit rGcee05eed2f47: [lldb] Warn when Mach-O files have overlapping segments (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144528/new/ htt

[Lldb-commits] [PATCH] D144311: [debugserver] Add one additional sleep before attaching after waiting

This revision was automatically updated to reflect the committed changes. Closed by commit rG97b579d31404: [debugserver] Add one additional sleep before attaching after waiting (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14

[Lldb-commits] [PATCH] D144688: [lldb] Fix watchpoint command function stopping behavior

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. I'm alright with this patch since it at least makes watchpoint commands consistent with breakpoint commands (w.r.t the `-F` flag). Comment at: lldb/source/Commands/Com

[Lldb-commits] [PATCH] D144688: [lldb] Fix {break, watch}point command function stopping behavior

bulbazord requested changes to this revision. bulbazord added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:1314-1324 +if (line.startswith("return ")) { + llvm::String

[Lldb-commits] [PATCH] D144688: [lldb] Fix {break, watch}point command function stopping behavior

bulbazord added inline comments. Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:1314-1324 +if (line.startswith("return ")) { + llvm::StringRef return_val = + line.substr(llvm::StringRef("return ").size()); + sstr.Print

[Lldb-commits] [PATCH] D144688: [lldb] Fix {break, watch}point command function stopping behaviour

bulbazord added inline comments. Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:1316 +auto_generated_function.AppendString("def __user_code():"); +for (int i = 0; i < num_lines; ++i) { + sstr.Clear(); delcyp

[Lldb-commits] [PATCH] D144688: [lldb] Fix {break, watch}point command function stopping behaviour

bulbazord accepted this revision. bulbazord added a comment. LGTM thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144688/new/ https://reviews.llvm.org/D144688 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

bulbazord added a comment. Seems like you've already got feedback about breaking ABI. Small nit: Comment at: lldb/bindings/interface/SBWatchpointDocstrings.i:30-31 +%feature("docstring", " +Returns true if the watchpoint is a variable watchpoint. Otherwise the +watchpoi

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

bulbazord added inline comments. Comment at: lldb/source/API/SBWatchpoint.cpp:319 + } + return false; +} delcypher wrote: > @bulbazord What I don't like about my implementation here is that the user > can't tell the difference between > > 1. there's no shared

[Lldb-commits] [PATCH] D143104: [lldb/Plugins] Add Attach capabilities to ScriptedProcess

bulbazord added a comment. I still think it's a little weird that you can create LaunchInfo or AttachInfo, call SetScriptedProcessDictionary, and still have the ScriptedMetadata be "invalid", but I suppose it makes no sense if there is no class name anyway. Just a few small things. Everything e

[Lldb-commits] [PATCH] D144224: [lldb] Extend SWIG SBProcess interface with WriteMemoryAsCString method

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. Thanks for adding those tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144224/new/ https://reviews.llvm.org/D144224 ___ lldb-c

[Lldb-commits] [PATCH] D145414: [lldb/Utility] Fix layering violation caused by ScriptedMetadata

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. LGTM. Comment at: lldb/include/lldb/Utility/ScriptedMetadata.h:33-38 - ScriptedMetadata(const OptionGroupPythonClassWithDict &option_group) { -auto opt_group = con

[Lldb-commits] [PATCH] D145450: [lldb] Respect empty arguments in target.run-args

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, clayborg, 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. Currently empty arguments are not respect

[Lldb-commits] [PATCH] D145450: [lldb] Respect empty arguments in target.run-args

This revision was automatically updated to reflect the committed changes. Closed by commit rGca8faf8f4659: [lldb] Respect empty arguments in target.run-args (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145450/new/ https://r

[Lldb-commits] [PATCH] D145136: Add a Debugger interruption mechanism in parallel to the Command Interpreter interruption

bulbazord added a comment. Small nits Comment at: lldb/include/lldb/API/SBCommandInterpreter.h:251-253 + /// Interrupts the command currently executing in the RunCommandInterpreter + /// thread. + bool InterruptCommand(); Some info on the return value would

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, clayborg, jasonmolenda, jingham, labath. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. MemoryCache::Read is not resilie

[Lldb-commits] [PATCH] D145547: When setting load addresses on darwin kernel kexts, handle case where in-memory load commands are not updated

bulbazord added inline comments. Comment at: lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp:816-819 + ObjectFileMachO *ondisk_objfile_macho = + llvm::dyn_cast_or_null( + m_module_sp ? m_module_sp->GetObjectFile() : nullpt

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

bulbazord added inline comments. Comment at: lldb/source/Target/Memory.cpp:133-141 // Check the L1 cache for a range that contain the entire memory read. If we - // find a range in the L1 cache that does, we use it. Else we fall back to - // reading memory in m_L2_cache_line

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

bulbazord added a comment. In D145624#4182424 , @clayborg wrote: > I would suggest checking the google stadia patch for the L1 > and L2 caches: > > https://github.com/googlestadia/vsi-lldb/blob/master/patches/llvm-project/00

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

bulbazord updated this revision to Diff 503942. bulbazord added a comment. - Addressed reviewer feedback - Simplified the case where we use L2 cache lines Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145624/new/ https://reviews.llvm.org/D145624 F

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

bulbazord marked 11 inline comments as done. bulbazord added inline comments. Comment at: lldb/source/Target/Memory.cpp:255-256 - // We need to read from the process + if (process_bytes_read < cache_line_byte_size) +data_buffer_heap_up->SetByteSize(process_byt

[Lldb-commits] [PATCH] D145295: [lldb] Move ScriptedProcess private state update to implementation

bulbazord added inline comments. Comment at: lldb/examples/python/scripted_process/scripted_process.py:166-167 -Returns: -lldb.SBError: An `lldb.SBError` with error code 0. -""" -return lldb.SBError() - -@abstractmethod -def should_st

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

bulbazord added inline comments. Comment at: lldb/source/Target/Memory.cpp:245-246 +if (m_invalid_ranges.FindEntryThatContains(cache_line_base_addr)) { + error.SetErrorStringWithFormat("memory read failed for 0x%" PRIx64, + cache_line_

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

bulbazord updated this revision to Diff 504873. bulbazord added a comment. Address small nits from Greg Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145624/new/ https://reviews.llvm.org/D145624 Files: lldb/include/lldb/Target/Memory.h lldb/so

[Lldb-commits] [PATCH] D145295: [lldb] Move ScriptedProcess private state update to implementation

bulbazord added a comment. No problem from me. Jonas? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145295/new/ https://reviews.llvm.org/D145295 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/

[Lldb-commits] [PATCH] D145450: [lldb] Respect empty arguments in target.run-args

bulbazord added inline comments. Comment at: lldb/source/Host/common/ProcessLaunchInfo.cpp:324-325 std::string safe_arg = Args::GetShellSafeArgument(m_shell, argv[i]); + if (safe_arg.empty()) +safe_arg = "\"\""; // Add a space to separat

[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

bulbazord added inline comments. Comment at: lldb/test/Shell/helper/build.py:254 +if args.objc_gnustep: +assert args.objc_gnustep_dir, "GNUstep libobjc2 runtime for Linux and Windows" +self.objc_gnustep_inc = os.path.join(args.objc_gnustep_dir, '

[Lldb-commits] [PATCH] D145136: Add a Debugger interruption mechanism in parallel to the Command Interpreter interruption

bulbazord added inline comments. Comment at: lldb/source/Core/Debugger.cpp:1973-1977 +HostThread &Debugger::SetIOHandlerThread(HostThread &new_thread) { + HostThread &old_host = m_io_handler_thread; + m_io_handler_thread = new_thread; + return old_host; +} JDe

[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] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

bulbazord added inline comments. Comment at: lldb/test/Shell/helper/toolchain.py:46 +if config.objc_gnustep_dir: + build_script_args.append('--objc-gnustep-dir="{0}"'.format(config.objc_gnustep_dir)) sgraenitz wrote: > bulbazord wrote: > > Why does

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

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, jingham, mib, labath, clayborg. Herald added a subscriber: emaste. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. The high lev

[Lldb-commits] [PATCH] D108335: [lldb] Move UnixSignals subclasses to lldbTarget

bulbazord abandoned this revision. bulbazord added a comment. This is made defunct by https://reviews.llvm.org/D146263 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108335/new/ https://reviews.llvm.org/D108335 _

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

This revision was automatically updated to reflect the committed changes. Closed by commit rGf341d7a4091a: [lldb] Make MemoryCache::Read more resilient (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145624/new/ https://review

[Lldb-commits] [PATCH] D146335: [lldb] Introduce CMake variable LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, aprantl, labath. 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 add the ability for the CMake con

[Lldb-commits] [PATCH] D146335: [lldb] Introduce CMake variable LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS

This revision was automatically updated to reflect the committed changes. Closed by commit rGc47da7f10946: [lldb] Introduce CMake variable LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14

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

bulbazord added a comment. I'm supportive of this idea but I would like to find a way to do it without introducing dependencies on plugins in non-plugin code if possible. Comment at: lldb/include/lldb/Core/DumpRegisterValue.h:12 +#include "Plugins/TypeSystem/Clang/TypeSystem

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