diff options
author | jade <lix@jade.fyi> | 2024-08-08 23:09:30 +0000 |
---|---|---|
committer | Gerrit Code Review <gerrit@localhost> | 2024-08-08 23:09:30 +0000 |
commit | 9682ab4f3859ca60b0b4525452b27297e31cb751 (patch) | |
tree | 140cf7817556db65c80693f8b89a76c722b6d726 | |
parent | 757041c3e74787c755b3de826078968119f706d6 (diff) | |
parent | a5f0954c290157875b4dfb79edcf651f55742dc2 (diff) |
Merge changes I6358a393,I2d9f276b,Idd096dc9 into main
* changes:
clang-tidy: write a lint for charptr_cast
tree-wide: automated migration to charptr_cast
clang-tidy: enforce the new rules
-rw-r--r-- | .clang-tidy | 14 | ||||
-rw-r--r-- | src/libstore/build/worker.cc | 3 | ||||
-rw-r--r-- | src/libstore/crypto.cc | 13 | ||||
-rw-r--r-- | src/libstore/sqlite.cc | 3 | ||||
-rw-r--r-- | src/libutil/charptr-cast.hh | 1 | ||||
-rw-r--r-- | src/libutil/compression.cc | 7 | ||||
-rw-r--r-- | src/libutil/file-descriptor.cc | 3 | ||||
-rw-r--r-- | src/libutil/tarfile.cc | 3 | ||||
-rw-r--r-- | subprojects/lix-clang-tidy/CharPtrCast.cc | 45 | ||||
-rw-r--r-- | subprojects/lix-clang-tidy/CharPtrCast.hh | 32 | ||||
-rw-r--r-- | subprojects/lix-clang-tidy/LixClangTidyChecks.cc | 2 | ||||
-rw-r--r-- | subprojects/lix-clang-tidy/meson.build | 3 |
12 files changed, 115 insertions, 14 deletions
diff --git a/.clang-tidy b/.clang-tidy index 0cc1f2520..87f6d0404 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -16,6 +16,20 @@ Checks: - -bugprone-unchecked-optional-access # many warnings, seems like a questionable lint - -bugprone-branch-clone + # all thrown exceptions must derive from std::exception + - hicpp-exception-baseclass + # capturing async lambdas are dangerous + - cppcoreguidelines-avoid-capturing-lambda-coroutines + # crimes must be appropriately declared as crimes + - cppcoreguidelines-pro-type-cstyle-cast + - lix-* + # This can not yet be applied to Lix itself since we need to do source + # reorganization so that lix/ include paths work. + - -lix-fixincludes + # This lint is included as an example, but the lib function it replaces is + # already gone. + - -lix-hasprefixsuffix + CheckOptions: bugprone-reserved-identifier.AllowedIdentifiers: '__asan_default_options' diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index aaa4e8907..a27cb0076 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -1,3 +1,4 @@ +#include "charptr-cast.hh" #include "machines.hh" #include "worker.hh" #include "substitution-goal.hh" @@ -539,7 +540,7 @@ void Worker::waitForInput() } else { printMsg(lvlVomit, "%1%: read %2% bytes", goal->getName(), rd); - std::string_view data(reinterpret_cast<char *>(buffer.data()), rd); + std::string_view data(charptr_cast<char *>(buffer.data()), rd); j->lastOutput = after; handleWorkResult(goal, goal->handleChildOutput(k, data)); } diff --git a/src/libstore/crypto.cc b/src/libstore/crypto.cc index e8ab15537..6f4a36735 100644 --- a/src/libstore/crypto.cc +++ b/src/libstore/crypto.cc @@ -1,3 +1,4 @@ +#include "charptr-cast.hh" #include "crypto.hh" #include "file-system.hh" #include "globals.hh" @@ -44,15 +45,15 @@ std::string SecretKey::signDetached(std::string_view data) const { unsigned char sig[crypto_sign_BYTES]; unsigned long long sigLen; - crypto_sign_detached(sig, &sigLen, reinterpret_cast<const unsigned char *>(data.data()), data.size(), - reinterpret_cast<const unsigned char *>(key.data())); + crypto_sign_detached(sig, &sigLen, charptr_cast<const unsigned char *>(data.data()), data.size(), + charptr_cast<const unsigned char *>(key.data())); return name + ":" + base64Encode(std::string(reinterpret_cast<char *>(sig), sigLen)); } PublicKey SecretKey::toPublicKey() const { unsigned char pk[crypto_sign_PUBLICKEYBYTES]; - crypto_sign_ed25519_sk_to_pk(pk, reinterpret_cast<const unsigned char *>(key.data())); + crypto_sign_ed25519_sk_to_pk(pk, charptr_cast<const unsigned char *>(key.data())); return PublicKey(name, std::string(reinterpret_cast<char *>(pk), crypto_sign_PUBLICKEYBYTES)); } @@ -85,9 +86,9 @@ bool verifyDetached(const std::string & data, const std::string & sig, if (sig2.size() != crypto_sign_BYTES) throw Error("signature is not valid"); - return crypto_sign_verify_detached(reinterpret_cast<unsigned char *>(sig2.data()), - reinterpret_cast<const unsigned char *>(data.data()), data.size(), - reinterpret_cast<const unsigned char *>(key->second.key.data())) == 0; + return crypto_sign_verify_detached(charptr_cast<unsigned char *>(sig2.data()), + charptr_cast<const unsigned char *>(data.data()), data.size(), + charptr_cast<const unsigned char *>(key->second.key.data())) == 0; } PublicKeys getDefaultPublicKeys() diff --git a/src/libstore/sqlite.cc b/src/libstore/sqlite.cc index 3114aad48..8d0bfcb11 100644 --- a/src/libstore/sqlite.cc +++ b/src/libstore/sqlite.cc @@ -1,3 +1,4 @@ +#include "charptr-cast.hh" #include "sqlite.hh" #include "globals.hh" #include "logging.hh" @@ -201,7 +202,7 @@ bool SQLiteStmt::Use::next() std::optional<std::string> SQLiteStmt::Use::getStrNullable(int col) { - auto s = reinterpret_cast<const char *>(sqlite3_column_text(stmt, col)); + auto s = charptr_cast<const char *>(sqlite3_column_text(stmt, col)); return s != nullptr ? std::make_optional<std::string>((s)) : std::nullopt; } diff --git a/src/libutil/charptr-cast.hh b/src/libutil/charptr-cast.hh index 990f2ec55..ec53d8924 100644 --- a/src/libutil/charptr-cast.hh +++ b/src/libutil/charptr-cast.hh @@ -134,6 +134,7 @@ template<typename To, typename From> requires charptr_cast_detail::IsCharCastable<From, To> inline To charptr_cast(From p) { + // NOLINTNEXTLINE(lix-charptrcast): stop the linter ever getting too clever and causing funny recursion return reinterpret_cast<To>(p); } diff --git a/src/libutil/compression.cc b/src/libutil/compression.cc index 6b0fa9d15..5152a2146 100644 --- a/src/libutil/compression.cc +++ b/src/libutil/compression.cc @@ -1,3 +1,4 @@ +#include "charptr-cast.hh" #include "compression.hh" #include "tarfile.hh" #include "signals.hh" @@ -160,7 +161,7 @@ struct BrotliDecompressionSource : Source size_t read(char * data, size_t len) override { - uint8_t * out = reinterpret_cast<uint8_t *>(data); + uint8_t * out = charptr_cast<uint8_t *>(data); const auto * begin = out; while (len && !BrotliDecoderIsFinished(state.get())) { @@ -172,7 +173,7 @@ struct BrotliDecompressionSource : Source } catch (EndOfFile &) { break; } - next_in = reinterpret_cast<const uint8_t *>(buf.get()); + next_in = charptr_cast<const uint8_t *>(buf.get()); } if (!BrotliDecoderDecompressStream( @@ -238,7 +239,7 @@ struct BrotliCompressionSink : ChunkedCompressionSink void writeInternal(std::string_view data) override { - auto next_in = reinterpret_cast<const uint8_t *>(data.data()); + auto next_in = charptr_cast<const uint8_t *>(data.data()); size_t avail_in = data.size(); uint8_t * next_out = outbuf; size_t avail_out = sizeof(outbuf); diff --git a/src/libutil/file-descriptor.cc b/src/libutil/file-descriptor.cc index 7c82988b3..be9f8c889 100644 --- a/src/libutil/file-descriptor.cc +++ b/src/libutil/file-descriptor.cc @@ -1,3 +1,4 @@ +#include "charptr-cast.hh" #include "file-system.hh" #include "finally.hh" #include "logging.hh" @@ -115,7 +116,7 @@ Generator<Bytes> drainFDSource(int fd, bool block) throw SysError("reading from file"); } else if (rd == 0) break; - else co_yield std::span{reinterpret_cast<char *>(buf.data()), (size_t) rd}; + else co_yield std::span{charptr_cast<char *>(buf.data()), (size_t) rd}; } } diff --git a/src/libutil/tarfile.cc b/src/libutil/tarfile.cc index f024149ec..316751533 100644 --- a/src/libutil/tarfile.cc +++ b/src/libutil/tarfile.cc @@ -1,6 +1,7 @@ #include <archive.h> #include <archive_entry.h> +#include "charptr-cast.hh" #include "file-system.hh" #include "logging.hh" #include "serialise.hh" @@ -19,7 +20,7 @@ static ssize_t callback_read(struct archive * archive, void * _self, const void *buffer = self->buffer.data(); try { - return self->source->read(reinterpret_cast<char *>(self->buffer.data()), self->buffer.size()); + return self->source->read(charptr_cast<char *>(self->buffer.data()), self->buffer.size()); } catch (EndOfFile &) { return 0; } catch (std::exception & err) { diff --git a/subprojects/lix-clang-tidy/CharPtrCast.cc b/subprojects/lix-clang-tidy/CharPtrCast.cc new file mode 100644 index 000000000..e76797f5d --- /dev/null +++ b/subprojects/lix-clang-tidy/CharPtrCast.cc @@ -0,0 +1,45 @@ +#include "CharPtrCast.hh" +#include <clang/AST/ExprCXX.h> +#include <clang/Basic/Diagnostic.h> +#include <clang/Tooling/Transformer/SourceCode.h> + +namespace nix::clang_tidy { +using namespace clang::ast_matchers; +using namespace clang; + +void CharPtrCastCheck::registerMatchers(ast_matchers::MatchFinder *Finder) { + Finder->addMatcher( + traverse(clang::TK_IgnoreUnlessSpelledInSource, + cxxReinterpretCastExpr(allOf( + hasDestinationType(qualType(pointsTo(isAnyCharacter()))), + has(expr(hasType(qualType(pointsTo(isAnyCharacter())))))))) + .bind("reinterpret-cast-expr"), + this); +} + +void CharPtrCastCheck::check( + const ast_matchers::MatchFinder::MatchResult &Result) { + const auto ReinterpretCastExpr = + Result.Nodes.getNodeAs<CXXReinterpretCastExpr>("reinterpret-cast-expr"); + const auto ToTypeSpan = ReinterpretCastExpr->getAngleBrackets(); + const auto & SM = Result.Context->getSourceManager(); + + auto Diag = + diag(ReinterpretCastExpr->getExprLoc(), + "reinterpret_cast used for trivially safe character pointer cast"); + Diag << ReinterpretCastExpr->getSourceRange(); + + auto Inside = tooling::getText(*ReinterpretCastExpr->getSubExprAsWritten(), + *Result.Context); + + Diag << Inserter.createIncludeInsertion(SM.getFileID(ReinterpretCastExpr->getExprLoc()), "charptr-cast.hh"); + + llvm::Twine Replacement = + "charptr_cast" + + tooling::getText(CharSourceRange(ToTypeSpan, true), *Result.Context) + + "(" + Inside + ")"; + Diag << FixItHint::CreateReplacement(ReinterpretCastExpr->getSourceRange(), + Replacement.str()); +} + +} // namespace nix::clang_tidy diff --git a/subprojects/lix-clang-tidy/CharPtrCast.hh b/subprojects/lix-clang-tidy/CharPtrCast.hh new file mode 100644 index 000000000..66883d055 --- /dev/null +++ b/subprojects/lix-clang-tidy/CharPtrCast.hh @@ -0,0 +1,32 @@ +#pragma once +///@file + +#include <clang-tidy/ClangTidyCheck.h> +#include <clang-tidy/utils/IncludeInserter.h> +#include <clang/ASTMatchers/ASTMatchFinder.h> +#include <llvm/ADT/StringRef.h> + +namespace nix::clang_tidy { + +using namespace clang; +using namespace clang::tidy; + +class CharPtrCastCheck : public ClangTidyCheck { + tidy::utils::IncludeInserter Inserter{ + Options.getLocalOrGlobal("IncludeStyle", + tidy::utils::IncludeSorter::IS_Google), + false}; + +public: + CharPtrCastCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + + void registerPPCallbacks(const SourceManager &, Preprocessor *PP, + Preprocessor *) override { + Inserter.registerPreprocessor(PP); + } + + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; +} // namespace nix::clang_tidy diff --git a/subprojects/lix-clang-tidy/LixClangTidyChecks.cc b/subprojects/lix-clang-tidy/LixClangTidyChecks.cc index b3503dd3a..283e0e7c8 100644 --- a/subprojects/lix-clang-tidy/LixClangTidyChecks.cc +++ b/subprojects/lix-clang-tidy/LixClangTidyChecks.cc @@ -2,6 +2,7 @@ #include <clang-tidy/ClangTidyModuleRegistry.h> #include "FixIncludes.hh" #include "HasPrefixSuffix.hh" +#include "CharPtrCast.hh" namespace nix::clang_tidy { using namespace clang; @@ -12,6 +13,7 @@ class NixClangTidyChecks : public ClangTidyModule { void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck<HasPrefixSuffixCheck>("lix-hasprefixsuffix"); CheckFactories.registerCheck<FixIncludesCheck>("lix-fixincludes"); + CheckFactories.registerCheck<CharPtrCastCheck>("lix-charptrcast"); } }; diff --git a/subprojects/lix-clang-tidy/meson.build b/subprojects/lix-clang-tidy/meson.build index ef0226420..43648a1c8 100644 --- a/subprojects/lix-clang-tidy/meson.build +++ b/subprojects/lix-clang-tidy/meson.build @@ -5,9 +5,10 @@ project('lix-clang-tidy', ['cpp', 'c'], llvm = dependency('Clang', version: '>= 17', modules: ['libclang']) sources = files( + 'CharPtrCast.cc', + 'FixIncludes.cc', 'HasPrefixSuffix.cc', 'LixClangTidyChecks.cc', - 'FixIncludes.cc', ) lix_clang_tidy = shared_module('lix-clang-tidy', sources, |