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

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

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

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

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

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

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

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

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

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] D150531: Fix start index for sprintf ovlerap check + tests

ArnaudBienner abandoned this revision. ArnaudBienner added a comment. Sorry about creating a new patch instead of updating the existing one. Closing that one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150531/new/ https://reviews.llvm.org/D15053

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

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

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

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] D150531: Fix start index for sprintf ovlerap check + tests

ArnaudBienner created this revision. Herald added subscribers: steakhal, martong, arphaman. Herald added a reviewer: NoQ. Herald added a project: All. ArnaudBienner requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Githu

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

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

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

[PATCH] D56366: New warning call-to-virtual-from-ctor-dtor when calling a virtual function from a constructor or a destructor

ArnaudBienner added a subscriber: xbolva00. ArnaudBienner added a comment. Thanks @xbolva00 for adding reviewers and @rjmccall for reviewing! @rjmccall as you might remember, we had a discussion on the mailing list ("Warning when calling virtual functions from constructor/desctructor?") and fr

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

ArnaudBienner added inline comments. Comment at: lib/Sema/SemaChecking.cpp:11103 + + if (auto *BO = dyn_cast(E)) { +BinaryOperator::Opcode Opc = BO->getOpcode(); Shouldn't that be "const auto*" instead? I'm surprised dyn_cast casts away const qualifier, but

[PATCH] D56366: New warning call-to-virtual-from-ctor-dtor when calling a virtual function from a constructor or a destructor

ArnaudBienner created this revision. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D56366 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaOverload.cpp test/Analysis/ctor.mm test/Analysis/dt

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

ArnaudBienner added a comment. OK, thanks Serge! :) For the record, I updated the patch one last time after it was accepted to remove my change to constant-expression-cxx1y.cpp since someone else did the same change in a separate commit meanwhile. Repository: rC Clang CHANGES SINCE LAST AC

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

ArnaudBienner updated this revision to Diff 180075. ArnaudBienner added a comment. Herald added a reviewer: serge-sans-paille. Make -Wstring-plus-int warns even if when the result is not out of bounds Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55382/new/ https

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

ArnaudBienner added a comment. Fixed two tests broken by this change: - test_diagnostics.py: AFAICT we are not testing all warnings here, but just that warnings are emitted correctly. The "+1" didn't seem to be useful, since the warning triggered was about the const char* being converted to an

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

ArnaudBienner updated this revision to Diff 178624. ArnaudBienner added a comment. Herald added a subscriber: arphaman. Update tests broken by this change Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55382/new/ https://reviews.llvm.org/D55382 Files: bindings/

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

ArnaudBienner reopened this revision. ArnaudBienner added a comment. This revision is now accepted and ready to land. Reopening since this broke some tests. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55382/new/ https://reviews.llvm.org/D55382 __

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

ArnaudBienner added a comment. Really sorry about breaking the build :( Thanks for reverting it. Actually, I think we could fix that test by removing the " +1": AFAICT this test is checking that warnings are correctly emitted from clang, but not testing all the warnings. So the conversion from c

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

ArnaudBienner added a comment. Thanks for the archeological work and finding the conversation related to the initial patch :) Interesting that the last case you mentioned is exactly the bug I had in my project (though in your case this would have been intended). Repository: rL LLVM CHANGES

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

ArnaudBienner added a comment. So I built Firefox with my patched version of clang and the only place it found a warning was in ffmpeg [1], a case you already reported in [2] when you implemented that patch, so no new positive AFAICT. Do you also want to try on Chromium and see how it goes? Or i

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

ArnaudBienner added a comment. I found the commit: [1] which links to a related discussion [2] but there isn't much details. I wasn't sure if there was other discussions. I will try to build Firefox with this change, and see how it goes (just need to find some time to do it). I'll keep you poste

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

ArnaudBienner added a comment. Nico: I've added you as reviewer since you originally implemented this warning (thanks for this BTW :) as said, it's really helpful), but feel free to delegate to someone else. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55382/ne

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

ArnaudBienner added a comment. I found this warning to be really useful but was confused that it is not always triggered. I initially discovered -Wstring-plus-char (and fixed some issues in the projects I work on where sometimes developers didn't realized the "+" wasn't doing to do concatenatio

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

ArnaudBienner created this revision. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D55382 Files: lib/Sema/SemaExpr.cpp test/SemaCXX/string-plus-int.cpp Index: test/SemaCXX/string-plus-int.cpp

[PATCH] D53807: Create a diagnostic group for warn_call_to_pure_virtual_member_function_from_ctor_dtor, so it can be turned into an error using Werror

ArnaudBienner added a comment. Thanks for the review! :) Could someone please land the patch for me? I don't have commit access. Repository: rC Clang https://reviews.llvm.org/D53807 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://l

[PATCH] D53807: Create a diagnostic group for warn_call_to_pure_virtual_member_function_from_ctor_dtor, so it can be turned into an error using Werror

ArnaudBienner added a comment. @rsmith do you have any thoughts about this change? Repository: rC Clang https://reviews.llvm.org/D53807 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D53807: Create a diagnostic group for warn_call_to_pure_virtual_member_function_from_ctor_dtor, so it can be turned into an error using Werror

ArnaudBienner added a reviewer: davide. ArnaudBienner added a comment. Hi Davide, I see you the last person who updated the test file related to this feature. Would you feel comfortable reviewing my patch? This is my first clang patch, so I apologize in advance if I missed obvious things :) R

[PATCH] D53807: Create a diagnostic group for warn_call_to_pure_virtual_member_function_from_ctor_dtor, so it can be turned into an error using Werror

ArnaudBienner updated this revision to Diff 171502. ArnaudBienner added a comment. Update tests Repository: rC Clang https://reviews.llvm.org/D53807 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td test/Misc/warning-flags.c test/SemaCXX/warn-p

[PATCH] D53807: Create a diagnostic group for warn_call_to_pure_virtual_member_function_from_ctor_dtor, so it can be turned into an error using Werror

ArnaudBienner created this revision. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D53807 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td Index: include/clang/Basic/DiagnosticSemaKinds.td ==