On 2016-10-16 18:38, Ole Streicher wrote:
> Hi Julien,
> On 16.10.2016 18:30, Julien Cristau wrote:
> >     Packages must autobuild without failure on all architectures on
> >     which they are supported. Packages must be supported on as many
> >     architectures as is reasonably possible. Packages are assumed to
> >     be supported on all architectures for which they have previously
> >     built successfully. Prior builds for unsupported architectures
> >     must be removed from the archive (contact -release or ftpmaster
> >     if this is the case).
> > 
> > As skimage no longer builds on architectures where it used to build (and
> > where it thus is assumed to supported), the "serious" severity for this
> > bug is correct.
> Yea, but it is on the decision of upstream and maintainer which
> platforms are supported by the package. If the maintainer is not able
> (or willing) to support the platforms that FTBFS, why shouldn't he
> remove them?

10 minutes of debugging on *i386* (not a fancy architecture) with GDB
shows that the problem is actually not limited to some unsupported
upstream architectures, but rather that it only work by chance on
architectures which pass the test. The code is buggy and will return
at best wrong values on amd64.

From skimage/measure/_ccomp.pyx:

| cdef void scanBG(DTYPE_t *data_p, DTYPE_t *forest_p, shape_info *shapeinfo,
|                  bginfo *bg):
|     """
|     Settle all background pixels now and don't bother with them later.
|     Since this only requires one linar sweep through the array, it is fast
|     and it makes sense to do it separately.
|     The result of this function is update of forest_p and bg parameter.
|     """
|     cdef DTYPE_t i, bgval = bg.background_val, firstbg
|     # We find the provisional label of the background, which is the index of
|     # the first background pixel
|     for i in range(shapeinfo.numels):
|         if data_p[i] == bgval:
|             firstbg = i
|             bg.background_node = firstbg
|             break
|     # And then we apply this provisional label to the whole background
|     for i in range(firstbg, shapeinfo.numels):
|         if data_p[i] == bgval:
|             forest_p[i] = firstbg

As you can see above, the variable firstbg is only initialized
conditionally in the first loop. It depends on the array values and the
and background value. Still it is used as the starting pointer on the
second loop, even if uninitialized. At best the pointer point to the
wrong address, at worst it generates an out of bound access causing the
segmentation fault.

The test uses the following array with bg.background_val = -1

|  x = np.array([[1, 0, 6],
|                [0, 0, 6],
|                [5, 5, 5]])

The problem has been introduced in the following upstream commit:

| commit d29831882daef9b7d7691becc3669349d642b547
| Author: Matěj Týč <matej....@gmail.com>
| Date:   Sat Nov 15 16:42:22 2014 +0100
|     Redone the background handling = considerable speedup


Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurel...@aurel32.net                 http://www.aurel32.net

Reply via email to