[lldb-dev] Advice on architectures with multiple address spaces

Fri May 25 05:35:31 PDT 2018

Just a small update. I did create the class AddressBase. Class Address 
inherits from it. When I compare it with my first implementation, it's 
cleaner than the additional argument in API. I also implemented all 
operators to the class AddressBase, so it behaves like addr_t if you're 
not interested in the address space.

I thought that I'd rename the class AddressBase to addr_t and see what 
would happen, but then I realized that LLDB has python support and I'm 
not sure how this works with classes, so, any advice here?

Btw. I still don't have a clear answer to my question below...

Zdenek

On 05/17/2018 03:45 PM, Zdenek Prikryl wrote:
> Greg, Jim, what's your opinion here?
>
> What about having the class Address (pretty much as it is right now) 
> and the
>
> struct AddressBase {
>   lldb::addr_t m_address;
>   lldb::as_t m_address_space;
>   ...
> }
>
> Another question is, which classes/code should use Address, 
> AddressBase, and addr_t. Do you have any idea here?
>
> Zdenek
>
> On 05/17/2018 01:38 PM, Pavel Labath wrote:
>> The Address class may be suitable for the higher layers of lldb, but I
>> don't think the it can ever be a blanket replacement for 
>> lldb::addr_t. It
>> has way too much smartness built-in. We use addr_t in a lot of places 
>> that
>> don't/shouldn't care about Targets, ExecutionContexts or Sections. 
>> All of
>> lldb-server is one of those places, but this is also true for any 
>> low-level
>> operation which only wants to work with real (virtual) addresses in the
>> process address space.
>>
>> On the other hand, replacing addr_t with a lighweight struct which is 
>> just
>> adds some sort of an address space identifier seems like a useful thing,
>> and would go a long way towards bringing Harward architecture support to
>> lldb-server. (Note that we would still need an addr_t or something of 
>> that
>> form to name the type of the "address" member of the struct, but pretty
>> much all of the apis that currently take addr_t, could that the new 
>> struct
>> instead).
>>
>>
>> On Thu, 17 May 2018 at 12:01, Zdenek Prikryl via lldb-dev <
>> lldb-dev at lists.llvm.org> wrote:
>>
>>
>>> On 04/19/2018 08:22 PM, Jim Ingham wrote:
>>>>> On Apr 19, 2018, at 10:54 AM, Greg Clayton <clayborg at gmail.com> 
>>>>> wrote:
>>>>>
>>>>>
>>>>>
>>>>>> On Apr 19, 2018, at 10:35 AM, Jim Ingham <jingham at apple.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On Apr 19, 2018, at 9:44 AM, Greg Clayton via lldb-dev <
>> lldb-dev at lists.llvm.org> wrote:
>>>>>>>
>>>>>>>
>>>>>>>> On Apr 19, 2018, at 6:51 AM, Zdenek Prikryl via lldb-dev <
>> lldb-dev at 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
>>> So, does it make sense to start with lldb::addr_t replacement? In other
>>> words, replace all instances of lldb::addr_t with Address. It'd be the
>>> first step and first patch towards to the ability to extend it in the
>>> future, right?
>>> Zdenek
>>
>>>>
>>>>>> 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 at lists.llvm.org
>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>>> _______________________________________________
>>> lldb-dev mailing list
>>> lldb-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>