[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 Ind

[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 *CommandObjectType

[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 shoul

[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-

[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 convenien

[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/ https://reviews.

[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 d

[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 LAS

[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, "Lo

[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 exte

[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 `sett

[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/ https://reviews.llvm.o

[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 the

[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 https://reviews.llvm.org/D149625/n

[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] 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/exa

[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 `Stat

[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 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 ht

[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

[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] 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 chan

[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 s

[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 ma

[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, wh

[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 t

[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 w

[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 https://reviews.llvm.org/D15

[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 "Mos

[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/ htt

[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] 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/sou

[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/inc

[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 subsequent

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGe19387e6936c: We can't let GetStackFrameCount get interrupted or it will give the (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150236

[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 diffe

[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 that

[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

[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 &diagnostic_manager, ExecutionCon

[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 script

[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 i

[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] 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&id=526852#toc Repository: rG LLVM Github Mono

[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, 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 request

[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 MachProcess::De

[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/ https://revi

[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 tr

[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 Githu

[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] 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 search

[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&id=527657#toc Repository:

[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 https://reviews.

[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 ConstStri

[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] 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] 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] 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 setti

[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] 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] 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 a

[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 another

[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 you're

[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: Don't 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&id=534775#toc Repository: r

[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 fi

[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 litt

[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, +std::forward(args)...)));

[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/ https://reviews.l

[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(); bulbaz

[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/include

[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), +m_interrupt_time(std::chrono:

[Lldb-commits] [PATCH] D106226: [lldb] Improve error message when "lldb attach" fails

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. This seems like a good idea, but the functionality belongs in the Linux Platform, not in the Target.cpp. We try really hard to keep platform specific details out of the generic fi

[Lldb-commits] [PATCH] D109101: [lldb] Add an option to specify a VFS overlay

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM Comment at: lldb/source/API/SBDebugger.cpp:878 // call the target triple version. - error = m_opaque_sp->GetTargetList().CreateTarget(*m_opaque_sp, filena

[Lldb-commits] [PATCH] D109249: [lldb] Add Getdescription function for SBInstruction.

jingham added a comment. Would it be better to make an API that takes an SBExecutionContext? If you knew the frame that held In D109249#2988808 , @labath wrote: > I'm not sure what's the exact use case here, but I /think/ that passing just > the targe

[Lldb-commits] [PATCH] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. In the future, can you generate patches with context (i.e. pass -U to "git diff" or "git show"), it's a lot easier to read patches with context. This doesn't seem like the righ

[Lldb-commits] [PATCH] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

jingham added a comment. BTW, do you know what change made this stop working? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109738/new/ https://reviews.llvm.org/D109738 ___ lldb-commits mailing list lldb

[Lldb-commits] [PATCH] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

jingham added a comment. In D109738#3001122 , @vadimcn wrote: >> In the future, can you generate patches with context (i.e. pass -U to >> "git diff" or "git show"), it's a lot easier to read patches with context. > > Sure thing, will do. > > This does

[Lldb-commits] [PATCH] D110298: Add "command multiword add" and the ability to add script commands into a user multiword hierarchy

jingham created this revision. jingham added reviewers: clayborg, JDevlieghere. Herald added subscribers: dang, mgorny. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. One of the ways that we organize all the functionality provided

[Lldb-commits] [PATCH] D110298: Add "command multiword add" and the ability to add script commands into a user multiword hierarchy

jingham added a comment. Note, I didn't do anything on the SB API side. We don't have a way to add commands of any sort through the API's, so we would have to do all that work along with adding multiword support, which is certainly fodder for a separate patch. Repository: rG LLVM Github Mo

[Lldb-commits] [PATCH] D110298: Add "command multiword add" and the ability to add script commands into a user multiword hierarchy

jingham added a comment. I also didn't mirror "command script list" - I'm not sure how useful that is and anyway, that's an orthogonal piece of work, and this patch is big enough already. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110298/new/

[Lldb-commits] [PATCH] D110298: Add "command multiword add" and the ability to add script commands into a user multiword hierarchy

jingham added a comment. The other slightly awkward choice I made was to have all the commands that take command paths (e.g. "command script add" and "command multiword add") take them with each component of the path as a separate argument. We have an argument type for commands, but the problem

[Lldb-commits] [PATCH] D110298: Add "command multiword add" and the ability to add script commands into a user multiword hierarchy

jingham added inline comments. Comment at: lldb/source/Commands/CommandCompletions.cpp:802 + // The only arguments constitute a command path, however, there might be + // options interspersed among the options, and we need to skip those. Do that + // by copying the args vect

[Lldb-commits] [PATCH] D110553: [lldb] Remove non-stop mode code

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. I don't think this patch added all that much value, and pretty much only worked if the non-stop threads never stopped... I think we'd have to start deeper in lldb to if we really want to su

[Lldb-commits] [PATCH] D110115: [lldb] Remove Expression's dependency on CPlusPlusLanguagePlugin

jingham added a comment. I'm fine with this, but I'll defer to Jonas since he had the last significant comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110115/new/ https://reviews.llvm.org/D110115 __

[Lldb-commits] [PATCH] D110601: Add -relative-to-command-file to "command source"

jingham created this revision. jingham added a reviewer: JDevlieghere. Herald added a subscriber: dang. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This option was recently added to "command script import" so that an "organizin

[Lldb-commits] [PATCH] D110601: Add -relative-to-command-file to "command source"

jingham added a comment. The one thing that makes me sad about this is that "command script import" chose "-c" as the short letter for this option, but "-c" was already taken for "command source", so I used -C for "command source". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[Lldb-commits] [PATCH] D110298: Add "command multiword add" and the ability to add script commands into a user multiword hierarchy

jingham added a comment. In D110298#3026579 , @clayborg wrote: > The changes look fine from a quick read. See inline comments The command add/command delete situation is a little fractured. We currently have: command alias/command unalias - Handles al

[Lldb-commits] [PATCH] D110787: Make "process attach -c" work again, add a test for it.

jingham created this revision. jingham added reviewers: jasonmolenda, clayborg, JDevlieghere. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. process attach -c stopped working at some point. Since there was no test for this featur

[Lldb-commits] [PATCH] D110601: Add -relative-to-command-file to "command source"

jingham updated this revision to Diff 376083. jingham added a comment. Made -C with an absolute path an error, added at test for it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110601/new/ https://reviews.llvm.org/D110601 Files: lldb/source/Co

[Lldb-commits] [PATCH] D110601: Add -relative-to-command-file to "command source"

This revision was automatically updated to reflect the committed changes. Closed by commit rG3bf3b96629e8: Add the --relative-to-command-file to "command source" so you can (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110601/n

[Lldb-commits] [PATCH] D110787: Make "process attach -c" work again, add a test for it.

This revision was automatically updated to reflect the committed changes. Closed by commit rG2303391d1f54: Make "process attach -c" work correctly, and add a test for it. (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110787/new

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

jingham added a comment. In D110804#3034878 , @clayborg wrote: > I would like to get a consensus on if we should move this functionality to > "statistics" or not. The reasons that I didn't do it were: > > - "statistics" as a top level method doesn't real

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

jingham added a comment. In D110804#3035052 , @clayborg wrote: > Ok, so how does this sound: > > - This new command will replace the existing "statistics dump" command and > emit JSON only by default > - The "statistics enable/disable" will stay in place

[Lldb-commits] [PATCH] D110298: Add "command multiword add" and the ability to add script commands into a user multiword hierarchy

jingham updated this revision to Diff 376406. jingham marked an inline comment as done. jingham added a comment. Addressed Jonas' review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110298/new/ https://reviews.llvm.org/D110298 Files:

[Lldb-commits] [PATCH] D110298: Add "command multiword add" and the ability to add script commands into a user multiword hierarchy

jingham marked 9 inline comments as done. jingham added a comment. Addresses Jonas' first round of comments. Comment at: lldb/source/Commands/CommandCompletions.cpp:811-813 + sort(to_delete.begin(), to_delete.end(), std::greater()); + for (size_t idx : to_delete) +args.De

[Lldb-commits] [PATCH] D110298: Add "command multiword add" and the ability to add script commands into a user multiword hierarchy

jingham updated this revision to Diff 376410. jingham added a comment. My brain wants llvm::Error to be false when there's an error, (probably because false is the bad state.) Fix a case where I didn't catch myself... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

jingham added a comment. Support for a scripting language in lldb has three distinct pieces. 1. Making the SB API's available to the scripting languages 2. Adding all the callbacks so the scripting language can be used for breakpoint callbacks, data formatters, etc. 3. Adding a REPL for that lan

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

jingham added a comment. In D111409#3052192 , @d-millar wrote: > Just to clarify my use case: I am one of the developers for a > reverse-engineering tool called Ghidra. Part of the tool is > debugging-support to allow cross-over between dynamic and st

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

jingham added a comment. Sounds good. When we started adding extension points for python, we made the implementations be free functions. But then over time we realized it was often more convenient if you had a object managing the callback, that way it could more easily store state over the in

[Lldb-commits] [PATCH] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

jingham added a comment. I realized I typed this all down a while ago but forgot to hit submit. I think I still agree with former me... In D109738#3002257 , @vadimcn wrote: > Hi Jim, > I think there's a bit of confusion going on here: > The original bu

<    5   6   7   8   9   10   11   12   13   14   >