llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Arseniy Zaostrovnykh (necto) <details> <summary>Changes</summary> The MSVC C runtime (UCRT) declares fopen without the restrict qualifiers of the standard prototype. From C99 on, the summaries of StdLibraryFunctionsChecker require restrict on such parameters, so the fopen summary is not applied: there is no argument constraint check and no "Assuming that 'fopen' is successful" path note. The same holds for any summary with restrict parameters when using UCRT implementation. To address this limitation, ignore the `restrict` qualifier from the C standard so that the UCRT signatures also fit the `Summary`. --- Full diff: https://github.com/llvm/llvm-project/pull/236747.diff 3 Files Affected: - (added) clang/test/Analysis/std-c-library-functions-restrict-msvc-path-notes.c (+37) - (added) clang/test/Analysis/std-c-library-functions-restrict-msvc.c (+46) - (modified) clang/test/Analysis/std-c-library-functions-restrict.c (+15) ``````````diff diff --git a/clang/test/Analysis/std-c-library-functions-restrict-msvc-path-notes.c b/clang/test/Analysis/std-c-library-functions-restrict-msvc-path-notes.c new file mode 100644 index 00000000000000..10735d82fea095 --- /dev/null +++ b/clang/test/Analysis/std-c-library-functions-restrict-msvc-path-notes.c @@ -0,0 +1,37 @@ +// RUN: %clang_analyze_cc1 %s -std=c89 \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.StdCLibraryFunctions \ +// RUN: -analyzer-output=text \ +// RUN: -triple x86_64-pc-windows-msvc -verify=expected,applied + +// RUN: %clang_analyze_cc1 %s -std=c99 \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.StdCLibraryFunctions \ +// RUN: -analyzer-output=text \ +// RUN: -triple x86_64-pc-windows-msvc -verify=expected,not-applied + +// RUN: %clang_analyze_cc1 %s -std=c11 \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.StdCLibraryFunctions \ +// RUN: -analyzer-output=text \ +// RUN: -triple x86_64-pc-windows-msvc -verify=expected,not-applied + +// The MSVC C runtime (UCRT) declares fopen without the restrict qualifiers of +// the standard prototype. From C99 on, the summary of fopen does not match +// this declaration, so the path misses the note about the successful call. +// +// FIXME: The summary should be applied in C99 and later too, and the note +// should be present as in C89. See std-c-library-functions-restrict-msvc.c. + +typedef struct _iobuf FILE; +FILE *fopen(char const *_FileName, char const *_Mode); + +void test_fopen_note(void) { + int *p = 0; // expected-note{{'p' initialized to a null pointer value}} + FILE *fp = fopen("file", "r"); // applied-note{{Assuming that 'fopen' is successful}} + if (fp) // applied-note{{'fp' is non-null}} + // not-applied-note@-1{{Assuming 'fp' is non-null}} + // expected-note@-2{{Taking true branch}} + *p = 1; // expected-warning{{Dereference of null pointer (loaded from variable 'p')}} + // expected-note@-1{{Dereference of null pointer (loaded from variable 'p')}} +} diff --git a/clang/test/Analysis/std-c-library-functions-restrict-msvc.c b/clang/test/Analysis/std-c-library-functions-restrict-msvc.c new file mode 100644 index 00000000000000..cdf7af0690d821 --- /dev/null +++ b/clang/test/Analysis/std-c-library-functions-restrict-msvc.c @@ -0,0 +1,46 @@ +// RUN: %clang_analyze_cc1 %s -std=c89 \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.StdCLibraryFunctions \ +// RUN: -triple x86_64-pc-windows-msvc -verify=applied + +// RUN: %clang_analyze_cc1 %s -std=c99 \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.StdCLibraryFunctions \ +// RUN: -triple x86_64-pc-windows-msvc -verify=not-applied + +// RUN: %clang_analyze_cc1 %s -std=c11 \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.StdCLibraryFunctions \ +// RUN: -triple x86_64-pc-windows-msvc -verify=not-applied + +// RUN: %clang_analyze_cc1 %s -std=c99 \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.StdCLibraryFunctions \ +// RUN: -triple x86_64-unknown-linux-gnu -verify=not-applied + +// RUN: %clang_analyze_cc1 %s -std=c99 \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.StdCLibraryFunctions \ +// RUN: -triple x86_64-w64-windows-gnu -verify=not-applied + +// The MSVC C runtime (UCRT) declares fopen without the restrict qualifiers of +// the standard prototype. In C89, the summary has no restrict either, so it is +// applied. From C99 on, the summary requires restrict and does not match, so +// the summary is not applied to the fopen of MSVC. +// +// FIXME: The summary should be applied in C99 and later too when targeting the +// MSVC environment. Its C runtime does not follow the standard prototype, but +// this declaration is still the fopen of the C library. +// +// On other targets (including MinGW, which has its own C library headers), the +// C library is expected to follow the standard prototype, so the summary is +// rightly not applied to a declaration without restrict. + +// not-applied-no-diagnostics + +typedef struct _iobuf FILE; +FILE *fopen(char const *_FileName, char const *_Mode); + +void test_fopen_null_filename(void) { + fopen(0, "r"); // applied-warning{{The 1st argument to 'fopen' is NULL but should not be NULL}} +} diff --git a/clang/test/Analysis/std-c-library-functions-restrict.c b/clang/test/Analysis/std-c-library-functions-restrict.c index 27e223c6e5b2f2..172e5460b91562 100644 --- a/clang/test/Analysis/std-c-library-functions-restrict.c +++ b/clang/test/Analysis/std-c-library-functions-restrict.c @@ -5,10 +5,21 @@ // RUN: -analyzer-config unix.StdCLibraryFunctions:DisplayLoadedSummaries=true \ // RUN: -triple i686-unknown-linux 2>&1 | FileCheck %s +// RUN: %clang_analyze_cc1 %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.StdCLibraryFunctions \ +// RUN: -analyzer-checker=debug.StdCLibraryFunctionsTester \ +// RUN: -analyzer-config unix.StdCLibraryFunctions:DisplayLoadedSummaries=true \ +// RUN: -triple x86_64-pc-windows-msvc 2>&1 | FileCheck %s --check-prefix=MSVC + // The signatures for these functions are the same and they specify their // parameter with the restrict qualifier. In C, the signature should match only // if the restrict qualifier is there on the parameter. Thus, the summary // should be loaded for the last two declarations only. +// FIXME: The MSVC environment should be an exception: the Microsoft C runtime +// omits the restrict qualifiers of the standard prototypes, so the signature +// should match regardless of the restrict qualifier, and the summary should be +// loaded for all three declarations. void __test_restrict_param_0(void *p); void __test_restrict_param_1(void *__restrict p); void __test_restrict_param_2(void *restrict p); @@ -17,6 +28,10 @@ void __test_restrict_param_2(void *restrict p); // CHECK: Loaded summary for: void __test_restrict_param_1(void *restrict p) // CHECK: Loaded summary for: void __test_restrict_param_2(void *restrict p) +// MSVC-NOT: Loaded summary for: void __test_restrict_param_0 +// MSVC: Loaded summary for: void __test_restrict_param_1(void *restrict p) +// MSVC: Loaded summary for: void __test_restrict_param_2(void *restrict p) + // Must have at least one call expression to initialize the summary map. int bar(void); void foo(void) { `````````` </details> https://github.com/llvm/llvm-project/pull/236747 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
