Re: [lldb-dev] LLDB problems on remote debugging

2020-04-30 Thread Greg Clayton via lldb-dev


> On Apr 29, 2020, at 9:28 PM, Rui Hong  wrote:
> 
> Hi LLDB devs,
> 
> First I would like to express my appreciation and thanks to you all 
> especially Greg Clayton and Ted Woodward! Your advice and guidance are quite 
> useful for me!
> 
> I've been working on other lldb problems and resume solving the "remote 
> loading" issue now. I now fully understand the "remote 
> connection/execution"(what happens after gdb-remote and how 'c' or 's' 
> controls the execution) and the only problem is how to load the program. My 
> simulator starts this way :./sim --port 1234It do not do the "loading 
> program" job when specified with --port right now. In order to fit the old 
> GDB style, I prefer to still let lldb to do the "loading binary" job.
> 
> To provide more information about my memory architecture : logically, the 
> memory is a whole(one address space), but divided into PM(program memory, 
> starting at address 0x0) and DM(data memory, starting at address 0x8000). 
> When loading program, the .text section should be loaded into PM and the 
> .data section should be loaded into DM, and nothing more. And yes, only one 
> executable.
> 
> I've tried "target modules load --load"(I'm using lldb 7.0.1 and it 
> implemented this command),

That LLDB is really old. I would highly recommend building top of tree LLDB for 
any real work.

> but the sections to load are not loadable(not PT_LOAD) and triggers no RSP 
> package exchange.

Are you not building your ELF file correctly? Why are there no PT_LOAD program 
headers? I would suggest fixing this?

> So I tried "memory write --infile", it triggers a "qMemoryRegionInfo:0" 
> packet to query memory region info and a "M" packet to write memory, but 
> these two packet are not supported by my simulator right now. My simulator 
> supports "X" packet not "M"! When using with the old GDB, "load" command in 
> GDB triggers a few "X" packets

So did you extract the section contents yourself into separate files so you can 
load the memory using "memory write --infile"? qMemoryRegionInfo is not 
required, so your stub can respond with "$#00" which means unsupported. This is 
there for people that have flash memory as someone implemented the ability to 
write to flash with some fancy packets (see 
ProcessGDBRemote::DoWriteMemory(...) for details).
> 
> So I want to know:
> 1. How to let lldb send "X" packet(perhaps a command) and where is the 
> corresponding code located?(to let my simulator support "M" packet is also 
> OK, but using the existing code that handles "X" packet is much easier)

We support the x packet, but right now it is hooked up only for memory reads 
since not a lot of clients do large memory writes. In order for the "x" packet 
for memory reads to be used, we try and detect if it is supported for "x" 
packet (binary memory read) by sending a "x0,0" packet. if "OK" is returned we 
say it is supported. So you will need to modify:

size_t ProcessGDBRemote::DoWriteMemory(addr_t addr, const void *buf, size_t 
size, Status &error);

in ProcessGDBRemote.cpp to try the "X" packet once, and if "$#00" is returned, 
set a bool in the ProcessGDBRemote class to know to not try and use the "X" 
packet again. You will see a mixture of some packets being directly sent by 
ProcessGDBRemote, and some are put into the GDBRemoteCommunicationClient class 
and an accessor is called to try and send the packet. I would suggest making a 
function:

size_t GDBRemoteCommunicationClient::WriteMemory(addr_t addr, const void *buf, 
size_t size, Status &error);

And have the GDBRemoteCommunicationClient keep track of wether the X packet is 
supported and always use it if it is. 

So the flow is:
1 - add a new instance variable to GDBRemoteCommunicationClient:
 LazyBool m_supports_X = eLazyBoolCalculate;

LazyBool is an enum:

enum LazyBool { eLazyBoolCalculate = -1, eLazyBoolNo = 0, eLazyBoolYes = 1 };

2 - Add a new size_t GDBRemoteCommunicationClient::WriteMemory(addr_t addr, 
const void *buf, size_t size, Status &error) function:

size_t GDBRemoteCommunicationClient::WriteMemory(addr_t addr, const void *buf, 
size_t size, Status &error) {
  if (m_supports_X != eLazyBoolNo) {
// Make packet and try sending the X packet
StreamString packet;
StringExtractorGDBRemote response;
packet.PutChar('X');
... // Fill in all of the args and the binary bytes
if (SendPacketAndWaitForResponse(packet.GetString(), response, false) == 
PacketResult::Success) {
  if (response.IsUnsupportedResponse())
m_supports_X = eLazyBoolNo;
  else if (response.IsOKResponse())
return size;
  else
return 0;
}
  }
  // Make and send the 'M' packet just like in 
ProcessGDBRemote::DoWriteMemory(...)

3 - Modify ProcessGDBRemote::DoWriteMemory() to call the new 
GDBRemoteCommunicationClient::WriteMemory() function.


> 2. What's the actual difference between "X" packet and "M" packet?(I can't 
> see any difference between them, from th

[lldb-dev] Question regarding argument types of "BreakpointHitCallback"

2020-04-30 Thread Vangelis Tsiatsianas via lldb-dev
Hello,

I would like to ask a question regarding "BreakpointHitCallback", which is 
declared as such:

bool (*BreakpointHitCallback)(void *baton,
  StoppointCallbackContext *context,
  lldb::user_id_t break_id,
  lldb::user_id_t break_loc_id);

Is there any particular reason that "break_id" and "break_loc_id" are of type 
"user_id_t" (64-bit unsigned) instead of "break_id_t" (32-bit signed), which is 
used both for "Stoppoint::m_bid" and "StoppointLocation::m_loc_id"?

This causes an issue mainly with internal breakpoints, since the callback of an 
internal breakpoint with (ID == 0xfffe) is called with (break_id == 
0xfffe), forcing the callback to cast the argument back to a 32-bit 
signed in order to use it correctly, e.g. when the IDs are stored and need to 
be looked up.

A small example attempting to illustrate the problem: 
https://godbolt.org/z/y8LbK2 


― Vangelis


___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] [cfe-dev] [llvm-dev] RFC: Switching from Bugzilla to Github Issues [UPDATED]

2020-04-30 Thread Tom Stellard via lldb-dev
On 04/29/2020 01:19 PM, David Blaikie wrote:
> Generally sounds pretty good to me - only variation on the theme (& certainly 
> imho dealer's choice at this point - if you/whoever ends up doing this 
> doesn't like the sound of it, they shouldn't feel they have to do it this 
> way) - maybe creating blank issues up to the current bugzilla PR number (& 
> maybe some padding) in a single/quick-ish (no idea how quickly those can be 
> created) window might help reduce the need for race conditions/shutting down 
> bug reporting, etc
> 

I think this is a really good suggestion.  It would take a lot of
pressure off the migration process.  The only potential downside is that
copying into blank issues might be slower than just moving the issues.
I have not tested this, so I don't know yet, but even if copying is slower
it doesn't really matter that much if there is nothing (e.g. re-enabling
repo access) that is waiting on the process to complete.

-Tom

> On Wed, Apr 29, 2020 at 8:25 AM Tom Stellard via cfe-dev 
> mailto:cfe-...@lists.llvm.org>> wrote:
> 
> Hi,
> 
> Thanks to everyone who provided feedback.  I would like to propose a
> more detailed plan based on the everyone's comments.  It seems like there 
> was a strong
> preference to maintain the bug ID numbers, so this proposal tries to 
> address that.
> 
> TLDR; This proposes to maintain bug ID numbers by overwriting existing 
> GitHub issues
> instead of creating new ones.  e.g. github.com/llvm/llvm-project/issues/1 
>  will
> be overwritten with data from llvm.org/PR1 .  There 
> will be some bugs that
> end up having their data copied into pull requests, which may be strange,
> but the data will be preserved and the IDs will be preserved and this 
> would
> only happen to very old bugs.
> 
> Proposal:
> 
> Detailed steps for doing the migration:
> 
> 
> * Weeks or days before the migration:
> 
> 1. Create a new GitHub repository called llvm-bug-archive and import bug
> data from bugzilla.
> 
> This step should not be under any kind of time pressure, so that the 
> conversion
> process can be debugged and refined.
> 
> 2. Install label notification system using GitHub actions and enable web 
> hook
> to send emails to llvm-bugs list.
> 
> * Day before the migration:
> 
> 3. Make bugzilla readonly.
> 
> 4. Import any new bugs created since the last import.
> 
> There may be commit access disruption during the migration, so
> completing these steps the day before will limit the amount of down time.
> 
> 5. Temporarily re-enable issues in the llvm-project repo and copy 
> existing issues
> to the llvm-bug-archive repo so they get higher IDs numbers than any
> existing PR.  Disable issues when done.
> 
> Note that we will copy issues instead of moving them, which means the 
> original
> issue will remain in tact.  This will allow us to retain the bug IDs
> for future use and not 'lose' a bug ID.
> 
> * Day of migration:
> 
> 6. Lockdown the repo as much as possible to prevent people from creating
> issues or pull requests.
> 
> Temporarily making the repo private may be one way to achieve this.  Other
> suggestions welcome.
> 
> 7. Copy issues with overlapping issues IDs from the llvm-bug-archive repo
> into the llvm-project repo.
> 
> Issues from the llvm-bug-archive repo that have the same ID number as
> existing issues in the llvm-project repo will be manually copied from
> the former to the later.  This will allow us to preserve the PR numbers
> from bugzilla.  Here is an example for how this would work:
> 
> - Delete comments and description from llvm-project issue #1.
> - Copy comments and description from llvm-bug-archive issue #1 into
>   llvm-project issue #1.
> 
> Since GitHub issue and pull requests share the same numbering sequence, 
> any
> PR# from bugzilla that maps to a pull request in the llvm-project repo 
> will
> need to have it's comments copied into a pull request.  These issues will 
> look slightly
> strange since there will be random commits attached to the issue.  
> However,
> all data will be preserved and more importantly the bug ID will be 
> preserved.
> 
> The issues URL can be used to access pull requests e.g.
> pull request #84 is accessible via github.com/llvm/llvm-project/issues/84 
> 
> so even with bugzilla data stored in pull requests, we will still be able 
> to do a simple redirect
> from llvm.org/PR###  to 
> github.com/llvm/llvm-project/issues/### 
> 
> 
> 
> 8. Once all the overlapping Issue IDs have been copied.  Move the rest of 
> the issues
> from the llvm-bug-archive repo to the llvm-project repo.
> 
> 

Re: [lldb-dev] LLDB problems on remote debugging

2020-04-30 Thread Ted Woodward via lldb-dev

On Apr 29, 2020, at 9:28 PM, Rui Hong 
mailto:hongru...@mails.ucas.ac.cn>> wrote:

First I would like to express my appreciation and thanks to you all especially 
Greg Clayton and Ted Woodward! Your advice and guidance are quite useful for me!

You’re welcome!

*4. by the way, does GDB has similar command like "log enable gdb-remote 
packets" in lldb to print all the RSP packet exchange?

“set debug remote 1”
Unlike with lldb, this doesn’t work when you do a run. It only works when you 
do a target remote (at least with the default gdb installed on Ubuntu 16.04).
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Question regarding argument types of "BreakpointHitCallback"

2020-04-30 Thread Greg Clayton via lldb-dev


> On Apr 30, 2020, at 8:50 AM, Vangelis Tsiatsianas via lldb-dev 
>  wrote:
> 
> Hello,
> 
> I would like to ask a question regarding "BreakpointHitCallback", which is 
> declared as such:
> 
> bool (*BreakpointHitCallback)(void *baton,
>   StoppointCallbackContext *context,
>   lldb::user_id_t break_id,
>   lldb::user_id_t break_loc_id);
> 
> Is there any particular reason that "break_id" and "break_loc_id" are of type 
> "user_id_t" (64-bit unsigned) instead of "break_id_t" (32-bit signed), which 
> is used both for "Stoppoint::m_bid" and "StoppointLocation::m_loc_id"?

I believe this callback predated the time when we added break_id and 
break_loc_id, and since arguments are part of the signature of C++ functions, 
we didn't change it in order to keep the public API from changing. Or this 
could have just been a mistake. Either way, we have a stable API and can't 
really change it.
> 
> This causes an issue mainly with internal breakpoints, since the callback of 
> an internal breakpoint with (ID == 0xfffe) is called with (break_id == 
> 0xfffe), forcing the callback to cast the argument back to a 
> 32-bit signed in order to use it correctly, e.g. when the IDs are stored and 
> need to be looked up.
> 
> A small example attempting to illustrate the problem: 
> https://godbolt.org/z/y8LbK2 
Sorry for the issue, but I think we are stuck with it now.


___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Question regarding argument types of "BreakpointHitCallback"

2020-04-30 Thread Vangelis Tsiatsianas via lldb-dev
Thank you for the answer, Greg.

I personally managed to work around the problem, although it confused me a bit 
at first and took a while to figure out the cause. May I suggest the addition 
of a note in the documentation of "{Breakpoint, Watchpoint}::{Invoke, 
Set}Callback()" and possibly other relevant functions as a warning to future 
developers that may stumble upon the same issue?

Regarding the public C++ API, would defining "break_id_t" as "int64_t" be a 
viable solution or that change would also break the API? It seems that making 
both types 64-bit alleviates the issue, despite the sign difference.


― Vangelis


> On 30 Apr 2020, at 22:22, Greg Clayton  wrote:
> 
> 
> 
>> On Apr 30, 2020, at 8:50 AM, Vangelis Tsiatsianas via lldb-dev 
>> mailto:lldb-dev@lists.llvm.org>> wrote:
>> 
>> Hello,
>> 
>> I would like to ask a question regarding "BreakpointHitCallback", which is 
>> declared as such:
>> 
>> bool (*BreakpointHitCallback)(void *baton,
>>   StoppointCallbackContext *context,
>>   lldb::user_id_t break_id,
>>   lldb::user_id_t break_loc_id);
>> 
>> Is there any particular reason that "break_id" and "break_loc_id" are of 
>> type "user_id_t" (64-bit unsigned) instead of "break_id_t" (32-bit signed), 
>> which is used both for "Stoppoint::m_bid" and "StoppointLocation::m_loc_id"?
> 
> I believe this callback predated the time when we added break_id and 
> break_loc_id, and since arguments are part of the signature of C++ functions, 
> we didn't change it in order to keep the public API from changing. Or this 
> could have just been a mistake. Either way, we have a stable API and can't 
> really change it.
>> 
>> This causes an issue mainly with internal breakpoints, since the callback of 
>> an internal breakpoint with (ID == 0xfffe) is called with (break_id == 
>> 0xfffe), forcing the callback to cast the argument back to a 
>> 32-bit signed in order to use it correctly, e.g. when the IDs are stored and 
>> need to be looked up.
>> 
>> A small example attempting to illustrate the problem: 
>> https://godbolt.org/z/y8LbK2 
> Sorry for the issue, but I think we are stuck with it now.
> 
> 

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Question regarding argument types of "BreakpointHitCallback"

2020-04-30 Thread Jim Ingham via lldb-dev


> On Apr 30, 2020, at 12:43 PM, Vangelis Tsiatsianas via lldb-dev 
>  wrote:
> 
> Thank you for the answer, Greg.
> 
> I personally managed to work around the problem, although it confused me a 
> bit at first and took a while to figure out the cause. May I suggest the 
> addition of a note in the documentation of "{Breakpoint, 
> Watchpoint}::{Invoke, Set}Callback()" and possibly other relevant functions 
> as a warning to future developers that may stumble upon the same issue?
> 
> Regarding the public C++ API, would defining "break_id_t" as "int64_t" be a 
> viable solution or that change would also break the API? It seems that making 
> both types 64-bit alleviates the issue, despite the sign difference.

Mangled names don’t encode typedef names, but the bare type.  For instance:

 > cat signatures.cpp 
#include 
#include 

typedef int64_t break_id_t;

struct Foo {
  void GiveMeABreak(break_id_t id) { printf("Got: %d\n", id); }
};

int
main()
{
  Foo myFoo;
  myFoo.GiveMeABreak(100);
  return 0;
}
 > clang++ -g -O0 signatures.cpp
 > nm a.out | grep GiveMeABreak
00010f50 T __ZN3Foo12GiveMeABreakEx
 > c++filt __ZN3Foo12GiveMeABreakEx
Foo::GiveMeABreak(long long)

So this change would change the mangled names of any methods taking a 
break_id_t, and mean old clients would get missing symbol errors when running 
with the new API’s.  So this isn’t something we can do.

Jim


> 
> 
> ― Vangelis
> 
> 
>> On 30 Apr 2020, at 22:22, Greg Clayton > > wrote:
>> 
>> 
>> 
>>> On Apr 30, 2020, at 8:50 AM, Vangelis Tsiatsianas via lldb-dev 
>>> mailto:lldb-dev@lists.llvm.org>> wrote:
>>> 
>>> Hello,
>>> 
>>> I would like to ask a question regarding "BreakpointHitCallback", which is 
>>> declared as such:
>>> 
>>> bool (*BreakpointHitCallback)(void *baton,
>>>   StoppointCallbackContext *context,
>>>   lldb::user_id_t break_id,
>>>   lldb::user_id_t break_loc_id);
>>> 
>>> Is there any particular reason that "break_id" and "break_loc_id" are of 
>>> type "user_id_t" (64-bit unsigned) instead of "break_id_t" (32-bit signed), 
>>> which is used both for "Stoppoint::m_bid" and "StoppointLocation::m_loc_id"?
>> 
>> I believe this callback predated the time when we added break_id and 
>> break_loc_id, and since arguments are part of the signature of C++ 
>> functions, we didn't change it in order to keep the public API from 
>> changing. Or this could have just been a mistake. Either way, we have a 
>> stable API and can't really change it.
>>> 
>>> This causes an issue mainly with internal breakpoints, since the callback 
>>> of an internal breakpoint with (ID == 0xfffe) is called with (break_id 
>>> == 0xfffe), forcing the callback to cast the argument back to a 
>>> 32-bit signed in order to use it correctly, e.g. when the IDs are stored 
>>> and need to be looked up.
>>> 
>>> A small example attempting to illustrate the problem: 
>>> https://godbolt.org/z/y8LbK2 
>> Sorry for the issue, but I think we are stuck with it now.
>> 
>> 
> 
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Question regarding argument types of "BreakpointHitCallback"

2020-04-30 Thread Vangelis Tsiatsianas via lldb-dev
I understand. Thank you very much for the thorough explanation, Jim! 🙂


― Vangelis


> On 30 Apr 2020, at 23:38, Jim Ingham  wrote:
> 
> 
> 
>> On Apr 30, 2020, at 12:43 PM, Vangelis Tsiatsianas via lldb-dev 
>> mailto:lldb-dev@lists.llvm.org>> wrote:
>> 
>> Thank you for the answer, Greg.
>> 
>> I personally managed to work around the problem, although it confused me a 
>> bit at first and took a while to figure out the cause. May I suggest the 
>> addition of a note in the documentation of "{Breakpoint, 
>> Watchpoint}::{Invoke, Set}Callback()" and possibly other relevant functions 
>> as a warning to future developers that may stumble upon the same issue?
>> 
>> Regarding the public C++ API, would defining "break_id_t" as "int64_t" be a 
>> viable solution or that change would also break the API? It seems that 
>> making both types 64-bit alleviates the issue, despite the sign difference.
> 
> Mangled names don’t encode typedef names, but the bare type.  For instance:
> 
>  > cat signatures.cpp 
> #include 
> #include 
> 
> typedef int64_t break_id_t;
> 
> struct Foo {
>   void GiveMeABreak(break_id_t id) { printf("Got: %d\n", id); }
> };
> 
> int
> main()
> {
>   Foo myFoo;
>   myFoo.GiveMeABreak(100);
>   return 0;
> }
>  > clang++ -g -O0 signatures.cpp
>  > nm a.out | grep GiveMeABreak
> 00010f50 T __ZN3Foo12GiveMeABreakEx
>  > c++filt __ZN3Foo12GiveMeABreakEx
> Foo::GiveMeABreak(long long)
> 
> So this change would change the mangled names of any methods taking a 
> break_id_t, and mean old clients would get missing symbol errors when running 
> with the new API’s.  So this isn’t something we can do.
> 
> Jim
> 
> 
>> 
>> 
>> ― Vangelis
>> 
>> 
>>> On 30 Apr 2020, at 22:22, Greg Clayton >> > wrote:
>>> 
>>> 
>>> 
 On Apr 30, 2020, at 8:50 AM, Vangelis Tsiatsianas via lldb-dev 
 mailto:lldb-dev@lists.llvm.org>> wrote:
 
 Hello,
 
 I would like to ask a question regarding "BreakpointHitCallback", which is 
 declared as such:
 
 bool (*BreakpointHitCallback)(void *baton,
   StoppointCallbackContext *context,
   lldb::user_id_t break_id,
   lldb::user_id_t break_loc_id);
 
 Is there any particular reason that "break_id" and "break_loc_id" are of 
 type "user_id_t" (64-bit unsigned) instead of "break_id_t" (32-bit 
 signed), which is used both for "Stoppoint::m_bid" and 
 "StoppointLocation::m_loc_id"?
>>> 
>>> I believe this callback predated the time when we added break_id and 
>>> break_loc_id, and since arguments are part of the signature of C++ 
>>> functions, we didn't change it in order to keep the public API from 
>>> changing. Or this could have just been a mistake. Either way, we have a 
>>> stable API and can't really change it.
 
 This causes an issue mainly with internal breakpoints, since the callback 
 of an internal breakpoint with (ID == 0xfffe) is called with (break_id 
 == 0xfffe), forcing the callback to cast the argument back to 
 a 32-bit signed in order to use it correctly, e.g. when the IDs are stored 
 and need to be looked up.
 
 A small example attempting to illustrate the problem: 
 https://godbolt.org/z/y8LbK2 
>>> Sorry for the issue, but I think we are stuck with it now.
>>> 
>>> 
>> 
>> ___
>> lldb-dev mailing list
>> lldb-dev@lists.llvm.org 
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev 
>> 
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev