[PATCH] D107078: [analyzer] Catch leaking stack addresses via stack variables

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 17 11:11:17 PDT 2021


NoQ added a comment.

> In D107078#2933682 <https://reviews.llvm.org/D107078#2933682>, @NoQ wrote:
>
>> In D107078#2927899 <https://reviews.llvm.org/D107078#2927899>, @steakhal wrote:
>>
>>> I did not fix the extra blue 'bubble' note issue, and I think that is out of the scope of this patch.
>>
>> This is an on-by-default checker. We should not knowingly regress it, even if temporarily.
>
> I'm not exactly sure what would I regress, but to be on the safe side I won't emit any extra notes this time.

Yup that's what i meant, you can't introduce a note and address problems with it in a follow-up patch; it should be either fully addressed immediately or go under a flag until fixed. I think we're good now that the note is not there.

I think i see one bug in the code but other than that i think we're good to go.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:325-326
+      if (ReferrerMemSpace && ReferredMemSpace) {
+        if (ReferredFrame == PoppedFrame &&
+            ReferrerFrame->isParentOf(PoppedFrame)) {
+          V.emplace_back(Referrer, Referred);
----------------
You probably meant `||`?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107078/new/

https://reviews.llvm.org/D107078



More information about the cfe-commits mailing list