[Lldb-commits] [PATCH] D75720: Make ThreadPlanTracer use {Process, TID} rather than Thread to find it's underlying thread...

jingham created this revision. jingham added reviewers: friss, clayborg. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This is the next patch in the sequence started by: https://reviews.llvm.org/D75711 though it isn't formally dependent on it. See the summary of that pa

[Lldb-commits] [PATCH] D75711: [NFC] Have ThreadPlans hold onto the Process & TID, rather than the Thread

jingham added a comment. In D75711#1909055 , @labath wrote: > I am somewhat worried about the direction this is taking. Maybe vanishing > threads are fine for os-level debugging (because there the "real" threads are > the cpu cores, and the os threads ar

[Lldb-commits] [PATCH] D75711: [NFC] Have ThreadPlans hold onto the Process & TID, rather than the Thread

jingham added a comment. In D75711#1910156 , @clayborg wrote: > As long as the keeping thread plans around for threads that aren't there is > only enabled when OS plug-in is around, I am fine with this direction. One > questions: do we currently only all

[Lldb-commits] [PATCH] D75711: [NFC] Have ThreadPlans hold onto the Process & TID, rather than the Thread

jingham updated this revision to Diff 248868. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75711/new/ https://reviews.llvm.org/D75711 Files: lldb/include/lldb/Target/ThreadPlan.h lldb/include/lldb/Target/ThreadPlanPython.h lldb/source/Plugin

[Lldb-commits] [PATCH] D75720: Make ThreadPlanTracer use {Process, TID} rather than Thread to find it's underlying thread...

jingham updated this revision to Diff 248869. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75720/new/ https://reviews.llvm.org/D75720 Files: lldb/include/lldb/Target/ThreadPlanTracer.h lldb/source/Target/ThreadPlanTracer.cpp Index: lldb/source

[Lldb-commits] [PATCH] D75711: [NFC] Have ThreadPlans hold onto the Process & TID, rather than the Thread

jingham added a comment. In D75711#1912230 , @labath wrote: > Thanks for the detailed response. I'll try to be equally understandable. > > In D75711#1910155 , @jingham wrote: > > > In D75711#1909055

[Lldb-commits] [PATCH] D75880: [NFC} Move ThreadPlans stacks into their own class, store it in Process by TID

jingham created this revision. jingham added reviewers: friss, clayborg, labath. Herald added subscribers: lldb-commits, mgorny. Herald added a project: LLDB. jingham added parent revisions: D75711: [NFC] Have ThreadPlans hold onto the Process & TID, rather than the Thread, D75720: Make ThreadPlan

[Lldb-commits] [PATCH] D75880: [NFC} Move ThreadPlans stacks into their own class, store it in Process by TID

jingham updated this revision to Diff 249257. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75880/new/ https://reviews.llvm.org/D75880 Files: lldb/include/lldb/Target/Process.h lldb/include/lldb/Target/Thread.h lldb/include/lldb/Target/ThreadP

[Lldb-commits] [PATCH] D75711: [NFC] Have ThreadPlans hold onto the Process & TID, rather than the Thread

jingham added a comment. In D75711#1914001 , @labath wrote: > In D75711#1912902 , @jingham wrote: > > > In D75711#1912230 , @labath wrote: > > > > > So this is technically no

[Lldb-commits] [PATCH] D75880: [NFC} Move ThreadPlans stacks into their own class, store it in Process by TID

jingham marked 9 inline comments as done. jingham added a comment. Addressed Pavel's review comments. Comment at: lldb/include/lldb/Target/ThreadPlanStack.h:113 + +class ThreadPlanStackMap { +public: labath wrote: > It's not clear to me what is the value of thi

[Lldb-commits] [PATCH] D75880: [NFC} Move ThreadPlans stacks into their own class, store it in Process by TID

jingham updated this revision to Diff 249515. jingham marked 3 inline comments as done. jingham added a comment. Addressed Pavel's review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75880/new/ https://reviews.llvm.org/D75880 Files: l

[Lldb-commits] [PATCH] D75880: [NFC} Move ThreadPlans stacks into their own class, store it in Process by TID

jingham updated this revision to Diff 249525. jingham added a comment. Fix some clang-format nits. I didn't change: condition1 && condition2 && condition3 to condition1 && condition2 && condition3 as it suggests. We use that all over the place in lldb, and I much prefer it. It makes

[Lldb-commits] [PATCH] D75880: [NFC} Move ThreadPlans stacks into their own class, store it in Process by TID

jingham updated this revision to Diff 249674. jingham added a comment. more reformatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75880/new/ https://reviews.llvm.org/D75880 Files: lldb/include/lldb/Target/Process.h lldb/include/lldb/Targe

[Lldb-commits] [PATCH] D75880: [NFC} Move ThreadPlans stacks into their own class, store it in Process by TID

jingham marked an inline comment as done. jingham added inline comments. Comment at: lldb/source/Target/ThreadPlanStack.cpp:373 + } +} labath wrote: > jingham wrote: > > labath wrote: > > > This is the usual place where one would add a llvm_unreachable. Some >

[Lldb-commits] [PATCH] D76011: Add a verification mechanism to CompilerType.

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Seems fine to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76011/new/ https://reviews.llvm.org/D76011 ___ lldb-commits mailing li

[Lldb-commits] [PATCH] D76009: [lldb/Target] Initialize new targets environment variables from target.env-vars

jingham added a comment. In D76009#1919554 , @friss wrote: > In D76009#1919103 , @labath wrote: > > > Actually, hang on. > > > > > One existing test had to be modified, because the initialization of > > > the envir

[Lldb-commits] [PATCH] D76009: [lldb/Target] Initialize new targets environment variables from target.env-vars

jingham added a comment. BTW, don't worry about the `OS_ACTIVITY_DT_MODE` env var. That's something you have to set when debugging or the Foundation loggging method (NSLog) will go to the system console not to the current terminal, so we force it on all processes on macOS... Repository: rG

[Lldb-commits] [PATCH] D76080: Adjust error_msg handling for expect_expr in lldbtest.py

jingham added a comment. The result from EvaluateExpression is pretty much always going to be valid, since the result holds the error. The correct way to do the first check is: self.assertTrue(eval_result.GetError().Fail(), "Unexpected success...") though you probably also want to make sure "e

[Lldb-commits] [PATCH] D75880: [NFC} Move ThreadPlans stacks into their own class, store it in Process by TID

jingham updated this revision to Diff 250025. jingham marked 6 inline comments as done. jingham added a comment. Removed ThreadPlanStackMap::Update, fixed a bug in PushPlan (can't std::move the ThreadPlan, THEN log it...) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[Lldb-commits] [PATCH] D75880: [NFC} Move ThreadPlans stacks into their own class, store it in Process by TID

jingham marked an inline comment as done. jingham added a comment. Removed Update till the next patch, and fixed a bug in PushPlan that preparing the next patch uncovered. Comment at: lldb/include/lldb/Target/ThreadPlanStack.h:126-130 +auto result = m_plans_list.find(tid);

[Lldb-commits] [PATCH] D75711: [NFC] Have ThreadPlans hold onto the Process & TID, rather than the Thread

jingham added a comment. Is this one okay as well? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75711/new/ https://reviews.llvm.org/D75711 ___ lldb-commits mailing list lldb-commits@lists.llvm.org htt

[Lldb-commits] [PATCH] D76105: [lldb/settings] Reset the inferior environment when target.inherit-env is toggled

jingham added a comment. If I'm following the logic correctly, if you run a target with inherit-env off, and then do: env VAR_IN_ENVIRONMENT=NOT_THE_ENVIRONMENTS_VALUE then turn inherit-env on, we will preserve the value you set it to, not the environment value, because you pass in false for

[Lldb-commits] [PATCH] D75711: [NFC] Have ThreadPlans hold onto the Process & TID, rather than the Thread

jingham added a comment. In D75711#1920642 , @clayborg wrote: > Everything looks good, just a question in inlined comment about having a > thread plan hold onto a pointer to a thread. Seems dangerous The way the ThreadPlanStacks will get used, every tim

[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. Thanks for doing this, it will be useful! I'll let Jonas comment on whether the Reproducer bindings are right, though it looks fine to me. One of the very common uses of an SBEnvi

[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

jingham added a comment. Yes, a key/value approach would be a better API. Looks like the Environment class would make that pretty easy to wrap up, as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76111/new/ https://reviews.llvm.org/D76111

[Lldb-commits] [PATCH] D76105: [lldb/settings] Reset the inferior environment when target.inherit-env is toggled

jingham added a comment. In D76105#1921273 , @labath wrote: > In D76105#1920753 , @friss wrote: > > > Yeah, this shortcoming was pretty obvious while writing the patch. I don't > > like it very much, it seems like

[Lldb-commits] [PATCH] D76188: [lldb/Target] Support more than 2 symbols in StackFrameRecognizer

jingham added a comment. In D76188#1924083 , @labath wrote: > Being able to match multiple symbols sounds useful in its own right, but the > motivating case is a bit shady. `raise`, `__GI_raise` and `gsignal` are all > aliases to one another (they have t

[Lldb-commits] [PATCH] D76216: Improve step over performance

jingham added a comment. This looks fine to me. The thread plans are self-healing, in the sense that they always check after every stop whether they are still relevant, (IsStale) and unship themselves if they aren't. For instance, if a step-over plan finds that the stack has unwound past its

[Lldb-commits] [PATCH] D76188: [lldb/Target] Support more than 2 symbols in StackFrameRecognizer

jingham added inline comments. Comment at: lldb/source/Commands/CommandObjectFrame.cpp:880 auto func = -RegularExpressionSP(new RegularExpression(m_options.m_function)); +RegularExpressionSP(new RegularExpression(m_options.m_symbols.front())); StackFram

[Lldb-commits] [PATCH] D76188: [lldb/Target] Support more than 2 symbols in StackFrameRecognizer

jingham added inline comments. Comment at: lldb/source/Commands/CommandObjectFrame.cpp:880 auto func = -RegularExpressionSP(new RegularExpression(m_options.m_function)); +RegularExpressionSP(new RegularExpression(m_options.m_symbols.front())); StackFram

[Lldb-commits] [PATCH] D76188: [lldb/Target] Support more than 2 symbols in StackFrameRecognizer

jingham added inline comments. Comment at: lldb/source/Commands/CommandObjectFrame.cpp:880 auto func = -RegularExpressionSP(new RegularExpression(m_options.m_function)); +RegularExpressionSP(new RegularExpression(m_options.m_symbols.front())); StackFram

[Lldb-commits] [PATCH] D76407: [lldb/testsuite] Remove "healthy process" test from TestProcessCrashInfo.py

jingham added a comment. This test just seems wrong to me. We can pretend that processes that haven't crashed don't have crash_info, and manually suppress the output in lldb if the stop state isn't a crash. But it seems useful to ask "what are the crash_info bits in flight right now" even if

[Lldb-commits] [PATCH] D76216: Improve step over performance

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Sorry, forgot to select the action... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76216/new/ https://reviews.llvm.org/D76216

[Lldb-commits] [PATCH] D76470: [lldb/Target] Rework the way the inferior environment is created

jingham added a comment. Is there any command-based way to see the entire environment that a process will get when it launches? By populating target.env-vars with the inherited environment there was a way to mostly do that, but I don't see that anymore. It seems a shame not to be able to see

[Lldb-commits] [PATCH] D76470: [lldb/Target] Rework the way the inferior environment is created

jingham added a comment. In D76470#1934146 , @friss wrote: > In D76470#1933910 , @jingham wrote: > > > Is there any command-based way to see the entire environment that a process > > will get when it launches? By

[Lldb-commits] [PATCH] D76532: Internal expressions should not increment the next result variable numbering

jingham created this revision. jingham added a reviewer: davide. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. For instance, breakpoints run an expression when evaluating a condition. If each of these expression evaluations incremented the result numbering, you could en

[Lldb-commits] [PATCH] D76470: [lldb/Target] Rework the way the inferior environment is created

jingham added a comment. Except for the suggestion to use eCommandRequiresTarget for the new command, this looks good. Comment at: lldb/source/Commands/CommandObjectTarget.cpp:690 +interpreter, "target show-launch-environment", +"Shows the environemtn b

[Lldb-commits] [PATCH] D76532: Internal expressions should not increment the next result variable numbering

This revision was automatically updated to reflect the committed changes. Closed by commit rG67d67ebe8f25: Internal expressions shouldn't increment the result variable numbering. (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76

[Lldb-commits] [PATCH] D76672: [lldb/Reproducers] Always collect the whole dSYM in the reproducer

jingham added a comment. Also, the dSYM bundle for a binary contains both the debug information and scripts that may be useful in debugging that binary - data formatters and the like. So even if the original debugging session crashed before they got around to using any of the facilities in the

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

jingham added a reviewer: clayborg. jingham added a comment. Greg originally designed the macOS equivalent of this, so I've added him. I totally agree that you should only do wide searches for source files when there's no way to get a narrower context. Even "source list" could use the current

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

jingham added a comment. IIUC, you should be able to test the actual effect you are trying to achieve by having a large source file, running "source list" to get it into the File Manager, then try to open the source file for writing in a process in a subshell that won't have the same mmap. Tha

[Lldb-commits] [PATCH] D76814: Preserve ThreadPlanStacks for unreported threads

jingham created this revision. jingham added reviewers: clayborg, labath, friss. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. jingham added a parent revision: D75880: [NFC} Move ThreadPlans stacks into their own class, store it in Process by TID. This is the final patch

[Lldb-commits] [PATCH] D76758: Augment lldb's symbol table with external symbols in Mach-O's dyld trie

jingham added inline comments. Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:1926 + bool IsSymbolAlreadyPresent() { +if (flags == LLDB_INVALID_ADDRESS) + return true; JDevlieghere wrote: > `return flags == LLDB_INVALID_ADDRESS` T

[Lldb-commits] [PATCH] D76814: Preserve ThreadPlanStacks for unreported threads

jingham updated this revision to Diff 252982. jingham marked 2 inline comments as done. jingham added a comment. Addressed most of Pavel's review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76814/new/ https://reviews.llvm.org/D76814 Fi

[Lldb-commits] [PATCH] D76814: Preserve ThreadPlanStacks for unreported threads

jingham added a comment. Addressed most of the review comments. I don't see that using file check would make the plan output checking any easier. The current function seems clear and easy to use. Trying to dynamically insert the passed in matches into a filecheck test file and then run file

[Lldb-commits] [PATCH] D76814: Preserve ThreadPlanStacks for unreported threads

jingham marked 2 inline comments as done. jingham added inline comments. Comment at: lldb/source/Target/TargetProperties.td:183 Desc<"A path to a python OS plug-in module file that contains a OperatingSystemPlugIn class.">; + def PluginReportsAllThreads: Property<"plugin-r

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

jingham accepted this revision. jingham added a comment. There was one missing use of GetLastFile (as opposed to doing it by hand.) Getting the default file is not used in any performance critical way that I'm aware of. It's mostly used for "break set -l" with no "-f" and list with no argument

[Lldb-commits] [PATCH] D76814: Preserve ThreadPlanStacks for unreported threads

jingham updated this revision to Diff 254372. jingham added a comment. Add a variable backing the std::condition_variable in the source for the stepping test to catch spurious wakeups. AssertTrue -> AssertIn in the ThreadPlanList test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D76814: Preserve ThreadPlanStacks for unreported threads

jingham marked 5 inline comments as done. jingham added inline comments. Comment at: lldb/source/Target/TargetProperties.td:183 Desc<"A path to a python OS plug-in module file that contains a OperatingSystemPlugIn class.">; + def PluginReportsAllThreads: Property<"plugin-r

[Lldb-commits] [PATCH] D76814: Preserve ThreadPlanStacks for unreported threads

jingham updated this revision to Diff 254664. jingham added a comment. I changed target.process.plugin-reports-all-threads to target.experimental.os-plugin-reports-all-threads. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76814/new/ https://revie

[Lldb-commits] [PATCH] D76814: Preserve ThreadPlanStacks for unreported threads

jingham added a comment. In D76814#1958988 , @labath wrote: > Looks fine to me, though I can't say I'm much of a thread plan expert (but > then again, I don't know if anyone except you is). That's not a great situation, but it is where we currently are.

[Lldb-commits] [PATCH] D75711: [NFC] Have ThreadPlans hold onto the Process & TID, rather than the Thread

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 rGe4598dc04a1f: Make ThreadPlans use TID and Process, rather than Thread *. (authored by jingham). Changed prior to commit:

[Lldb-commits] [PATCH] D75880: [NFC} Move ThreadPlans stacks into their own class, store it in Process by TID

This revision was automatically updated to reflect the committed changes. Closed by commit rG61e8e6882de7: Move thread plan stacks into the Process, indexed by TID. (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75880/new/ http

[Lldb-commits] [PATCH] D75720: Make ThreadPlanTracer use {Process, TID} rather than Thread to find it's underlying thread...

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 rG2c1c57a1df81: Make ThreadPlanTracers use TID & Process rather than Thread *. (authored by jingham). Repository: rG LLVM

[Lldb-commits] [PATCH] D76814: Preserve ThreadPlanStacks for unreported threads

This revision was automatically updated to reflect the committed changes. Closed by commit rG1893065d7bf5: Allow the ThreadPlanStackMap to hold the thread plans for threads that were not… (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[Lldb-commits] [PATCH] D77450: Fix LLDB debug builds

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Ah, great, thanks. Looks like the standalone Xcode debug build doesn't pass LDB_CONFIGURATION_DEBUG. That's a little odd, but the patch is fine. Repository: rG LLVM Github Monorepo CHA

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

jingham added a comment. It's great to add this capability. But if you pass a nullptr, then the exact previous command will be repeated. That's sometimes what you want, but often not. For instance: (lldb) source list -f foo -l 10 ... (lldb) You DON'T want those same source lines to be dumpe

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

jingham added a comment. For instance, maybe instead of passing in a bool, you could pass in the repeat command? So when you create the command, if you pass in None, you get no repeat, if you pass in "" you get the exact command line repeated, and if you pass in anything else, that will be the

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

jingham added a comment. ValueObjects know their execution context, and so they can get to their frame, if they have one. The language of the frame seems like the best thing to clue off here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77153/ne

[Lldb-commits] [PATCH] D77582: [CommandInterpreter] Implement UserCommandExists

jingham added a comment. In D77582#1967031 , @labath wrote: > In D77582#1966962 , @teemperor wrote: > > > I think the part of the patch that adds documentation to the existing > > method should go in (In general al

[Lldb-commits] [PATCH] D77662: [WIP][lldb/test] Make TestLoadUnload compatible with windows

jingham added inline comments. Comment at: lldb/test/API/functionalities/load_unload/TestLoadUnload.py:419 self.copy_shlibs_to_remote() +env_cmd_string = "settings set target.env-vars " + self.dylibPath + "=" + self.getBuildArtifact(".") +self.runCmd(env

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

jingham added a comment. This looks good to me, except that instead of leaving all the other variants of AddCommand, they should funnel through the one that takes the most arguments. It was poor form to leave two around and more so now that there's three. I'm beginning to think we need an SBAd

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

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/D77444/new/ https://reviews.llvm.org/D77444 _

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. The code looks good to me. I'll defer to Adrian as to whether the test is robust, though it also seems reasonable to deal with a failure caused by llvm changes when it happens. CHANGES SI

[Lldb-commits] [PATCH] D96539: [lldb] Add step target to ThreadPlanStepInRange constructor

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. I must have thought there would be a case where you wouldn't be able to figure out the target right when you made the plan, but since that possibility never arose, this simplification is fin

[Lldb-commits] [PATCH] D96688: [lldb] Minor refinements to ThreadPlan::RestoreThreadState (NFC)

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. It is possible that the stub could fail to restore the checkpointed state, though apparently it either never has or never has mattered because no one if setting or checking the bool return.

[Lldb-commits] [PATCH] D96666: [lldb] Remove unused ThreadPlan tracer utilities (NFC)

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. At some point I had the idea that there would be a whole plugin architecture where you could add observers on all the stops & steps the lldb's execution control did. But for lldb command li

[Lldb-commits] [PATCH] D96687: [lldb] Lower GetRealStopInfo into ThreadPlanCallFunction (NFC)

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. It does seem odd to peek behind the back of a thread plan to figure out how to proceed, so making this special to the UserExpression thread plans, which are special in a bunch of ways, isn't

[Lldb-commits] [PATCH] D96839: Add a callout to the LanguageRuntime to override the normal UnwindPlan used for a frame

jingham added inline comments. Comment at: lldb/include/lldb/Target/LanguageRuntime.h:187 + static lldb::UnwindPlanSP + GetAsyncedFrameUnwindPlan(lldb_private::Thread &thread, +lldb_private::RegisterContext *regctx); JDevlieghere wro

[Lldb-commits] [PATCH] D96917: [lldb] Rename {stop, run}_vote to report_{stop, run}_vote

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Herald added a subscriber: JDevlieghere. Sure, that's clearer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96917/new/ https://reviews.llvm.o

[Lldb-commits] [PATCH] D97076: [lldb] Refine ThreadPlan::ShouldAutoContinue

jingham added a comment. This seems like a fine improvement. One little nit, I would ask the current plan ShouldAutoContinue before popping it. Popping the plan does call WillPop, so the plan does have a chance to react to being popped, and you don't know what it will do. So to be on the saf

[Lldb-commits] [PATCH] D97076: [lldb] Refine ThreadPlan::ShouldAutoContinue

jingham added inline comments. Comment at: lldb/include/lldb/Target/ThreadPlan.h:354-359 + /// Returns whether this thread plan overrides the `ShouldStop` of previous + /// plans. + /// + /// When processing the thread plan stack, this function gives plans the + /// ability

[Lldb-commits] [PATCH] D48177: Suppress SIGSEGV on Android when the program will recover

jingham added a comment. Another thing that slightly bugs me about this patch is now we have the Architecture with special purpose code to modify the stop reason, and the Platform ditto. I wonder if it wouldn't be better to have a way to register interest in modifying stop infos, and then let

[Lldb-commits] [PATCH] D97165: [lldb] Add deref support and tests to shared_ptr synthetic

jingham added a comment. This looks great. It's a little weird to use "SBValue.value" to get a string version of something that's a pointer and then have to do regex compares. It's more direct to use "unsigned" and compare against 0x0. Comment at: lldb/test/API/functionali

[Lldb-commits] [PATCH] D97165: [lldb] Add deref support and tests to shared_ptr synthetic

jingham accepted this revision. jingham added a comment. LGTM at this point. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97165/new/ https://reviews.llvm.org/D97165 ___ lldb-commits mailing list lldb-co

[Lldb-commits] [PATCH] D97249: [lldb] Support debugging utility functions

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This will be really handy! The code for UserExpressions ends up doing this same thing (over in ClangExpressionParser::ParseInternal. But given how light-weight creating the file is I'm pre

[Lldb-commits] [PATCH] D97249: [lldb] Support debugging utility functions

jingham added a comment. I am pretty sure other Unix-es share the code to dlopen images that backs "Process::LoadImage". It is trivial compared to the ObjC ones, but is does have a couple of lines of code, 'cause it tries the dlopen and then if that fails, calls dlerror to fetch the error. Th

[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

jingham added a comment. I haven't thought about the details of this implementation closely yet. How will this handled nested progress bars? For instance, if I'm Indexing (maybe even on multiple threads) and then one of the threads results in a debug symbols fetch request (e.g. dsymForUUID).

[Lldb-commits] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

jingham added a comment. While next to the binary seems like a better guess than the cwd, it does seem awkward that if the binary & dwo's aren't in the same place, you have to move files to get that heuristic to work, whereas you could just "cd" into wherever the dwo's got dumped to make the cw

[Lldb-commits] [PATCH] D97985: [lldb] Rename QueueFundamentalPlan to QueueBasePlan (NFC)

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Herald added a subscriber: JDevlieghere. Yeah, apparently I got tired of typing Fundamental pretty quickly... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

jingham added a comment. This way of doing progress is going to look odd in anything that uses multiple debuggers. If I'm reading the code aright, if I have two debuggers, and a target in Debugger A starts doing something that would cause progress traffic, both debuggers will show activity. G

[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

jingham added a comment. In D97739#2607993 , @clayborg wrote: > I agree that we should avoid SBEvent after thinking about it. First off, doing long running operations on a thread that is the one handling the major lldb events is obviously a bad idea. Se

[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

jingham added a comment. In D97739#2610521 , @clayborg wrote: > In D97739#2608510 , @friss wrote: > >> In D97739#2607961 , @clayborg wrote: >> >>> In D97739#2607869

[Lldb-commits] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

jingham added a comment. I bet gdb uses the debugger's CWD because that's the practice working ON gdb (at least back in the day when I worked on it). You built gdb, cd'ed into the gdb build directory and ran the debugger on it from there. gdb's build also produced a debugging-gdb specific .gd

[Lldb-commits] [PATCH] D98653: [lldb] Refactor variable paths to support languages with non-pointer "this" (NFC)

jingham added inline comments. Comment at: lldb/include/lldb/Symbol/CompilerDeclContext.h:77 /// in a struct, union or class. - bool IsClassMethod(lldb::LanguageType *language_ptr, - bool *is_instance_method_ptr, - ConstString *lang

[Lldb-commits] [PATCH] D98914: Make target.process.stop-on-sharedlibrary-events setting work

jingham created this revision. jingham added a reviewer: JDevlieghere. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This setting is supposed to make us stop when we hit a shared library event. The shared library event callbacks

[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

jingham added a comment. Thanks for doing this! The event version looks pretty clean to me. I think we should go that way. I don't think we should have two ways, that seems confusing and still leaves us calling unknown user code in the middle of symbol updates... Repository: rG LLVM Gith

[Lldb-commits] [PATCH] D98822: [lldb] follow-fork/vfork support [WIP]

jingham added a comment. The gdb model - since gdb only supports one debugee per gdb - is to either follow the fork or follow the parent. It would be more in keeping with lldb's model to make a new target for the child side of the fork, and use that to follow the child. That way you can conti

[Lldb-commits] [PATCH] D98914: Make target.process.stop-on-sharedlibrary-events setting work

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9406d4313881: Make the stop-on-sharedlibrary-events setting work. (authored by jingham). Changed prior to commit: https://reviews.llvm.org/D98914?

[Lldb-commits] [PATCH] D98914: Make target.process.stop-on-sharedlibrary-events setting work

jingham added a comment. I added a couple more tests for having a breakpoint at the load site before committing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98914/new/ https://reviews.llvm.org/D98914

[Lldb-commits] [PATCH] D98822: [lldb] follow-fork/vfork support [WIP]

jingham added a comment. In D98822#2640881 , @labath wrote: > In D98822#2637871 , @jingham wrote: > >> The gdb model - since gdb only supports one debugee per gdb - is to either >> follow the fork or follow the par

[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

jingham added a comment. This seems like the right way to go. I made a couple trivial comment comments. But also: the feature has the problem that when multiple debuggers are present, there's no way to tell what debugger actually triggered the work, and in that case all Debuggers will get the

[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

jingham added inline comments. Comment at: lldb/include/lldb/API/SBDebugger.h:46 + static const char *GetBroadcasterClass(); + clayborg wrote: > @jingham: do we need this GetBroadcasterClass()? Or would this only be used > to listen to all debugger events as

[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

jingham added inline comments. Comment at: lldb/source/API/SBDebugger.cpp:857 +error = m_opaque_sp->GetTargetList().CreateTarget( +*m_opaque_sp, filename, arch, eLoadDependentsYes, platform_sp, +target_sp); clayborg wrote: > I subm

[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

jingham accepted this revision. jingham added a comment. This looks good to me. Comment at: lldb/source/API/SBDebugger.cpp:857 +error = m_opaque_sp->GetTargetList().CreateTarget( +*m_opaque_sp, filename, arch, eLoadDependentsYes, platform_sp, +ta

[Lldb-commits] [PATCH] D99331: [TESTS] Fix TestInlineStepping with ccac compiler

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. Raphael is right. The default values of the target.process.thread.step-avoid-regexp are set such that if lldb finds itself in a function inlined from std:: it will automatically s

[Lldb-commits] [PATCH] D99694: Add support for getting signed ObjC tagged pointer values

jingham created this revision. jingham added a reviewer: JDevlieghere. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. The ObjC runtime offers both signed & unsigned tagged pointer value accessors to tagged pointer providers, but ll

[Lldb-commits] [PATCH] D99694: Add support for getting signed ObjC tagged pointer values

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4d9039c8dc2d: Add support for fetching signed values from tagged pointers. (authored by jingham). Changed prior to commit: https://reviews.llvm.or

[Lldb-commits] [PATCH] D99828: Create setting to disable LanguageRuntime provided UnwindPlans

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This seems okay to me. Putting it in the process is right. We do "disable X" in a bunch of other places when enabled is the default, so that seems right. And I don't think there's much be

[Lldb-commits] [PATCH] D96715: [lldb] Decouple IsMasterPlan and OkayToDiscard (NFC)

jingham added a comment. I don't understand how DiscardPlansConsultingMasterPlan in this patch works. With your changes, every plan but the ObjC trampoline finder returns false from OkayToDiscard. So if you are trying to discard plans up to the innermost master plan, you start from the younges

<    8   9   10   11   12   13   14   15   16   17   >