scipopt / papilo

Parallel Presolve for Integer and Linear Optimization
GNU Lesser General Public License v3.0
64 stars 17 forks source link

Avoid out-of-bounds array access #48

Closed jamesjer closed 6 months ago

jamesjer commented 9 months ago

This is my attempt at a fix for https://github.com/scipopt/papilo/issues/47. It avoids reading element 0 of an empty array. If that array should not be empty in the first place, then some other fix is needed.

jamesjer commented 9 months ago

I should have mentioned that this does in fact prevent the segfaults reported in https://github.com/scipopt/papilo/issues/47, and also silences valgrind.

alexhoen commented 9 months ago

Hi James, thanks for your proposed fix and your investigation. First, to provide some context what the ParallelColumns is doing:

For each column/variable, the hashes are computed and then sorted so that the same hashes are adjacent in the vector. Nevertheless, it can happen that variables were already fixed prior to presolving. Fixed variables (column size = 0) are only deleted if enough changes in the matrix justify the matrix update. Hence, it can happen that fixed variables are still present when executing ParallelColumn Detection These fixed variables can then be ignored when sorting the hashes.

Hence, I would slightly reformulate your code by adding the check below. Thank you very much for your investigation


if( cflags[a].test( ColFlag::kFixed ) && cflags[b].test( ColFlag::kFixed ) )
             return a < b;
          if( cflags[a].test( ColFlag::kFixed ) )
             return true;
          if( cflags[b].test( ColFlag::kFixed ) )
             return false;
          assert(constMatrix.getColumnCoefficients( a ).getLength() > 0);
          assert(constMatrix.getColumnCoefficients( b ).getLength() > 0);
alexhoen commented 6 months ago

merged into the mirrored repository