CPP: Add a test of common mistakes using locking classes.

[geoffw0]

(follows on from discussion with @jbj on #1192)

Add a test showing a couple of common locking pitfalls, which we want to ensure we have good test coverage for:

• Bug #6 in Curiously Recurring C++ Bugs at Facebook, where a variable is declared for the lock but a difficult to spot typo results in the default constructor being called rather than the intended one
• this bug, where what appears to be a variable declaration is in fact a function declaration

Note that AV Rule 107 has been downgraded to a recommendation.

[jbj]

If you guard this in if (m), then none of the bad examples below will segfault. That makes it a much more scary bug.

[geoffw0]

Updated.

[geoffw0]

So as I see it there are two issues:

(1) AV Rule 107.ql ('Function declared in block') is only a recommendation following https://github.com/Semmle/ql/pull/1192/files. It may be worth making a more specialized version as a warning.

(the other relevant query, LocalVariableHidesGlobalVariable.ql, is a warning)

(2) [As far as I'm aware] the result in 'test7' is not flagged by any query.

[geoffw0]

(2) could take the form of a LocalVariableHidesMemberVariable.ql query (which you warned might be noisy, but I don't yet follow why) or a 'generalised "expression has no effect"' as you described in the older discussion (which I suspect might work out quite complicated, but could potentially spot more problems).

[jbj]

Noisy or not, LocalVariableHidesMemberVariable.ql would at most be a recommendation. We want something that's at least a warning. I agree that 'generalised "expression has no effect"' could become very complicated.

How about giving an alert when

1. A local variable shadows a field or a global,
2. the local variable is default-constructed and never modified,
3. the type of the local variable has a non-trivial destructor, and
4. the type of the local variable has a constructor that accepts an argument of the type of the shadowed field.

I think Bug #6 satisfies all four conditions, but it's possible we could drop some of them and find related bugs.