[clang] [Clang][AST] Fix a crash on attaching doc comments (PR #78716)

Dmitri Gribenko via cfe-commits cfe-commits at lists.llvm.org
Sat Jan 20 16:04:42 PST 2024


gribozavr wrote:

Thank you for the fix.

Here's what happens here.

This function receives the just-parsed decls, and its aim is to the comment in the same vicinity (I know this because I am the original author of the code).

This first loop over `Decls` identifies the file in which we will be looking for comments. Then the code retrieves the comments in that file (`auto CommentsInThisFile = Comments.getCommentsInFile(File);`). At this point we have really committed which file we are looking in.

Now, the next loop over `Decls` disturbs this decision this by adjusting the decl to the template. That decl could be in a different file.

The call `const auto DeclLocs = getDeclLocsForCommentSearch(D, SourceMgr);` retrieves source locations for the adjusted decl. The adjusted decl could be in a different file from the comments that we retrieved in `CommentsInThisFile`.

Next, we are passing the mismatched `DeclLocs` and `CommentsInThisFile` into `getRawCommentForDeclNoCacheImpl`, which crashes when they come from different files.

I think the bug was introduced as a consequence of f31d8df1c8c69 and b67d3702577d4, while the recent source location change merely unmasked it.

So, this makes me wondering if there is a better way to structure this code to make the mismatch between `DeclLocs` and `CommentsInThisFile` impossible. I think a better way would be to merge the two loops over `Decls` and call `Comments.getCommentsInFile` for the source location from `DeclLocs`.

WDYT @chenshanzhi ? We can merge this change as is, since it is a good crash fix, but it would be great if you could make the code more robust by merging the loops.

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


More information about the cfe-commits mailing list