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
"misc-new-delete-overloads");
+ CheckFactories.registerCheck<NoIntToPtrCheck>("misc-no-inttoptr");
CheckFactories.registerCheck<NoRecursionCheck>("misc-no-recursion");
----------------
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
7.20.1.4p1. 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)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91055/new/
https://reviews.llvm.org/D91055
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
