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 "SizeofContainerCheck.h" #include "SizeofExpressionCheck.h" @@ -117,6 +118,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 SizeofContainerCheck.cpp SizeofExpressionCheck.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,42 @@ +//===--- 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 { + +/// Check for cast of a pointer to wider (even unsigned) integer. This will +/// sign-extend the pointer which happens on 32-bit hosts for 64-bit integers. +/// +/// \code +/// void *p=(void *)0x80000000; +/// printf( "%p" "\n", p ); // 0x80000000 +/// printf("0x%" PRIxPTR "\n", (uintptr_t)p ); // 0x80000000 +/// 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 +/// \endcode +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; +}; + +} // 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,54 @@ +//===--- 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) { + if (!getLangOpts().CPlusPlus && !getLangOpts().C99) + return; + + Finder->addMatcher(castExpr(unless(isInTemplateInstantiation())).bind("cast"), + this); +} + +void PointerCastWideningCheck::check(const MatchFinder::MatchResult &Result) { + const auto *MatchedCast = Result.Nodes.getNodeAs("cast"); + + if (MatchedCast->getCastKind() != CK_PointerToIntegral) + return; + + QualType DestType = MatchedCast->getType(); + for (QualType DestTypeCheck = DestType;;) { + if (DestTypeCheck.getAsString() == "uintptr_t") + return; + QualType DestTypeNext = + DestTypeCheck.getSingleStepDesugaredType(*Result.Context); + if (DestTypeNext == DestTypeCheck) + break; + DestTypeCheck = DestTypeNext; + } + + 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; +} + +} // 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 @@ Without the null terminator it can result in undefined behaviour when the string is read. +- New :doc:`bugprone-pointer-cast-widening + ` check + + Check for cast of a pointer to wider (even unsigned) integer. This will + sign-extend the pointer which happens on 32-bit hosts for 64-bit integers. + - New :doc:`cert-mem57-cpp ` 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 +============================== + +Check for cast of a pointer to wider (even unsigned) integer. This will +sign-extend 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 @@ -62,6 +62,7 @@ bugprone-multiple-statement-macro bugprone-not-null-terminated-result bugprone-parent-virtual-call + bugprone-pointer-cast-widening bugprone-posix-return bugprone-sizeof-container bugprone-sizeof-expression 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,17 @@ +// RUN: %check_clang_tidy %s bugprone-pointer-cast-widening %t + +#include + +void test(void) { + void *p = 0; + uint64_t u64 = (uint64_t)p; + // CHECK-MESSAGES: :[[@LINE-1]]:18: 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 u64r = reinterpret_cast(p); + // CHECK-MESSAGES: :[[@LINE-1]]:19: 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] + intptr_t ip = reinterpret_cast(p); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not use cast of a pointer 'void *' to non-uintptr_t 'intptr_t' (aka 'long') which may sign-extend [bugprone-pointer-cast-widening] + uintptr_t up = (uintptr_t)p; + typedef uintptr_t t1; + typedef t1 t2; + t2 t = (t2)p; +}