[clang-tidy] Fix null pointer dereference in readability-identifier-naming

[markstegeman]

Summary

readability-identifier-naming causes a null pointer dereference when checking an identifier introduced by a structured binding whose right hand side is an undeclared identifier.

Running the check on a file that is just the following results in a crash:

auto [left] = right;

[markstegeman]

I haven't yet been able to figure out how to properly add a test for this bug, since the expected result is a failure to compile while the current test/clang-tidy/readability-identifier-naming.cpp must compile to pass.

Any suggestions?

[aaron.ballman]

In D62404#1516243, @markstegeman wrote:

I haven't yet been able to figure out how to properly add a test for this bug, since the expected result is a failure to compile while the current test/clang-tidy/readability-identifier-naming.cpp must compile to pass.

Any suggestions?

The way we usually handle this is to create a separate test file that exhibits the previously crashing code with a comment saying the test case used to crash. IIRC, you can use a CHECK line to check that you get the expected error.

[markstegeman]

Added a test.

[hokein]

clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp

804

We are risky of dereferencing a nullptr, as the TypePtr can be null now.

[madsravn]

clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp

804

Are we not checking for null in the line where TypePtr gets declared? Or am I misunderstanding?

[aaron.ballman]

LGTM!

clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp

804

I don't see a null pointer dereference here for TypePtr; I think the code is fine as-is.

[markstegeman]

Great. Can someone commit this for me?

[hokein]

clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp

804

Ah, I missed the if statement, it is guarded by the if statement.

clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp

 
     // Fix type aliases in value declarations
     if (const auto *Value = Result.Nodes.getNodeAs<ValueDecl>("decl")) {
-      if (const auto *Typedef =
-              Value->getType().getTypePtr()->getAs<TypedefType>()) {
+      if (const auto *TypePtr = Value->getType().getTypePtrOrNull()) {
+        if (const auto *Typedef = TypePtr->getAs<TypedefType>()) {
           addUsage(NamingCheckFailures, Typedef->getDecl(),
                    Value->getSourceRange());
+        }
       }
     }