-
-
Notifications
You must be signed in to change notification settings - Fork 29
BUG: amos/amos.h: fix scope of l in seri
#92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
e6c80f2
8ef7b48
71da72e
1beee0b
ce75f4b
ba780bd
a973ddd
8a3cb53
9f9fac1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,6 +62,28 @@ TEST_CASE("amos besi vectorized", "[amos][xsf_tests]") { | |
| } | ||
| } | ||
|
|
||
| TEST_CASE("amos seri buffer overflow gh-92", "[amos][xsf_tests]") { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to run this test on
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, I tried again and for me the test fails if I revert the fix. I reverted the fix and pushed to see what CI says.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, looks like you are using macOS? Then CI agrees with you. On Linux CI reproduces the failure I observe. So the effect of the bug might be ill-defined in C++ and gcc/clang give different results. Could this be the origin of the differences? Would you be willing to merge if I revert the last commit? Looks like we agree that the fix is correct and we have a regression test at least for some of the CI jobs.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think those are memory bugs. I would expect CI to fail for all three Operating Systems. |
||
| using std::complex; | ||
|
|
||
| // parameters for besh which trigger overflow in seri | ||
| const complex<double> z{14.0, -3.0}; | ||
| const double fnu = 1.0; | ||
| const int m = 1; | ||
| const int n = 260; | ||
| const int kode = 1; | ||
|
|
||
| // allocate n+1 elements, initialize extra to sentinel to detect overflow | ||
| const complex<double> sentinel{12345.67, 98765.43}; | ||
| std::vector<complex<double>> cy(n + 1, sentinel); | ||
|
|
||
| int ierr = 0; | ||
| int nz = xsf::amos::besh(z, fnu, kode, m, n, cy.data(), &ierr); | ||
|
|
||
| // check if the extra element (index n) was touched | ||
| CAPTURE(cy[n]); | ||
| CHECK(cy[n] == sentinel); | ||
|
JaRoSchm marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| TEST_CASE("amos asyi buffer overflow gh-158", "[amos][xsf_tests]") { | ||
| using std::complex; | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, this seems to fix the translation error.