labath marked an inline comment as done.
labath added a comment.
In D66744#1646514 <https://reviews.llvm.org/D66744#1646514>, @jankratochvil
wrote:
> > {Read,Write}[GF]PR now no longer check whether the supplied buffers are
> > null, because they never are
>
> But then there is `NativeRegisterContextLinux_x86_64::GetFPRBuffer()`. Which
> is never used so maybe together with
> `NativeRegisterContextLinux_x86_64::GetFPRSize()` they could be just
> `assert(0);`. It can be also considered as a different cleanup patch.
Hm.. I didn't notice that. I'll put that in separately. Ideally, I'd say these
functions should be used, but that may require more cleanups in the x86
register context, such as detecting the register type earlier on instead of
just when the operations fail. Overall, there's a lot of room for improvement
in this area...
> I did not test these patches but at least I am finally setting up now a
> (silent) buildbot for `s390x`+`ppc64le` (I haven't found a suitable ARM offer
> yet).
That would be awesome. Btw, we already have some arm (32&64) lldb bots set up
by linaro, though they don't seem to be in a particularly good shape right
now...
================
Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp:918
- return ReadRegisterSet(&ioVec, buf_size, NT_PRSTATUS);
+ return ReadRegisterSet(&ioVec, GetGPRSize(), NT_PRSTATUS);
#endif // __arm__
----------------
jankratochvil wrote:
> This keeps the existing bug in place as the size should be rather
> `sizeof(ioVec)`. But then `sizeof(ioVec)` is also not much useful as it is
> used only for `PtraceDisplayBytes`. It is probably considered as a different
> cleanup patch so this patch does not touch it.
>
Yeah, I noticed that. I am keeping that for some other patch. Ideally, I'd like
to refactor this even more so that one does not have to construct the iovec
manually, and fix PtraceDisplayBytes to show the contents of the iovec, but I
don't have a clear idea on the best way to do that.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66744/new/
https://reviews.llvm.org/D66744
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
