From: Aaron Ballman Date: Sun, 24 Apr 2016 13:30:21 +0000 (+0000) Subject: Improve diagnostic checking for va_start to also warn on other instances of undefined... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=2340dc028030dadc7a65ca5182cfd8e267b49d2c;p=clang Improve diagnostic checking for va_start to also warn on other instances of undefined behavior, such as a parameter declared with the register keyword in C, or a parameter of a type that undergoes default argument promotion. This helps cover some more of the CERT secure coding rule EXP58-CPP. Pass an object of the correct type to va_start (https://www.securecoding.cert.org/confluence/display/cplusplus/EXP58-CPP.+Pass+an+object+of+the+correct+type+to+va_start). git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@267338 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 053dca3de8..f24bf87011 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -7435,8 +7435,10 @@ def err_ms_va_start_used_in_sysv_function : Error< def warn_second_arg_of_va_start_not_last_named_param : Warning< "second argument to 'va_start' is not the last named parameter">, InGroup; -def warn_va_start_of_reference_type_is_undefined : Warning< - "'va_start' has undefined behavior with reference types">, InGroup; +def warn_va_start_type_is_undefined : Warning< + "passing %select{an object that undergoes default argument promotion|" + "an object of reference type|a parameter declared with the 'register' " + "keyword}0 to 'va_start' has undefined behavior">, InGroup; def err_first_argument_to_va_arg_not_of_type_va_list : Error< "first argument to 'va_arg' is of type %0 and not 'va_list'">; def err_second_parameter_to_va_arg_incomplete: Error< diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index fb11adb451..f70c06377e 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -2702,6 +2702,7 @@ bool Sema::SemaBuiltinVAStartImpl(CallExpr *TheCall) { // block. QualType Type; SourceLocation ParamLoc; + bool IsCRegister = false; if (const DeclRefExpr *DR = dyn_cast(Arg)) { if (const ParmVarDecl *PV = dyn_cast(DR->getDecl())) { @@ -2718,15 +2719,21 @@ bool Sema::SemaBuiltinVAStartImpl(CallExpr *TheCall) { Type = PV->getType(); ParamLoc = PV->getLocation(); + IsCRegister = + PV->getStorageClass() == SC_Register && !getLangOpts().CPlusPlus; } } if (!SecondArgIsLastNamedArgument) Diag(TheCall->getArg(1)->getLocStart(), diag::warn_second_arg_of_va_start_not_last_named_param); - else if (Type->isReferenceType()) { - Diag(Arg->getLocStart(), - diag::warn_va_start_of_reference_type_is_undefined); + else if (IsCRegister || Type->isReferenceType() || + Type->isPromotableIntegerType() || + Type->isSpecificBuiltinType(BuiltinType::Float)) { + unsigned Reason = 0; + if (Type->isReferenceType()) Reason = 1; + else if (IsCRegister) Reason = 2; + Diag(Arg->getLocStart(), diag::warn_va_start_type_is_undefined) << Reason; Diag(ParamLoc, diag::note_parameter_type) << Type; } diff --git a/test/Sema/varargs-x86-64.c b/test/Sema/varargs-x86-64.c index e3ded6fbf6..0929c0d914 100644 --- a/test/Sema/varargs-x86-64.c +++ b/test/Sema/varargs-x86-64.c @@ -26,11 +26,11 @@ void __attribute__((ms_abi)) g2(int a, int b, ...) { __builtin_ms_va_start(ap, b); } -void __attribute__((ms_abi)) g3(float a, ...) { +void __attribute__((ms_abi)) g3(float a, ...) { // expected-note 2{{parameter of type 'float' is declared here}} __builtin_ms_va_list ap; - __builtin_ms_va_start(ap, a); - __builtin_ms_va_start(ap, (a)); + __builtin_ms_va_start(ap, a); // expected-warning {{passing an object that undergoes default argument promotion to 'va_start' has undefined behavior}} + __builtin_ms_va_start(ap, (a)); // expected-warning {{passing an object that undergoes default argument promotion to 'va_start' has undefined behavior}} } void __attribute__((ms_abi)) g5() { diff --git a/test/Sema/varargs.c b/test/Sema/varargs.c index 82fa42b2c1..457d84c212 100644 --- a/test/Sema/varargs.c +++ b/test/Sema/varargs.c @@ -18,12 +18,11 @@ void f2(int a, int b, ...) __builtin_va_start(ap, b); } -void f3(float a, ...) -{ +void f3(float a, ...) { // expected-note 2{{parameter of type 'float' is declared here}} __builtin_va_list ap; - __builtin_va_start(ap, a); - __builtin_va_start(ap, (a)); + __builtin_va_start(ap, a); // expected-warning {{passing an object that undergoes default argument promotion to 'va_start' has undefined behavior}} + __builtin_va_start(ap, (a)); // expected-warning {{passing an object that undergoes default argument promotion to 'va_start' has undefined behavior}} } @@ -83,3 +82,15 @@ void f10(int a, ...) { i = __builtin_va_start(ap, a); // expected-error {{assigning to 'int' from incompatible type 'void'}} __builtin_va_end(ap); } + +void f11(short s, ...) { // expected-note {{parameter of type 'short' is declared here}} + __builtin_va_list ap; + __builtin_va_start(ap, s); // expected-warning {{passing an object that undergoes default argument promotion to 'va_start' has undefined behavior}} + __builtin_va_end(ap); +} + +void f12(register int i, ...) { // expected-note {{parameter of type 'int' is declared here}} + __builtin_va_list ap; + __builtin_va_start(ap, i); // expected-warning {{passing a parameter declared with the 'register' keyword to 'va_start' has undefined behavior}} + __builtin_va_end(ap); +} diff --git a/test/Sema/varargs.cpp b/test/Sema/varargs.cpp deleted file mode 100644 index 48a7b2fdf1..0000000000 --- a/test/Sema/varargs.cpp +++ /dev/null @@ -1,7 +0,0 @@ -// RUN: %clang_cc1 -fsyntax-only -verify %s - -class string; -void f(const string& s, ...) { // expected-note {{parameter of type 'const string &' is declared here}} - __builtin_va_list ap; - __builtin_va_start(ap, s); // expected-warning {{'va_start' has undefined behavior with reference types}} -} diff --git a/test/SemaCXX/varargs.cpp b/test/SemaCXX/varargs.cpp new file mode 100644 index 0000000000..6a1883786a --- /dev/null +++ b/test/SemaCXX/varargs.cpp @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -fsyntax-only -std=c++03 -verify %s + +class string; +void f(const string& s, ...) { // expected-note {{parameter of type 'const string &' is declared here}} + __builtin_va_list ap; + __builtin_va_start(ap, s); // expected-warning {{passing an object of reference type to 'va_start' has undefined behavior}} +} + +void g(register int i, ...) { + __builtin_va_list ap; + __builtin_va_start(ap, i); // okay +}