Skip to content

Update Catch missing format attributes #106649

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Update Catch missing format attributes #106649

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions clang/lib/Sema/SemaDeclAttr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5339,9 +5339,9 @@ void Sema::DiagnoseMissingFormatAttributes(Stmt *Body,

// Check if there are more than one format type found. In that case do not
// emit diagnostic.
const FormatAttr *FirstAttr = MissingFormatAttributes[0];
const clang::IdentifierInfo *AttrType = MissingFormatAttributes[0]->getType();
if (llvm::any_of(MissingFormatAttributes, [&](const FormatAttr *Attr) {
return FirstAttr->getType() != Attr->getType();
return AttrType != Attr->getType();
}))
return;

Expand Down Expand Up @@ -5416,15 +5416,18 @@ Sema::GetMissingFormatAttributes(Stmt *Body, const FunctionDecl *FDecl) {
// If child expression is function, check if it is format function.
// If it is, check if parent function misses format attributes.

unsigned int ChildFunctionFormatArgumentIndexOffset =
checkIfMethodHasImplicitObjectParameter(ChildFunction) ? 2 : 1;

if (!ChildFunction->hasAttr<FormatAttr>())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check was added because there is llvm::any_of later and one pointer can only be assigned inside llvm::any_of. This pointer is later called if format attribute was caught. This change prevents throwing error that unitialized value was used.

continue;

// If child function is format function and format arguments are not
// relevant to emit diagnostic, save only information about format type
// (format index and first-to-check argument index are set to -1).
// Information about format type is later used to determine if there are
// more than one format type found.

unsigned int ChildFunctionFormatArgumentIndexOffset =
checkIfMethodHasImplicitObjectParameter(ChildFunction) ? 2 : 1;

// Check if function has format attribute with forwarded format string.
IdentifierInfo *AttrType;
const ParmVarDecl *FormatArg;
Expand Down Expand Up @@ -5465,7 +5468,7 @@ Sema::GetMissingFormatAttributes(Stmt *Body, const FunctionDecl *FDecl) {
continue;

// Check if format string argument is parent function parameter.
unsigned int StringIndex = 0;
int StringIndex = 0;
if (!llvm::any_of(FDecl->parameters(), [&](const ParmVarDecl *Param) {
if (Param != FormatArg)
return false;
Expand Down Expand Up @@ -5506,16 +5509,13 @@ Sema::GetMissingFormatAttributes(Stmt *Body, const FunctionDecl *FDecl) {
}

// Get first argument index
unsigned FirstToCheck = [&]() -> unsigned {
int FirstToCheck = [&]() -> unsigned {
if (!FDecl->isVariadic())
return 0;
const auto *FirstToCheckArg =
dyn_cast<DeclRefExpr>(Args[NumArgs - 1]->IgnoreParenCasts());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed dynamic cast to DeclRefAttr of last function argument. In C++ mode casting to DeclRefAttr resulted in unrecognizing argument type as va_list for several architectures and OS.

if (!FirstToCheckArg)
return 0;

if (FirstToCheckArg->getType().getCanonicalType() !=
Context.getBuiltinVaListType().getCanonicalType())
const auto *FirstToCheckArg = Args[NumArgs - 1]->IgnoreParenCasts();
if (!FirstToCheckArg ||
FirstToCheckArg->getType().getCanonicalType() !=
Context.getBuiltinVaListType().getCanonicalType())
return 0;
return NumOfParentFunctionParams + FunctionFormatArgumentIndexOffset;
}();
Expand Down
64 changes: 27 additions & 37 deletions clang/test/Sema/attr-format-missing.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,11 @@
// RUN: %clang_cc1 -fsyntax-only -x c++ -verify=expected,cpp_diagnostics -Wmissing-format-attribute %s
// RUN: %clang_cc1 -fsyntax-only -x c++ -verify=expected,cpp_diagnostics -std=c++2b -Wmissing-format-attribute %s
// RUN: %clang_cc1 -fsyntax-only -x c++ -verify=expected,cpp_diagnostics -std=c++23 -Wmissing-format-attribute %s
// RUN: not %clang_cc1 -fsyntax-only -x c++ -Wmissing-format-attribute -fdiagnostics-parseable-fixits -triple x86_64-linux %s 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-LIN64
// RUN: not %clang_cc1 -fsyntax-only -x c++ -Wmissing-format-attribute -fdiagnostics-parseable-fixits -triple x86_64-windows %s 2>&1 | FileCheck %s --check-prefixes=CHECK
// RUN: not %clang_cc1 -fsyntax-only -x c++ -Wmissing-format-attribute -fdiagnostics-parseable-fixits -triple i386-windows %s 2>&1 | FileCheck %s --check-prefixes=CHECK
// RUN: not %clang_cc1 -fsyntax-only -x c++ -Wmissing-format-attribute -fdiagnostics-parseable-fixits -triple i386-windows %s 2>&1 | FileCheck %s --check-prefixes=CHECK
// RUN: not %clang_cc1 -fsyntax-only -x c++ -Wmissing-format-attribute -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s

#ifndef __cplusplus
typedef unsigned short char16_t;
typedef unsigned int char32_t;
typedef __CHAR16_TYPE__ char16_t;
typedef __CHAR32_TYPE__ char32_t;
typedef __WCHAR_TYPE__ wchar_t;
#endif

Expand Down Expand Up @@ -84,16 +81,16 @@ void f7(const char *out, ... /* args */) // #f7
{
va_list args;

vscanf(out, &args[0]); // expected-warning@#f7 {{diagnostic behavior may be improved by adding the 'scanf' format attribute to the declaration of 'f7'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:6-[[@LINE-5]]:6}:"__attribute__((format(scanf, 1, 0)))"
vscanf(out, args); // expected-warning@#f7 {{diagnostic behavior may be improved by adding the 'scanf' format attribute to the declaration of 'f7'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:6-[[@LINE-5]]:6}:"__attribute__((format(scanf, 1, 2)))"
}

void f8(const char *out, ... /* args */) // #f8
{
va_list args;

vscanf(out, &args[0]); // expected-no-warning@#f8
vprintf(out, &args[0]); // expected-no-warning@#f8
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that second argument is not passed properly.

vscanf(out, args); // expected-no-warning@#f8
vprintf(out, args); // expected-no-warning@#f8
}

void f9(const char out[], ... /* args */) // #f9
Expand All @@ -109,16 +106,18 @@ void f10(const wchar_t *out, ... /* args */) // #f10
{
va_list args;
vscanf(out, args);
#if __SIZEOF_WCHAR_T__ == 4
// c_diagnostics-warning@-2 {{incompatible pointer types passing 'const wchar_t *' (aka 'const int *') to parameter of type 'const char *'}}
#if (defined(__aarch64__) && !defined(_WIN64)) || (defined(__arm__) && !defined(_WIN32))
// c_diagnostics-warning@-2 {{incompatible pointer types passing 'const wchar_t *' (aka 'const unsigned int *') to parameter of type 'const char *'}}
#elif __SIZEOF_WCHAR_T__ == 4
// c_diagnostics-warning@-4 {{incompatible pointer types passing 'const wchar_t *' (aka 'const int *') to parameter of type 'const char *'}}
#else
// c_diagnostics-warning@-4 {{incompatible pointer types passing 'const wchar_t *' (aka 'const unsigned short *') to parameter of type 'const char *'}}
// c_diagnostics-warning@-6 {{incompatible pointer types passing 'const wchar_t *' (aka 'const unsigned short *') to parameter of type 'const char *'}}
#endif
// c_diagnostics-note@#vscanf {{passing argument to parameter here}}
// c_diagnostics-warning@#f10 {{diagnostic behavior may be improved by adding the 'scanf' format attribute to the declaration of 'f10'}}
// cpp_diagnostics-error@-8 {{no matching function for call to 'vscanf'}}
// cpp_diagnostics-note@#vscanf {{candidate function not viable: no known conversion from 'const wchar_t *' to 'const char *' for 1st argument}}
// C-CHECK: fix-it:"{{.*}}":{[[@LINE-13]]:6-[[@LINE-13]]:6}:"__attribute__((format(scanf, 1, 2)))"
// cpp_diagnostics-error@-11 {{no matching function for call to 'vscanf'}}
// cpp_diagnostics-note@#vscanf {{candidate function not viable: no known conversion from 'const wchar_t *' to 'const char *' for 1st argument}}
}

void f11(const wchar_t *out, ... /* args */) // #f11
Expand All @@ -139,16 +138,18 @@ void f13(const wchar_t *out, ... /* args */) // #f13
{
va_list args;
vscanf(out, args);
#if __SIZEOF_WCHAR_T__ == 4
// c_diagnostics-warning@-2 {{incompatible pointer types passing 'const wchar_t *' (aka 'const int *') to parameter of type 'const char *'}}
#else
// c_diagnostics-warning@-4 {{incompatible pointer types passing 'const wchar_t *' (aka 'const unsigned short *') to parameter of type 'const char *'}}
#if (defined(__aarch64__) && !defined(_WIN64)) || (defined(__arm__) && !defined(_WIN32))
// c_diagnostics-warning@-2 {{incompatible pointer types passing 'const wchar_t *' (aka 'const unsigned int *') to parameter of type 'const char *'}}
#elif (defined(__x86_64__) && !defined(_WIN64)) || __SIZEOF_WCHAR_T__ == 4
// c_diagnostics-warning@-4 {{incompatible pointer types passing 'const wchar_t *' (aka 'const int *') to parameter of type 'const char *'}}
#elif __SIZEOF_WCHAR_T__ == 2
// c_diagnostics-warning@-6 {{incompatible pointer types passing 'const wchar_t *' (aka 'const unsigned short *') to parameter of type 'const char *'}}
#endif
// c_diagnostics-note@#vscanf {{passing argument to parameter here}}
// cpp_diagnostics-error@-7 {{no matching function for call to 'vscanf'}}
// cpp_diagnostics-error@-9 {{no matching function for call to 'vscanf'}}
// cpp_diagnostics-note@#vscanf {{candidate function not viable: no known conversion from 'const wchar_t *' to 'const char *' for 1st argument}}
// expected-warning@#f13 {{diagnostic behavior may be improved by adding the 'scanf' format attribute to the declaration of 'f13'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-13]]:6-[[@LINE-13]]:6}:"__attribute__((format(scanf, 1, 2)))"
// CHECK: fix-it:"{{.*}}":{[[@LINE-15]]:6-[[@LINE-15]]:6}:"__attribute__((format(scanf, 1, 2)))"
vscanf((const char *) out, args);
vscanf((char *) out, args);
}
Expand Down Expand Up @@ -327,9 +328,9 @@ void f33(char *out, va_list args) // #f33
void f34(char *out, ... /* args */) // #f34
{
va_list args;
scanf(out, args); // expected-no-warning@#f34
scanf(out, args);
{
scanf(out, args); // expected-no-warning@#f34
scanf(out, args);
}
}

Expand All @@ -341,7 +342,7 @@ void f35(char* ch, const char *out, ... /* args */) // #f35
int a;
printf(out, a); // expected-warning@#f35 {{diagnostic behavior may be improved by adding the 'printf' format attribute to the declaration of 'f35'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-7]]:6-[[@LINE-7]]:6}:"__attribute__((format(printf, 2, 0)))"
printf(out, 1); // no warning because first command above emitted same warning with same fix-it text
printf(out, 1); // no warning because first command above emitted same warning and fix-it
printf(out, args); // expected-warning@#f35 {{diagnostic behavior may be improved by adding the 'printf' format attribute to the declaration of 'f35'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-10]]:6-[[@LINE-10]]:6}:"__attribute__((format(printf, 2, 3)))"
}
Expand Down Expand Up @@ -387,17 +388,6 @@ void f40(char *out, ... /* args */) // #f40
void f41(char *out, ... /* args */) // #f41
{
va_list args;
char *ch;
vscanf("%s", ch);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Char * was passed to second parameter which accepts va_list type.

#if defined(__x86_64__) && defined(__linux__)
// c_diagnostics-warning@-2 {{incompatible pointer types passing 'char *' to parameter of type 'struct __va_list_tag *'}}
// c_diagnostics-note@#vscanf {{passing argument to parameter here}}
// cpp_diagnostics-error@-4 {{no matching function for call to 'vscanf'}}
// cpp_diagnostics-note@#vscanf {{candidate function not viable: no known conversion from 'char *' to '__va_list_tag *' for 2nd argument}}
#endif
vprintf(out, args);
#if defined(__x86_64__) && defined(__linux__)
// cpp_diagnostics-warning@#f41 {{diagnostic behavior may be improved by adding the 'printf' format attribute to the declaration of 'f41'}}
// CHECK-LIN64: fix-it:"{{.*}}":{[[@LINE-14]]:6-[[@LINE-14]]:6}:"__attribute__((format(printf, 1, 2)))"
#endif
vscanf("%s", args); // expected-no-warning@#f41
vprintf(out, args); // expected-no-warning@#f41
}
Loading