forked from llvm/llvm-project
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[clang-tidy] Add bugprone-suspicious-stringview-data-usage check (llv…
…m#83716) This check identifies suspicious usages of std::string_view::data() that could lead to reading out-of-bounds data due to inadequate or incorrect string null termination. Closes llvm#80854
- Loading branch information
Showing
9 changed files
with
268 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
97 changes: 97 additions & 0 deletions
97
clang-tools-extra/clang-tidy/bugprone/SuspiciousStringviewDataUsageCheck.cpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
//===--- SuspiciousStringviewDataUsageCheck.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 "SuspiciousStringviewDataUsageCheck.h" | ||
#include "../utils/Matchers.h" | ||
#include "../utils/OptionsUtils.h" | ||
#include "clang/AST/ASTContext.h" | ||
#include "clang/ASTMatchers/ASTMatchFinder.h" | ||
|
||
using namespace clang::ast_matchers; | ||
|
||
namespace clang::tidy::bugprone { | ||
|
||
SuspiciousStringviewDataUsageCheck::SuspiciousStringviewDataUsageCheck( | ||
StringRef Name, ClangTidyContext *Context) | ||
: ClangTidyCheck(Name, Context), | ||
StringViewTypes(utils::options::parseStringList(Options.get( | ||
"StringViewTypes", "::std::basic_string_view;::llvm::StringRef"))), | ||
AllowedCallees( | ||
utils::options::parseStringList(Options.get("AllowedCallees", ""))) {} | ||
|
||
void SuspiciousStringviewDataUsageCheck::storeOptions( | ||
ClangTidyOptions::OptionMap &Opts) { | ||
Options.store(Opts, "StringViewTypes", | ||
utils::options::serializeStringList(StringViewTypes)); | ||
Options.store(Opts, "AllowedCallees", | ||
utils::options::serializeStringList(AllowedCallees)); | ||
} | ||
|
||
bool SuspiciousStringviewDataUsageCheck::isLanguageVersionSupported( | ||
const LangOptions &LangOpts) const { | ||
return LangOpts.CPlusPlus; | ||
} | ||
|
||
std::optional<TraversalKind> | ||
SuspiciousStringviewDataUsageCheck::getCheckTraversalKind() const { | ||
return TK_AsIs; | ||
} | ||
|
||
void SuspiciousStringviewDataUsageCheck::registerMatchers(MatchFinder *Finder) { | ||
|
||
auto AncestorCall = anyOf( | ||
cxxConstructExpr(), callExpr(unless(cxxOperatorCallExpr())), lambdaExpr(), | ||
initListExpr( | ||
hasType(qualType(hasCanonicalType(hasDeclaration(recordDecl())))))); | ||
|
||
auto DataMethod = | ||
cxxMethodDecl(hasName("data"), | ||
ofClass(matchers::matchesAnyListedName(StringViewTypes))); | ||
|
||
auto SizeCall = cxxMemberCallExpr( | ||
callee(cxxMethodDecl(hasAnyName("size", "length"))), | ||
on(ignoringParenImpCasts( | ||
matchers::isStatementIdenticalToBoundNode("self")))); | ||
|
||
auto DescendantSizeCall = expr(hasDescendant( | ||
expr(SizeCall, hasAncestor(expr(AncestorCall).bind("ancestor-size")), | ||
hasAncestor(expr(equalsBoundNode("parent"), | ||
equalsBoundNode("ancestor-size")))))); | ||
|
||
Finder->addMatcher( | ||
cxxMemberCallExpr( | ||
on(ignoringParenImpCasts(expr().bind("self"))), callee(DataMethod), | ||
expr().bind("data-call"), | ||
hasParent(expr(anyOf( | ||
invocation( | ||
expr().bind("parent"), unless(cxxOperatorCallExpr()), | ||
hasAnyArgument( | ||
ignoringParenImpCasts(equalsBoundNode("data-call"))), | ||
unless(hasAnyArgument(ignoringParenImpCasts(SizeCall))), | ||
unless(hasAnyArgument(DescendantSizeCall)), | ||
hasDeclaration(namedDecl( | ||
unless(matchers::matchesAnyListedName(AllowedCallees))))), | ||
initListExpr(expr().bind("parent"), | ||
hasType(qualType(hasCanonicalType(hasDeclaration( | ||
recordDecl(unless(matchers::matchesAnyListedName( | ||
AllowedCallees))))))), | ||
unless(DescendantSizeCall)))))), | ||
this); | ||
} | ||
|
||
void SuspiciousStringviewDataUsageCheck::check( | ||
const MatchFinder::MatchResult &Result) { | ||
const auto *DataCallExpr = | ||
Result.Nodes.getNodeAs<CXXMemberCallExpr>("data-call"); | ||
diag(DataCallExpr->getExprLoc(), | ||
"result of a `data()` call may not be null terminated, provide size " | ||
"information to the callee to prevent potential issues") | ||
<< DataCallExpr->getCallee()->getSourceRange(); | ||
} | ||
|
||
} // namespace clang::tidy::bugprone |
38 changes: 38 additions & 0 deletions
38
clang-tools-extra/clang-tidy/bugprone/SuspiciousStringviewDataUsageCheck.h
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
//===--- SuspiciousStringviewDataUsageCheck.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_SUSPICIOUSSTRINGVIEWDATAUSAGECHECK_H | ||
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSSTRINGVIEWDATAUSAGECHECK_H | ||
|
||
#include "../ClangTidyCheck.h" | ||
|
||
namespace clang::tidy::bugprone { | ||
|
||
/// Identifies suspicious usages of std::string_view::data() that could lead to | ||
/// reading out-of-bounds data due to inadequate or incorrect string null | ||
/// termination. | ||
/// | ||
/// For the user-facing documentation see: | ||
/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/suspicious-stringview-data-usage.html | ||
class SuspiciousStringviewDataUsageCheck : public ClangTidyCheck { | ||
public: | ||
SuspiciousStringviewDataUsageCheck(StringRef Name, ClangTidyContext *Context); | ||
void registerMatchers(ast_matchers::MatchFinder *Finder) override; | ||
void check(const ast_matchers::MatchFinder::MatchResult &Result) override; | ||
void storeOptions(ClangTidyOptions::OptionMap &Opts) override; | ||
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override; | ||
std::optional<TraversalKind> getCheckTraversalKind() const override; | ||
|
||
private: | ||
std::vector<llvm::StringRef> StringViewTypes; | ||
std::vector<llvm::StringRef> AllowedCallees; | ||
}; | ||
|
||
} // namespace clang::tidy::bugprone | ||
|
||
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSSTRINGVIEWDATAUSAGECHECK_H |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
58 changes: 58 additions & 0 deletions
58
...ools-extra/docs/clang-tidy/checks/bugprone/suspicious-stringview-data-usage.rst
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
.. title:: clang-tidy - bugprone-suspicious-stringview-data-usage | ||
|
||
bugprone-suspicious-stringview-data-usage | ||
========================================= | ||
|
||
Identifies suspicious usages of ``std::string_view::data()`` that could lead to | ||
reading out-of-bounds data due to inadequate or incorrect string null | ||
termination. | ||
|
||
It warns when the result of ``data()`` is passed to a constructor or function | ||
without also passing the corresponding result of ``size()`` or ``length()`` | ||
member function. Such usage can lead to unintended behavior, particularly when | ||
assuming the data pointed to by ``data()`` is null-terminated. | ||
|
||
The absence of a ``c_str()`` method in ``std::string_view`` often leads | ||
developers to use ``data()`` as a substitute, especially when interfacing with | ||
C APIs that expect null-terminated strings. However, since ``data()`` does not | ||
guarantee null termination, this can result in unintended behavior if the API | ||
relies on proper null termination for correct string interpretation. | ||
|
||
In today's programming landscape, this scenario can occur when implicitly | ||
converting an ``std::string_view`` to an ``std::string``. Since the constructor | ||
in ``std::string`` designed for string-view-like objects is ``explicit``, | ||
attempting to pass an ``std::string_view`` to a function expecting an | ||
``std::string`` will result in a compilation error. As a workaround, developers | ||
may be tempted to utilize the ``.data()`` method to achieve compilation, | ||
introducing potential risks. | ||
|
||
For instance: | ||
|
||
.. code-block:: c++ | ||
|
||
void printString(const std::string& str) { | ||
std::cout << "String: " << str << std::endl; | ||
} | ||
|
||
void something(std::string_view sv) { | ||
printString(sv.data()); | ||
} | ||
|
||
In this example, directly passing ``sv`` to the ``printString`` function would | ||
lead to a compilation error due to the explicit nature of the ``std::string`` | ||
constructor. Consequently, developers might opt for ``sv.data()`` to resolve the | ||
compilation error, albeit introducing potential hazards as discussed. | ||
|
||
.. option:: StringViewTypes | ||
|
||
Option allows users to specify custom string view-like types for analysis. It | ||
accepts a semicolon-separated list of type names or regular expressions | ||
matching these types. Default value is: | ||
`::std::basic_string_view;::llvm::StringRef`. | ||
|
||
.. option:: AllowedCallees | ||
|
||
Specifies methods, functions, or classes where the result of ``.data()`` is | ||
passed to. Allows to exclude such calls from the analysis. Accepts a | ||
semicolon-separated list of names or regular expressions matching these | ||
entities. Default value is: empty string. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
56 changes: 56 additions & 0 deletions
56
clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-stringview-data-usage.cpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-suspicious-stringview-data-usage %t -- -- -isystem %clang_tidy_headers | ||
#include <string> | ||
|
||
struct View { | ||
const char* str; | ||
}; | ||
|
||
struct Pair { | ||
const char* begin; | ||
const char* end; | ||
}; | ||
|
||
struct ViewWithSize { | ||
const char* str; | ||
std::string_view::size_type size; | ||
}; | ||
|
||
void something(const char*); | ||
void something(const char*, unsigned); | ||
void something(const char*, unsigned, const char*); | ||
void something_str(std::string, unsigned); | ||
|
||
void invalid(std::string_view sv, std::string_view sv2) { | ||
std::string s(sv.data()); | ||
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: result of a `data()` call may not be null terminated, provide size information to the callee to prevent potential issues | ||
std::string si{sv.data()}; | ||
// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: result of a `data()` call may not be null terminated, provide size information to the callee to prevent potential issues | ||
std::string_view s2(sv.data()); | ||
// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: result of a `data()` call may not be null terminated, provide size information to the callee to prevent potential issues | ||
something(sv.data()); | ||
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: result of a `data()` call may not be null terminated, provide size information to the callee to prevent potential issues | ||
something(sv.data(), sv.size(), sv2.data()); | ||
// CHECK-MESSAGES: :[[@LINE-1]]:39: warning: result of a `data()` call may not be null terminated, provide size information to the callee to prevent potential issues | ||
something_str(sv.data(), sv.size()); | ||
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: result of a `data()` call may not be null terminated, provide size information to the callee to prevent potential issues | ||
View view{sv.data()}; | ||
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: result of a `data()` call may not be null terminated, provide size information to the callee to prevent potential issues | ||
} | ||
|
||
void valid(std::string_view sv) { | ||
std::string s1(sv.data(), sv.data() + sv.size()); | ||
std::string s2(sv.data(), sv.data() + sv.length()); | ||
std::string s3(sv.data(), sv.size() + sv.data()); | ||
std::string s4(sv.data(), sv.length() + sv.data()); | ||
std::string s5(sv.data(), sv.size()); | ||
std::string s6(sv.data(), sv.length()); | ||
something(sv.data(), sv.size()); | ||
something(sv.data(), sv.length()); | ||
ViewWithSize view1{sv.data(), sv.size()}; | ||
ViewWithSize view2{sv.data(), sv.length()}; | ||
Pair view3{sv.data(), sv.data() + sv.size()}; | ||
Pair view4{sv.data(), sv.data() + sv.length()}; | ||
Pair view5{sv.data(), sv.size() + sv.data()}; | ||
Pair view6{sv.data(), sv.length() + sv.data()}; | ||
const char* str{sv.data()}; | ||
} |