[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-31 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment. TBH we don't really have processes there. And its a bit of a mess, regarding CSA. Usually we forgot to update the release notes, and at best we aggregate the changes just prior a release for the release notes. But we haven't done it for the last two releases for sure.

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-31 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner added a comment. @steakhal should the release notes page be updated to add this? I think this could be beneficial for the users to be aware of that change. If yes, who is responsible for doing that? Developers? (me) or someone else is taking care of reviewing the list of changes s

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-31 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner added a comment. Thanks @steakhal for the proposal :) but I pushed it myself: https://github.com/llvm/llvm-project/commit/ce97312d109b21acb97d3ea243e214f20bd87cfc I used to have svn access, and Chris kindly give me write access to the git repository. Repository: rG LLVM Github

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-31 Thread Arnaud Bienner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGce97312d109b: Implement BufferOverlap check for sprint/snprintf (authored by ArnaudBienner). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150430/new/ https

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-30 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner updated this revision to Diff 526743. ArnaudBienner added a comment. - clang-format fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150430/new/ https://reviews.llvm.org/D150430 Files: clang/lib/StaticAnalyzer/Checkers/CStringChec

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-30 Thread Balázs Benics via Phabricator via cfe-commits
steakhal accepted this revision. steakhal added a comment. Can you commit this change or should I do it on your behalf? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150430/new/ https://reviews.llvm.org/D150430

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-29 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner updated this revision to Diff 526463. ArnaudBienner added a comment. - Code review comments: change back the string buffer check + run clang format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150430/new/ https://reviews.llvm.org/D15

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-29 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment. Ah, I can see that pre-merge bots failed due to clang-format violations. Please make all the touched lines as clang-formatted prior to committing anything. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150430/new/ https:/

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-29 Thread Balázs Benics via Phabricator via cfe-commits
steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Yea, it still looks okay. Let me know if you don't have commit access. In that case, also let me know what should be used for the commit author. Repository: rG LLVM Github Monorepo CHA

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-26 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner added a comment. @steakhal did you get a chance to look at my comment? I would really love to see this merged upstream if you think this could be a beneficial change :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150430/new/ https

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-17 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner added a comment. Giving this a second thought, I feel like the initial check: if (!ArgExpr->getType()->isAnyPointerType() || !ArgExpr->getType()->getPointeeType()->isAnyCharacterType()) is better than the new one. To me it reads like "expr type is a pointer and it points t

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-16 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment. In D150430#4347365 , @ArnaudBienner wrote: > Thanks for the review :) > > I implemented the suggested changes. > > Just one question: `PointeeTy.isNull()`: is this guaranteed to always work > even though `getType()->isAnyPointe

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-16 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner added a comment. Thanks for the review :) I implemented the suggested changes. Just one question: `PointeeTy.isNull()`: is this guaranteed to always work even though `getType()->isAnyPointerType() == false`? I'm assuming yes since the tests still pass, but wanted to confirm this

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-16 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner updated this revision to Diff 522738. ArnaudBienner added a comment. - Code review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150430/new/ https://reviews.llvm.org/D150430 Files: clang/lib/StaticAnalyzer/Checkers/CString

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-16 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment. Here is the migrated version of the mentioned discussion: https://discourse.llvm.org/t/static-or-dynamic-code-analysis-for-undefined-behavior-in-sprintf/59624 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150430/new/ http

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-16 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment. I like it. How about an implementation like this: void CStringChecker::evalSprintfCommon(CheckerContext &C, const CallExpr *CE, bool IsBounded) const { ProgramStateRef State = C.getState(); DestinationArgExpr Dest = {CE

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-14 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner updated this revision to Diff 522020. ArnaudBienner added a comment. Updating D150430 : Implement BufferOverlap check for sprint/snprintf Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150430/new/ htt

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-14 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner updated this revision to Diff 522019. ArnaudBienner added a comment. Updating D150430 : Implement BufferOverlap check for sprint/snprintf Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150430/new/ htt

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-14 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner updated this revision to Diff 522018. ArnaudBienner added a comment. Fix start index for sprintf ovlerap check + tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150430/new/ https://reviews.llvm.org/D150430 Files: clang/lib/Stat

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-12 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2385 + // sprintf(char *buffer, const char* format, ... /* format arguments */); + unsigned int format_arguments_start_idx = 3; + // snprintf case: one extra extra arguments f

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

2023-05-12 Thread Arnaud Bienner via Phabricator via cfe-commits
ArnaudBienner created this revision. Herald added subscribers: steakhal, martong. Herald added a reviewer: NoQ. Herald added a project: All. ArnaudBienner edited the summary of this revision. ArnaudBienner added a subscriber: dergachev.a. ArnaudBienner published this revision for review. Herald add