mbenfield updated this revision to Diff 384451.
mbenfield added a comment.
Renamed to diagnose_as_builtin (since two people suggested that name). Let me
know if a name mentioning fortify is preferred.
Validation in handleDiagnoseAsBuiltinAttr:
- first argument is a builtin function
- number of arguments matches number of parameters of that function
- subsequent arguments are integers
- indices are in bounds for the parameters of the declared function
- types match for declared function and builtin function
Added diagnostics for the above errors.
Added some consts.
Moved tests to new file attr-diagnose-as-builtin.c.
Added many test cases.
Fixed diagnostic around checkUInt32Argument.
Clarified documentation.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112024/new/
https://reviews.llvm.org/D112024
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/ParsedAttr.h
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/Sema/attr-diagnose-as-builtin.c
Index: clang/test/Sema/attr-diagnose-as-builtin.c
===================================================================
--- /dev/null
+++ clang/test/Sema/attr-diagnose-as-builtin.c
@@ -0,0 +1,71 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
+// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
+
+typedef unsigned long size_t;
+
+void *test_memcpy(const void *src, size_t c, void *dst) __attribute__((diagnose_as_builtin(__builtin_memcpy, 3, 1, 2))) {
+ return __builtin_memcpy(dst, src, c);
+}
+
+void call_test_memcpy() {
+ char bufferA[10];
+ char bufferB[11];
+ test_memcpy(bufferB, 10, bufferA);
+ test_memcpy(bufferB, 11, bufferA); // expected-warning {{'memcpy' will always overflow; destination buffer has size 10, but size argument is 11}}
+}
+
+void failure_function_doesnt_exist() __attribute__((diagnose_as_builtin(__function_that_doesnt_exist))) {} // expected-error {{use of undeclared identifier '__function_that_doesnt_exist'}}
+
+void not_a_builtin() {}
+
+void failure_not_a_builtin() __attribute__((diagnose_as_builtin(not_a_builtin))) {} // expected-error {{'diagnose_as_builtin' attribute requires parameter 1 to be a builtin function}}
+
+void failure_too_many_parameters(void *dst, const void *src, size_t count, size_t nothing) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3, 4))) {} // expected-error {{'diagnose_as_builtin' attribute references function __builtin_memcpy, which requires exactly 3 arguments}}
+
+void failure_too_few_parameters(void *dst, const void *src) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2))) {} // expected-error{{'diagnose_as_builtin' attribute references function __builtin_memcpy, which requires exactly 3 arguments}}
+
+void failure_not_an_integer(void *dst, const void *src, size_t count) __attribute__((diagnose_as_builtin(__builtin_memcpy, "abc", 2, 3))) {} // expected-error{{'diagnose_as_builtin' attribute requires parameter 2 to be an integer constant}}
+
+void failure_not_a_builtin2() __attribute__((diagnose_as_builtin("abc"))) {} // expected-error{{'diagnose_as_builtin' attribute requires parameter 1 to be a builtin function}}
+
+void failure_parameter_index_bounds(void *dst, const void *src) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3))) {} // expected-error{{'diagnose_as_builtin' attribute references parameter 3, but the function failure_parameter_index_bounds has only 2 parameters}}
+
+void failure_parameter_types(double dst, const void *src, size_t count) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3))) {} // expected-error{{'diagnose_as_builtin' attribute parameter types do not match: parameter 1 of function 'failure_parameter_types' has type 'double', but parameter 1 of function '__builtin_memcpy' has type 'void *'}}
+
+void to_redeclare(void *dst, const void *src, size_t count);
+
+void use_to_redeclare() {
+ char src[10];
+ char dst[9];
+ // We shouldn't get an error yet.
+ to_redeclare(dst, src, 10);
+}
+
+void to_redeclare(void *dst, const void *src, size_t count) __attribute((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3)));
+
+void error_to_redeclare() {
+ char src[10];
+ char dst[9];
+ // Now we get an error.
+ to_redeclare(dst, src, 10); // expected-warning {{'memcpy' will always overflow; destination buffer has size 9, but size argument is 10}}
+}
+
+// Make sure this works even with extra qualifiers and the pass_object_size attribute.
+void *memcpy2(void *const dst __attribute__((pass_object_size(0))), const void *src, size_t copy_amount) __attribute((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3)));
+
+void call_memcpy2() {
+ char buf1[10];
+ char buf2[11];
+ memcpy2(buf1, buf2, 11); // expected-warning {{'memcpy' will always overflow; destination buffer has size 10, but size argument is 11}}
+}
+
+#ifdef __cplusplus
+template <class T>
+void some_templated_function(int x) {
+ return;
+}
+
+void failure_template(int x) __attribute__((diagnose_as_builtin(some_templated_function, 1))) {} // expected-error{{'diagnose_as_builtin' attribute requires parameter 1 to be a builtin function}}
+
+void failure_template_instantiation(int x) __attribute__((diagnose_as_builtin(some_templated_function<int>, 1))) {} // expected-error{{'diagnose_as_builtin' attribute requires parameter 1 to be a builtin function}}
+#endif
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -1001,6 +1001,84 @@
};
}
+static void handleDiagnoseAsBuiltinAttr(Sema &S, Decl *D,
+ const ParsedAttr &AL) {
+ const auto *DeclFD = dyn_cast_or_null<FunctionDecl>(D);
+ if (!DeclFD)
+ // User will have already been warned.
+ return;
+
+ const auto DiagnoseType = [&](unsigned Index, AttributeArgumentNType T) {
+ SourceLocation Loc = [&]() {
+ auto Union = AL.getArg(Index - 1);
+ if (Union.is<Expr *>()) {
+ return Union.get<Expr *>()->getBeginLoc();
+ } else {
+ return Union.get<IdentifierLoc *>()->Loc;
+ }
+ }();
+
+ S.Diag(Loc, diag::err_attribute_argument_n_type) << AL << Index << T;
+ };
+
+ FunctionDecl *AttrFD = [&]() -> FunctionDecl * {
+ if (!AL.isArgExpr(0))
+ return nullptr;
+ auto *F = dyn_cast_or_null<DeclRefExpr>(AL.getArgAsExpr(0));
+ if (!F)
+ return nullptr;
+ return dyn_cast_or_null<FunctionDecl>(F->getFoundDecl());
+ }();
+
+ if (!AttrFD || !AttrFD->getBuiltinID(true)) {
+ DiagnoseType(1, AANT_ArgumentBuiltinFunction);
+ return;
+ }
+
+ if (AttrFD->getNumParams() != AL.getNumArgs() - 1) {
+ S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments_for)
+ << AL << AttrFD->getName() << AttrFD->getNumParams();
+ return;
+ }
+
+ SmallVector<unsigned, 8> Indices;
+
+ for (unsigned I = 1; I < AL.getNumArgs(); ++I) {
+ if (!AL.isArgExpr(I)) {
+ DiagnoseType(I + 1, AANT_ArgumentIntegerConstant);
+ return;
+ }
+
+ const Expr *IndexExpr = AL.getArgAsExpr(I);
+ uint32_t Index;
+
+ if (!checkUInt32Argument(S, AL, IndexExpr, Index, I + 1, false)) {
+ return;
+ }
+
+ if (Index > DeclFD->getNumParams()) {
+ S.Diag(AL.getLoc(), diag::err_attribute_bounds_for_function)
+ << AL << Index << DeclFD->getName() << DeclFD->getNumParams();
+ return;
+ }
+
+ const auto T1 = AttrFD->getParamDecl(I - 1)->getType();
+ const auto T2 = DeclFD->getParamDecl(Index - 1)->getType();
+
+ if (T1.getCanonicalType().getUnqualifiedType() !=
+ T2.getCanonicalType().getUnqualifiedType()) {
+ S.Diag(IndexExpr->getBeginLoc(), diag::err_attribute_parameter_types)
+ << AL << Index << DeclFD << T2 << I << AttrFD << T1;
+ return;
+ }
+
+ Indices.push_back(Index - 1);
+ }
+
+ D->addAttr(::new (S.Context) DiagnoseAsBuiltinAttr(
+ S.Context, AL, AttrFD, Indices.data(), Indices.size()));
+}
+
static void handleDiagnoseIfAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
S.Diag(AL.getLoc(), diag::ext_clang_diagnose_if);
@@ -8032,6 +8110,9 @@
case ParsedAttr::AT_DiagnoseIf:
handleDiagnoseIfAttr(S, D, AL);
break;
+ case ParsedAttr::AT_DiagnoseAsBuiltin:
+ handleDiagnoseAsBuiltinAttr(S, D, AL);
+ break;
case ParsedAttr::AT_NoBuiltin:
handleNoBuiltinAttr(S, D, AL);
break;
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -594,32 +594,64 @@
isConstantEvaluated())
return;
- unsigned BuiltinID = FD->getBuiltinID(/*ConsiderWrappers=*/true);
+ bool UseDAIAttr = false;
+ const FunctionDecl *UseDecl = FD;
+
+ const auto *DAIAttr = FD->getAttr<DiagnoseAsBuiltinAttr>();
+ if (DAIAttr) {
+ UseDecl = DAIAttr->getFunction();
+ assert(UseDecl && "Missing FunctionDecl in DiagnoseAsBuiltin attribute!");
+ UseDAIAttr = true;
+ }
+
+ unsigned BuiltinID = UseDecl->getBuiltinID(/*ConsiderWrappers=*/true);
+
if (!BuiltinID)
return;
const TargetInfo &TI = getASTContext().getTargetInfo();
unsigned SizeTypeWidth = TI.getTypeWidth(TI.getSizeType());
- auto ComputeExplicitObjectSizeArgument =
+ const auto TranslateIndex = [&](unsigned Index) -> Optional<unsigned> {
+ if (UseDAIAttr) {
+ if (Index >= DAIAttr->argIndices_size())
+ return llvm::None;
+ return DAIAttr->argIndices_begin()[Index];
+ }
+ return Index;
+ };
+
+ const auto ComputeExplicitObjectSizeArgument =
[&](unsigned Index) -> Optional<llvm::APSInt> {
+ auto IndexOptional = TranslateIndex(Index);
+ if (!IndexOptional)
+ return llvm::None;
+ const unsigned NewIndex = IndexOptional.getValue();
Expr::EvalResult Result;
- Expr *SizeArg = TheCall->getArg(Index);
+ Expr *SizeArg = TheCall->getArg(NewIndex);
if (!SizeArg->EvaluateAsInt(Result, getASTContext()))
return llvm::None;
- return Result.Val.getInt();
+ auto Integer = Result.Val.getInt();
+ Integer.setIsUnsigned(true);
+ return Integer;
};
- auto ComputeSizeArgument = [&](unsigned Index) -> Optional<llvm::APSInt> {
+ const auto ComputeSizeArgument =
+ [&](unsigned Index) -> Optional<llvm::APSInt> {
// If the parameter has a pass_object_size attribute, then we should use its
// (potentially) more strict checking mode. Otherwise, conservatively assume
// type 0.
int BOSType = 0;
if (const auto *POS =
- FD->getParamDecl(Index)->getAttr<PassObjectSizeAttr>())
+ UseDecl->getParamDecl(Index)->getAttr<PassObjectSizeAttr>())
BOSType = POS->getType();
- const Expr *ObjArg = TheCall->getArg(Index);
+ const auto IndexOptional = TranslateIndex(Index);
+ if (!IndexOptional)
+ return llvm::None;
+ const unsigned NewIndex = IndexOptional.getValue();
+
+ const Expr *ObjArg = TheCall->getArg(NewIndex);
uint64_t Result;
if (!ObjArg->tryEvaluateObjectSize(Result, getASTContext(), BOSType))
return llvm::None;
@@ -628,8 +660,14 @@
return llvm::APSInt::getUnsigned(Result).extOrTrunc(SizeTypeWidth);
};
- auto ComputeStrLenArgument = [&](unsigned Index) -> Optional<llvm::APSInt> {
- Expr *ObjArg = TheCall->getArg(Index);
+ const auto ComputeStrLenArgument =
+ [&](unsigned Index) -> Optional<llvm::APSInt> {
+ const auto IndexOptional = TranslateIndex(Index);
+ if (!IndexOptional)
+ return llvm::None;
+ const unsigned NewIndex = IndexOptional.getValue();
+
+ const Expr *ObjArg = TheCall->getArg(NewIndex);
uint64_t Result;
if (!ObjArg->tryEvaluateStrLen(Result, getASTContext()))
return llvm::None;
@@ -768,7 +806,8 @@
}
if (!SourceSize || !DestinationSize ||
- SourceSize.getValue().ule(DestinationSize.getValue()))
+ llvm::APSInt::compareValues(SourceSize.getValue(),
+ DestinationSize.getValue()) <= 0)
return;
StringRef FunctionName = getASTContext().BuiltinInfo.getName(BuiltinID);
Index: clang/include/clang/Sema/ParsedAttr.h
===================================================================
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -1097,6 +1097,7 @@
AANT_ArgumentString,
AANT_ArgumentIdentifier,
AANT_ArgumentConstantExpr,
+ AANT_ArgumentBuiltinFunction,
};
/// These constants match the enumerated choices of
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2952,6 +2952,15 @@
def err_attribute_wrong_number_arguments : Error<
"%0 attribute %plural{0:takes no arguments|1:takes one argument|"
":requires exactly %1 arguments}1">;
+def err_attribute_wrong_number_arguments_for : Error <
+ "%0 attribute references function %1, which %plural{0:takes no arguments|1:takes one argument|"
+ ":requires exactly %2 arguments}2">;
+def err_attribute_bounds_for_function : Error<
+ "%0 attribute references parameter %1, but the function %2 has only %3 parameters">;
+def err_attribute_parameter_types : Error<
+ "%0 attribute parameter types do not match: parameter %1 of function %2 has type %3, "
+ "but parameter %4 of function %5 has type %6">;
+
def err_attribute_too_many_arguments : Error<
"%0 attribute takes no more than %1 argument%s1">;
def err_attribute_too_few_arguments : Error<
@@ -3005,7 +3014,7 @@
"%0 attribute cannot be applied to sizeless type %1">;
def err_attribute_argument_n_type : Error<
"%0 attribute requires parameter %1 to be %select{int or bool|an integer "
- "constant|a string|an identifier|a constant expression}2">;
+ "constant|a string|an identifier|a constant expression|a builtin function}2">;
def err_attribute_argument_type : Error<
"%0 attribute requires %select{int or bool|an integer "
"constant|a string|an identifier}1">;
Index: clang/include/clang/Basic/AttrDocs.td
===================================================================
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -5929,6 +5929,34 @@
}];
}
+def DiagnoseAsBuiltinDocs : Documentation {
+ let Category = DocCatFunction;
+ let Content = [{
+The ``diagnose_as_builtin` attribute indicates that Fortify diagnostics are to
+be applied to the declared function as if it were the function specified by the
+attribute. The builtin function whose diagnostics are to be mimicked should be
+given. In addition, the order in which arguments should be applied must also
+be given.
+
+For example, the attribute can be used as follows.
+
+ .. code-block:: c
+
+ __attribute__((diagnose_as_builtin(__builtin_memset, 3, 2, 1)))
+ void *mymemset(int n, int c, void *s) {
+ // ...
+ }
+
+This indicates that calls to ``mymemset`` should be diagnosed as if they were
+calls to ``__builtin_memset``. The arguments ``3, 2, 1`` indicate by index the
+order in which arguments of ``mymemset`` should be applied to
+``__builtin_memset``. The third argument should be applied first, then the
+second, and then the first. Thus (when Fortify warnings are enabled) the call
+``mymemset(n, c, s)`` will diagnose overflows as if it were the call
+``__builtin_memset(s, c, n)``;
+}];
+}
+
def ArmSveVectorBitsDocs : Documentation {
let Category = DocCatType;
let Content = [{
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -3822,6 +3822,15 @@
let Documentation = [ReleaseHandleDocs];
}
+def DiagnoseAsBuiltin : InheritableAttr {
+ let Spellings = [Clang<"diagnose_as_builtin">];
+ let Args = [DeclArgument<Function, "Function">,
+ VariadicUnsignedArgument<"ArgIndices">];
+ let Subjects = SubjectList<[Function]>;
+ let LateParsed = 1;
+ let Documentation = [DiagnoseAsBuiltinDocs];
+}
+
def Builtin : InheritableAttr {
let Spellings = [];
let Args = [UnsignedArgument<"ID">];
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
