pixfun trunk_gcc5.2_sanitize failure

[Kurt Schwehr]

I am about to move pixelfunctions.c -> pixelfunctions.cpp before looking into this.

​https://travis-ci.org/rouault/gdal_coverage/builds/143054859

Running gcore/pixfun.py...
  TEST: pixfun_real_c ... success
  TEST: pixfun_real_r ... success
=================================================================
[1m[31m==28374==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x615000077ae0 at pc 0x7f1cc53ef5d5 bp 0x7ffd5c965c50 sp 0x7ffd5c965400
[1m[0m[1m[34mREAD of size 16 at 0x615000077ae0 thread T0[1m[0m
    #0 0x7f1cc53ef5d4 in __asan_memcpy (/home/travis/build/rouault/gdal_coverage/install-gcc-5.2.0/lib64/libasan.so.2.0.0+0x8b5d4)
    #1 0x7f1cb0afb652 in GDALCopyWords /home/travis/build/rouault/gdal_coverage/gdal/gcore/rasterio.cpp:2447
    #2 0x7f1cb0115a45 in ImagPixelFunc /home/travis/build/rouault/gdal_coverage/gdal/frmts/vrt/pixelfunctions.c:166
    #3 0x7f1cb069194b in VRTDerivedRasterBand::IRasterIO(GDALRWFlag, int, int, int, int, void*, int, int, GDALDataType, long long, long long, GDALRasterIOExtraArg*) /home/travis/build/rouault/gdal_coverage/gdal/frmts/vrt/vrtderivedrasterband.cpp:391
    #4 0x7f1cb0a3fc3c in GDALRasterBand::RasterIO(GDALRWFlag, int, int, int, int, void*, int, int, GDALDataType, long long, long long, GDALRasterIOExtraArg*) /home/travis/build/rouault/gdal_coverage/gdal/gcore/gdalrasterband.cpp:286
    #5 0x7f1cb0a40105 in GDALRasterIOEx /home/travis/build/rouault/gdal_coverage/gdal/gcore/gdalrasterband.cpp:349
    #6 0x7f1c9b947e5e in BandRasterIONumPy(void*, int, double, double, double, double, PyArrayObject*, int, GDALRIOResampleAlg, int (*)(double, char const*, void*), void*) extensions/gdal_array_wrap.cpp:4028
    #7 0x7f1c9b948e0e in _wrap_BandRasterIONumPy extensions/gdal_array_wrap.cpp:4853
    #8 0x4f95a4 in PyEval_EvalFrameEx (/usr/bin/python2.7+0x4f95a4)
    #9 0x5008c1 in PyEval_EvalCodeEx (/usr/bin/python2.7+0x5008c1)
    #10 0x4f9ab7 in PyEval_EvalFrameEx (/usr/bin/python2.7+0x4f9ab7)
    #11 0x5008c1 in PyEval_EvalCodeEx (/usr/bin/python2.7+0x5008c1)
    #12 0x4f9ab7 in PyEval_EvalFrameEx (/usr/bin/python2.7+0x4f9ab7)
    #13 0x4f9d01 in PyEval_EvalFrameEx (/usr/bin/python2.7+0x4f9d01)
    #14 0x4f9d01 in PyEval_EvalFrameEx (/usr/bin/python2.7+0x4f9d01)
    #15 0x4f9d01 in PyEval_EvalFrameEx (/usr/bin/python2.7+0x4f9d01)
    #16 0x5008c1 in PyEval_EvalCodeEx (/usr/bin/python2.7+0x5008c1)
    #17 0x4f9255 in PyRun_StringFlags (/usr/bin/python2.7+0x4f9255)
    #18 0x501558 in PyRun_SimpleStringFlags (/usr/bin/python2.7+0x501558)
    #19 0x49a4c9 in Py_Main (/usr/bin/python2.7+0x49a4c9)
    #20 0x7f1cc31487ec in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x217ec)
    #21 0x41bb30  (/usr/bin/python2.7+0x41bb30)

[1m[32m0x615000077ae0 is located 0 bytes to the right of 480-byte region [0x615000077900,0x615000077ae0)
[1m[0m[1m[35mallocated by thread T0 here:[1m[0m
    #0 0x7f1cc53fb02a in malloc (/home/travis/build/rouault/gdal_coverage/install-gcc-5.2.0/lib64/libasan.so.2.0.0+0x9702a)
    #1 0x7f1cb0d17d43 in VSIMalloc /home/travis/build/rouault/gdal_coverage/gdal/port/cpl_vsisimple.cpp:524
    #2 0x7f1cb0d184dc in VSIMallocVerbose /home/travis/build/rouault/gdal_coverage/gdal/port/cpl_vsisimple.cpp:932
    #3 0x7f1cb06909d5 in VRTDerivedRasterBand::IRasterIO(GDALRWFlag, int, int, int, int, void*, int, int, GDALDataType, long long, long long, GDALRasterIOExtraArg*) /home/travis/build/rouault/gdal_coverage/gdal/frmts/vrt/vrtderivedrasterband.cpp:344
    #4 0x7f1cb0a3fc3c in GDALRasterBand::RasterIO(GDALRWFlag, int, int, int, int, void*, int, int, GDALDataType, long long, long long, GDALRasterIOExtraArg*) /home/travis/build/rouault/gdal_coverage/gdal/gcore/gdalrasterband.cpp:286
    #5 0x7f1cb0a40105 in GDALRasterIOEx /home/travis/build/rouault/gdal_coverage/gdal/gcore/gdalrasterband.cpp:349
    #6 0x7f1c9b947e5e in BandRasterIONumPy(void*, int, double, double, double, double, PyArrayObject*, int, GDALRIOResampleAlg, int (*)(double, char const*, void*), void*) extensions/gdal_array_wrap.cpp:4028

[Even Rouault]

Yes this is discussed in ​https://github.com/OSGeo/gdal/pull/141

[Kurt Schwehr]

Proposed cleanup of pixelfunctions.cpp before looking at the buffer overflow:

​https://gist.github.com/schwehr/80cc125706666a4201f4834ac7a703ec

Changes:

• Sort includes
• #include <math.h> -> #include <cmath>
• Format fixes
• Fit to 80 cols
• Localize variables
• Combine definition and initialization
• Add const where possible
• Deprecated GDALGetDataTypeSize( eSrcType ) / 8 -> GDALGetDataTypeSizeBytes()
• Define loop variables in for
• /* */ -> for non --- comments
• C casts -> C++ casts
• Spelling: multilpy -> multiply

[Even Rouault]

I'd suggest first that Antonio or Julien fix the buffer overflow and other issues I've raised in the PR and when things have settled down you can apply your cleanup.

[Kurt Schwehr]

I was thinking that hardening the code will make it easier to debug and fix the issues. I find issues are easier to see when the code is as clean and protected as possible. e.g. Analyzers like coverity are more likely to spot issues when scopes and possibilities are restricted.

Either way, the reworked version of the file is in that gist. I'll let you all figure out what you want to do.

[Even Rouault]

Buffer overflow was fixed in r34611 + r34612

I've applied Kurt's cleanups in r34618