> On Apr 19, 2018, at 10:54 AM, Greg Clayton <clayb...@gmail.com> wrote:
> 
> 
> 
>> On Apr 19, 2018, at 10:35 AM, Jim Ingham <jing...@apple.com> wrote:
>> 
>> 
>> 
>>> On Apr 19, 2018, at 9:44 AM, Greg Clayton via lldb-dev 
>>> <lldb-dev@lists.llvm.org> wrote:
>>> 
>>> 
>>> 
>>>> On Apr 19, 2018, at 6:51 AM, Zdenek Prikryl via lldb-dev 
>>>> <lldb-dev@lists.llvm.org> wrote:
>>>> 
>>>> Hi lldb developers,
>>>> 
>>>> I've been researching using lldb + gdbserver stub that is based on Harvard 
>>>> architecture with multiple address spaces (one program, multiple data). 
>>>> The commonly adopted approach is that everything is mapped to a single 
>>>> "virtual" address space. The stub reads/writes from/to the right memory 
>>>> based on the "virtual" addresses. But I'd like to use real addresses with 
>>>> address space id instead. So, I've started looking at what has to be 
>>>> changed.
>>>> 
>>>> I've enhanced read/write commands (e.g. memory read --as <id> ...) and RSP 
>>>> protocol (new packet) so that the stub can read/write properly. That 
>>>> wasn't that complicated.
>>> 
>>> It might be nice to add a new RSP protocol packet that asks for the address 
>>> space names/values:
>>> 
>>> qGetAddressSpaces
>>> 
>>> which would return something like:
>>> 
>>> 1:text;2:data1,3:data2
>>> 
>>> or it would return not supported. If we get a valid return value from 
>>> qGetAddressSpaces, then it enables the use of the new packet you added 
>>> above. Else it defaults to using the old memory read functions.
>>> 
>>> 
>>>> 
>>>> Now I've hit an issue with expressions (LLVMUserExpression.cpp) and local 
>>>> variables (DWARFExpressions.cpp). There is a lot of memory read/write 
>>>> functions that take just an address argument. Is the only way to go to 
>>>> patch all these calls? Has anybody solved it differently?
>>> 
>>> My quick take is that any APIs that take just a lldb::addr_t would need to 
>>> take something like:
>>> 
>>> struct SpaceAddress {
>>> static constexpr uint32_t kNoSpace = 0;
>>> lldb::addr_t addr;
>>> uint32_t space;
>>> };
>>> 
>> 
>> I'm curious why you are suggesting another kind of address, rather than 
>> adding this functionality to Address?  When you actually go to resolve an 
>> Address in a target with a process you should have everything you need to 
>> know to give it the proper space.  Then fixing the expression evaluator (and 
>> anything else that needs fixing) would be a matter of consistently using 
>> Address rather than lldb::addr_t.  That seems general goodness, since 
>> converting to an lldb::addr_t loses information.
> 
> If we accept lldb_private::Address in all APIs that take a lldb::addr_t 
> currently, then we need to always be able to get to the target in case we 
> need to add code to resolve the address everywhere. I am thinking of 
> SpaceAddress as an augmented lldb::addr_t instead of a section + offset style 
> address. Also, there will be addresses in the code and data that do not exist 
> in actual sections. Not saying that you couldn't use lldb_private::Address. I 
> am open to suggestions though. So your though it remove all API that take 
> lldb::addr_t and use lldb_private::Address everywhere all the time?

It has always bugged me that we have these two ways of specifying addresses.  
Are there many/any places that have to resolve an Address to a real address in 
a process that don't have a Target readily available?  That would surprise me.  
I would much rather centralize on one way than adding a third.

Jim


>> 
>> Jim
>> 
>> 
>>> We would need a default value for "space" (feel free to rename) that 
>>> indicates the default address space as most of our architectures would not 
>>> need this support. If we added a constructor like:
>>> 
>>> SpaceAddress(lldb::addr_t a) : addr(a), space(kNoSpace) {}
>>> 
>>> Then all usages of the APIs that used to take just a "lldb::addr_t" would 
>>> implicitly call this constructor and continue to act as needed. Then we 
>>> would need to allow lldb_private::Address objects to resolve to a 
>>> SpaceAddress:
>>> 
>>> SpaceAddress lldb_private::Address::GetSpaceAddress(Target *target) const;
>>> 
>>> Since each lldb_private::Address has a section and each section knows its 
>>> address space. Then the tricky part is finding all locations in the 
>>> expression parser and converting those to track and use SpaceAddress. We 
>>> would probably need to modify the allocate memory packets in the RSP 
>>> protocol to be able to allocate memory in any address space as well.
>>> 
>>> I didn't spend much time think about correct names above, so feel free to 
>>> suggest alternate naming. 
>>> 
>>> Best advice:
>>> - make things "just work" to keep changes to a minimum and allowing 
>>> lldb::addr_t to implicitly convert to a SpaceAddress easily
>>> - when modifying RSP, make sure to check for existence of new feature 
>>> before enabling it
>>> - query for address space names so when we dump SpaceAddress we can show 
>>> something that means something to the user. This means we would need to 
>>> query the address space names from the current lldb_private::Process for 
>>> display.
>>> 
>>> Submitting might go easier if we break it down into chunks:
>>> 1 - add SpaceAddress and modify all needed APIs to use it
>>> 2 - add ProcessGDBRemote changes that enable this support
>>> 
>>> It will be great to support this as a first class citizen within LLDB. You 
>>> might ask the Hexagon folks if they have done anything in case they already 
>>> support this is some shape or form.
>>> 
>>> Greg Clayton
>>> 
>>> _______________________________________________
>>> lldb-dev mailing list
>>> lldb-dev@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

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

Reply via email to