- User Since
- Sep 2 2014, 12:05 PM (317 w, 21 h)
Mon, Sep 28
Thanks for the suggestions Greg, solid ideas. Let me rework this a bit.
Sun, Sep 27
Fri, Sep 25
Thanks for the feedback!
Wed, Sep 2
Ah sorry lost track of this one; that looks correct, thanks for the fix.
Aug 29 2020
Hahaha I remember looking at this constructor and expecting to find an uninitialized ivar, but it was initialized correctly and I went to look for another place where the bug might be. I think I see why there's some confusion -- this change landed Thrusday:
Ah, so we've still got a bug in x86AssemblyInspectionEngine::AugmentUnwindPlanFromCallSite somehow for abort(). This method takes an UnwindPlan from eh_frame instructions (which says it is not a trap handler - correct), and adds some epilogue unwind rows at the end if needed and sets a couple of flags. I'm not sure how the trap handler lazybool is getting set to True in the process of this conversion. :/ I'll read through this method more closely Monday and try to spot how that is happening.
Aug 28 2020
Hi, could you try
Hi, thanks so much for showing all the unwind plans, sorry for dropping off the thread for a day.
Aug 25 2020
Minor followup on the 'image show-unwind' output -- I just landed a patch to print when a function or unwindplan are marked as being a trap handler.
separate from the fact that UnwindPlan::Dump does not print m_plan_is_for_signal_trap for an UnwindPlan, what we need to see from a real trap handler -- in lldb's terminology -- is that we have restore rules for all of the registers.
If I understand correctly, in
Hi, I'll review this problem / suggested patch in a bit but I think it might be helpful to outline what the intention of all this is. Let's say we have a stack of
Aug 24 2020
LGTM, that call to dispatch_get_global_queue QOS_CLASS_UNSPECIFIED does not look valid.
Aug 19 2020
This looks good to me, getting any logic out of the Makefile templates that we can, that's a good improvement for maintainability.
Aug 15 2020
Aug 14 2020
Aug 10 2020
Nice set of changes, and you're right it should be a lower-case "q" packet, not an upper-case "Q" packet like I suggested, it's just querying information from the remote side.
This is mostly fine. You need to escape the completed filenames you send back. You should have a test case where multiple matches are returned, like "a" matches "abc1" and "abc2".
Aug 6 2020
LGTM, it's hard to keep all the supported behaviors in my head but I think this is right.
Aug 5 2020
Update to std::unique_ptr -- thanks for the reminder!
Made the change in Xcode and the indentation was off.
Jul 30 2020
Jul 21 2020
With this patch, debugging a translated app will only work when we invoke debugserver with --fd for its communication back to lldb. e.g. running 'debugserver localhost:2000 --attach=translated-app' will pass -fd=-1 as a command line argument to /Library/Apple/usr/libexec/oah/debugserver. Doing this properly would involve passing the same hostspec:portnum that debugserver received in this case -- but could we at least error out in DNBProcessAttach if communcation_fd==-1 so it's clear that this is not yet a supported workflow?
Jul 15 2020
The rewrite of the ObjectFileMachO parts is very nice. LGTM.
Jul 14 2020
Jun 29 2020
Sorry for the delay, this looks good to me.
Jun 26 2020
Sorry yes this patch is fine. To be honest, on x86_64 at least, as you've pointed out, maybe we should just live on eh_frame completely without going through any of these augmentation checks at all. AFAIK gdb is living off of eh_frame exclusively, it may be that all the compilers have caught on to that and are emitting asynchronous unwind information in eh_frame.
Jun 25 2020
Hi Pavel, sorry I've been doing a bunch of random things today and haven't really had a chance to look at this yet. eh_frame is so problematic for lldb, we never know what we're getting. I keep thinking we should take over a few augmentation string characters so that the producer can definitively indicate one of:
Jun 24 2020
LGTM. With p_flag, we only need to evaluate (processInfo.kp_proc.p_flag & P_TRANSLATED) as a boolean, but that's a style pref as much as anything.
Jun 15 2020
Jun 4 2020
Hi Jan, I noticed our sanitizer bot started getting failures in
Jun 1 2020
Nice cleanup of a long standing undefined behavior warning.
May 28 2020
This looks good to me, thanks Raphael.
May 21 2020
I skimmed it, I guess the one question that comes to mind is how this will behave if we an operating system thread provider plugin active, where it may not actually list all of the threads that exist -- where we try to be tolerant of threads disappearing and then re-appearing later -- and the thread running an inferior function call got scheduled out while the function call was still happening. Pretty unlikely scenario, but I thought I'd throw it out there.
May 11 2020
Thanks for the feedback.
May 9 2020
May 8 2020
May 7 2020
May 5 2020
Apr 16 2020
Apr 14 2020
Update to address mistake Greg identified; also remove two unused variables that were in this method before my changes.
Apr 13 2020
Apr 6 2020
Apr 2 2020
Mar 31 2020
I was a little worried about the case where we have an 8-byte region being watched that is 4-byte aligned (so it's spanning two 8-byte watchpoints), but a quick test of that with this patch shows it works correctly. This is fine with me.
Mar 27 2020
Updated the patch to incorporate Jonas and Greg's suggestions.
Yea, this looks good, I don't think any of those other search methods were valid any longer.
Mar 26 2020
Mar 25 2020
Thanks Adrian, great comments, incorporated most of them. The use of the block to search the trie entries when eliminating duplication between nlist/tries was a good idea, much cleaner now.
Update patch with Adrian's suggestions.
Mar 24 2020
Mar 21 2020
(and if you're still seeing mystery reads, put a breakpoint on ProcessGDBRemote::DoReadMemory and see who is doing it)
Mar 19 2020
nice catch, lgtm.
Nice patch, thanks for doing this. I wouldn't have added the fallback code in DNBArchMachARM64::NumSupportedHardwareBreakpoints to handle the case where the sysctl fails - it doesn't fail - but there's no harm in it.
Mar 18 2020
Mar 17 2020
Nice fix, this looks good to me but I haven't been following this area of code too closely before now.