GNATS-1110: in_cksum_hdr error in PC386 BSP

[Till Straumann]

I believe I caught this one.
Thanks to Danilliu for reporting this and his/(her?)
perseverance.

It is definitely a toolchain/optimization issue:

The BSD code does

header->checksum = 0;
header->checksum = ip_cksum_hdr(&header);

The inline ip_cksum_hdr() routine has inline assembly
and gcc doesn't consider that the assembly actually
could read 'header->checksum' (which it does) so it thinks the
header->checksum = 0 assignment is unnecessary
and optimizes it away :-(

Confirmed by disassembly of Danilliu's broken
and a working binary.

[ As soon as any 'real' subroutine (like a printf)
is called from ip_cksum_hdr() gcc realizes that
the header->checksum=0 assigment could have
side-effects and the problem goes away].

A memory barrier could resolve this.


-- Till


Ralf Corsepius wrote:

>On Wed, 2006-06-28 at 09:54 -0500, Eric Norum wrote:
>  
>
>>On Jun 28, 2006, at 9:39 AM, Ralf Corsepius wrote:
>>
>>    
>>
>>>On Wed, 2006-06-28 at 08:47 -0500, Eric Norum wrote:
>>>
>>>
>>>      
>>>
>>>>The checksum routines are very heavily used so it's really
>>>>important  
>>>>that they be as fast as possible.  Do you have an estimate of the
>>>> 
>>>>relative times of the old and new versions of this code?
>>>>        
>>>>
>>>heavily used and broken don't align smoothly.
>>>      
>>>
>>Yes, I think that they might in this case.
>>1) This problem may have just shown up as a result of the change from
>>gcc-3.x to gcc-4.x.
>>2) Joel has reported for some time that qemu has been broken.
>>    
>>
>Somebody might want to test on real hardware.
>
>  
>
>>>Either these routines have been heavily used, then they must have
>>>been
>>>working, or not - Then I don't understand why nobody has tripped
>>>over
>>>this issue in the years this code is in place.
>>>      
>>>
>>Because in previous years they were using a different tool chain.
>>    
>>
>It's always a comfortable excuse to accuse the toolchain :-)
>
>Did you check the asm being generated?
>
>  
>
>>>IMO, the PR is way to vague to give any trust in it.
>>>      
>>>
>>Others have reported problems with networking on the pc386.
>>    
>>
>This doesn't mean much, esp. because most networking on little endian
>targets always is a magnitude difficult than on big endian ones.
>
>  
>
>>  This is the first time I've seen a patch though.
>>    
>>
>I presume, explicitly testing this checksum code on real hardware with
>somebody being familiar to i386s isn't much of a problem.
>
>Ralf
>
>
>
>  
>