This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Add bugprone-undefined-memory-manipulation check.
ClosedPublic

Authored by rnkovacs on Jul 6 2017, 5:50 AM.

Details

Summary

Finds calls of memory manipulation functions memset(), memcpy() and memmove() on not TriviallyCopyable objects, as these always result in an undefined behavior.

Moved to module bugprone created by D32700.

Hits on LLVM codebase:

Diff Detail

Repository
rL LLVM

Event Timeline

rnkovacs created this revision.Jul 6 2017, 5:50 AM
whisperity edited the summary of this revision. (Show Details)Jul 6 2017, 5:57 AM
rnkovacs added a subscriber: szepet.Jul 6 2017, 6:55 AM
alexfh edited edge metadata.Jul 11 2017, 7:51 AM

As usual, please run the check at least on LLVM+Clang and include a brief summary of results into the description of the patch.

Another thing is that at this point I'd already create a bugprone module (if nobody has a better name) and place the check there (together with the other memset-related check).

rnkovacs updated this revision to Diff 106185.Jul 12 2017, 5:28 AM
rnkovacs retitled this revision from [clang-tidy] Add misc-undefined-memory-manipulation check. to [clang-tidy] Add bugprone-undefined-memory-manipulation check..
rnkovacs edited the summary of this revision. (Show Details)
  • Moved to module bugprone.
  • Added summary of hits on LLVM in the description.
alexfh requested changes to this revision.Jul 12 2017, 8:28 AM
alexfh added inline comments.
clang-tidy/bugprone/UndefinedMemoryManipulationCheck.cpp
22 ↗(On Diff #106185)

nit: No need for the parentheses.

39 ↗(On Diff #106185)

What about memset?

test/clang-tidy/bugprone-undefined-memory-manipulation.cpp
34 ↗(On Diff #106185)

Remove stray semicolons after closing braces of function bodies.

This revision now requires changes to proceed.Jul 12 2017, 8:28 AM
rnkovacs updated this revision to Diff 106234.Jul 12 2017, 8:56 AM
rnkovacs edited edge metadata.

Removed redundant parens and stray semicolons.

rnkovacs marked 2 inline comments as done.Jul 12 2017, 8:59 AM
rnkovacs added inline comments.
clang-tidy/bugprone/UndefinedMemoryManipulationCheck.cpp
39 ↗(On Diff #106185)

memset has a bit different signature than the other two. It has a fill value as its second argument instead of a source object.

alexfh accepted this revision.Jul 12 2017, 9:01 AM

LG

clang-tidy/bugprone/UndefinedMemoryManipulationCheck.cpp
39 ↗(On Diff #106185)

Ah, now I get it. We're inspecting the second argument here.

This revision is now accepted and ready to land.Jul 12 2017, 9:01 AM
This revision was automatically updated to reflect the committed changes.

FYI, bugprone-undefined-memory-manipulation crashes on some of our code. I guess, this happens with dependent types, but I don't have an isolated repro yet.

The top of stack trace is:

clang::CXXRecordDecl::isTriviallyCopyable()
(unknown)
clang::tidy::bugprone::(anonymous namespace)::internal::matcher_isNotTriviallyCopyableMatcher::matches()

Dependent types seem to work, but we did manage to produce a crash on incomplete types. I created D35790 for that. I hope it's the same problem you encountered.