[PATCH] D135375: [analyzer] Initialize regions returned by CXXNew to undefined

Mon Oct 17 07:00:46 PDT 2022

martong added a comment.

In D135375#3849261 <https://reviews.llvm.org/D135375#3849261>, @Szelethus wrote:

> Some early results:
>
> https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?review-status=Unreviewed&review-status=Confirmed%20bug&detection-status=New&detection-status=Reopened&detection-status=Unresolved&is-unique=on&run=%2acxxnew_baseline&newcheck=%2acxxnew_patched&diff-type=New
>
> I'm still waiting on a few projects, but this is something. A few reports from `alpha.security.ArrayBound`, I still need to look into that.
>
> This <https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?review-status=Unreviewed&review-status=Confirmed%20bug&detection-status=New&detection-status=Reopened&detection-status=Unresolved&is-unique=on&run=%2acxxnew_baseline&newcheck=%2acxxnew_patched&diff-type=New&report-hash=ba1039b9da740e4d12fd6f121cc1b88c&report-filepath=%2aqimage_conversions.cpp&report-id=459361> report from `core.uninitialized.Assign` is interesting. First off, I'd appreciate if the "storing uninitialized value" was placed //inside// the notes about the call to `QScopedArrayPointer`'s constructor. Otherwise, the report looks correct.
>
> Not too happy about this <https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?review-status=Unreviewed&review-status=Confirmed%20bug&detection-status=New&detection-status=Reopened&detection-status=Unresolved&is-unique=on&run=%2acxxnew_baseline&newcheck=%2acxxnew_patched&diff-type=New&report-hash=0fa7641b6d8982587cef9512fa7121bb&report-filepath=%2aqimage.cpp&report-id=460856> report by `core.CallAndMessage`. Message 9 is placed on the line
>
>   applyColorTransform(d->colorSpace.transformationToColorSpace(colorSpace));
>
> Messag 10 talks about what happens in `applyColorTransform`, but the important thing happens at the evaluation of the argument, which is not described, so this isn't a very good bug report, can't tell whether its false.

Yes, I agree, this bug-report is hard to follow. Seems like the `colorSpace` argument from step 1 is flowing through step 10. I wonder if `colorSpace` at step 1 is evaluated as `Undefined`? Could that happen? Did you have this report before your change?

> Reports from `core.UndefinedBinaryOperatorResult` here <https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?review-status=Unreviewed&review-status=Confirmed%20bug&detection-status=New&detection-status=Reopened&detection-status=Unresolved&is-unique=on&run=%2acxxnew_baseline&newcheck=%2acxxnew_patched&diff-type=New&report-hash=7b5ed42f3de9290c469b6bf1176b0159&report-filepath=%2aqdistancefield.cpp&report-id=459055>

This seems to be justified, at step 11 we initialize the variable with a negative value. I think, this has nothing to do with your changes.

> and `alpha.core.Conversion` here <https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?review-status=Unreviewed&review-status=Confirmed%20bug&detection-status=New&detection-status=Reopened&detection-status=Unresolved&is-unique=on&run=%2acxxnew_baseline&newcheck=%2acxxnew_patched&diff-type=New&report-hash=86d047abf4d2ea2cbdde1f7293eab9a1&report-filepath=%2aqpixmapfilter.cpp&report-id=460416> are interesting, because there doesn't need to be caused by uninitialized dynamic memory, but rather the fact that the memory is being modeled at all. I suspect this is due us maybe discarding array size information or something similar when the value held in `UnknownVal`?
>
> The rest of the reports seem nobrainder gains.
>
> Interstingly, 3 reports are lost <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?review-status=Unreviewed&review-status=Confirmed%20bug&detection-status=New&detection-status=Reopened&detection-status=Unresolved&is-unique=on&run=%2acxxnew_baseline&newcheck=%2acxxnew_patched&diff-type=Resolved> from `alpha.security.ArrayBound`, but I'm not sure how seriously do we take this checker.

I think these new reports and the lost reports are the result of the changed analysis paths. With your patch, there are new bug reports which introduce sink nodes which were not there previously. Consequently, the set of analyzed paths and thus the new results can be different.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135375