[PATCH] D105821: [analyzer] [WIP] Model destructor for std::unique_ptr

Deep Majumder via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 22 06:38:45 PDT 2021


RedDocMD added a comment.

> But before we go there we should decide whether we want to actually go for inlining (or otherwise default-evaluating) these destructors. If we do, we should probably not spend too much time on improving invalidation in the checker, because default evaluation would do that properly for us anyway (well, it doesn't really dodge any problems such as the absence of the necessary AST so we'll probably have to solve all these problems anyway, just in a different setting). So it's great that we've fixed `evalCall` for destructors, this could definitely land as a separate patch (tested via `debug.AnalysisOrder`), but we really need to think what to do next here. So I recommend gathering some data to see if proper destructor evaluation is actually a real problem.

`MallocChecker` doesn't seem to mind not evaluating destructors properly. With the current version of the patch, the following code doesn't emit any warnings:

  class SillyPtr {
  	int *ptr;
  	bool wasMalloced;
  public:
  	SillyPtr(int *ptr, bool wasMalloced = false) : 
  			ptr(ptr), 
  			wasMalloced(wasMalloced) {}
  	~SillyPtr() {
  		if (wasMalloced) free(ptr);
  		else delete ptr;
  	}
  };
  
  void foo() {
  	int *ptr = new int(13);
  	SillyPtr silly(ptr);
  	// No leak here!
  }

I am going to remove the debug dumps and run this patch on the projects in the `clang/utils/analyzer/projects` folder. If I don't find any false positives being caused due to this lack of modelling, then I think we can defer the proper handling of destructors (ie, finish up the invalidation) and move on to the other remaining problems (notes on get for an instance).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105821



More information about the cfe-commits mailing list