Neon alignment/any check bug

GoogleCodeExporter commented 9 years ago

I422ToBGRA has this code

#if defined(HAS_I422TOBGRAROW_NEON)
  if (TestCpuFlag(kCpuHasNEON)) {
    I422ToBGRARow = I422ToBGRARow_Any_NEON;
    if (IS_ALIGNED(width, 16)) {
      I422ToBGRARow = I422ToBGRARow_NEON;
    }
  }
#endif

but it should have this code.
#if defined(HAS_I422TOBGRAROW_NEON)
  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
    I422ToBGRARow = I422ToBGRARow_Any_NEON;
    if (IS_ALIGNED(width, 8)) {
      I422ToBGRARow = I422ToBGRARow_NEON;
    }
  }
#endif

This will cause a memory stomp for images <= 8

Original issue reported on code.google.com by fbarch...@google.com on 20 Oct 2014 at 6:02

GoogleCodeExporter commented 9 years ago

A search for functions that check neon but not width >= 8 shows a few others 
that should be manually inspected for correctness

c:\src\libyuv2\trunk\source>findstr kCpuHasNEON *.cc
compare.cc:  if (TestCpuFlag(kCpuHasNEON)) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 32)) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && halfwidth >= 16) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 32)) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 16) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert.cc:    if (TestCpuFlag(kCpuHasNEON) && width >= 16) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 16) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 16) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 16) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 16) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 16) {
convert_from.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 16) {
convert_from.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 16) {
convert_from.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 16) {
convert_from.cc:  if (TestCpuFlag(kCpuHasNEON) && halfwidth >= 16) {
convert_from.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 16) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 32) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 16) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && halfwidth >= 16) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 16) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && halfwidth >= 16) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 16) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 16) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 16) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 16) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 16) {
convert_from_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
cpu_id.cc:    return kCpuHasNEON;
cpu_id.cc:        return kCpuHasNEON;
cpu_id.cc:        return kCpuHasNEON;
cpu_id.cc:  cpu_info_ = kCpuHasNEON;
cpu_id.cc:  cpu_info_ = kCpuHasNEON;
cpu_id.cc:    cpu_info_ &= ~kCpuHasNEON;
format_conversion.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
format_conversion.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
format_conversion.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 16) {
format_conversion.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
format_conversion.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 32)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 32)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 16)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 4)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 16)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 16)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 8)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 8)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 8)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 8)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 8)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 8)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 4) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 4) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && width >= 8) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 8)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 16)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 8)) {
rotate.cc:  if (TestCpuFlag(kCpuHasNEON)) {
rotate.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 16)) {
rotate.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 32)) {
rotate.cc:  if (TestCpuFlag(kCpuHasNEON)) {
rotate.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 8)) {
rotate_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(height, 4)) {  // 
Width of dest.
rotate_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width, 4)) {
rotate_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(width * 4, 32)) {
scale.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(dst_width, 16)) {
scale.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(dst_width, 16)) {
scale.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(dst_width, 8)) {
scale.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(dst_width, 8)) {
scale.cc:  if (TestCpuFlag(kCpuHasNEON) && (dst_width % 24 == 0)) {
scale.cc:  if (TestCpuFlag(kCpuHasNEON) && (dst_width % 24 == 0)) {
scale.cc:  if (TestCpuFlag(kCpuHasNEON) && (dst_width % 12 == 0)) {
scale.cc:  if (TestCpuFlag(kCpuHasNEON) && (dst_width % 12 == 0)) {
scale.cc:  if (TestCpuFlag(kCpuHasNEON) && src_width >= 16) {
scale.cc:  if (TestCpuFlag(kCpuHasNEON) && src_width >= 16) {
scale.cc:  if (TestCpuFlag(kCpuHasNEON) && dst_width >= 16) {
scale.cc:  if (TestCpuFlag(kCpuHasNEON) && dst_width >= 16) {
scale_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(dst_width, 8)) {
scale_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(dst_width, 8)) {
scale_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && IS_ALIGNED(dst_width, 4)) {
scale_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && clip_src_width >= 16) {
scale_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && dst_width >= 4) {
scale_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && src_width >= 8) {
scale_argb.cc:  if (TestCpuFlag(kCpuHasNEON) && dst_width >= 4) {
scale_common.cc:  if (TestCpuFlag(kCpuHasNEON) && dst_width_bytes >= 16) {
scale_common.cc:  if (TestCpuFlag(kCpuHasNEON) && dst_width_bytes >= 16) {

Original comment by fbarch...@google.com on 20 Oct 2014 at 6:04

GoogleCodeExporter commented 9 years ago

planar_functions.cc definitely has this issue.  Need to add && width >= 8 to 
many any functions.

Original comment by fbarch...@google.com on 22 Oct 2014 at 6:59

Added labels: Priority-High
Removed labels: Priority-Medium

GoogleCodeExporter commented 9 years ago

These are the potentially dangerous ones

c:\src\libyuv2\trunk\source>findstr Cpu.*NEON\)\) *cc
compare.cc:  if (TestCpuFlag(kCpuHasNEON)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON)) {
planar_functions.cc:  if (TestCpuFlag(kCpuHasNEON)) {
rotate.cc:  if (TestCpuFlag(kCpuHasNEON)) {
rotate.cc:  if (TestCpuFlag(kCpuHasNEON)) {

Original comment by fbarch...@google.com on 23 Oct 2014 at 10:10

GoogleCodeExporter commented 9 years ago

fixed in r1138
but next step would be move the if to row_any so calling code can 
unconditionally use the any function.

Original comment by fbarch...@google.com on 24 Oct 2014 at 12:55

Changed state: Started

GoogleCodeExporter commented 9 years ago

Original comment by fbarch...@google.com on 1 Nov 2014 at 1:06

Changed state: Fixed

foolab / libyuv

Neon alignment/any check bug #373