aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorjade <lix@jade.fyi>2024-08-08 23:09:30 +0000
committerGerrit Code Review <gerrit@localhost>2024-08-08 23:09:30 +0000
commit9682ab4f3859ca60b0b4525452b27297e31cb751 (patch)
tree140cf7817556db65c80693f8b89a76c722b6d726
parent757041c3e74787c755b3de826078968119f706d6 (diff)
parenta5f0954c290157875b4dfb79edcf651f55742dc2 (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-tidy14
-rw-r--r--src/libstore/build/worker.cc3
-rw-r--r--src/libstore/crypto.cc13
-rw-r--r--src/libstore/sqlite.cc3
-rw-r--r--src/libutil/charptr-cast.hh1
-rw-r--r--src/libutil/compression.cc7
-rw-r--r--src/libutil/file-descriptor.cc3
-rw-r--r--src/libutil/tarfile.cc3
-rw-r--r--subprojects/lix-clang-tidy/CharPtrCast.cc45
-rw-r--r--subprojects/lix-clang-tidy/CharPtrCast.hh32
-rw-r--r--subprojects/lix-clang-tidy/LixClangTidyChecks.cc2
-rw-r--r--subprojects/lix-clang-tidy/meson.build3
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,