[PATCH] D40574: Bounds check argument_with_type_tag attribute.

[Aaron Ballman via Phabricator via cfe-commits]

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<const Expr*> 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