Skip to content

Commit a43af60

Browse files
author
Rik
committed
Fix ASAN bug in liboctave svd code caused by use of std::vector::reserve() (bug #67248)
* svd.cc: Replace all instances of std::vector::reserve() with std::vector::resize(). Guaranteed creation of scratch pad memory of the correct size, but it unnecessarily initializes each value to 0.
1 parent 394cc28 commit a43af60

File tree

1 file changed

+29
-20
lines changed

1 file changed

+29
-20
lines changed

liboctave/numeric/svd.cc

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ svd<Matrix>::gesvd (char& jobu, char& jobv, F77_INT m, F77_INT n,
358358
GESVD_REAL_STEP (dgesvd, DGESVD);
359359

360360
lwork = static_cast<F77_INT> (work[0]);
361-
work.reserve (lwork);
361+
work.resize (lwork);
362362

363363
GESVD_REAL_STEP (dgesvd, DGESVD);
364364
}
@@ -375,7 +375,7 @@ svd<FloatMatrix>::gesvd (char& jobu, char& jobv, F77_INT m, F77_INT n,
375375
GESVD_REAL_STEP (sgesvd, SGESVD);
376376

377377
lwork = static_cast<F77_INT> (work[0]);
378-
work.reserve (lwork);
378+
work.resize (lwork);
379379

380380
GESVD_REAL_STEP (sgesvd, SGESVD);
381381
}
@@ -394,7 +394,7 @@ svd<ComplexMatrix>::gesvd (char& jobu, char& jobv, F77_INT m, F77_INT n,
394394
GESVD_COMPLEX_STEP (zgesvd, ZGESVD, F77_DBLE_CMPLX_ARG);
395395

396396
lwork = static_cast<F77_INT> (work[0].real ());
397-
work.reserve (lwork);
397+
work.resize (lwork);
398398

399399
GESVD_COMPLEX_STEP (zgesvd, ZGESVD, F77_DBLE_CMPLX_ARG);
400400
}
@@ -414,7 +414,7 @@ svd<FloatComplexMatrix>::gesvd (char& jobu, char& jobv, F77_INT m,
414414
GESVD_COMPLEX_STEP (cgesvd, CGESVD, F77_CMPLX_ARG);
415415

416416
lwork = static_cast<F77_INT> (work[0].real ());
417-
work.reserve (lwork);
417+
work.resize (lwork);
418418

419419
GESVD_COMPLEX_STEP (cgesvd, CGESVD, F77_CMPLX_ARG);
420420
}
@@ -450,7 +450,7 @@ svd<Matrix>::gesdd (char& jobz, F77_INT m, F77_INT n, double *tmp_data,
450450
GESDD_REAL_STEP (dgesdd, DGESDD);
451451

452452
lwork = static_cast<F77_INT> (work[0]);
453-
work.reserve (lwork);
453+
work.resize (lwork);
454454

455455
GESDD_REAL_STEP (dgesdd, DGESDD);
456456
}
@@ -466,7 +466,7 @@ svd<FloatMatrix>::gesdd (char& jobz, F77_INT m, F77_INT n, float *tmp_data,
466466
GESDD_REAL_STEP (sgesdd, SGESDD);
467467

468468
lwork = static_cast<F77_INT> (work[0]);
469-
work.reserve (lwork);
469+
work.resize (lwork);
470470

471471
GESDD_REAL_STEP (sgesdd, SGESDD);
472472
}
@@ -495,7 +495,7 @@ svd<ComplexMatrix>::gesdd (char& jobz, F77_INT m, F77_INT n,
495495
GESDD_COMPLEX_STEP (zgesdd, ZGESDD, F77_DBLE_CMPLX_ARG);
496496

497497
lwork = static_cast<F77_INT> (work[0].real ());
498-
work.reserve (lwork);
498+
work.resize (lwork);
499499

500500
GESDD_COMPLEX_STEP (zgesdd, ZGESDD, F77_DBLE_CMPLX_ARG);
501501
}
@@ -524,7 +524,7 @@ svd<FloatComplexMatrix>::gesdd (char& jobz, F77_INT m, F77_INT n,
524524
GESDD_COMPLEX_STEP (cgesdd, CGESDD, F77_CMPLX_ARG);
525525

526526
lwork = static_cast<F77_INT> (work[0].real ());
527-
work.reserve (lwork);
527+
work.resize (lwork);
528528

529529
GESDD_COMPLEX_STEP (cgesdd, CGESDD, F77_CMPLX_ARG);
530530
}
@@ -581,7 +581,7 @@ svd<Matrix>::gejsv (char& joba, char& jobu, char& jobv,
581581
F77_INT& info)
582582
{
583583
lwork = gejsv_lwork<Matrix>::optimal (joba, jobu, jobv, m, n);
584-
work.reserve (lwork);
584+
work.resize (lwork);
585585

586586
GEJSV_REAL_STEP (dgejsv, DGEJSV);
587587
}
@@ -598,7 +598,7 @@ svd<FloatMatrix>::gejsv (char& joba, char& jobu, char& jobv,
598598
F77_INT& info)
599599
{
600600
lwork = gejsv_lwork<FloatMatrix>::optimal (joba, jobu, jobv, m, n);
601-
work.reserve (lwork);
601+
work.resize (lwork);
602602

603603
GEJSV_REAL_STEP (sgejsv, SGEJSV);
604604
}
@@ -616,18 +616,18 @@ svd<ComplexMatrix>::gejsv (char& joba, char& jobu, char& jobv,
616616
{
617617
F77_INT lrwork = -1; // work space size query
618618
std::vector<double> rwork (1);
619-
work.reserve (2);
619+
work.resize (2);
620620

621621
GEJSV_COMPLEX_STEP (zgejsv, ZGEJSV, F77_DBLE_CMPLX_ARG);
622622

623623
lwork = static_cast<F77_INT> (work[0].real ());
624-
work.reserve (lwork);
624+
work.resize (lwork);
625625

626626
lrwork = static_cast<F77_INT> (rwork[0]);
627-
rwork.reserve (lrwork);
627+
rwork.resize (lrwork);
628628

629629
F77_INT liwork = static_cast<F77_INT> (iwork[0]);
630-
iwork.reserve (liwork);
630+
iwork.resize (liwork);
631631

632632
GEJSV_COMPLEX_STEP (zgejsv, ZGEJSV, F77_DBLE_CMPLX_ARG);
633633
}
@@ -645,18 +645,18 @@ svd<FloatComplexMatrix>::gejsv (char& joba, char& jobu, char& jobv,
645645
{
646646
F77_INT lrwork = -1; // work space size query
647647
std::vector<float> rwork (1);
648-
work.reserve (2);
648+
work.resize (2);
649649

650650
GEJSV_COMPLEX_STEP (cgejsv, CGEJSV, F77_CMPLX_ARG);
651651

652652
lwork = static_cast<F77_INT> (work[0].real ());
653-
work.reserve (lwork);
653+
work.resize (lwork);
654654

655655
lrwork = static_cast<F77_INT> (rwork[0]);
656-
rwork.reserve (lrwork);
656+
rwork.resize (lrwork);
657657

658658
F77_INT liwork = static_cast<F77_INT> (iwork[0]);
659-
iwork.reserve (liwork);
659+
iwork.resize (liwork);
660660

661661
GEJSV_COMPLEX_STEP (cgejsv, CGEJSV, F77_CMPLX_ARG);
662662
}
@@ -758,9 +758,18 @@ svd<T>::svd (const T& a, svd::Type type, svd::Driver driver)
758758
P *vt = m_right_sm.rwdata ();
759759

760760
// Query _GESVD for the correct dimension of WORK.
761-
762761
F77_INT lwork = -1;
763-
762+
// FIXME: A variable-sized scratchpad is required for Fortran SVD routines.
763+
// Octave calls LAPACK routines with lwork of -1 initially which causes
764+
// LAPACK to calculate the size of the required scratchpad and return that
765+
// value in the first entry of the "work" array. The temporary buffer was
766+
// grown to the desired size using std::vector::reserve(). However, this
767+
// leads to undefined behavior outside the C++ specification. To work around
768+
// this (7/4//2025) all of the calls were changed to std::vector::resize().
769+
// This is correct, but unnecessarily initializes all values of the array.
770+
// The code should be re-architected to avoid this, possibly by moving
771+
// this scratchpad memory out of this function and in to the lower-level
772+
// routines where the memory is used.
764773
std::vector<P> work (1);
765774

766775
const F77_INT f77_int_one = static_cast<F77_INT> (1);

0 commit comments

Comments
 (0)