steakhal marked 8 inline comments as done.
steakhal added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/IncorrectPointerCastCheck.cpp:69-70
+ "cast from %0 to %1 may lead to access memory based on invalid "
+ "memory layout; pointed to type is strictly aligned than the "
+ "allocated type")
+ << SrcPointedType << DestPointedType;
----------------
lebedev.ri wrote:
> "*more* strictly aligned" perhaps?
> Similarly, specify the actual values.
Which metrics should I use? Bits or char units? For now I will stick to bits.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/IncorrectPointerCastCheck.cpp:98-107
+ } else if (WarnForDifferentSignedness &&
+ ((SrcPointedType->isSignedIntegerType() &&
+ DestPointedType->isUnsignedIntegerType()) ||
+ (SrcPointedType->isUnsignedIntegerType() &&
+ DestPointedType->isSignedIntegerType()))) {
+ diag(castExpr->getBeginLoc(),
+ "cast from %0 to %1 may lead to access memory based on invalid "
----------------
lebedev.ri wrote:
> What does this mean?
> How can signedness affect memory layout?
You are right, signess difference is **explicitly** allowed in bitcasts. It has
nothing to do with the actual memory layout. I thought it might be useful for
detecting those cases too. It should be in a different checker in the future.
For now I've removed the related code from this checker.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/IncorrectPointerCastCheck.h:18
+
+/// Warns for cases when pointer is cast and the pointed to type is
incompatible
+/// with allocated memory area type. This may lead to access memory based on
----------------
lebedev.ri wrote:
> is cast*ed*
> s/pointed to/new/
Casted fixed now.
What do you think about **destination type** and **original type** instead of
**new type**?
================
Comment at: clang-tools-extra/clang-tidy/bugprone/IncorrectPointerCastCheck.h:19
+/// Warns for cases when pointer is cast and the pointed to type is
incompatible
+/// with allocated memory area type. This may lead to access memory based on
+/// invalid memory layout.
----------------
lebedev.ri wrote:
> How do you know anything about the actual `allocated memory area type`.
> You probably want to talk about the original type before cast.
You are right, fixed.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D48866/new/
https://reviews.llvm.org/D48866
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
