Szelethus planned changes to this revision.
Szelethus added a comment.

Here's how I'm planning to approach this issue:

- Split `MallocChecker` and `CStringChecker` up properly, introduce 
`MallocCheckerBase` and `CStringCheckerBase`. I'm a little unsure about how 
I'll do this, but I'll share my thoughts when I come up with something clever.
- Some checkers do not register themselves in all cases[1], which should 
probably be handled at a higher level, preferably in `Checkers.td`. Warnings 
could be emitted when a checker is explicitly enabled but will not be 
registered.
- Once all checkers in all cases properly register themselves, refactor this 
patch to introduce dependencies in between the subcheckers of `MallocChecker` 
and `CStringChecker`. This will imply that every registry function registers 
one, and only one checker.
- Move `CheckerManager::registerChecker<T>` out of the registry functions, 
rename them from `register##CHECKERNAME` to `setup##CHECKERNAME`, and supply 
the //mutable// checker object and the //immutable// `CheckerManager`. The 
downside is that all checkers will have to be default constructible, and their 
fields will have to be set up in `setup##CHECKERNAME`, but I see this as an 
acceptable tradeoff for the extra security.
  - This will make it impossible to affect other checkers. I'm a little unsure 
whether this is achievable considering how intertwined everything is in 
`MallocChecker`, but even if the supplied `CheckerManager` will have to 
non-const, `getCurrentCheckerName` and `setCurrentCheckerName` could be 
eliminated for sure.
- Properly document why `CheckerRegistry` and `CheckerManager` are separate 
entities, how are the tblgen files processed, how are dependencies handled, and 
so on.
- Rebase my local checker option-related branches, and celebrate. Unless 
another skeleton falls out of the closet. You never know I guess.

[1]

  void ento::registerObjCDeallocChecker(CheckerManager &Mgr) {
    const LangOptions &LangOpts = Mgr.getLangOpts();
    // These checker only makes sense under MRR.
    if (LangOpts.getGC() == LangOptions::GCOnly || LangOpts.ObjCAutoRefCount)
      return;
  
    Mgr.registerChecker<ObjCDeallocChecker>();
  }


Repository:
  rC Clang

https://reviews.llvm.org/D54438



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to