gcc miscompiles csum_tcpudp_magic() on ARMv5

[Willy Tarreau]

On Thu, Dec 12, 2013 at 03:18:18PM +0000, Måns Rullgård wrote:
> Willy Tarreau <w at 1wt.eu> writes:
> 
> >> >> Hmmm aren't you passing a 16-bit register directly to the ASM for
> >> >> being used as a 32-bit one ? This seems hasardous to me since
> >> >> nowhere you tell gcc how you're going to use the register.
> >> >
> >> > this is exactly what I'm complaining about, the arm code for
> >> > csum_tcpudp_nofold() in the kernel does exactly that.
> >> >
> >> >> Could you check if that fixes it :
> >> >> 
> >> >>  static inline uint32_t asm_add(uint16_t len, uint32_t sum)
> >> >>  {
> >> >>          uint32_t len32 = len;
> >> >
> >> > or change the asm_add() proto to take an "uint32_t len" instead, and yes
> >> > of course that fixes it.
> >> 
> >> It's a bug.  Please report it to the gcc developers.
> >
> > Here I don't agree with the generalization (and believe me I swear all the
> > day about gcc's bugs). It's a matter of ABI and availability or not of 16
> > bit registers or not. If the ASM supports 16-bit regs and the compiler is
> > allowed to emit 16-bit regs, then gcc will have no way to know whether it
> > must zero-extend the value first. If it's specified that "r" is necessarily
> > a 32-bit register then it should.
> 
> ARM has *only* 32-bit registers.
> 
> > Maybe the issue is in the ABI itself, I don't know if 16-bit values
> > are supposed to be zero-extended only when they're converted to 32-bit
> > or also when they're passed as arguments. The fact that it works
> > without inline may simply be a side effect of the different code (eg:
> > 16 lower bits of the register copied into another one).
> >
> > So one needs to look at the specs of the ABI to know where the 16-bit value
> > is supposed to be converted to 32-bit, then the faulty component must be
> > fixed (gcc or kernel code).
> 
> The ABI for function calls sign/zero-extends all arguments prior to the
> call.

OK then that's pretty clear, there's no ambiguity, thanks for the precision.

Willy