- User Since
- Sep 2 2014, 12:05 PM (263 w, 18 h)
Thu, Sep 12
Hi Aleksandr, nice job getting this working. I read over the patch and had a couple of initial questions.
Sorry for taking so long to look at this one -- it looks like a good solution to this.
Tue, Sep 10
Mon, Sep 9
LGTM, nice job with the test case, I see myself copying that in the future. :)
Fri, Sep 6
Thu, Sep 5
We don't need to keep this command around. It was originated by Tamas in 2015,
but never got used for anything - Tamas was working on unwind things back then iirc, and may it looks like he thought it would be useful for that. On Darwin systems we do ship with a python command for diagnosing unwind problems ("script import lldb.diagnose"; "diagnose-unwind") which can be used when the current backtrace looks like it may be missing stack frames, or truncate early. The unwinder hasn't seen a lot of active development for years, so we don't have cause to use this very often.
Tue, Sep 3
Thu, Aug 29
LGTM. In debugserver we have the definition for the V registers invaliding the D and S registers it contains. If the user modifies v10, we want any cached s10 and d10 values to be marked as invalid / refresh them. The same thing with the X and W registers. But the definitions you're rewriting already had this as incorrect, you've just replicated it more compactly, so I wouldn't call that a reason to hold off. Let's give Pavel a chance to look at this too.
Tue, Aug 27
Mon, Aug 26
Thu, Aug 22
Wed, Aug 21
Tue, Aug 20
Aug 15 2019
Aug 14 2019
Overall this changeset looks good to me. A few small comments.
Aug 6 2019
Aug 5 2019
Yeah, it's not currently hooked up to anything; I'm returning to remote-platform testing soon, if we've lost some necessary functionality I can re-add it, but this option right now isn't doing anything.
Aug 1 2019
This looks good, this is in line with what we discussed, thanks for taking it on! Sorry for the delay at looking this over, it has been a little crazy this week.
fwiw this behavior is documented here,
I'm sure this is fine, I read the code through and I've realized I haven't had to do anything with bitfields in lldb up to now.
Jul 29 2019
I'm not sure it's really relevant when lit is the driver, but I would often use --no-multiprocess with dotest to see the full trace output of a single test while it was running. In multiprocess mode that wasn't displayed iirc. That's my only use of it. I do use --threads 1 when doing a testsuite run against a device because there's some buggy behavior in the lldb testsuite when running multiple tests simultaneously on a remote device (at least a remote darwin device). But I'm sure you're not talking about removing that control.
Jul 24 2019
LGTM. I don't think the use of the posix_spawn API are important for the test case to exercise. If posix_spawn had a problem on darwin systems, lldb/debugserver's process launching would turn it up pretty quick.
Jul 23 2019
LGTM. There was another problem I was discussing with someone the other week (I can't remember exactly the details) where we'd need to retrieve the Xcode install path so this would have been needed there too.
Jul 22 2019
LGTM. scripts/sort-pbxproj.rb can go too.
Jul 18 2019
Jul 17 2019
Hm, that could be really helpful! I think I'd like to do that as a separate piece of work a little bit later, but I'll look at that.
I know there are a lot of specific notes floating around right now, but two quick suggestions.
Jul 16 2019
Sorry yes, I should have marked this as approved. I don't have a linux machine handy so I haven't looked into the problem Jan reports with the change -- fwiw if you turn on unwind logging ('log enable lldb unwind' before you backtrace at a stop), it will tell you which unwind rule it is using to find the caller's return pc value as it goes up the stack. It's a little tricky to read at first, but once you get the hang of it you can usually spot the problem quickly and understand why lldb is behaving the way it is. But just looking at the source changes you're making, I'm fine with this.
Jul 15 2019
Ah, regarding the S bit, looks good to me. I think I misunderstood the original problem you were working on. On Darwin systems, we have our friend sigtramp. When an async interrupt is received, the kernel saves the register context to stack and then calls sigtramp which does some instructions and then calls the trap handler function. Backing up the pc by 1 for __sigtramp is fine, because it did a normal CALL instruction to the trap handler.
Jul 8 2019
Thanks Jonas, yes we need to come up with a perf testing infrastructure in lldb but there isn't a lot of value in this original attempt from c. 2013.
Jul 2 2019
Jun 26 2019
Jun 25 2019
Jun 24 2019
Jun 21 2019
This looks good. Over in RegisterContextLLDB::GetFullUnwindPlanForFrame there's a behaves_like_zeroth_frame bool with similar setup -- in that case, it's trying to decide what type of UnwindPlan to pick for the frame, either one that is synchronous ("valid only at call sites / throwable locations") or asynchronous ("valid at every instruction"). For a behaves_like_zeroth_frame it needs to pick an asynchronous unwind plan. You may want to look at that too.
Jun 13 2019
Jun 12 2019
Jun 11 2019
Jun 4 2019
Jun 3 2019
May 31 2019
May 22 2019
Does your feature print *all* the registers including floating point/vector registers? Or just the main general purpose registers? In debugserver, we expedite the register values for the stopping thread:
May 21 2019
May 16 2019
My one concern is how this will work in a swift-lldb where we have a swift ValueObject, the print description fails, and we fall back to the ObjC print description. That's a change in behavior, right? I imagine worst case is that the objc print description will fail to produce anything and we'll have the same behavior as we do today.
May 10 2019
May 9 2019
Looks good to me.
May 2 2019
I had some more revisions on this patch, but I finally understood that I was solving the wrong problem with this patchset. lldb should not fetch class names up front when scanning the objective c class tables, they should only be read lazily as we evaluate an expression. The fact that we're reading names during the table scan is the problem that needs to be fixed.
Apr 26 2019
Apr 25 2019
Fix one small bug in the jitted expressions. Increase the estimated size of class names that lldb uses to pre-allocate the string pool buffer. Added a method to read the string pool out of the inferior, to reduce code duplication, and have it read the last name to get the full size of the string pool correct.
Apr 24 2019
Apr 22 2019
Sorry for the delay in looking at this. It looks like a good change, an improvement on the original for sure. I've never seen multiple eh_frame FDEs for a single function, so that's fine. The .ARM.exidx format and Apple's compact unwind format can theoretically have multiple different unwind plans for a single function - but I've never seen it done in practice. More commonly, a single unwind plan will be used for multiple functions that have the same unwind state. (these two formats don't describe prologue/epilogues - they only describe the unwind state in the middle of the function where exceptions can be thrown)
Apr 21 2019
Apr 17 2019
Thanks for digging in to this Pavel. I haven't had time to look over the patch today, but I'd like to; I'll make time for it in the next couple days if that's OK.
Apr 15 2019
Agreed, this change is correct. I double checked this in the http://infocenter.arm.com/help/topic/com.arm.doc.ihi0038b/IHI0038B_ehabi.pdf doc (mod 24 Nov 2015) and the only difference in Section 9.3 "Frame unwinding instructions" Table 4 and this code is in the comment - they now refer to this as 'Pop VFP double-precision registers D-D[8+nnn] saved (as if) by VPUSH (see remark d)'. But there's no benefit in updating this one comment while the others are using the older comments (this code was originally written against the 30 Nov 2012 version of the doc).
Apr 8 2019
Apr 5 2019
Added a testcase, TestModuleLoadedNotifys.
Apr 4 2019
Thanks Pavel, I'll convert this to use Expected<> while I'm working on it. I'm writing a test case right now, and looking at how DynamicPluginDarwin works more closely for adding & removing binaries, e.g. there's also no way to batch remove libraries and have a group broadcast notification be sent. There was also a bug in my original patch for calling LoadScriptingResourceForModule() to find a python resource file in a dSYM bundle - it is currently called from Target::ModuleUpdated when a binary is added to the target's ModuleList, but I'm suppressing that call with this change, so I need to move that over to ModulesDidLoad.
Apr 3 2019
Remove const bool notify's. Rename method to Target::GetOrCreateModule. Refine the method headerdoc a bit.
OK looked things over a bit.
Apr 2 2019
Thanks for looking the patch over.
Apr 1 2019
Mar 27 2019
Looks good. Removing that printf is good. Could you also remove the printf("mach_header:...") in the same function?
Mar 26 2019
Sorry for not weighing in earlier, yes this is good.