[clang-tidy] bugprone-argument-comment: SourceLocation valid judgment avoid emitting coredump in isInSystemHeader

[Sockke]

Summary

If the Node has an invalid location, it will trigger assert in isInSystemHeader(...).

void test() {
  __builtin_va_list __args;
  // __builtin_va_list has no defination in any source file and its CXXConstructorDecl has invalid sourcelocation
}

coredump with "Assertion `Loc.isValid() && "Can't get file characteristic of invalid loc!"' failed." in getFileCharacteristic(SourceLocation)

[aaron.ballman]

Can you add a test case that shows we've fixed the issue? (Note, you cannot include iostream directly in a test because that will use the platform STL and we want our tests to have consistent behavior regardless of platform.)

[Sockke]

Update!

[aaron.ballman]

LGTM, thank you! Do you need me to commit on your behalf? I'm happy to do so, but given the number of quality submissions you've had, I'm wondering if you'd like to obtain commit access of your own? (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access has the details of what that entails.)

[Sockke]

In D111625#3060978, @aaron.ballman wrote:

LGTM, thank you! Do you need me to commit on your behalf? I'm happy to do so, but given the number of quality submissions you've had, I'm wondering if you'd like to obtain commit access of your own? (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access has the details of what that entails.)

Thanks a lot! It's my pleasure to obtain commit access of my own. I will send the application as required later. Before that, I would appreciate it if you could commit it on my behalf. Thanks again!
Name: liuke
Email: [e-mail]

[aaron.ballman]

In D111625#3061736, @Sockke wrote:

In D111625#3060978, @aaron.ballman wrote:

LGTM, thank you! Do you need me to commit on your behalf? I'm happy to do so, but given the number of quality submissions you've had, I'm wondering if you'd like to obtain commit access of your own? (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access has the details of what that entails.)

Thanks a lot! It's my pleasure to obtain commit access of my own. I will send the application as required later. Before that, I would appreciate it if you could commit it on my behalf. Thanks again!
Name: liuke
Email: [e-mail]

Sounds great! I've commit on your behalf in ea72b55b5c7c281cb21bb7bd50e6e039ca63dfe8, thank you for the fix!

clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp

 namespace clang {
 namespace tidy {
 namespace bugprone {
 namespace {
 AST_MATCHER(Decl, isFromStdNamespaceOrSystemHeader) {
   if (const auto *D = Node.getDeclContext()->getEnclosingNamespaceContext())
     if (D->isStdNamespace())
       return true;
+  if (Node.getLocation().isInvalid())
+    return false;
   return Node.getASTContext().getSourceManager().isInSystemHeader(
       Node.getLocation());
 }
 } // namespace
 
 ArgumentCommentCheck::ArgumentCommentCheck(StringRef Name,
                                            ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),

clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment.cpp

 } // namespace regular_header
 
 namespace system_header {
 #include "system-header-with-decl.h"
 void test() {
   my_system_header_function(/*not_arg=*/1);
 }
 } // namespace system_header
+
+void testInvalidSlocCxxConstructExpr() {
+  __builtin_va_list __args;
+  // __builtin_va_list has no defination in any source file
+}