Skip to content

Avoid overflow in Luksan restart counters - #657

Open
CAOShurong wants to merge 2 commits into
stevengj:masterfrom
CAOShurong:codex/luksan-restart-overflow
Open

CAOShurong wants to merge 2 commits into
stevengj:masterfrom
CAOShurong:codex/luksan-restart-overflow

Conversation

@CAOShurong

@CAOShurong CAOShurong commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #632.

The 999*n restart interval overflows a 32-bit integer at 2,149,634 variables. LBFGS, VAR1/VAR2 and all four truncated-Newton variants then return NLOPT_FAILURE after the first objective evaluation.

Use long long for the private restart counter and shared interval arithmetic, and clamp the LBFGS history length before converting it to int. The public dimension API is unchanged.

The default regression checks the overflow boundaries without large allocations and runs all seven algorithms on four variables. The large public-API cases remain available with t_luksan_restart --large.

Validation on Windows/MSVC: 89 C/C++ CTest cases and 53 C-only AddressSanitizer cases pass. The default regression passes 13 cases and peaks at about 5.4 MiB. The opt-in stress test passes all 27 cases with the fixed installed library and fails seven against the unmodified library. A deliberately narrowed arithmetic helper also fails the boundary checks.

Use wider private restart counters and arithmetic for LBFGS, VAR and
truncated Newton algorithms. Keep the history-size conversion after
clamping, and test all seven algorithms across the 999*n overflow boundary.
@stevengj

Copy link
Copy Markdown
Owner

This seems reasonable, but the test allocates an awful lot of memory.

@CAOShurong

Copy link
Copy Markdown
Contributor Author

The default test now checks the shared restart arithmetic at the overflow boundaries without allocating large vectors, plus four-variable solves for all seven algorithms. The large public-API cases are still available with --large. Both modes pass; the default test peaks at about 5.4 MiB on Windows, versus 259 MiB for the opt-in stress cases.

@Sinkleberg Sinkleberg left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The new static inline at luksan.h:12 breaks the C89 build. I reproduced it with GCC 14.3, and the Linux job log shows the same compiler error before the workflow continues to the MinGW build. Could you move the helper into a C file and leave a prototype in the header? The overflow boundary tests passed locally.

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.

The product of two integers can overflow in L-BFGS

3 participants