flac dev - Sep 2007 - Re: 1.2.0: Test suite failures on LP64 archs?

Christian Weisgerber <naddy@mips.inka.de> wrote:
> #0  0x0000000040d18810 in
FLAC__lpc_compute_residual_from_qlp_coefficients_wide
>     (data=0x49e4c014, data_len=110, qlp_coeff=0x7f7ffffece70, order=1, 
>     lp_quantization=14, residual=0x4fced000) at lpc.c:745
> 745                                                     residual[i] >
data[i] - (FLAC__int32)((qlp_coeff[0] * (FLAC__int64)data[i-1]) >>
> lp_quantization);
> (gdb) i loc
> i = 0
I have figured out the problem now.  The index variable i is defined
as unsigned int.  Underflowing it causes the value to go to 0xffffffff
which is _not_ the same as -1 when added to a pointer on 64-bit
architectures.

Specifically in the case of the crash above, the generated code
performs an access at offset 0x3fffffffc (0xffffffff * sizeof(int)).
This problem is hidden on 32-bit archs due to truncation.

The bug was introduced in 1.2.0 with the fully unrolled functions
in libFLAC/lpc.c.

-- 
Christian "naddy" Weisgerber                         
naddy@mips.inka.de

Christian Weisgerber <naddy@mips.inka.de> wrote:
> The bug was introduced in 1.2.0 with the fully unrolled functions
> in libFLAC/lpc.c.
I guess the use of unsigned index variables was an accident.  Changing
them back to int makes FLAC pass the "thorough" test suite on amd64.

--- src/libFLAC/lpc.c.orig	Sun Sep  2 00:13:36 2007
+++ src/libFLAC/lpc.c	Sun Sep  2 00:14:01 2007
@@ -569,7 +569,7 @@ void FLAC__lpc_compute_residual_from_qlp_coefficients_
 }
 #else /* fully unrolled version for normal use */
 {
-	unsigned i;
+	int i;
 	FLAC__int64 sum;
 
 	FLAC__ASSERT(order > 0);
@@ -1099,7 +1099,7 @@ void FLAC__lpc_restore_signal_wide(const FLAC__int32 r
 }
 #else /* fully unrolled version for normal use */
 {
-	unsigned i;
+	int i;
 	FLAC__int64 sum;
 
 	FLAC__ASSERT(order > 0);
-- 
Christian "naddy" Weisgerber                         
naddy@mips.inka.de

thanks, I had fixed that same problem in nearby functions but
somehow missed that one...

--- Christian Weisgerber <naddy@mips.inka.de> wrote:
> Christian Weisgerber <naddy@mips.inka.de> wrote:
> 
> > The bug was introduced in 1.2.0 with the fully unrolled functions
> > in libFLAC/lpc.c.
> 
> I guess the use of unsigned index variables was an accident. 
> Changing
> them back to int makes FLAC pass the "thorough" test suite on
amd64.
> 
> --- src/libFLAC/lpc.c.orig	Sun Sep  2 00:13:36 2007
> +++ src/libFLAC/lpc.c	Sun Sep  2 00:14:01 2007
> @@ -569,7 +569,7 @@ void
> FLAC__lpc_compute_residual_from_qlp_coefficients_
>  }
>  #else /* fully unrolled version for normal use */
>  {
> -	unsigned i;
> +	int i;
>  	FLAC__int64 sum;
>  
>  	FLAC__ASSERT(order > 0);
> @@ -1099,7 +1099,7 @@ void FLAC__lpc_restore_signal_wide(const
> FLAC__int32 r
>  }
>  #else /* fully unrolled version for normal use */
>  {
> -	unsigned i;
> +	int i;
>  	FLAC__int64 sum;
>  
>  	FLAC__ASSERT(order > 0);
> -- 
> Christian "naddy" Weisgerber                         
> naddy@mips.inka.de
> _______________________________________________
> Flac-dev mailing list
> Flac-dev@xiph.org
> http://lists.xiph.org/mailman/listinfo/flac-dev
> 


       
____________________________________________________________________________________
Yahoo! oneSearch: Finally, mobile search 
that gives answers, not web links. 
http://mobile.yahoo.com/mobileweb/onesearch?refer=1ONXIC