[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

Aaron Ballman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 23 06:47:41 PST 2020

aaron.ballman added a comment.

In D91055#2410447 <https://reviews.llvm.org/D91055#2410447>, @lebedev.ri wrote:

> Ping.
> If there's no pushback within next 24h i will commit this and wait for post-commit review (if any).
> Thanks.

FWIW, that's not the way we usually do things, so please don't push and hope for post-commit review on something people have expressed questions/concerns over.

Comment at: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp:40
+    CheckFactories.registerCheck<NoIntToPtrCheck>("misc-no-inttoptr");
Is `misc` really the right place for this? If this is related to optimizer behavior, perhaps it should live in `performance`?

Comment at: clang-tools-extra/clang-tidy/misc/NoIntToPtrCheck.cpp:26
+  const auto *MatchedCast = Result.Nodes.getNodeAs<CastExpr>("x");
+  diag(MatchedCast->getBeginLoc(), "avoid integer to pointer casts");
This doesn't really tell the user what's wrong with their code or how to fix it. There are valid use cases for casting an integer into a pointer, after all. For instance, if I'm doing this cast because I'm trying to read a register bank:
volatile int *registers = (volatile int *)0xFEEDFACE;
what is wrong with my code and what should I do to silence this diagnostic?

Intuitively, one would think that use of `intptr_t` or `uintptr_t` would have some impact on this given the standard's requirements for that type in C17 Could there be a certain cast path that can be used to silence the diagnostic because it's safe enough for the optimizer's needs?

Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-no-inttoptr.rst:6
+Diagnoses every integer to pointer cast.
char buffer[16];
sprintf(buffer, "%p", ptr);

intptr_t val;
sscanf(buffer, "%" SCNxPTR, &val);
Do you think this check should catch such constructs under the same reasoning as other conversions that hide provenance?

Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-no-inttoptr.rst:12
+if you got that integral value via a pointer-to-integer cast originally,
+the new pointer will lack the provenance information from the original pointer.
Does this run afoul of the C++ standard's requirements for pointer traceability: http://eel.is/c++draft/basic.stc.dynamic.safety ?

Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-no-inttoptr.cpp:3
+// can not implicitly cast int to a pointer.
+// can not use static_cast<>() to cast integer to a pointer.
can not -> cannot
(same below)

  rG LLVM Github Monorepo



More information about the llvm-commits mailing list