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 {
----------------
nit: `namespace {`
================
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 buffer is used.
// - Evaluate strlen of strcpy arguments, use as object size.
----------------
Can you delete this comment now?
================
Comment at: clang/lib/Sema/SemaChecking.cpp:383
bool IsChkVariant = false;
+ llvm::APSInt UsedSize = llvm::APSInt::get(-1);
unsigned SizeIndex, ObjectIndex;
----------------
`Optional<APSInt>` might be a better way of representing this?
================
Comment at: clang/lib/Sema/SemaChecking.cpp:388
return;
+ case Builtin::BI__builtin___sprintf_chk: {
+ if (auto *StrE = dyn_cast<StringLiteral>(
----------------
Can't you do this for `Builtin::sprintf` as well? The diagnostic would still be
worth issuing when `_FORTIFY_SOURCE` is disabled.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:392
+ EstimateSizeFormatHandler H(StrE);
+ StringRef StrRef = StrE->getString();
+ const char *Str = StrRef.data();
----------------
Will this assert on: `sprintf(buf, L"foo");`? Not that that makes any sense,
but we shouldn't crash.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:399
+ size_t StrLen =
+ std::min(std::max(TypeSize, size_t(1)) - 1, StrRef.size());
+ if (!analyze_format_string::ParsePrintfString(
----------------
I'm not sure why you're using `min` here, can you add a comment?
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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
