Home Original page

Fix buffer overflow in LM transition model LoadRestart causing segfauIt #2606 by guptapratykshh · Pull Request #2707 · su2code/SU2

@guptapratykshh

Proposed Changes

Fixed buffer overflow in the Langtry-Menter model where derived variables were incorrectly read from the restart file, causing segmentation faults. This ensures the discrete adjoint solver runs correctly and simulations can be restarted without crashing.

Related Work

Resolves #2606.

PR Checklist

Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.

  • I am submitting my contribution to the develop branch.
  • My contribution generates no new compiler warnings (try with --warnlevel=3 when using meson).
  • My contribution is commented and consistent with SU2 style (https://su2code.github.io/docs_v7/Style-Guide/).
  • I used the pre-commit hook to prevent dirty commits and used pre-commit run --all to format old commits.
  • I have added a test case that demonstrates my contribution, if necessary.
  • I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp), if necessary.

@guptapratykshh

pcarruscag

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please modify one of the LM tests to use a restart file.

Comment on lines +552 to +553

* completes. Previously, this code incorrectly tried to read them from indices [index+2] and [index+3],
* which caused a buffer overflow since only nVar=2 solution variables are stored per point. ---*/

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not necessary to document what was wrong before, just the present state is enough.

* completes. Previously, this code incorrectly tried to read them from indices [index+2] and [index+3],
* which caused a buffer overflow since only nVar=2 solution variables are stored per point. ---*/
* completes. ---*/

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

pcarruscag

Comment on lines +240 to +242

tutorial_trans_flatplate_T3A.test_vals = [-5.808996, -2.070606, -3.969765, -0.277943, -1.953093, 1.708472, -3.514943, 0.357411]
tutorial_trans_flatplate_T3A.test_vals_aarch64 = [-5.808996, -2.070606, -3.969765, -0.277943, -1.953289, 1.708472, -3.514943, 0.357411]
tutorial_trans_flatplate_T3A.no_restart = True
# Restart test enabled to verify LM transition model restart functionality (issue #2606)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it suspicious that you get the same results after turning on the restart?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, that is exactly what we want to see, the residuals picked up right where they left off instead of resetting, which confirms state loaded correctly

the tiny variation is just standard noise from recomputing derived fields, but fact that it runs stable

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no

pcarruscag

Comment on lines +240 to +242

tutorial_trans_flatplate_T3A.test_vals = [-5.808996, -2.070606, -3.969765, -0.277943, -1.953093, 1.708472, -3.514943, 0.357411]
tutorial_trans_flatplate_T3A.test_vals_aarch64 = [-5.808996, -2.070606, -3.969765, -0.277943, -1.953289, 1.708472, -3.514943, 0.357411]
tutorial_trans_flatplate_T3A.no_restart = True
# Restart test enabled to verify LM transition model restart functionality (issue #2606)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no

@guptapratykshh

@rois1995

This might also be linked to #2615. In that case, if I recall correctly, the problem was that when restarting, some secondary variables in the transition model were not correctly initialized (this only occurred with WRT_RESTART_COMPACT= YES).

@guptapratykshh

@pcarruscag

The way to test a restart is:

  • Run a case to convergence (low residuals!) to create a restart file.
  • Restarting from that file should have the same residuals on the first iteration that the previous case had on the last iteration.

@guptapratykshh

@guptapratykshh