papilo icon indicating copy to clipboard operation
papilo copied to clipboard

Avoid out-of-bounds array access

Open jamesjer opened this issue 11 months ago • 2 comments

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 avatar Mar 07 '24 04:03 jamesjer

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.

jamesjer avatar Mar 07 '24 04:03 jamesjer

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 avatar Mar 07 '24 08:03 alexhoen

merged into the mirrored repository

alexhoen avatar May 16 '24 11:05 alexhoen