[Lldb-commits] [PATCH] D158010: [lldb] Allow synthetic providers in C++ and fix linking problems

jingham added a comment. Sorry, I got distracted, but after the fact I also agree this is reasonable. We might actually be able to support externally adding C++ based synthetic providers, though I think the best way to do that is to add a way to make an SBTypeSynthetic from the appropriate C++

[Lldb-commits] [PATCH] D158237: Change LLGSTest.cpp to run environment_check inferior with stdin/stdout disabled, to work around ASAN CI bot issue

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This seems good to me. Tests that aren't using stdio for a purpose shouldn't have to worry about random output. But wait on my acceptance a bit to see if Pavel (or anyone else) has a

[Lldb-commits] [PATCH] D158205: [lldb] Fix performance regression after adding GNUstep ObjC runtime

jingham accepted this revision. jingham added inline comments. Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp:49 + if (TT.isOSBinFormatELF()) +return filename.starts_with("libobjc.so"); + if (TT.isOSWindows())

[Lldb-commits] [PATCH] D157556: Replace the singleton "ShadowListener" with a primary and N secondary listeners

This revision was not accepted when it landed; it landed in state "Needs Revision". This revision was automatically updated to reflect the committed changes. Closed by commit rG2e7aa2ee34eb: Replace the singleton ShadowListener with a primary and N secondary Listeners (authored by jingham).

[Lldb-commits] [PATCH] D157556: Replace the singleton "ShadowListener" with a primary and N secondary listeners

jingham marked 4 inline comments as done. jingham added a comment. I implemented Alex's suggestion for a GetListeners filter, but doing that triggered some bugs due to the fact that we weren't really pruning the m_listeners correctly as listeners come and go. I fixed some bugs there along

[Lldb-commits] [PATCH] D157556: Replace the singleton "ShadowListener" with a primary and N secondary listeners

jingham updated this revision to Diff 550495. jingham added a comment. Respond to review comments. In the course of implementing Alex's suggestion to add a filter to GetListeners, I noticed that there we were being lax in our dealing with removed listeners. RemoveListener didn't actually do

[Lldb-commits] [PATCH] D158010: [lldb] Allow synthetic providers in C++ and fix linking problems

jingham added inline comments. Comment at: lldb/source/Commands/CommandObjectType.cpp:2174 -#if LLDB_ENABLE_PYTHON - electriclilies wrote: > rriddle wrote: > > Why is this dropped? > Walter and I want to use the synthetic types from C++, but right now it's >

[Lldb-commits] [PATCH] D157756: Support corefiles that put the uuid of their binary in a fixed address in low memory; load that binary

jingham accepted this revision. jingham 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/D157756/new/ https://reviews.llvm.org/D157756

[Lldb-commits] [PATCH] D157640: [lldb] Improve error message when trying to debug a non-debuggable process

jingham added a comment. Not sure if codesign exists on iOS devices, but at least we could do this on macOS. I thought we'd be able to tell from the Team Identifier whether it was a platform binary, but for /bin/ls & Co I get "not set" for the Team Identifier. CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D157640: [lldb] Improve error message when trying to debug a non-debuggable process

jingham added a comment. In this function we have the path to the binary. We could spawn `codesign -d -entitlements -` and then we would know whether it had that entitlement. Maybe that's more work than you wanted to do here, however. CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D157556: Replace the singleton "ShadowListener" with a primary and N secondary listeners

jingham created this revision. jingham added reviewers: mib, bulbazord, clayborg, JDevlieghere. Herald added a project: All. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Allow a Broadcaster to have one "Primary" listener, and

[Lldb-commits] [PATCH] D157512: [lldb][PlatformDarwin] Only parse SDK from debug-info if target language is Objective-C

jingham added a comment. It's fairly common for AppKit developers to use access to static methods under NSApplication to see the state of the application. Since you aren't actually accessing the frame variables for this purpose, it's also pretty common for those folks to do: (lldb) process

[Lldb-commits] [PATCH] D157022: Fix the NSIndexSet formatter for macOS Sonoma

jingham marked an inline comment as done. jingham added inline comments. Comment at: lldb/source/Plugins/Language/ObjC/Cocoa.cpp:303 +return false; + count = llvm::popcount(bitfield); + break; bulbazord wrote: > This clobbers the

[Lldb-commits] [PATCH] D157022: Fix the NSIndexSet formatter for macOS Sonoma

jingham updated this revision to Diff 547048. jingham added a comment. Remove redundant code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157022/new/ https://reviews.llvm.org/D157022 Files: lldb/source/Plugins/Language/ObjC/Cocoa.cpp Index:

[Lldb-commits] [PATCH] D157022: Fix the NSIndexSet formatter for macOS Sonoma

jingham added inline comments. Comment at: lldb/source/Plugins/Language/ObjC/Cocoa.cpp:278 + && descriptor->GetTaggedPointerInfo(nullptr, nullptr, )) { +count = __builtin_popcountll(payload); +break; bulbazord wrote: > I'm pretty sure

[Lldb-commits] [PATCH] D157022: Fix the NSIndexSet formatter for macOS Sonoma

jingham updated this revision to Diff 546976. jingham marked an inline comment as done. jingham added a comment. Use llvm::popcount instead of using the builtin directly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157022/new/

[Lldb-commits] [PATCH] D157022: Fix the NSIndexSet formatter for macOS Sonoma

jingham created this revision. jingham added reviewers: JDevlieghere, bulbazord, mib. Herald added a subscriber: arphaman. Herald added a project: All. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Just mutatis mutandis for the

[Lldb-commits] [PATCH] D156822: [lldb] Make IR interpretation interruptible

jingham added a comment. LGTM. Thanks for adding this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156822/new/ https://reviews.llvm.org/D156822 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D155334: [lldb] Make frame var --regex always search globals

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

[Lldb-commits] [PATCH] D155334: [lldb] Make frame var --regex always search globals

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. LGTM, with a couple suggestions. You did add a check for requested scope when applying the regex which was missing in the original implementation. That's correct, but I don't

[Lldb-commits] [PATCH] D154992: Add a generic Process method to dump plugin history

This revision was automatically updated to reflect the committed changes. Closed by commit rG8402ad23104b: Add a generic Process method to dump plugin history. (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154992/new/

[Lldb-commits] [PATCH] D154992: Add a generic Process method to dump plugin history

jingham created this revision. jingham added reviewers: jasonmolenda, JDevlieghere. Herald added a project: All. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. I need to dump the gdb-remote communication history when we fail the

[Lldb-commits] [PATCH] D154643: [lldb] Prevent crash when completing ambiguous subcommands

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This looks like it might be the place where we also say "`set s` could either be `set` or `show`". But that's actually done elsewhere so this change is okay. We should maybe make sure we

[Lldb-commits] [PATCH] D154752: runCmd should print the command before running it in case of crashes

This revision was automatically updated to reflect the committed changes. Closed by commit rG01e3393b94d1: Split up the runCmd trace printing to print the command before running. (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D154752: runCmd should print the command before running it in case of crashes

jingham created this revision. jingham added reviewers: JDevlieghere, mib, bulbazord. Herald added a project: All. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. runCmd (and expect which uses runCmd) run the command, and when that

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

This revision was automatically updated to reflect the committed changes. Closed by commit rG2b0c88654212: Refine the reporting mechanism for interruption. (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154542/new/

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

jingham updated this revision to Diff 537908. jingham added a comment. Add some std;:move's, Jonas agrees all the cool kids are doing that these days. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154542/new/ https://reviews.llvm.org/D154542

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

jingham added inline comments. Comment at: lldb/include/lldb/Core/Debugger.h:455-456 +InterruptionReport(std::string function_name, std::string description) : +m_function_name(function_name), +m_description(description), +

[Lldb-commits] [PATCH] D154649: [lldb] Fix dead lock issue when loading modules in Scripted Process

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Yes, that makes sense. lldb always updates its shared library state in reaction to a stop, so it's much safer to have the scripted process emulate this behavior. Plus, refreshing the

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

jingham added inline comments. Comment at: lldb/include/lldb/Core/Debugger.h:455-456 +InterruptionReport(std::string function_name, std::string description) : +m_function_name(function_name), +m_description(description), +

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

jingham updated this revision to Diff 537551. jingham added a comment. Protect InterruptRequested from null function & format strings. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154542/new/ https://reviews.llvm.org/D154542 Files:

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

jingham added inline comments. Comment at: lldb/include/lldb/Core/Debugger.h:430-431 + template + bool InterruptRequested(const char *cur_func, + const char *formatv, Args &&... args) { +bool ret_val = InterruptRequested();

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

jingham updated this revision to Diff 537544. jingham added a comment. Address review comments: Made the in_process target list shared pointers. Removed sstream includes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154542/new/

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

jingham added inline comments. Comment at: lldb/include/lldb/Core/Debugger.h:435-436 + ReportInterruption(InterruptionReport(cur_func, +llvm::formatv(formatv, +

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

jingham created this revision. jingham added reviewers: JDevlieghere, mib, bulbazord, labath. Herald added a project: All. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This patch enhances the interruption features I added a

[Lldb-commits] [PATCH] D153922: [lldb] Duplicate Target::Launch resuming logic into CommandObjectPlatformProcessLaunch

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. The fact that you had to duplicate code from Target::Launch means the various paths to Launching a process aren't properly factored out. However, that seems like a lot just to get Dave's

[Lldb-commits] [PATCH] D153657: Don't allow ValueObject::Cast from a smaller type to a larger

This revision was automatically updated to reflect the committed changes. Closed by commit rGf05e2fb013f0: Dont allow SBValue::Cast to cast from a smaller type to a larger, (authored by jingham). Changed prior to commit: https://reviews.llvm.org/D153657?vs=534036=534775#toc Repository: rG

[Lldb-commits] [PATCH] D153657: Don't allow ValueObject::Cast from a smaller type to a larger

jingham added a comment. In D153657#4445292 , @bulbazord wrote: > I think this patch is probably okay to do, but it does break a supported use > case that I'm aware of: One way you can do "object oriented programming" in C > is to do exactly what

[Lldb-commits] [PATCH] D153657: Don't allow ValueObject::Cast from a smaller type to a larger

jingham created this revision. jingham added reviewers: JDevlieghere, kastiglione, mib. Herald added a project: All. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. SBValue::Cast actually allows casting from a struct type to

[Lldb-commits] [PATCH] D153648: [lldb] Remove broken shared cache duplicate class handling

jingham added a comment. There's similar code for the runtime versions 12-15 which actually looks correct. It seems to claim that the duplicate classes are all stuffed after the capacity in the classOffsets array and that the one that won will indeed be in that list but will not be marked as

[Lldb-commits] [PATCH] D153079: Add an llvm::report_fatal_error for when the darwin kernel says we've finished an insn-step but the thread doesn't think it was insn-stepping

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM. Let's see if we can get this to actually trap the concurrent test failures. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153079/new/

[Lldb-commits] [PATCH] D151951: [lldb][NFCI] Change return type of Properties::GetExperimentalSettingsName

jingham accepted this revision. jingham 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/D151951/new/ https://reviews.llvm.org/D151951

[Lldb-commits] [PATCH] D151966: [lldb] Default can_create to true in GetChildMemberWithName (NFC)

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. It's OK to retain this as the default, and as you say, taking it out would be a trivial patch after this work. The control does allow you to do "Have I already made this child" before

[Lldb-commits] [PATCH] D152031: [lldb] Default can_create to true in GetChildAtIndex (NFC)

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. I can dream up a few speculative uses of `can_create => false` but they are all a little far-fetched. Certainly passing `true` is the dominant mode, so making it a default seems fine to me

[Lldb-commits] [PATCH] D152579: [lldb] Symtab::SectionFileAddressesChanged should clear the file address map instead of name map

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. That does look like a thinko. Nothing about a file's section addresses changing will change the name to symbol map as that isn't dependent on load addresses. Repository: rG LLVM Github

[Lldb-commits] [PATCH] D151597: [lldb][NFCI] Remove use of ConstString from IOHandler

jingham added a comment. I don't think we need to support the case where the "end line token" doesn't have to stand on its own line. It's reasonable for lldb to impose this policy, and if we are going to do that then requiring the "\n" after the end line token also seems odd. The way this is

[Lldb-commits] [PATCH] D152324: [lldb][NFCI] Change return type of PersistentExpressionState::GetNextPersistentVariableName

jingham added a comment. I wonder about this one. In every instance where the API is used, its result is turned into a ConstString first. That's because this variable name lives in the same slot as normal variable names, which come from the debug information and so tend to be in the

[Lldb-commits] [PATCH] D152573: [lldb][NFCI] Remove use of ConstString from Listener

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM, if no one has found a use for this way of filtering by this point, there probably isn't a good one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D151940: Fix regex & startsWith name lookup in SBTarget::FindGlobalVariables

This revision was automatically updated to reflect the committed changes. Closed by commit rG22667e3220de: Fix regex startsWith name lookup in SBTarget::FindGlobalVariables (authored by jingham). Changed prior to commit: https://reviews.llvm.org/D151940?vs=527606=527657#toc Repository: rG

[Lldb-commits] [PATCH] D151940: Fix regex & startsWith name lookup in SBTarget::FindGlobalVariables

jingham created this revision. jingham added reviewers: aprantl, jasonmolenda, mib, JDevlieghere. Herald added a project: All. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. There were two bugs here. 1. eMatchTypeStartsWith

[Lldb-commits] [PATCH] D151861: Don't emit a bunch of spurious "Unknown platform 0" warnings from debugserver

This revision was automatically updated to reflect the committed changes. Closed by commit rG267a4cda8248: Prevent some spurious error messages in the debugserver logs. (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151861/new/

[Lldb-commits] [PATCH] D151843: Add EXC_SYSCALL to the allowable ignored exceptions for Darwin

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG620dc1224ff9: Add EXC_SYSCALL to the set of ignorable mach exceptions. (authored by jingham). Repository: rG LLVM

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set name to targets

jingham added a comment. I'd also maybe call this the Target "Label" not the Name. We have a fairly strong use of Name for breakpoint names, and this doesn't have that character at all. Also, if they are Labels, I think it's legit for us to keep them unique, which I think is more sane than

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set name to targets

jingham added a comment. Make sure we do something sensible with "target select " if the user has given the same name to two targets (or disallow doing that, one or the other). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151859/new/

[Lldb-commits] [PATCH] D151861: Don't emit a bunch of spurious "Unknown platform 0" warnings from debugserver

jingham created this revision. jingham added a reviewer: jasonmolenda. Herald added a project: All. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. The code that was trying to get the platform string was passing the

[Lldb-commits] [PATCH] D151765: [lldb] Introduce the FileSpecBuilder abstraction

jingham added a comment. Why did you choose to have a separate FileSpecBuilder class, rather than making FileSpec smarter about the structure of the path (e.g. have an array of StringRef's into the paths for each component.) We could do the parse once the first time a path element was

[Lldb-commits] [PATCH] D151843: Add EXC_SYSCALL to the allowable ignored exceptions for Darwin

jingham created this revision. jingham added reviewers: JDevlieghere, mib. Herald added a project: All. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. EXC_SYSCALL has the same problem as EXC_BAD_INSTRUCTION and EXC_BAD_ACCESS,

[Lldb-commits] [PATCH] D151392: Fix SBValue::FindValue for file static variables

This revision was automatically updated to reflect the committed changes. Closed by commit rG14186773e79b: Fix SBValue::FindValue for file static variables (authored by jingham). Changed prior to commit: https://reviews.llvm.org/D151392?vs=525391=526852#toc Repository: rG LLVM Github

[Lldb-commits] [PATCH] D151392: Fix SBValue::FindValue for file static variables

jingham created this revision. jingham added reviewers: JDevlieghere, mib. Herald added a project: All. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This was just a thinko. The API StackFrame::GetVariableList takes a bool for

[Lldb-commits] [PATCH] D151292: lldb WIP/RFC: Adding support for address fixing on AArch64 with high and low memory addresses

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM The help string for the setting seems clear. There's also some logic to handle the setting vrs. the values we find from the stub which you describe in the comment to the review, but

[Lldb-commits] [PATCH] D151043: [lldb] Add "Trace" stop reason in Scripted Thread

jingham added a comment. If a Scripted process is able to provide private events and enough info to lldb's lower level calls (read memory, registers) to work the thread-plan machinery then it won't need to generate the inferred stop reasons like PlanComplete and Instrumentation. But the

[Lldb-commits] [PATCH] D150619: [lldb] Delay removal of persistent results

jingham added a comment. This seems fine in general, with one quibble: IIUC, the "indirect" removal of persistent results which you are trying to avoid happens here: lldb::ExpressionResults UserExpression::Execute(DiagnosticManager _manager, ExecutionContext _ctx,

[Lldb-commits] [PATCH] D149914: [lldb] Refactor ObjCLanguage::MethodName

jingham added a comment. LGTM, Adrian's comment is still outstanding however. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149914/new/ https://reviews.llvm.org/D149914 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D149379: [lldb] Add tests for command removal

jingham added a comment. In D149379#4331059 , @wallace wrote: > The alias still works because it still holds a reference to it. I could add > that as a test The alias works but the original command is gone? Interesting. We should definitely test

[Lldb-commits] [PATCH] D149914: [lldb] Refactor ObjCLanguage::MethodName

jingham added a comment. Your test measured setting a found simple breakpoint. That should measure filling all the names caches - we do that the first time you try to set a breakpoint of any sort. But doesn't measure the effects on lookup. I am guessing you will find the same "not much

[Lldb-commits] [PATCH] D150236: Thread::GetStackFrameCount should forestall interruption

This revision was automatically updated to reflect the committed changes. Closed by commit rGe19387e6936c: We cant let GetStackFrameCount get interrupted or it will give the (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D150236: Thread::GetStackFrameCount should forestall interruption

jingham marked 2 inline comments as done. jingham added a comment. I added the enum, that's a good idea. I agree that we should centralize internal reporting of Interrupt events, but as your "longer term" suggests, that's a design task and orthogonal to this patch. I'll do that in a

[Lldb-commits] [PATCH] D150236: Thread::GetStackFrameCount should forestall interruption

jingham updated this revision to Diff 521380. jingham added a comment. Address Review Comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150236/new/ https://reviews.llvm.org/D150236 Files: lldb/include/lldb/Target/StackFrameList.h

[Lldb-commits] [PATCH] D150236: Thread::GetStackFrameCount should forestall interruption

jingham updated this revision to Diff 521183. jingham added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150236/new/ https://reviews.llvm.org/D150236 Files: lldb/include/lldb/Target/StackFrameList.h

[Lldb-commits] [PATCH] D150236: Thread::GetStackFrameCount should forestall interruption

jingham added inline comments. Comment at: lldb/include/lldb/Target/StackFrameList.h:103-104 - void GetFramesUpTo(uint32_t end_idx); + /// Gets frames up to end_idx (which can be greater than the actual number of + /// frames.) Returns true if the function was interrupted,

[Lldb-commits] [PATCH] D150315: Make sure the "Relevant Frame" gets selected before the initial stop printing

This revision was automatically updated to reflect the committed changes. Closed by commit rG7b5dc63fc446: When the Debugger runs HandleProcessEvent it should allow (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150315/new/

[Lldb-commits] [PATCH] D150315: Make sure the "Relevant Frame" gets selected before the initial stop printing

jingham created this revision. jingham added reviewers: mib, JDevlieghere. Herald added a project: All. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. When the Debugger runs HandleProcessEvent it should allow selecting the

[Lldb-commits] [PATCH] D150239: ObjectFileMachO: Prioritize the TEXT segment as the mach header segment, regardless of the order the segments appear in the file

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM for support of something that really should hurt a little more than you are making it... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D150236: Thread::GetStackFrameCount should forestall interruption

jingham created this revision. jingham added reviewers: JDevlieghere, mib, bulbazord. Herald added a project: All. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. We can't let GetStackFrameCount get interrupted or it will give the

[Lldb-commits] [PATCH] D140630: [lldb-vscode] Add data breakpoint support

jingham added a comment. In D140630#4276855 , @cimacmillan wrote: > Ping. Greg's been out since late March, and isn't expected back for a while still. I am entirely unfamiliar with the lldb-vscode part of lldb, so I don't feel comfortable reviewing

[Lldb-commits] [PATCH] D149914: [lldb] Refactor ObjCLanguage::MethodName

jingham added a comment. Most of this is fine. I wonder about avoiding caching the full name and name w/o category & selector name. One of the main uses of this class is to take incoming ObjC names from the ConstString pool, chop them up into full name, name w/o category, and selectorName,

[Lldb-commits] [PATCH] D149379: [lldb] Add tests for command removal

jingham added a comment. What happens if you remove a command that had an alias bound to it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149379/new/ https://reviews.llvm.org/D149379 ___ lldb-commits

[Lldb-commits] [PATCH] D150228: [lldb][NFCI] Replace dw_form_t with llvm::dwarf::Form

jingham added a comment. Apparently a similar change was made with dw_tag_t, in the line below your first deletion I see: typedef llvm::dwarf::Tag dw_tag_t; It seems weird to have dw_tag_t but lvm::dwarf::Form. If there's a good reason to use the more verbose form, we should probably do the

[Lldb-commits] [PATCH] D150157: [lldb] Mark most SBAPI methods involving private types as protected or private

jingham added inline comments. Comment at: lldb/unittests/API/SBCommandInterpreterTest.cpp:24 SBDebugger::Initialize(); m_dbg = SBDebugger::Create(/*source_init_files=*/false); } bulbazord wrote: > jingham wrote: > > It isn't clear to me how the

[Lldb-commits] [PATCH] D150157: [lldb] Mark most SBAPI methods involving private types as protected or private

jingham added a comment. This seems like a pretty non-intrusive way of protecting the lldb_private side of the SB API construction. Looking at the patch makes it seem like we've been semi-randomly assorting members of the SB classes to "protected" and "private". We have NO intentions of ever

[Lldb-commits] [PATCH] D150219: [lldb][NFCI] Remove n^2 loops and simplify iterator usage

jingham accepted this revision. jingham added a comment. You are inconsistent in a couple of places about whether you re-look up m_event_map.end or use the version you captured in a variable, which is a little confusing. Other than that this looks equivalent Comment at:

[Lldb-commits] [PATCH] D149692: Allow scripted thread plans to modify the stop reason shown when the plan completes

jingham added a comment. Committed as: c2be702104284cb3facf31124494b9a400296179 . I forgot to put the differential revision cookie in the commit message... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D149692: Allow scripted thread plans to modify the stop reason shown when the plan completes

jingham added a comment. In D149692#4313715 , @mib wrote: > This looks good to me, although I'm wondering whether instead of passing a > string, we should pass a dictionary. This way the user could either fill the > dictionary with a simple stop reason

[Lldb-commits] [PATCH] D149692: Allow scripted thread plans to modify the stop reason shown when the plan completes

jingham marked an inline comment as done. jingham added inline comments. Comment at: lldb/bindings/python/python-wrapper.swig:390 + if (PyErr_Occurred()) { +printf("Error occured for call to %s.\n", + method_name); mib wrote: > If we passed a

[Lldb-commits] [PATCH] D149692: Allow scripted thread plans to modify the stop reason shown when the plan completes

jingham updated this revision to Diff 518878. jingham added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149692/new/ https://reviews.llvm.org/D149692 Files: lldb/bindings/python/python-wrapper.swig

[Lldb-commits] [PATCH] D149692: Allow scripted thread plans to modify the stop reason shown when the plan completes

jingham created this revision. jingham added reviewers: JDevlieghere, delcypher, mib. Herald added a project: All. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. We were just printing some fairly ugly boiler plate, for instance:

[Lldb-commits] [PATCH] D149625: [lldb] Refactor SBFileSpec::GetDirectory

jingham added a comment. The old code had the side-effect of NOT resolving the path of the SBFileSpec in order to get its directory. I am not sure whether that was on purpose or not, however. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. jingham marked an inline comment as done. Closed by commit rG930c8ac5f561: Improve

[Lldb-commits] [PATCH] D149482: [lldb] Change ObjectValueDictionary to use a StringMap

jingham added a comment. I think something went wrong here. This was a patch for OptionValueDictionary, but it seems to have become a patch for OpenInExternalEditor? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149482/new/

[Lldb-commits] [PATCH] D149482: [lldb] Change ObjectValueDictionary to use a StringMap

jingham added a comment. No reason for these strings to be in the ConstString pool, so that part is fine. But if we're going to use this to store things like the env-vars, having no ordering guarantees when we dump them seems likely to be a bit annoying. If the order of element output in

[Lldb-commits] [PATCH] D149472: [lldb] Refactor host::OpenFileInExternalEditor

jingham added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:387-388 + static std::once_flag g_once_flag; + std::call_once(g_once_flag, [&]() { +if (const char *external_editor = ::getenv("LLDB_EXTERNAL_EDITOR")) { + LLDB_LOG(log, "Looking for

[Lldb-commits] [PATCH] D149472: [lldb] Refactor host::OpenFileInExternalEditor

jingham added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:411-412 kLSLaunchDefaults | kLSLaunchDontAddToRecents | kLSLaunchDontSwitch; - - char *external_editor = ::getenv("LLDB_EXTERNAL_EDITOR"); - - if (external_editor) { -LLDB_LOGF(log,

[Lldb-commits] [PATCH] D149394: Finish the job of D145569

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG47f72aede163: Make the TSan report capture data structure anonymous. (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE

[Lldb-commits] [PATCH] D149394: Finish the job of D145569

jingham created this revision. jingham added reviewers: Michael137, JDevlieghere. Herald added a project: All. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. The InstrumentationRuntime's defined a data structure to gather report

[Lldb-commits] [PATCH] D149312: [lldb] Create a way to force remove commands

jingham added a comment. In D149312#4302449 , @wallace wrote: > Sure You can probably just add something to TestCommandDelete.py. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149312/new/

[Lldb-commits] [PATCH] D149312: [lldb] Create a way to force remove commands

jingham added a comment. This also seems a bit Quixotic to me. Unless you are also planning to remove the `script` command, there's nothing you can do in the command line that you can't do with the SB API so you aren't actually removing any functionality, just making it slightly less

[Lldb-commits] [PATCH] D149312: [lldb] Create a way to force remove commands

jingham added a comment. This was added without a test. Can you add a test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149312/new/ https://reviews.llvm.org/D149312 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

jingham added inline comments. Comment at: lldb/source/Expression/LLVMUserExpression.cpp:339 + if (stack_frame_size == 0) +stack_frame_size = arch == llvm::Triple::msp430 ? 512 : 512 * 1024; This doesn't seem appropriate in generic code. You

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

jingham marked 2 inline comments as done. jingham added inline comments. Comment at: lldb/source/Commands/CommandObjectType.cpp:879-885 +const char *CommandObjectTypeFormatterDelete::g_short_help_template = +"Delete an existing %s for a type."; +const char

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

jingham updated this revision to Diff 516590. jingham added a comment. Adopt suggestions from Alex. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148282/new/ https://reviews.llvm.org/D148282 Files: lldb/source/Commands/CommandObjectType.cpp

  1   2   3   4   5   6   7   8   9   10   >