- User Since
- Sep 23 2014, 5:24 PM (306 w, 11 h)
Wed, Jul 29
Remember to do the action thingie...
Tue, Jul 28
This LGTM with a couple of quibbles but I haven't been working on the platform support recently so we should wait till people who have done so weight in.
I'm a little curious how you would specify the path to your binary on a remote platform using the command line. But this way certainly wasn't it...
Fri, Jul 24
This is doing the right thing. Ismail and I talked a little off-line and I allayed his concerns.
This overall change makes sense to me.
Wed, Jul 22
The current lldb behavior is that when you start up lldb, it reads the editline command-history file (so the history of the previous session), and loads it into it's internal command history, which you can get to by up and down arrows or ^R searches. But it doesn't prime the list that the "command history" command has access to. So command history starts out empty. But IMO, that's just bug. If a command shows up in scrolling through with up-arrow, it should also be listed in the history command wherever it lives. This really reduces the utility of the "history" command... I think we do want to show this. We might want mark in the "command history" output where the imported commands end and the new commands start, but we really should make them available.
Tue, Jul 21
I think you missed two more places to pass a target, then this looks good to me. How tedious, thanks for doing it...
I pointed out a couple of places where you have either at hand or near at hand a useful exe_ctx that you could pass instead of nullptr.
The !text test is correct, since you intend to pass *text in as a ValueObject &. But I wouldn't add the GetValueAsUnsigned check, that seems confusing. The NSStringSummaryProvider is returning a bool to tell you whether it succeeded or not, so it seems odd to pre-judge one of it's error states.
The patch lost seems to have lost everything but the PPC, SystemZ and MIPS code...
Mon, Jul 20
Yes, the only way you can get your hands on an SBThreadPlan is in the process of queueing one, so the ThreadPlan stack will always be the one to manage the lifecycle of a plan. And if an SBThreadPlan represents a ThreadPlan that has been discarded, it should convert back to an empty SBThreadPlan rather than keeping the underlying plan alive. So this is correct.
Fri, Jul 17
Address review comments
Thu, Jul 16
Address review comments.
BTW, note that target delete has a -c option which does:
And if you really are trying to keep out-of-date modules from building up, DeleteTarget is not the correct time to do this. After all, this most commonly happens in command-line lldb when people do:
Note, you want to be careful how you reap executables whose binary has changed. If you are debugging with the debug info in .o files, and you change one .cpp file & rebuild, you can throw away the Module for the executable, but you don't want to throw away the modules that back the .o files that haven't changed.
Wed, Jul 15
We certainly don't want to clear the shared module cache when the Debugger or the SBDebugger is destroyed. Most IDE's that use LLDB as a library use a debugger per debugging session, and destroy the debugger when the debugging session ends. As it stands now, when the first debugging session ends we don't remove any of the already parsed modules, so that the second debugging session is much faster. That would not be true if you did RemoveOrphanedSharedModules in Debugger::Destroy. 99% of all the libraries that get loaded by an app don't change over an Xcode run, and are in fact shared among most of the targets you might debug on a given platform. So clearing the cache after every run would result in a lot of pointless reparsing of system libraries.
This looks good. In the breakpoint case, I added a --dummy option so that you could set breakpoints directly on the dummy target. You would do that if you were going to do something that would make new targets, and want to ensure that the breakpoints get set in all the new targets. That's probably a very little used feature, so while it would be nice to maintain consistency amongst commands that add to the dummy target, that's definitely extra-credit.
Tue, Jul 14
The change looks pretty straightforward, and having the recognizers be per target seems like a better model to me. The one difficulty with adding a target to the recognizers is that you can't add a recognizer in your .lldbinit file, since you don't have a target yet. If anybody has gotten around to doing that yet (the recognizers aren't brand new at this point) will be broken by this change.
Mon, Jul 13
Couple of minor comments.
Fri, Jul 10
Thu, Jul 9
IIRC from when Shafik and I were looking at this the ObjC runtime data lists the same offset for all the members of the bitfield: the offset of the first member of the bitfield. Apparently the ObjC runtime doesn't need to know where the individual members are dynamically - the offsets are baked into the code or something? So to find where the member actually is you have to take the offset into the field - which is given in the DWARF, and add it to the dynamic offset you get from the runtime for the first member.
I want to be careful not to conflate lldb's general event behavior with the way the command interpreter happens to run things.
I think the proper way to gate this sort of "race" is to use the process state. If the current process state is "stopped" then that means we're done with the stop processing and ready to allow commands that require the process to be stopped.
I am not selling this as the correct long-term structure for ValueObjects. So far as I can see, in all cases it should be possible to tell a ValueObject where it's going to find both its own store and the storage for pointer children when the ValueObject is created. It's not like that's a feature we discover as we go through the process of creating the ValueObject. That isn't how it is currently done. Now we almost always start off an incorrect setting, and often goes through several rounds of resetting before it settles on the final value. That's what I meant by saying it is "frustratingly self-healing".
Wed, Jul 8
Tue, Jul 7
Other than a quibble about using _sp suffix, this is fine.
This is fine. If there were a universal convention (e.g. : separated lists) for environment variables, it would be reasonable for "append" behavior to extend rather than replace extant variables, but in the absence of the universality of such a convention, the best we can do is overwrite.
Jun 16 2020
Jun 15 2020
Just to be clear, the lldb -> gdb command map doesn't prescribe behavior for lldb commands. It just suggests the analogous command in gdb. We are still free to implement lldb behavior however seems best to us.
Jun 11 2020
I need to read this in more detail at this point but I'm caught up in something else.
Jun 10 2020
This looks okay to me though I'm not very familiar with the llvm file system interfaces.
Thanks, this is looking good. I have a bunch of nits, but nothing substantial.
Jun 8 2020
I thought I was just repeating your original description of the problem in a scenario orchestrated by breakpoint actions. I must have missed something in your description if it's true that the scenario I described doesn't match your initial problem. But regardless, if there was a problem you should be able to actually drive lldb, either through the command-line or the SB API to the point of failure. And if so, you should be able to write a test that does the same thing.
Humm... So you'll have to test the continue behavior instead, which after all was your original issue. That shouldn't be too hard, however. Just make a breakpoint action that calls "thread suspend" on its thread and returns false for should_stop the first time it is called, and just returns true every time thereafter. Then the program should stop at the second hit of the breakpoint rather than continuing to the exit.
Jun 4 2020
Adding a ShouldStop to that if test doesn't seem right. You should still run the stop actions even if the thread doesn't want to stop.
It bugs me a little bit that we are doing work masking the stop info when in fact the ShouldStop mechanism is the one that shouldn't be consulting threads that were suspended by the user during the last run.
Sorry, now that I'm thinking about this more, I'm confused as to why you are seeing the symptoms you describe. Thread::ShouldStop starts with:
Jun 3 2020
The one scenario I can think of where this might do the wrong thing is:
May 29 2020
Looks even better.
I think "This resolves the SBAddress" is better than "This resolves SBAddress". Other than that LGTM.
Ooh, test would be good. Just use it in a python command.
May 28 2020
I don't think that test is right. There's no guarantee that your GetExecutable().GetFilename() runs before or after the dlopen in the process has run. If it runs before you aren't testing anything.
May 27 2020
This change makes the eArgTypePlugin be the completion for process plugins. Shouldn't we change the enum name and completion name to indicate that?
May 26 2020
Note that there is no guarantee that ObjC names won't be mangled. They aren't on macOS because the macOS linker grew up along with objC so all the weird characters it introduces into the symbol namespace ([,],-,+, ,etc...) are allowed. Way back in the day when WebObjects ran on Windows ObjC names weren't legal, and were mangled. Just saying...