[-Wunsafe-buffer-usage] Emit a warning if pointer returned by vector::data and array::data is cast to larger type #111910
Conversation
This file contains hidden or 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
…:data and array::data is cast to larger type Emit a warning when the raw pointer retrieved from std::vector and std::array instances are cast to a larger type. Such a cast followed by a field dereference to the resulting pointer could cause an OOB access. This is similar to the existing span::data warning. (rdar://136704278)
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang-analysis @llvm/pr-subscribers-clang Author: Malavika Samak (malavikasamak) ChangesEmit a warning when the raw pointer retrieved from std::vector and std::array instances are cast to a larger type. Such a cast followed by a field dereference to the resulting pointer could cause an OOB access. This is similar to the existing span::data warning. (rdar://136704278) Full diff: https://github.com/llvm/llvm-project/pull/111910.diff 4 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f4a2d4a3f0656a..301a0d46d88390 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12497,7 +12497,7 @@ def warn_unsafe_buffer_variable : Warning<
InGroup<UnsafeBufferUsage>, DefaultIgnore;
def warn_unsafe_buffer_operation : Warning<
"%select{unsafe pointer operation|unsafe pointer arithmetic|"
- "unsafe buffer access|function introduces unsafe buffer manipulation|unsafe invocation of span::data|"
+ "unsafe buffer access|function introduces unsafe buffer manipulation|unsafe invocation of %1|"
"field %1 prone to unsafe buffer manipulation}0">,
InGroup<UnsafeBufferUsage>, DefaultIgnore;
def warn_unsafe_buffer_libc_call : Warning<
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 97f1c4f16b8f4c..5e0ec9ecc92ea4 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1499,8 +1499,11 @@ class DataInvocationGadget : public WarningGadget {
}
static Matcher matcher() {
- Matcher callExpr = cxxMemberCallExpr(
- callee(cxxMethodDecl(hasName("data"), ofClass(hasName("std::span")))));
+
+ Matcher callExpr = cxxMemberCallExpr(callee(
+ cxxMethodDecl(hasName("data"),
+ ofClass(anyOf(hasName("std::span"), hasName("std::array"),
+ hasName("std::vector"))))));
return stmt(
explicitCastExpr(anyOf(has(callExpr), has(parenExpr(has(callExpr)))))
.bind(OpTag));
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 6496a33b8f5a50..c76733e9a774b6 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2279,9 +2279,18 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
QualType srcType = ECE->getSubExpr()->getType();
const uint64_t sSize =
Ctx.getTypeSize(srcType.getTypePtr()->getPointeeType());
+
if (sSize >= dSize)
return;
+ if (const auto *CE = dyn_cast<CXXMemberCallExpr>(
+ ECE->getSubExpr()->IgnoreParens())) {
+ D = CE->getMethodDecl();
+ }
+
+ if (!D)
+ return;
+
MsgParam = 4;
}
Loc = Operation->getBeginLoc();
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
index 08707d7ff545d8..0228e42652bd95 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
@@ -32,38 +32,68 @@ void foo(int *p){}
namespace std{
template <typename T> class span {
- T *elements;
+ T *elements;
- span(T *, unsigned){}
+ span(T *, unsigned){}
- public:
+ public:
- constexpr span<T> subspan(size_t offset, size_t count) const {
- return span<T> (elements+offset, count); // expected-warning{{unsafe pointer arithmetic}}
- }
+ constexpr span<T> subspan(size_t offset, size_t count) const {
+ return span<T> (elements+offset, count); // expected-warning{{unsafe pointer arithmetic}}
+ }
- constexpr T* data() const noexcept {
- return elements;
- }
+ constexpr T* data() const noexcept {
+ return elements;
+ }
+
+ constexpr T* hello() const noexcept {
+ return elements;
+ }
+ };
+
+ template <typename T> class vector {
+
+ T *elements;
+
+ public:
+
+ vector(size_t n) {
+ elements = new T[n];
+ }
+
+ constexpr T* data() const noexcept {
+ return elements;
+ }
+
+ ~vector() {
+ delete[] elements;
+ }
+ };
+
+ template <class T, size_t N>
+ class array {
+ T elements[N];
+
+ public:
+
+ constexpr const T* data() const noexcept {
+ return elements;
+ }
+
+ };
-
- constexpr T* hello() const noexcept {
- return elements;
- }
-};
-
template <typename T> class span_duplicate {
- span_duplicate(T *, unsigned){}
+ span_duplicate(T *, unsigned){}
- T array[10];
+ T array[10];
- public:
+ public:
- T* data() {
- return array;
- }
+ T* data() {
+ return array;
+ }
-};
+ };
}
using namespace std;
@@ -89,21 +119,28 @@ void cast_without_data(int *ptr) {
float *p = (float*) ptr;
}
-void warned_patterns(std::span<int> span_ptr, std::span<Base> base_span, span<int> span_without_qual) {
- A *a1 = (A*)span_ptr.data(); // expected-warning{{unsafe invocation of span::data}}
- a1 = (A*)span_ptr.data(); // expected-warning{{unsafe invocation of span::data}}
+void warned_patterns_span(std::span<int> span_ptr, std::span<Base> base_span, span<int> span_without_qual) {
+ A *a1 = (A*)span_ptr.data(); // expected-warning{{unsafe invocation of 'data'}}
+ a1 = (A*)span_ptr.data(); // expected-warning{{unsafe invocation of 'data'}}
- a1 = (A*)(span_ptr.data()); // expected-warning{{unsafe invocation of span::data}}
- A *a2 = (A*) (span_without_qual.data()); // expected-warning{{unsafe invocation of span::data}}
+ a1 = (A*)(span_ptr.data()); // expected-warning{{unsafe invocation of 'data'}}
+ A *a2 = (A*) (span_without_qual.data()); // expected-warning{{unsafe invocation of 'data'}}
- a2 = (A*) span_without_qual.data(); // expected-warning{{unsafe invocation of span::data}}
+ a2 = (A*) span_without_qual.data(); // expected-warning{{unsafe invocation of 'data'}}
// TODO:: Should we warn when we cast from base to derived type?
- Derived *b = dynamic_cast<Derived*> (base_span.data());// expected-warning{{unsafe invocation of span::data}}
+ Derived *b = dynamic_cast<Derived*> (base_span.data());// expected-warning{{unsafe invocation of 'data'}}
// TODO:: This pattern is safe. We can add special handling for it, if we decide this
// is the recommended fixit for the unsafe invocations.
- A *a3 = (A*)span_ptr.subspan(0, sizeof(A)).data(); // expected-warning{{unsafe invocation of span::data}}
+ A *a3 = (A*)span_ptr.subspan(0, sizeof(A)).data(); // expected-warning{{unsafe invocation of 'data'}}
+}
+
+void warned_patterns_array(std::array<int, 5> array_ptr, std::array<Base, 10> base_span, span<int> span_without_qual) {
+ const A *a1 = (A*)array_ptr.data(); // expected-warning{{unsafe invocation of 'data'}}
+ a1 = (A*)array_ptr.data(); // expected-warning{{unsafe invocation of 'data'}}
+
+ a1 = (A*)(array_ptr.data()); // expected-warning{{unsafe invocation of 'data'}}
}
void not_warned_patterns(std::span<A> span_ptr, std::span<Base> base_span) {
|
malavikasamak
requested review from
devincoughlin,
jkorous-apple,
t-rasmud and
ziqingluo-90
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/705 Here is the relevant piece of the build log for the reference |
malavikasamak
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Nov 12, 2024
…:data and array::data is cast to larger type (llvm#111910) Emit a warning when the raw pointer retrieved from std::vector and std::array instances are cast to a larger type. Such a cast followed by a field dereference to the resulting pointer could cause an OOB access. This is similar to the existing span::data warning. (rdar://136704278) Co-authored-by: MalavikaSamak <[email protected]> (cherry picked from commit e913a33)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Emit a warning when the raw pointer retrieved from std::vector and std::array instances are cast to a larger type. Such a cast followed by a field dereference to the resulting pointer could cause an OOB access. This is similar to the existing span::data warning.
(rdar://136704278)