[lldb-dev] RegisterContextLLDB::ReadGPRValue isn't endian-aware

[Greg Clayton]

Just keep asking questions when you get them and we will do our best to let you know what you need to know as you run into it. There is quite a bit of code and it takes a while to get your head around things. It is always best to start with a small part of LLDB and venture out as you go. 

With PPC you will need to keep an eye out for byte ordering issues. All the targets we have had so far are little endian. The first big endian target with PowerPC will probably expose all the places we didn't get this right...

Greg

On Aug 29, 2013, at 7:57 PM, Félix Cloutier <felixcca at yahoo.ca> wrote:

> Yup, that fixes it. Thanks! Still not too familiar with the architecture.
> 
> Félix
> 
> Le 2013-08-29 à 13:00:15, Greg Clayton <gclayton at apple.com> a écrit :
> 
>> 
>> On Aug 28, 2013, at 8:26 PM, Félix Cloutier <felixcca at yahoo.ca> wrote:
>> 
>>> Hello people,
>>> 
>>> I'm using lldb on an Intel Mac (little endian), and working on a stub for a program that emulates a PowerPC (big endian).
>>> 
>>> The GDB protocol says this about commands for reading registers:
>>> 
>>>> `XX...'
>>>> Each byte of register data is described by two hex digits. The bytes with the register are transmitted in target byte order. The size of each register and their position within the `g' packet are determined by the GDB internal macros REGISTER_RAW_SIZE and REGISTER_NAME macros. The specification of several standard g packets is specified below.
>>> 
>>> So I'm doing just that, replying in big endian. However, RegisterContextLLDB::ReadGPRValue isn't endian-aware: 
>> 
>> It is, see below.
>> 
>>> GetRegisterContext()->ReadRegister (where RegisterContext is a GDBRemoteRegisterContext) reads my big endian response and writes it one byte at a time in the register values buffer, effectively swapping it.
>>> 
>>> From my analysis, GDBRemoteRegisterContext::ReadRegister could do the swapping transparently, but GetRegisterSet (among others?) would still return swapped values; otherwise, using the register metadata, it should be possible to do the swapping when the values are read.
>>> 
>>> How hard would it be to fix lldb?
>> 
>> See below for a one line fix.
>> 
>>> Would I be better off doing a quick hack in my own stub to throw little endian values at lldb in the short term?
>> 
>> No.
>> 
>> Data that potentially needs to be byte swapped is always passed around in DataExtractor objects that do have the endianness built into them. On the GDBRemoteRegisterContext::ReadRegisterBytes it does:
>> 
>> 
>>   if (&data != &m_reg_data)
>>   {
>>       // If we aren't extracting into our own buffer (which
>>       // only happens when this function is called from
>>       // ReadRegisterValue(uint32_t, Scalar&)) then
>>       // we transfer bytes from our buffer into the data
>>       // buffer that was passed in
>>       data.SetByteOrder (m_reg_data.GetByteOrder());
>>       data.SetData (m_reg_data, reg_info->byte_offset, reg_info->byte_size);
>>   }
>> 
>> 
>> "data" is an out parameter that gets filled in with a pointer to the bytes. "m_reg_data.GetByteOrder()" is probably not correctly set to be the endian-ness of the process. 
>> 
>> This might fix your issues:
>> 
>> % svn diff
>> Index: source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
>> ===================================================================
>> --- source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp	(revision 189367)
>> +++ source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp	(working copy)
>> @@ -54,7 +54,7 @@
>>    // Make a heap based buffer that is big enough to store all registers
>>    DataBufferSP reg_data_sp(new DataBufferHeap (reg_info.GetRegisterDataByteSize(), 0));
>>    m_reg_data.SetData (reg_data_sp);
>> -
>> +    m_reg_data.SetByteOrder(thread.GetProcess()->GetByteOrder());
>> }
>> 
>> //----------------------------------------------------------------------
>> 
>> 
>> 
>