-
Notifications
You must be signed in to change notification settings - Fork 304
Get a clean -fsanitize=integer run of make check
#3568
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 all commits
1e547b9
dad6cb3
9ed73fc
4b7b9ee
c7ca89c
0c690ff
1201ae9
9ac48b1
3ca168c
bfc6944
d53cb36
121b633
f95be14
32a204b
4b99bb8
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 |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| # This is a set of exceptions to use in | ||
| # '-fsanitize-ignorelist=yourdir/fsanitize_ignorelist.txt' | ||
| # | ||
| # So far it's been tested and found to be complete for | ||
| # -fsanitize=integer on clang++ 14 with g++ 12.1.0 headers | ||
| fun:*basic_string_view*rfind* | ||
| fun:*basic_string*compare* | ||
| src:*hashing.h | ||
| src:*hashword.h | ||
| src:*libHilbert*SetBits* | ||
| src:*fpoptimizer* | ||
| src:*fp_opcode_add* | ||
| src:*bytecodesynth* | ||
| src:*fparser.cc | ||
| src:*Eigen/* |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -125,8 +125,8 @@ NameBasedIO::is_parallel_file_format (std::string_view name) | |
| { | ||
| return ((name.rfind(".xda") < name.size()) || | ||
| (name.rfind(".xdr") < name.size()) || | ||
| (name.rfind(".nem") + 4 == name.size()) || | ||
| (name.rfind(".n") + 2 == name.size()) || | ||
| (name.rfind(".nem") == name.size() - 4) || | ||
| (name.rfind(".n") == name.size() - 2) || | ||
|
Comment on lines
+128
to
+129
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'd have guessed the sanitizer wouldn't like subtracting from an unsigned type that could possibly be 0? Oh, but std::string::rfind() returns std::string::npos which is effectively max size_t so adding to that would also be bad... not sure which is best actually.
Member
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. Yeah; the saving grace here is that |
||
| (name.rfind(".cp") < name.size()) | ||
| ); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1717,7 +1717,7 @@ dof_id_type DofMap::n_local_constrained_dofs() const | |
| const DofConstraints::const_iterator lower = | ||
| _dof_constraints.lower_bound(this->first_dof()), | ||
| upper = | ||
| _dof_constraints.upper_bound(this->end_dof()-1); | ||
| _dof_constraints.lower_bound(this->end_dof()); | ||
|
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. This looks like an actual bugfix... was that suggested by the sanitizer?
Member
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. It wasn't suggested by the sanitizer; that only reports filename and line number and what type of behavior was detected, and leaves it up to the developer to figure out how to fix it. Maybe in LLVM 30. But ... yeah, this looks like a real bug fix. I recently spent a ton of time fixing a bug that was triggered by an unexpected corner case with element deletion, just like this one could easily have been, and as annoying as the sanitizer is I have to say this fix alone made me feel like the annoyance was worth it so far. |
||
|
|
||
| return cast_int<dof_id_type>(std::distance(lower, upper)); | ||
| } | ||
|
|
@@ -2086,7 +2086,7 @@ void DofMap::process_mesh_constraint_rows(const MeshBase & mesh) | |
|
|
||
| if (was_previously_constrained.count(constrained_id)) | ||
| { | ||
| for (auto q : IntRange<unsigned int>(0, n_adjoint_rhs+1)) | ||
| for (auto q : IntRange<int>(0, n_adjoint_rhs+1)) | ||
| { | ||
| DenseMatrix<Number> K(1,1); | ||
| DenseVector<Number> F(1); | ||
|
|
||
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.
How was this working before with an unsigned type and this cutesy
b-->0BS? Whenb==0the test fails and then you post-decrement so the number wraps to max size_t, but we never see it because the loop is done?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.
Precisely. It's apparently a popular idiom, too; this is what
-fsanitize=integerwas complaining about in a couple places in libstdc++basic_stringcode.