Busybox 1.16.x dnsd alignment problems

[Lars Reemts]

Unfortunately
# define move_from_unaligned16(v, u16p) {void* src=(void*)(u16p);memcpy(&(v), src, 2);}

also didn't do it. The only solution I've found working is doing it the
hard way by hand:

# define move_from_unaligned16(v, u16p) do { \
        *((uint8_t*)&v) = *((uint8_t*)u16p); \
        *(((uint8_t*)&v)+1) = *(((uint8_t*)u16p)+1); \
} while (0)



Lars Reemts schrieb:
>> Are you saying that here:
>>     
>
>   
>> # define move_from_unaligned16(v, u16p) (memcpy(&(v), (u16p), 2))
>>     
>
>   
>> memcpy may be opportunistic and if u16p has type uin16_t,
>> memcpy will be optimized to assignment?
>>     
>
> Definitely yes.
>
>   
>> Please find a way which works for your arch and compiler,
>> and let me know. Parhaps this?
>>     
>
>   
>> # define move_from_unaligned16(v, u16p) (memcpy(&(v), (void*)(u16p), 2))
>>     
>
> This also was my first idea, but it doesn't work. Seems like gcc tries
> to be super clever, ignoring the cast and using the original pointer
> type instead.
> Maybe something like
>
> # define move_from_unaligned16(v, u16p) {void*
> src=(void*)(u16p);memcpy(&(v), src, 2);}
>
> will do the trick. I will try it tomorrow.
>
>
> Denys Vlasenko schrieb:
>   
>> On Wed, Apr 14, 2010 at 6:02 AM, Lars Reemts <lars at reemts.de> wrote:
>>   
>>     
>>> Hello,
>>>
>>> I'm using busybox on a AT91SAM9261 (arm920t) processor with gcc 4.3.4.
>>> Yesterday I ran into some problems using dnsd. DNS queries from a
>>> windows host were not properly decoded and hence not answered. Digging
>>> into the code the problem turned out to be two different problems, both
>>> related to halfword alignment.
>>>
>>> The first one (in dnsd_main()) is related to the alignment of buffer
>>> where the incoming network packets are stored. It was declared and
>>> allocated as uint8_t buf[] in dnsd.c:462. This tells the compiler that
>>> the buffer is containing byte data and gives it the freedom to allocate
>>> the buffer at any address. In my setup it starts at an odd address. When
>>> processing an incoming packet later on in process_packet() the buffer is
>>> accessed as containing 2 byte data, which leads to alignment problems.
>>> Allocating the buffer as uint16_t buf[] solves the problem for me.
>>>
>>> The second one (in process_packet()) was particularly hard to track
>>> down. It boils down to move_from_unaligned16(), which is #defined as 2
>>> byte memcpy() in platform.h not working correctly for 16 bit source
>>> pointer types. For these the compiler assumes the pointer to already be
>>> aligned and replaces the 2 byte memcpy() by a half word load and a half
>>> word store assembler instruction. The solution is to ensure the source
>>> pointer is pointing to a single byte data type.
>>>     
>>>       
>> Are you saying that here:
>>
>> # define move_from_unaligned16(v, u16p) (memcpy(&(v), (u16p), 2))
>>
>> memcpy may be opportunistic and if u16p has type uin16_t,
>> memcpy will be optimized to assignment? If so, this
>> should be prevented from happening. We WANT to be damn sure
>> any address, however badly unaligned, will work in this macro.
>>
>> Please find a way which works for your arch and compiler,
>> and let me know. Parhaps this?
>>
>> # define move_from_unaligned16(v, u16p) (memcpy(&(v), (void*)(u16p), 2))
>>
>>   
>>     
>>> Please have a look at the attached patch which solves both problems. I'm
>>> not sure whether the solution for the second problem should be done in
>>> platform.h.
>>>     
>>>       
>> +       /* allocate the buffer as uint16_t[] to make sure it is
>> aligned at a uint16 boundary */
>> +       uint16_t _buf[MAX_PACK_LEN/2 + 1];
>> +       uint8_t *buf = (uint8_t*)_buf;
>>
>> Unfortunately gcc is too stupid to not generate bigger code for this.
>> So I went with slapping ALIGN4 on the byte array instead.
>>
>> The fix is here:
>> http://busybox.net/downloads/fixes-1.16.1/busybox-1.16.1-dnsd.patch
>>   
>>     
>
>
>   


-- 
Lars Reemts, Lange Wieke 1, 26817 Rhauderfehn, Germany
Tel: +49 4952 942290, Fax: +49 4952 942291
mailto: lars at reemts.de