[clang] [analyzer] Untangle subcheckers of CStringChecker (PR #113312)

[via cfe-commits]

================
@@ -838,8 +860,15 @@ void CStringChecker::emitUninitializedReadBug(CheckerContext &C,
 void CStringChecker::emitOutOfBoundsBug(CheckerContext &C,
                                         ProgramStateRef State, const Stmt *S,
                                         StringRef WarningMsg) const {
+  // FIXME: This is absurd. If a checker is disabled, we just emit the bug
----------------
isuckatcs wrote:

After digging further, I think one of these names is never used. Below is the only callsite of the function.

```c++
ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C,
                                              ProgramStateRef state,
                                              AnyArgExpr Buffer, SVal Element,
                                              AccessKind Access,
                                              CharKind CK) const {

  ...
  if (StOutBound && !StInBound) {
    // These checks are either enabled by the CString out-of-bounds checker
    // explicitly or implicitly by the Malloc checker.
    // In the latter case we only do modeling but do not emit warning.
    if (!Filter.CheckCStringOutOfBounds)
      return nullptr;

    // Emit a bug report.
    ErrorMessage Message =
        createOutOfBoundErrorMsg(CurrentFunctionDescription, Access);
    emitOutOfBoundsBug(C, StOutBound, Buffer.Expression, Message);
    return nullptr;
  }

  ...
}
```

It is clear that if `Filter.CheckCStringOutOfBounds` is not enabled, the bug is never emited, so it can only be that check. 

I was doing my homework properly, so here is the full story. Initially it's been only one name, and [2ff57bc](https://github.com/llvm/llvm-project/commit/2ff57bcd18d3514c3f5baed6d34e02f885e81b28) introduced the other one. Later [D48831](https://reviews.llvm.org/D48831) has basically reverted the change, because the warning couldn't be disabled. 

As for why the other name became `Filter.CheckCStringNullArg` in the first place, I have the following theory. `CheckLocation()` is only called from `CheckBufferAccess()`, where it is guarded by `Filter.CheckCStringOutOfBounds` and `evalStrcpyCommon()`, where the message with the second name can come from. 

In `evalStrcpyCommon()` we call `checkNonNull()`, `CheckOverlap()`, `checkAdditionOverflow()` and `CheckLocation()`. Before the first patch, only `checkNonNull()` and `CheckLocation()` could report any warning when `Filter.CheckCStringOutOfBounds` was disabled, however `checkNonNull()` only reports if `Filter.CheckCStringNullArg` is enabled. 

The author of the commit must have thought there is a connection. Maybe the malloc checker only enabled `CStringNullArg` by default (though this no longer seems to be the case) and it looked like that checker is causing the warnings. In theory the warning would have been reported regardless of which checks were enabled or disabled I think.

https://github.com/llvm/llvm-project/pull/113312