- User Since
- Jun 4 2018, 3:35 AM (119 w, 6 d)
Jun 25 2020
Jun 23 2020
Jun 22 2020
Hello! Thanks for the patch, generally it looks good to me, the only thing I worry about is scoped enums (please, see the comment below).
Apr 27 2020
Exactly! Removed unneeded wrapper.
Apr 24 2020
The problem here is that in the case of success GetCString() returns nullptr, and we fail on concatenation with None even if the expression was evaluated successfully.
Thanks! Fixed. The only thing is that GetCString() can return nullptr, which leads to None in Python, so I made a simple wrapper for that.
Apr 23 2020
Hello, Pavel! Thanks for the review. Yes, this method looks like a better fit, I just didn't notice it before.
Mar 31 2020
Sorry for the long delay, LGTM now, thanks!
Mar 30 2020
Hello! But does the format of an exception directory for aarch64 differ completely from x86-64 one? Can we somehow adopt the existing parser to support it? If we can, I think that this check looks good (we encapsulate the difference inside PECallFrameInfo). Or may be it is worth to make a different parser for aarch64? Then I think the check would look better in ObjectFilePECOFF::CreateEHCallFrameInfo (and then we will choose a right exception directory parser depending on an architecture). What do you think on this?
Oct 28 2019
Sorry, I'm OOO today, so I can't take a precise look on this now. But I'm not sure, isn't race condition possible here on the m_session_data reset?
LGTM, thank you! Can you send me an example of binary on which unwinding fails with a crash, please? It is a very interesting case...
Oct 15 2019
@stella.stamenova I've fixed it with r374888, thanks again for pointing to that. The fix of unwind can fix some stepping issues, because stepping logic uses call stack information actively.
Sorry, I didn't see Pavel's message. In this case I have no objections to the patch if other people also won't have.
Oct 11 2019
Oct 10 2019
Thanks a lot for the review!
Oct 9 2019
Hi! Could you choose some other names for com1.o and com2.o, please? :) They are reserved names on Windows, so there are problems with checking out this change.
Hello! I am sorry for a delay with reply, I was OOO. Thanks all for the review!
Oct 1 2019
I've made it in the way similar to Zachary have made for the SymbolFileNativePDB plugin. An environment variable could be more convenient e.g. to run a bunch of tests using the lldb-test option.
Sep 30 2019
Thank you! I've added a switch in a manner similar to one that Zachary has chosen for SymbolFilePDB/NativePDB plugins here: D68258
Sep 24 2019
Gentle ping! What do you think of this?
Update the patch due to Pavel's request.
Sep 13 2019
Hi Jason, thanks for the review!
Update due to the requests.
Sep 12 2019
Thanks all for taking a look!
Update diff due to Pavel's requests.
Sep 10 2019
It's a good point, thank you! I had the same thoughts when I done it, but I'm not completely sure. The problem is that an object file can't be completely responsible for choosing an unwind plan, because some plans are produced by symbol files (and an object file knows nothing about that, if I understand right). So we can move only a part of the plan-choosing-functionality from FuncUnwinders to ObjectFiles, but will it be better? In that case both FuncUnwinders and ObjectFiles will be responsible for managing plans, won't it add an additional complexity to the solution?
Sep 9 2019
Sep 5 2019
Determine whether 8-byte watchpoints are supported or not statically.
Sep 4 2019
Check bitness of the target when checking the watchpoint size.
Address the requested changes.
Address the requested changes.
Aug 25 2019
Jul 17 2019
Jul 16 2019
That makes sense, thank you!
Jul 15 2019
LGTM, thanks! Sorry for the long delay with the reply, I had days off.
LGTM! So the other functions (e.g. GetOrCreateDeclContextForUid, GetParentDeclContext etc.) will be also replaced in the future in a corresponding manner?
I do not fully understand how this works... Does the second fgets sets error to EINTR?
Jul 11 2019
Jun 18 2019
May 16 2019
Ah, yes, sure, it will just be moved on the lldb-server side. Thanks again for the clarification!
It sounds strange to me... If we don't have symbols for a function, then we can't even know amount of its arguments, so how can we retrieve them? Also e.g. on Windows x86 both stdcall, ccall, thiscall and fastcall are commonly used, so it would be strange to use some "default" ABI...
@lanza Hello! AFAIU, these values have nothing to do with the Microsoft x64 calling convention. They are used by ABISysV_x86_64, which specifies ABI for working with code injectable into a process being debugged. It requires six arguments to be passed through registers (see GetArgumentValues).
May 13 2019
Apr 29 2019
Apr 23 2019
Apr 22 2019
Apr 21 2019
Thanks for the review! Sorry, I've forgot to include context in the updated patch. Yes, it's exactly the case that is broken, I've left FIXME there until it will be fixed.
Apr 19 2019
Apr 18 2019
Thank you for the comments, I agree with you. I've fixed the patch.
Apr 17 2019
Apr 14 2019
Sorry for the long delay with the response, I had days off. LGTM, thanks!