Skip to content

Comments

Fix off-by-one in GEFullMat::backSubstituteIntoDU()#13

Open
fuzzybear3 wants to merge 1 commit intoFreeCAD:mainfrom
fuzzybear3:fix-off-by-one-index
Open

Fix off-by-one in GEFullMat::backSubstituteIntoDU()#13
fuzzybear3 wants to merge 1 commit intoFreeCAD:mainfrom
fuzzybear3:fix-off-by-one-index

Conversation

@fuzzybear3
Copy link

Body:

Summary

If you look at this PR #12 It had already recommended this change but seemed to have been overlooked.

Some info from Claude:

  • Fix out-of-bounds vector access in GEFullMat::backSubstituteIntoDU() where answerX->at(n) and rowi->at(n) accessed index n in
    vectors of size n (valid range: 0 to n-1)

Details

GEFullMat::backSubstituteIntoDU() initializes the back-substitution correctly on line 23 using n - 1:
answerX->at(n - 1) = rightHandSideB->at(m - 1) / matrixA->at(m - 1)->at(n - 1);

But the first iteration of the loop used n instead of n - 1:
double sum = answerX->at(n) * rowi->at(n); // out of bounds

This is the same class of bug fixed in GESpMatFullPv::backSubstituteIntoDU() and causes a vector::_M_range_check exception at runtime.

GEFullMat is the base class for GEFullMatParPv and GEFullMatFullPv, neither of which override backSubstituteIntoDU(), so this affects
both full-pivot and partial-pivot dense matrix solves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant