[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

This revision was automatically updated to reflect the committed changes. Closed by commit rG6d9eb5332913: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'. (authored by balazske). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133119/new/

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

martong accepted this revision. martong added a comment. In D133119#3827432 , @balazske wrote: > I added a simple detection of create a copy of `p` before `p = realloc(p, > ...)`. This can remove the warning at very obvious cases when a copy of `p` > is

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

balazske marked an inline comment as done. balazske added a comment. I added a simple detection of create a copy of `p` before `p = realloc(p, ...)`. This can remove the warning at very obvious cases when a copy of `p` is created (but even if the copy is made inside an `if` branch for example).

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

balazske updated this revision to Diff 464291. balazske added a comment. Added simple check for create of copy of pointer before the `realloc` call. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133119/new/ https://reviews.llvm.org/D133119 Files:

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

steakhal accepted this revision. steakhal added a comment. LGTM, no objection here. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-realloc-usage.cpp:81-85 +void warn_if_copy_exists(void *p) { + void *q = p; + p = realloc(p, 111); + // CHECK-MESSA

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

balazske updated this revision to Diff 463812. balazske added a comment. rebase to current main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133119/new/ https://reviews.llvm.org/D133119 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyM

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

balazske updated this revision to Diff 463776. balazske added a comment. added test, added a comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133119/new/ https://reviews.llvm.org/D133119 Files: clang-tools-extra/clang-tidy/bugprone/Bugprone

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

balazske updated this revision to Diff 463575. balazske marked 6 inline comments as done. balazske added a comment. Implemented review comments, changed warning text, changed documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133119/new/

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousReallocUsageCheck.cpp:89-90 +return; + diag(Call->getBeginLoc(), + "memory block passed to 'realloc' may leak if 'realloc' fails"); +} whisperity wrote: > I

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousReallocUsageCheck.cpp:89-90 +return; + diag(Call->getBeginLoc(), + "memory block passed to 'realloc' may leak if 'realloc' fails"); +} I have some reservatio

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

whisperity added a comment. I agree with @martong that `$ = realloc($, ...)` is indicative of something going wrong, even if the developer has saved the original value somewhere. Are we trying to catch this with Tidy specifically for this reason (instead of CSA's stdlib checker, or some taint-r

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

martong accepted this revision. martong added a reviewer: whisperity. martong added a subscriber: whisperity. martong added a comment. This revision is now accepted and ready to land. We had a discussion about this with @dkrupp . We think that the `p = realloc(p, var)` construct in itself is an e

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

balazske added reviewers: steakhal, martong. balazske added a comment. Herald added a subscriber: rnkovacs. I see that there are problems to solve with this patch. If a global variable is used at `realloc` or the pointer is a function parameter, it may have aliases that are not checkable. In the

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

steakhal added a comment. Consider adding a test case demonstrating if the checker recognizes and suppresses the check if there are multiple variables referring to the memory being reallocated. If the check does not handle that case, we should probably state this limitation explicitly. And mark

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

balazske created this revision. Herald added subscribers: carlosgalvezp, steakhal, martong, gamesh411, Szelethus, dkrupp, xazax.hun, mgorny. Herald added a project: All. balazske requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits.