diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -35,6 +35,7 @@ #include "MultipleStatementMacroCheck.h" #include "NotNullTerminatedResultCheck.h" #include "ParentVirtualCallCheck.h" +#include "PointerCastWideningCheck.h" #include "PosixReturnCheck.h" #include "SignedCharMisuseCheck.h" #include "SizeofContainerCheck.h" @@ -118,6 +119,8 @@ "bugprone-not-null-terminated-result"); CheckFactories.registerCheck( "bugprone-parent-virtual-call"); + CheckFactories.registerCheck( + "bugprone-pointer-cast-widening"); CheckFactories.registerCheck( "bugprone-posix-return"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -27,6 +27,7 @@ MultipleStatementMacroCheck.cpp NotNullTerminatedResultCheck.cpp ParentVirtualCallCheck.cpp + PointerCastWideningCheck.cpp PosixReturnCheck.cpp SignedCharMisuseCheck.cpp SizeofContainerCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/PointerCastWideningCheck.h b/clang-tools-extra/clang-tidy/bugprone/PointerCastWideningCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/PointerCastWideningCheck.h @@ -0,0 +1,38 @@ +//===--- PointerCastWideningCheck.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_BUGPRONE_POINTER_CAST_WIDENING_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_POINTER_CAST_WIDENING_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Checks for cast of a pointer to wider (even unsigned) integer as it +/// sign-extends the pointer which happens on 32-bit hosts for 64-bit integers. +class PointerCastWideningCheck : public ClangTidyCheck { +public: + PointerCastWideningCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + bool find_type(const llvm::StringSet<> &strset, QualType CheckType, + const ASTContext &Context) const; + void check_pointer(const CastExpr *MatchedCast, const ASTContext &Context); + void check_integral(const CastExpr *MatchedCast, const ASTContext &Context); +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_POINTER_CAST_WIDENING_H diff --git a/clang-tools-extra/clang-tidy/bugprone/PointerCastWideningCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/PointerCastWideningCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/PointerCastWideningCheck.cpp @@ -0,0 +1,93 @@ +//===--- PointerCastWideningCheck.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 "PointerCastWideningCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace bugprone { + +void PointerCastWideningCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(castExpr(anyOf(hasCastKind(CK_PointerToIntegral), + hasCastKind(CK_IntegralCast)), + unless(isInTemplateInstantiation())) + .bind("cast"), + this); +} + +bool PointerCastWideningCheck::find_type(const llvm::StringSet<> &strset, + QualType CheckType, + const ASTContext &Context) const { + for (;;) { + std::string str = CheckType.getAsString(); + if (strset.count(str)) + return true; + QualType CheckTypeNext = CheckType.getSingleStepDesugaredType(Context); + if (CheckTypeNext == CheckType) + return false; + CheckType = CheckTypeNext; + } +} + +void PointerCastWideningCheck::check_pointer(const CastExpr *MatchedCast, + const ASTContext &Context) { + QualType DestType = MatchedCast->getType(); + + static const auto intptrs = llvm::StringSet<>{"uintptr_t", "intptr_t"}; + if (find_type(intptrs, DestType, Context)) + return; + + QualType SourceType = MatchedCast->getSubExpr()->getType(); + + diag(MatchedCast->getBeginLoc(), "do not use cast of a pointer %0 to " + "non-uintptr_t %1 which may sign-extend") + << SourceType << DestType; +} + +void PointerCastWideningCheck::check_integral(const CastExpr *MatchedCast, + const ASTContext &Context) { + QualType SourceType = MatchedCast->getSubExpr()->getType(); + QualType DestType = MatchedCast->getType(); + + uint64_t SourceBits = Context.getTypeSize(SourceType); + uint64_t DestBits = Context.getTypeSize(DestType); + if (DestBits <= SourceBits) + return; + + static const auto intptr = llvm::StringSet<>{"intptr_t"}; + if (!find_type(intptr, SourceType, Context)) + return; + + diag(MatchedCast->getBeginLoc(), "do not use cast of pointer-like %0 to " + "a wider type %1 which will sign-extend") + << SourceType << DestType; +} + +void PointerCastWideningCheck::check(const MatchFinder::MatchResult &Result) { + const CastExpr *MatchedCast = Result.Nodes.getNodeAs("cast"); + + switch (MatchedCast->getCastKind()) { + case CK_PointerToIntegral: + check_pointer(MatchedCast, *Result.Context); + break; + case CK_IntegralCast: + check_integral(MatchedCast, *Result.Context); + break; + default: + break; + } +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -94,6 +94,12 @@ Finds function calls where it is possible to cause a not null-terminated result. +- New :doc:`bugprone-pointer-cast-widening + ` check + + Checks for cast of a pointer to wider (even unsigned) integer as it + sign-extends the pointer which happens on 32-bit hosts for 64-bit integers. + - New :doc:`bugprone-signed-char-misuse ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-pointer-cast-widening.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-pointer-cast-widening.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-pointer-cast-widening.rst @@ -0,0 +1,26 @@ +.. title:: clang-tidy - bugprone-pointer-cast-widening + +bugprone-pointer-cast-widening +============================== + +Checks for cast of a pointer to wider (even unsigned) integer as it +sign-extends the pointer which happens on 32-bit hosts for 64-bit integers. +The buggy behavior are 32-bit addresses printed like ``0xffffffffd56b5d60``. + +Casting from 32-bit ``void *`` to ``uint64_t`` requires an intermediate cast to +``uintptr_t`` otherwise the pointer gets sign-extended: + +.. code-block:: c++ + + void *p=(void *)0x80000000; + printf( "%p" "\n", p );//0x80000000 + printf("0x%" PRIxPTR "\n", (uintptr_t) p );//0x80000000 + printf("0x%" PRIxPTR "\n",reinterpret_cast(p));//0x80000000 + // ^^^^^^^^^^^^^^^^^^^^^^^^^^^ recommended + printf("0x%" PRIx64 "\n",reinterpret_cast(p));//0xffffffff80000000 + printf("0x%" PRIx64 "\n", (uint64_t ) p );//0xffffffff80000000 + printf("0x%" PRIx64 "\n", (uint64_t)(uintptr_t) p );//0x80000000 + +The only supported cast from a pointer to integer is ``uintptr_t``. + +Casting to ``int64_t`` is discouraged as it leads to the sign-extensions again. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -70,6 +70,7 @@ `bugprone-multiple-statement-macro `_, `bugprone-not-null-terminated-result `_, "Yes" `bugprone-parent-virtual-call `_, "Yes" + `bugprone-pointer-cast-widening `_, "Yes" `bugprone-posix-return `_, "Yes" `bugprone-signed-char-misuse `_, `bugprone-sizeof-container `_, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-pointer-cast-widening.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-pointer-cast-widening.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-pointer-cast-widening.cpp @@ -0,0 +1,38 @@ +// RUN: %check_clang_tidy %s bugprone-pointer-cast-widening %t + +// +typedef __UINT64_TYPE__ uint64_t; +typedef __INTPTR_TYPE__ intptr_t; +typedef __UINTPTR_TYPE__ uintptr_t; + +// Do not assume ull_ns::uintptr_t to be uintptr_t. +namespace ull_ns { +typedef unsigned long long uintptr_t; +} +// This is how std::uintptr_t is defined in : +namespace cstdint_ns { +using ::uintptr_t; +} + +void test() { + void *p = 0; + uint64_t u64cstyle = (uint64_t)p; + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not use cast of a pointer 'void *' to non-uintptr_t 'uint64_t' (aka 'unsigned long') which may sign-extend [bugprone-pointer-cast-widening] + // uint64_t u64implicit = p; // error: cannot initialize a variable of type 'uint64_t' (aka 'unsigned long') with an lvalue of type 'void *' [clang-diagnostic-error] + uint64_t u64reinterpret = reinterpret_cast(p); + // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: do not use cast of a pointer 'void *' to non-uintptr_t 'uint64_t' (aka 'unsigned long') which may sign-extend [bugprone-pointer-cast-widening] + // uint64_t u64static = static_cast(p); // error: static_cast from 'void *' to 'uint64_t' (aka 'unsigned long') is not allowed + uintptr_t up = (uintptr_t)p; + auto cup = (cstdint_ns::uintptr_t)p; + auto wup = (ull_ns::uintptr_t)p; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use cast of a pointer 'void *' to non-uintptr_t 'ull_ns::uintptr_t' (aka 'unsigned long long') which may sign-extend [bugprone-pointer-cast-widening] + typedef uintptr_t t1; + typedef t1 t2; + t2 t = (t2)p; + intptr_t ip = reinterpret_cast(p); + __uint128_t u64ipimplicit = ip; + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: do not use cast of pointer-like 'intptr_t' (aka 'long') to a wider type '__uint128_t' (aka 'unsigned __int128') which will sign-extend [bugprone-pointer-cast-widening] + __uint128_t u64ipstatic = static_cast<__uint128_t>(ip); + // CHECK-MESSAGES: :[[@LINE-1]]:54: warning: do not use cast of pointer-like 'intptr_t' (aka 'long') to a wider type '__uint128_t' (aka 'unsigned __int128') which will sign-extend [bugprone-pointer-cast-widening] + // __uint128_t u64ipreinterpret = reinterpret_cast<__uint128_t>(ip); // error: reinterpret_cast from 'intptr_t' (aka 'long') to '__uint128_t' (aka 'unsigned __int128') is not allowed +}