lebedev.ri created this revision. lebedev.ri added reviewers: aaron.ballman, njames93, JonasToth. lebedev.ri added a project: LLVM. Herald added subscribers: cfe-commits, xazax.hun, mgorny. Herald added a project: clang. lebedev.ri requested review of this revision.
While casting an (integral) pointer to an integer is obvious - you just get the integral value of the pointer, casting an integer to an (integral) pointer is deceivingly different. While you will get a pointer with that integral value, 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. So while (integral) pointer to integer casts are effectively no-ops, and are transparent to the optimizer, integer to (integral) pointer casts are *NOT* transparent, and may conceal information from optimizer. While that may be the intention, it is not always so. For example, let's take a look at a routine to align the pointer up to the multiple of 16: The obvious, naive implementation for that is: char* src(char* maybe_underbiased_ptr) { uintptr_t maybe_underbiased_intptr = (uintptr_t)maybe_underbiased_ptr; uintptr_t aligned_biased_intptr = maybe_underbiased_intptr + 15; uintptr_t aligned_intptr = aligned_biased_intptr & (~15); return (char*)aligned_intptr; // warning: avoid integer to pointer casts [misc-no-inttoptr] } The check will rightfully diagnose that cast. But when provenance concealment is not the goal of the code, but an accident, this example can be rewritten as follows, without using integer to pointer cast: char* tgt(char* maybe_underbiased_ptr) { uintptr_t maybe_underbiased_intptr = (uintptr_t)maybe_underbiased_ptr; uintptr_t aligned_biased_intptr = maybe_underbiased_intptr + 15; uintptr_t aligned_intptr = aligned_biased_intptr & (~15); uintptr_t bias = aligned_intptr - maybe_underbiased_intptr; return maybe_underbiased_ptr + bias; } See also: - D71499 <https://reviews.llvm.org/D71499> - Juneyoung Lee, Chung-Kil Hur, Ralf Jung, Zhengyang Liu, John Regehr, and Nuno P. Lopes. 2018. Reconciling High-Level Optimizations and Low-Level Code in LLVM. Proc. ACM Program. Lang. 2, OOPSLA, Article 125 (November 2018), 28 pages. <https://www.cs.utah.edu/~regehr/oopsla18.pdf> Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D91055 Files: clang-tools-extra/clang-tidy/misc/CMakeLists.txt clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp clang-tools-extra/clang-tidy/misc/NoIntToPtrCheck.cpp clang-tools-extra/clang-tidy/misc/NoIntToPtrCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst clang-tools-extra/docs/clang-tidy/checks/misc-no-inttoptr.rst clang-tools-extra/test/clang-tidy/checkers/misc-no-inttoptr.c clang-tools-extra/test/clang-tidy/checkers/misc-no-inttoptr.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/misc-no-inttoptr.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/misc-no-inttoptr.cpp @@ -0,0 +1,16 @@ +// RUN: %check_clang_tidy %s misc-no-inttoptr %t + +// can not implicitly cast int to a pointer. +// can not use static_cast<>() to cast integer to a pointer. +// can not use dynamic_cast<>() to cast integer to a pointer. +// can not use const_cast<>() to cast integer to a pointer. + +void *t0(long long int x) { + return (void *)x; + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr] +} + +void *t1(int x) { + return reinterpret_cast<void *>(x); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr] +} Index: clang-tools-extra/test/clang-tidy/checkers/misc-no-inttoptr.c =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/misc-no-inttoptr.c @@ -0,0 +1,66 @@ +// RUN: %check_clang_tidy %s misc-no-inttoptr %t + +void *t0(char x) { + return x; + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr] +} +void *t1(signed char x) { + return x; + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr] +} +void *t2(unsigned char x) { + return x; + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr] +} + +void *t3(short x) { + return x; + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr] +} +void *t4(unsigned short x) { + return x; + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr] +} +void *t5(signed short x) { + return x; + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr] +} + +void *t6(int x) { + return x; + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr] +} +void *t7(unsigned int x) { + return x; + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr] +} +void *t8(signed int x) { + return x; + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr] +} + +void *t9(long x) { + return x; + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr] +} +void *t10(unsigned long x) { + return x; + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr] +} +void *t11(signed long x) { + return x; + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr] +} + +void *t12(long long x) { + return x; + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr] +} +void *t13(unsigned long long x) { + return x; + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr] +} +void *t14(signed long long x) { + return x; + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: avoid integer to pointer casts [misc-no-inttoptr] +} Index: clang-tools-extra/docs/clang-tidy/checks/misc-no-inttoptr.rst =================================================================== --- /dev/null +++ clang-tools-extra/docs/clang-tidy/checks/misc-no-inttoptr.rst @@ -0,0 +1,45 @@ +.. title:: clang-tidy - misc-no-inttoptr + +misc-no-inttoptr +================ + +Diagnoses every integer to pointer cast. + +While casting an (integral) pointer to an integer is obvious - you just get +the integral value of the pointer, casting an integer to an (integral) pointer +is deceivingly different. While you will get a pointer with that integral value, +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. + +So while (integral) pointer to integer casts are effectively no-ops, +and are transparent to the optimizer, integer to (integral) pointer casts +are *NOT* transparent, and may conceal information from optimizer. + +While that may be the intention, it is not always so. For example, +let's take a look at a routine to align the pointer up to the multiple of 16: +The obvious, naive implementation for that is: + +.. code-block:: c++ + + char* src(char* maybe_underbiased_ptr) { + uintptr_t maybe_underbiased_intptr = (uintptr_t)maybe_underbiased_ptr; + uintptr_t aligned_biased_intptr = maybe_underbiased_intptr + 15; + uintptr_t aligned_intptr = aligned_biased_intptr & (~15); + return (char*)aligned_intptr; // warning: avoid integer to pointer casts [misc-no-inttoptr] + } + +The check will rightfully diagnose that cast. + +But when provenance concealment is not the goal of the code, but an accident, +this example can be rewritten as follows, without using integer to pointer cast: + +.. code-block:: c++ + + char* + tgt(char* maybe_underbiased_ptr) { + uintptr_t maybe_underbiased_intptr = (uintptr_t)maybe_underbiased_ptr; + uintptr_t aligned_biased_intptr = maybe_underbiased_intptr + 15; + uintptr_t aligned_intptr = aligned_biased_intptr & (~15); + uintptr_t bias = aligned_intptr - maybe_underbiased_intptr; + return maybe_underbiased_ptr + bias; + } Index: clang-tools-extra/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -201,6 +201,7 @@ `misc-definitions-in-headers <misc-definitions-in-headers.html>`_, "Yes" `misc-misplaced-const <misc-misplaced-const.html>`_, `misc-new-delete-overloads <misc-new-delete-overloads.html>`_, + `misc-no-inttoptr <misc-no-inttoptr.html>`_, `misc-no-recursion <misc-no-recursion.html>`_, `misc-non-copyable-objects <misc-non-copyable-objects.html>`_, `misc-non-private-member-variables-in-classes <misc-non-private-member-variables-in-classes.html>`_, Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -118,6 +118,11 @@ Alias to the :doc:`bugprone-signal-handler <clang-tidy/checks/bugprone-signal-handler>` check. +- New :doc:`misc-no-inttoptr + <clang-tidy/checks/misc-no-inttoptr>` check. + + Diagnoses every integer to pointer cast. + - New :doc:`readability-function-cognitive-complexity <clang-tidy/checks/readability-function-cognitive-complexity>` check. @@ -139,8 +144,8 @@ Now renames overridden virtual methods if the method they override has a style violation. - - Added support for specifying the style of scoped ``enum`` constants. If + + Added support for specifying the style of scoped ``enum`` constants. If unspecified, will fall back to the style for regular ``enum`` constants. - Removed `google-runtime-references` check because the rule it checks does Index: clang-tools-extra/clang-tidy/misc/NoIntToPtrCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/misc/NoIntToPtrCheck.h @@ -0,0 +1,34 @@ +//===--- NoIntToPtrCheck.h - clang-tidy -------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NOINTTOPTRCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NOINTTOPTRCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace misc { + +/// Diagnoses every integer to pointer cast. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/misc-no-inttoptr.html +class NoIntToPtrCheck : public ClangTidyCheck { +public: + NoIntToPtrCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace misc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NOINTTOPTRCHECK_H Index: clang-tools-extra/clang-tidy/misc/NoIntToPtrCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/misc/NoIntToPtrCheck.cpp @@ -0,0 +1,31 @@ +//===--- NoIntToPtrCheck.cpp - clang-tidy ---------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "NoIntToPtrCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace misc { + +void NoIntToPtrCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(castExpr(hasCastKind(CK_IntegralToPointer)).bind("x"), + this); +} + +void NoIntToPtrCheck::check(const MatchFinder::MatchResult &Result) { + const auto *MatchedCast = Result.Nodes.getNodeAs<CastExpr>("x"); + diag(MatchedCast->getBeginLoc(), "avoid integer to pointer casts"); +} + +} // namespace misc +} // namespace tidy +} // namespace clang Index: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp +++ clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp @@ -12,6 +12,7 @@ #include "DefinitionsInHeadersCheck.h" #include "MisplacedConstCheck.h" #include "NewDeleteOverloadsCheck.h" +#include "NoIntToPtrCheck.h" #include "NoRecursionCheck.h" #include "NonCopyableObjects.h" #include "NonPrivateMemberVariablesInClassesCheck.h" @@ -36,6 +37,7 @@ CheckFactories.registerCheck<MisplacedConstCheck>("misc-misplaced-const"); CheckFactories.registerCheck<NewDeleteOverloadsCheck>( "misc-new-delete-overloads"); + CheckFactories.registerCheck<NoIntToPtrCheck>("misc-no-inttoptr"); CheckFactories.registerCheck<NoRecursionCheck>("misc-no-recursion"); CheckFactories.registerCheck<NonCopyableObjectsCheck>( "misc-non-copyable-objects"); Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/misc/CMakeLists.txt +++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt @@ -8,6 +8,7 @@ MiscTidyModule.cpp MisplacedConstCheck.cpp NewDeleteOverloadsCheck.cpp + NoIntToPtrCheck.cpp NoRecursionCheck.cpp NonCopyableObjects.cpp NonPrivateMemberVariablesInClassesCheck.cpp
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits