[PATCH] D40574: Bounds check argument_with_type_tag attribute.
aaron.ballman closed this revision. aaron.ballman added a comment. In https://reviews.llvm.org/D40574#939693, @mattd wrote: > Thanks for the approval Aaron. I do not have commit access, would you mind > submitting this on my behalf? Happy to do so -- I've commit in r319383, thank you for the patch! https://reviews.llvm.org/D40574 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40574: Bounds check argument_with_type_tag attribute.
mattd marked an inline comment as done. mattd added a comment. Thanks for the approval Aaron. I do not have commit access, would you mind submitting this on my behalf? https://reviews.llvm.org/D40574 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40574: Bounds check argument_with_type_tag attribute.
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Thanks, LGTM! https://reviews.llvm.org/D40574 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40574: Bounds check argument_with_type_tag attribute.
mattd marked 6 inline comments as done. mattd added inline comments. Comment at: test/Sema/error-type-safety.cpp:3-4 + +static const int test_void + __attribute__((type_tag_for_datatype(test, void))) = 0; + aaron.ballman wrote: > Is this declaration necessary? Hi Aaron, thanks for the input. Yes, defining the tag is necessary in order to test the argument index. If the type tag is not satisfied, the code path checking argument tag index will never be reached. My update will test the bounds and is a little more comprehensive. https://reviews.llvm.org/D40574 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40574: Bounds check argument_with_type_tag attribute.
mattd updated this revision to Diff 124783.
mattd added a comment.
- Removed the new lines.
- Added white space between the data type and pointer character.
- Updated the test to check the exact bounds, and over-bounds cases
- Combined the diagnostic messages into one via 'select'
https://reviews.llvm.org/D40574
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/Sema/SemaChecking.cpp
test/Sema/error-type-safety.cpp
Index: test/Sema/error-type-safety.cpp
===
--- test/Sema/error-type-safety.cpp
+++ test/Sema/error-type-safety.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#define INT_TAG 42
+
+static const int test_in
+ __attribute__((type_tag_for_datatype(test, int))) = INT_TAG;
+
+// Argument index: 1, Type tag index: 2
+void test_bounds_index(...)
+ __attribute__((argument_with_type_tag(test, 1, 2)));
+
+// Argument index: 3, Type tag index: 1
+void test_bounds_arg_index(...)
+ __attribute__((argument_with_type_tag(test, 3, 1)));
+
+void test_bounds()
+{
+ // Test the boundary edges (ensure no off-by-one) with argument indexing.
+ test_bounds_index(1, INT_TAG);
+
+ test_bounds_index(1); // expected-error {{type tag index 2 is greater than the number of arguments specified}}
+ test_bounds_arg_index(INT_TAG, 1); // expected-error {{argument index 3 is greater than the number of arguments specified}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -2754,7 +2754,7 @@
// Type safety checking.
if (FDecl) {
for (const auto *I : FDecl->specific_attrs())
-CheckArgumentWithTypeTag(I, Args.data());
+CheckArgumentWithTypeTag(I, Args, Loc);
}
}
@@ -12329,10 +12329,18 @@
}
void Sema::CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
-const Expr * const *ExprArgs) {
+const ArrayRef ExprArgs,
+SourceLocation CallSiteLoc) {
const IdentifierInfo *ArgumentKind = Attr->getArgumentKind();
bool IsPointerAttr = Attr->getIsPointer();
+ // Retrieve the argument representing the 'type_tag'.
+ if (Attr->getTypeTagIdx() >= ExprArgs.size()) {
+// Add 1 to display the user's specified value.
+Diag(CallSiteLoc, diag::err_tag_index_out_of_range)
+<< 0 << Attr->getTypeTagIdx() + 1;
+return;
+ }
const Expr *TypeTagExpr = ExprArgs[Attr->getTypeTagIdx()];
bool FoundWrongKind;
TypeTagData TypeInfo;
@@ -12346,6 +12354,13 @@
return;
}
+ // Retrieve the argument representing the 'arg_idx'.
+ if (Attr->getArgumentIdx() >= ExprArgs.size()) {
+// Add 1 to display the user's specified value.
+Diag(CallSiteLoc, diag::err_tag_index_out_of_range)
+<< 1 << Attr->getArgumentIdx() + 1;
+return;
+ }
const Expr *ArgumentExpr = ExprArgs[Attr->getArgumentIdx()];
if (IsPointerAttr) {
// Skip implicit cast of pointer to `void *' (as a function argument).
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -10455,7 +10455,8 @@
/// \brief Peform checks on a call of a function with argument_with_type_tag
/// or pointer_with_type_tag attributes.
void CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
-const Expr * const *ExprArgs);
+const ArrayRef ExprArgs,
+SourceLocation CallSiteLoc);
/// \brief Check if we are taking the address of a packed field
/// as this may be a problem if the pointer value is dereferenced.
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7919,6 +7919,8 @@
"'type_tag_for_datatype' attribute requires the initializer to be "
"an %select{integer|integral}0 constant expression "
"that can be represented by a 64 bit integer">;
+def err_tag_index_out_of_range : Error<
+ "%select{type tag|argument}0 index %1 is greater than the number of arguments specified">;
def warn_type_tag_for_datatype_wrong_kind : Warning<
"this type tag was not designed to be used with this function">,
InGroup;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40574: Bounds check argument_with_type_tag attribute.
aaron.ballman added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7922-7925
+def err_type_tag_out_of_range : Error<
+ "type tag index %0 is greater than the number of arguments specified">;
+def err_arg_tag_out_of_range : Error<
+ "argument tag index %0 is greater than the number of arguments specified">;
These should be combined into a single diagnostic: `%select{type|argument}0 tag
index %1 is greater than the number of arguments specified`
Comment at: include/clang/Sema/Sema.h:10458
void CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
-const Expr * const *ExprArgs);
+const ArrayRef ExprArgs,
+SourceLocation CallSiteLoc);
`const Expr *` (space before the `*`). Same below in the definition.
Comment at: lib/Sema/SemaChecking.cpp:12345
const Expr *TypeTagExpr = ExprArgs[Attr->getTypeTagIdx()];
+
bool FoundWrongKind;
Spurious newline?
Comment at: lib/Sema/SemaChecking.cpp:12366
const Expr *ArgumentExpr = ExprArgs[Attr->getArgumentIdx()];
+
if (IsPointerAttr) {
Spurious newline?
Comment at: test/Sema/error-type-safety.cpp:3-4
+
+static const int test_void
+ __attribute__((type_tag_for_datatype(test, void))) = 0;
+
Is this declaration necessary?
Comment at: test/Sema/error-type-safety.cpp:16
+ test_invalid_arg_index(0); // expected-error {{argument tag index 99 is
greater than the number of arguments specified}}
+}
Can you also add a test for a boundary case so we can be sure there are no
off-by-one errors introduced later?
https://reviews.llvm.org/D40574
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40574: Bounds check argument_with_type_tag attribute.
mattd created this revision.
Fixes: PR28520
Perform a bounds check on a function's argument list, before accessing any
index value specified by an 'argument_with_type_tag' attribute.
https://reviews.llvm.org/D40574
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/Sema/SemaChecking.cpp
test/Sema/error-type-safety.cpp
Index: test/Sema/error-type-safety.cpp
===
--- test/Sema/error-type-safety.cpp
+++ test/Sema/error-type-safety.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+static const int test_void
+ __attribute__((type_tag_for_datatype(test, void))) = 0;
+
+void test_invalid_type_tag_index(int datatype, ...)
+ __attribute__((argument_with_type_tag(test,1,99)));
+
+void test_invalid_arg_index(int datatype, ...)
+ __attribute__((argument_with_type_tag(test,99,1)));
+
+void test_bounds()
+{
+ test_invalid_type_tag_index(0); // expected-error {{type tag index 99 is greater than the number of arguments specified}}
+ test_invalid_arg_index(0); // expected-error {{argument tag index 99 is greater than the number of arguments specified}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -2754,7 +2754,7 @@
// Type safety checking.
if (FDecl) {
for (const auto *I : FDecl->specific_attrs())
-CheckArgumentWithTypeTag(I, Args.data());
+CheckArgumentWithTypeTag(I, Args, Loc);
}
}
@@ -12329,11 +12329,20 @@
}
void Sema::CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
-const Expr * const *ExprArgs) {
+const ArrayRef ExprArgs,
+SourceLocation CallSiteLoc) {
const IdentifierInfo *ArgumentKind = Attr->getArgumentKind();
bool IsPointerAttr = Attr->getIsPointer();
+ // Retrieve the argument representing the 'type_tag'.
+ if (Attr->getTypeTagIdx() >= ExprArgs.size()) {
+// Add 1 to display the user's specified value.
+Diag(CallSiteLoc, diag::err_type_tag_out_of_range)
+<< Attr->getTypeTagIdx() + 1;
+return;
+ }
const Expr *TypeTagExpr = ExprArgs[Attr->getTypeTagIdx()];
+
bool FoundWrongKind;
TypeTagData TypeInfo;
if (!GetMatchingCType(ArgumentKind, TypeTagExpr, Context,
@@ -12346,7 +12355,15 @@
return;
}
+ // Retrieve the argument representing the 'arg_idx'.
+ if (Attr->getArgumentIdx() >= ExprArgs.size()) {
+// Add 1 to display the user's specified value.
+Diag(CallSiteLoc, diag::err_arg_tag_out_of_range)
+<< Attr->getArgumentIdx() + 1;
+return;
+ }
const Expr *ArgumentExpr = ExprArgs[Attr->getArgumentIdx()];
+
if (IsPointerAttr) {
// Skip implicit cast of pointer to `void *' (as a function argument).
if (const ImplicitCastExpr *ICE = dyn_cast(ArgumentExpr))
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -10455,7 +10455,8 @@
/// \brief Peform checks on a call of a function with argument_with_type_tag
/// or pointer_with_type_tag attributes.
void CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
-const Expr * const *ExprArgs);
+const ArrayRef ExprArgs,
+SourceLocation CallSiteLoc);
/// \brief Check if we are taking the address of a packed field
/// as this may be a problem if the pointer value is dereferenced.
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7919,6 +7919,10 @@
"'type_tag_for_datatype' attribute requires the initializer to be "
"an %select{integer|integral}0 constant expression "
"that can be represented by a 64 bit integer">;
+def err_type_tag_out_of_range : Error<
+ "type tag index %0 is greater than the number of arguments specified">;
+def err_arg_tag_out_of_range : Error<
+ "argument tag index %0 is greater than the number of arguments specified">;
def warn_type_tag_for_datatype_wrong_kind : Warning<
"this type tag was not designed to be used with this function">,
InGroup;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
