[PATCH] D71566: New checks for fortified sprintf

This revision was automatically updated to reflect the committed changes. Closed by commit rG6d485ff455ea: Improve static checks for sprintf and __builtin___sprintf_chk (authored by serge-sans-paille). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71

[PATCH] D71566: New checks for fortified sprintf

serge-sans-paille added a comment. In D71566#1839462 , @aaron.ballman wrote: > Continues to LG, do you need me to commit on your behalf? I was waiting for a last LG as I did some changes, thanks for all the quick iteration, I **really** enjoyed working

[PATCH] D71566: New checks for fortified sprintf

aaron.ballman added a comment. Continues to LG, do you need me to commit on your behalf? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71566/new/ https://reviews.llvm.org/D71566 ___ cfe-commits mailing

[PATCH] D71566: New checks for fortified sprintf

merge_guards_bot added a comment. {icon times-circle color=red} Unit tests: fail. 62134 tests passed, 9 failed and 811 were skipped. failed: LLVM.MC/AArch64/ete-sysregs.s failed: LLVM.MC/AArch64/trace-regs.s failed: LLVM.MC/Disassembler/AArch64/ete.txt failed: LLVM.MC/Disassembler/AArch6

[PATCH] D71566: New checks for fortified sprintf

serge-sans-paille updated this revision to Diff 239920. serge-sans-paille added a comment. clang-format update. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71566/new/ https://reviews.llvm.org/D71566 Files: clang/include/clang/Basic/DiagnosticS

[PATCH] D71566: New checks for fortified sprintf

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 62135 tests passed, 0 failed and 812 were skipped. {icon check-circle color=green} clang-tidy: pass. {icon times-circle color=red} clang-format: fail. Please format your changes with clang-format by running `gi

[PATCH] D71566: New checks for fortified sprintf

serge-sans-paille updated this revision to Diff 239913. serge-sans-paille added a comment. Remove extra new lines. Add more test cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71566/new/ https://reviews.llvm.org/D71566 Files: clang/include

[PATCH] D71566: New checks for fortified sprintf

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D71566#1834472 , @serge-sans-paille wrote: > In D71566#1832394 , @aaron.ballman > wrote: > >

[PATCH] D71566: New checks for fortified sprintf

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 62112 tests passed, 0 failed and 808 were skipped. {icon check-circle color=green} clang-tidy: pass. {icon check-circle color=green} clang-format: pass. Build artifacts

[PATCH] D71566: New checks for fortified sprintf

serge-sans-paille updated this revision to Diff 239654. serge-sans-paille added a comment. Remove useless assert Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71566/new/ https://reviews.llvm.org/D71566 Files: clang/include/clang/Basic/Diagnostic

[PATCH] D71566: New checks for fortified sprintf

serge-sans-paille added a comment. In D71566#1832394 , @aaron.ballman wrote: > (There are still some minor whitespace nits to resolve as well.) Strange, everything is passed through clang-format-diff :-/ Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D71566: New checks for fortified sprintf

aaron.ballman added a comment. (There are still some minor whitespace nits to resolve as well.) Comment at: clang/lib/Sema/SemaChecking.cpp:743 + } + assert(UsedSize && "Found a size estimate"); + if (UsedSize.getValue().ule(ObjectSize)) I think this asserti

[PATCH] D71566: New checks for fortified sprintf

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 62043 tests passed, 0 failed and 783 were skipped. {icon question-circle color=gray} clang-tidy: unknown. {icon times-circle color=red} clang-format: fail. Please format your changes with clang-format by runnin

[PATCH] D71566: New checks for fortified sprintf

serge-sans-paille updated this revision to Diff 239242. serge-sans-paille added a comment. More tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71566/new/ https://reviews.llvm.org/D71566 Files: clang/include/clang/Basic/DiagnosticSemaKinds.t

[PATCH] D71566: New checks for fortified sprintf

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 62028 tests passed, 0 failed and 783 were skipped. {icon question-circle color=gray} clang-tidy: unknown. {icon times-circle color=red} clang-format: fail. Please format your changes with clang-format by runnin

[PATCH] D71566: New checks for fortified sprintf

serge-sans-paille updated this revision to Diff 239197. serge-sans-paille marked an inline comment as done. serge-sans-paille added a comment. Finer grain lower bound computations, and simpler control-flow for the checking function. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D71566: New checks for fortified sprintf

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:745-746 +def warn_fortify_source_format_overflow : Warning< + "'%0' will always overflow; destination buffer has size %1," + " but format string expands to at least %2">, + InGrou

[PATCH] D71566: New checks for fortified sprintf

merge_guards_bot added a comment. {icon times-circle color=red} Unit tests: fail. 61975 tests passed, 1 failed and 783 were skipped. failed: LLVM.Bindings/Go/go.test {icon question-circle color=gray} clang-tidy: unknown. {icon times-circle color=red} clang-format: fail. Please format your ch

[PATCH] D71566: New checks for fortified sprintf

serge-sans-paille updated this revision to Diff 238868. serge-sans-paille added a comment. Update size computation + test case involving null byte inside the format string. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71566/new/ https://reviews.l

[PATCH] D71566: New checks for fortified sprintf

erik.pilkington added reviewers: aaron.ballman, rsmith. erik.pilkington added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:422 + size_t StrLen = + std::min(std::max(TypeSize, size_t(1)) - 1, FormatStrRef.size()); + if (!analyze_format_string::Pa

[PATCH] D71566: New checks for fortified sprintf

serge-sans-paille added a comment. @erik.pilkington up? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71566/new/ https://reviews.llvm.org/D71566 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D71566: New checks for fortified sprintf

serge-sans-paille added a comment. @erik.pilkington up? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71566/new/ https://reviews.llvm.org/D71566 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D71566: New checks for fortified sprintf

serge-sans-paille updated this revision to Diff 234327. serge-sans-paille added a comment. Support %% and %c Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71566/new/ https://reviews.llvm.org/D71566 Files: clang/include/clang/Basic/DiagnosticSema

[PATCH] D71566: New checks for fortified sprintf

serge-sans-paille marked 2 inline comments as done. serge-sans-paille added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:392 + EstimateSizeFormatHandler H(StrE); + StringRef StrRef = StrE->getString(); + const char *Str = StrRef.data();

[PATCH] D71566: New checks for fortified sprintf

serge-sans-paille updated this revision to Diff 234282. serge-sans-paille marked an inline comment as done. serge-sans-paille added a comment. - Prevent assert upon wide string format - More test cases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7

[PATCH] D71566: New checks for fortified sprintf

serge-sans-paille marked 7 inline comments as done. serge-sans-paille added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:370 // FIXME: There are some more useful checks we could be doing here: // - Analyze the format string of sprintf to see how much of buf

[PATCH] D71566: New checks for fortified sprintf

serge-sans-paille updated this revision to Diff 234262. serge-sans-paille added a comment. Take remarks into account: - support sprintf when the buffer has a known static size - use llvm::Optional - some extra minor tweaks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D71566: New checks for fortified sprintf

erik.pilkington added a comment. Thanks for working on this! I think this would be a really useful diagnostic to have. Comment at: clang/lib/Sema/SemaChecking.cpp:311 } +class EstimateSizeFormatHandler +: public analyze_format_string::FormatStringHandler { ---

[PATCH] D71566: New checks for fortified sprintf

serge-sans-paille created this revision. serge-sans-paille added a reviewer: erik.pilkington. serge-sans-paille added a project: clang. Herald added subscribers: cfe-commits, dexonsmith. Implement a pessimistic evaluator of the minimal required size for a buffer based on the format string, and co