[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

This revision was automatically updated to reflect the committed changes. Closed by commit rGaf64b31959f6: Add target.xml support for qXfer request. (authored by omjavaid). Changed prior to commit: https://reviews.llvm.org/D74217?vs=245167=245686#toc Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

omjavaid added a comment. In D74217#1882539 , @PatriosTheGreat wrote: > Hi Muhammad, > Thank you for review. > Could you or Pavel commit this to master, since I don't have commit access? Hi Patrios, There was some problem with our buildbot and it

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

PatriosTheGreat added a comment. Hi Muhammad, Thank you for review. Could you or Pavel commit this to master, since I don't have commit access? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74217/new/ https://reviews.llvm.org/D74217 ___

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

omjavaid accepted this revision. omjavaid added a comment. This revision is now accepted and ready to land. I have tested this not sure why it actually failed on the buildbot. The test seems to pass independently. Lets see if the new version fixes the issue. CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

PatriosTheGreat updated this revision to Diff 245167. PatriosTheGreat added a comment. In this revision I fix value_regnums and invalidate_regnums serialization in target.xml. However I'm not so sure that this is the cause of problem with ARM tests. >From tests log it seems like there are some

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

labath added a comment. Please update the current one to keep the context together. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74217/new/ https://reviews.llvm.org/D74217 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

PatriosTheGreat added a comment. Hi Muhammad, Thank you for finding this issue. After I fix that, could I update current review or should I create new one? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74217/new/ https://reviews.llvm.org/D74217

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

labath added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2814 +if (!encoding.empty()) + response.Printf("encoding=\"%s\" ", encoding.str().c_str()); + jarin wrote: > labath wrote: > >

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

jarin added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2814 + +if (reg_info->invalidate_regs && reg_info->invalidate_regs[0]) { + response.PutCString("invalidate_regnums=\""); I know this is

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

jarin added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py:69 +self.assertEqual(q_info_reg["offset"], xml_info_reg.get("offset")) +

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

jarin added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:465 + response.PutChar(','); +response.Printf("%" PRIx32, *reg_num); + } I think this is not correct: target.xml expects this to be

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

omjavaid requested changes to this revision. omjavaid added a comment. This revision now requires changes to proceed. I have reverted this change temporarily. Please look at the test failure. I ll also do the same at my end. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

omjavaid reopened this revision. omjavaid added a comment. This revision is now accepted and ready to land. This breaks LLDB AArch64 Linux buildbot http://lab.llvm.org:8011/builders/lldb-aarch64-ubuntu/builds/1713 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

labath added a comment. Committed as aedc196101e33bd58f7443c5b93398418ce55edf . I've updated the commit message to include a better description of what is being changed. Repository: rG LLVM Github Monorepo CHANGES SINCE

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

This revision was automatically updated to reflect the committed changes. Closed by commit rGaedc196101e3: [lldb/lldb-server] Add target.xml support for qXfer request. (authored by PatriosTheGreat, committed by labath). Changed prior to commit:

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

PatriosTheGreat added a comment. Hi Pavel, Thank you for review. Could you also commit this to master, since I don't have commit access? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74217/new/ https://reviews.llvm.org/D74217 ___

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good. Thanks for doing this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74217/new/ https://reviews.llvm.org/D74217 ___

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

PatriosTheGreat updated this revision to Diff 244152. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74217/new/ https://reviews.llvm.org/D74217 Files: lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/Makefile

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

labath added a comment. Just one more thing I didn't catch in the previous round :(. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:461-481 +static void CollectRegNums(const RegisterInfo *reg_info, +

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

PatriosTheGreat updated this revision to Diff 244130. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74217/new/ https://reviews.llvm.org/D74217 Files: lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/Makefile

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

labath added a comment. Just a couple of comments to make this code more llvm-y. Without those, we have just moved the place which constructs the temporary std::string. I haven't highlighted all the uses, but overall I'd recommend using the `Format` function or something similar instead of

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

PatriosTheGreat updated this revision to Diff 243932. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74217/new/ https://reviews.llvm.org/D74217 Files: lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/Makefile

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

labath added a comment. I think this is looking pretty good overall. I just have a bunch of stylistic remarks... Comment at: lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/main.cpp:1 +#include + I don't think the choice

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

PatriosTheGreat updated this revision to Diff 243579. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74217/new/ https://reviews.llvm.org/D74217 Files: lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/Makefile

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

labath added a comment. Yeah, we should fill in the other attributes too. As that will probably noticeably increase the amount of code, it would be good to put this stuff into a separate function (or two..). For a test, you could create some kind of a lldb-server test

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

jarin added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2819 +response.Printf(""); +const int registersCount = 128; +for (int reg_index = 0; reg_index < registersCount; reg_index++) { As