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

Thu May 17 06:45:11 PDT 2018

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