> On Jul 15, 2014, at 3:51 AM, Matthew Gardiner <[email protected]> wrote:
>
> Greg Clayton wrote:
>> A few questions:
>>
>> In PlatformKalimba::GetProcessInfo() the code is:
>>
>>
>> bool
>> PlatformKalimba::GetProcessInfo (lldb::pid_t pid, ProcessInstanceInfo
>> &process_info)
>> {
>> bool success = false;
>> if (IsHost())
>> {
>> success = Platform::GetProcessInfo (pid, process_info);
>> }
>> else
>> {
>> if (m_remote_platform_sp)
>> success = m_remote_platform_sp->GetProcessInfo (pid,
>> process_info);
>> }
>> return success;
>> }
>>
>>
>> Can "kalimba" processes actually run locally on your host machine as native
>> apps? I would guess that PlatformKalimba is for remote debugging only and
>> would expect your code to return false if "IsHost()" is true in that case.
>> If you are running a simulator that uses local processes on the host
>> machine, then this is correct.
>>
> We can't run kalimba ELFs on the host. We do have a simulator (which acts on
> a file resulting from turning an ELF into a bespoke target memory view) to
> run on the host, but we'd connect to the sim, in the "same" way as a real
> device, except that the CSR-type SPI wire packets, move over TCP/IP not on a
> real wire as such. So debugging kalimba on a simulator is still remote
> debugging. I'll modify my implementation as follows:
>
> bool success = false;
> if (IsHost())
> {
> - success = Platform::GetProcessInfo (pid, process_info);
> + success = false;
> }
> else
> {
>
> which I believe to be your suggestion.
>
>> In your GetSupportedArchitectureAtIndex you should only return true if "idx
>> == 0":
>>
>> bool
>> PlatformKalimba::GetSupportedArchitectureAtIndex (uint32_t idx, ArchSpec
>> &arch)
>> {
>> if (idx == 0)
>> {
>> arch = ArchSpec("kalimba-csr-unknown");
>> return true;
>> }
>> return false;
>> }
>>
>> GetSupportedArchitectureAtIndex gets used when you select your platform and
>> then load an executable. It will search for valid architectures by iterating
>> through the architectures using an increasing index. This is needed for
>> cases like iOS where the iOS device might be "armv7s", but when we load a
>> binary, we will start with "armv7s" and then back up to looking for "armv7",
>> armv6", etc.
>
> Ok, thanks for this, I'll modify my next patch attempt accordingly.
>
>> You will need to fill out:
>>
>> size_t
>> PlatformDarwin::GetSoftwareBreakpointTrapOpcode (Target &target,
>> BreakpointSite *bp_site)
>> {
>> }
>>
>> You mainly need to figure out which opcode your target will use as a
>> breakpoint instruction and return its size and fill in the bytes for the
>> instruction, see PlatformDarwin::GetSoftwareBreakpointTrapOpcode() for an
>> example.
>
> Software breakpoints aren't properly supported on kalimba. We did have them
> in the earlier variants, but I'm told that these never worked well: there
> were issues in the pipeline implementation and so on.
>
> So I think that my original implementation of "return 0" was correct.
>
> Kalimba architectures do, however, support setting up to 8 hardware
> breakpoints. That is, we have dedicated debug registers in the hardware.
>
> By the way what sort of thing should PlatformKalimba do for
> CalculateTrapHandlerSymbolNames? Should that be a no-op, not a
> m_trap_handlers.push_back (ConstString ("_sigtramp"));?
If there are functions that can asynchronously interrupt any function at any
instruction, you should list them. Why? Because stack backtracking rules get
broken if you don't. You can just leave them empty if you don't know.
>>
>> In PlatformKalimba::LaunchProcess() you end up launching a host process
>> after checking IsHost(), is this your intention? Are you running a simulator?
>
> Ok. So is it correct to merely do something like this:
>
> if (IsHost())
> {
> error.SetErrorString ("native execution is not possible");
> }
>
> ?
>> Same thing for PlatformKalimba::Attach().
>>
>>
> Ok. So I'll make a similar change to the one made to LaunchProcess.
Sounds good.
>
>
> In response to Greg's review and my current thoughts I'm attaching the latest
> version of the PlatformKalimba.cpp. Is this file now a good enough first
> attempt? Or are there any further areas to complete?
>
> In summary I've
>
> 1. Modified GetProcessInfo to return false if IsHost()==true.
> 2. Implemented GetSupportedArchitectureAtIndex as per Greg's suggestion.
> 3. Modified LaunchProcess and Attach to merely set up an error if
> IsHost()==true.
> 4. Retained my previous implementation for GetSoftwareBreakpointTrapOpcode
> with reasoning.
> 5. Remove m_trap_handlers.push_back (ConstString ("_sigtramp")); from
> CalculateTrapHandlerSymbolNames.
Should be good to go after your above changes.
_______________________________________________
lldb-dev mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
