[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

2020-09-15 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Post-commit LGTM on the post-commit changes! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87081/new/ https://reviews.llvm.org/D87081 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

2020-09-15 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. > Actually, the test added here could have catched that regression. Correction: It is the new BufferSize argument constraint in `fread`'s summary and the existing test `test_fread_uninitialized` in `std-c-library-functions.c` that could have catched that regression. (No

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

2020-09-15 Thread Gabor Marton via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa012bc4c42e4: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite (authored by martong). Changed prior to commit: https://reviews.llvm.org/D87081?vs=291606&id=291919#toc Re

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

2020-09-15 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. There is a blatant regression we introduced in D87240 . Actually, the test added here could have catched that regression. See https://reviews.llvm.org/D87240#inline-812062 I am putting back the modeling dependency with this patch. Repos

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

2020-09-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. @balazske may have some closing words. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1262 Getline(LongLongTy

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

2020-09-14 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 291606. martong added a comment. - Add tests to verify compatibility of the two checkers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87081/new/ https://reviews.llvm.org/D87081 Files: clang/lib/StaticAnalyz

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

2020-09-07 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. In D87081#2258637 , @Szelethus wrote: > The patch looks great, in fact, it demonstrates how well thought out your > summary crafting machinery is. > > In D87081#2258579 , @martong wrote: >

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

2020-09-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. The patch looks great, in fact, it demonstrates how well thought out your summary crafting machinery is. In D87081#2258579 , @martong wrote: > However, in a similar case with the CallAndMessage Checker, we decided to > list th

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

2020-09-07 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. In D87081#2256636 , @balazske wrote: > This checker will make an additional assumption on `fread` and `fwrite` with > the ReturnValueCondition. There is nothing new in that. This assumption described by the `.Case` has been here

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

2020-09-04 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment. This checker will make an additional assumption on `fread` and `fwrite` with the ReturnValueCondition. The return value is constrained by `StreamChecker` too but it splits the error (if returned value is less that arg 3) and non-error cases into separate branches. I th

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

2020-09-03 Thread Gabor Marton via Phabricator via cfe-commits
martong created this revision. martong added reviewers: steakhal, balazske, Szelethus, NoQ, vsavchenko. Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, whisperity. Herald ad