[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

Kristóf Umann via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 27 03:10:27 PDT 2020


Szelethus added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1687
+  if (ArgValKnown) {
+    if (!KernelZeroSizePtrValue)
+      KernelZeroSizePtrValue =
----------------
balazske wrote:
> Szelethus wrote:
> > Szelethus wrote:
> > > martong wrote:
> > > > balazske wrote:
> > > > > martong wrote:
> > > > > > martong wrote:
> > > > > > > This is a bit confusing for me. Perhaps alternatively we could have a free function `isInitialized(KernelZero...)` instead. Or maybe having a separate bool variable to indicate whether it was initialized could be cleaner?
> > > > > > Another idea: Adding a helper struct to contain the bool `initialized`? E.g. (draft):
> > > > > > ```
> > > > > > struct LazyOptional {
> > > > > >   bool initialized = false;
> > > > > >   Opt<int> value;
> > > > > >   Opt& get();
> > > > > >   void set(const Opt&);
> > > > > > };
> > > > > > ```
> > > > > It may be OK to have a function `lazyInitKernelZeroSizePtrValue`?
> > > > I don't insist, given we have a better described type for `KernelZeroSizePtrValue` (e.g. having a `using` `template`)
> > > `AnalysisManager` has access to the `Preprocessor`, and it is also responsible for the construction of the `CheckerManager` object. This would make moving such code to the checker registry function! I'll gladly take this issue off your hand and patch it in once rG2aac0c47aed8d1eff7ab6d858173c532b881d948 settles :)
> > Just pushed rG4dc8472942ce60f29de9587b9451cc3d49df7abf. It it settles (no buildbots complain), you could rebase and the lazy initialization could be a thing of the past! :)
> This makes it possible to have a generic one-time initialization function in the checker (`CheckerBase`)? The functionality is needed in more than one checker, at least for the bug types it can be used in almost every checker (that has bugtypes).
I'm not sure I understand what you mean -- do you want to add `CheckerManager` to `CheckerBase`'s constructor?

In any case, I remember reading something around `CheckerBase`'s or `BugType`'s implementation the way the checker receives its name is a bit tricky and not a part of the construction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76830





More information about the cfe-commits mailing list