[PATCH] D85417: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 7 05:54:18 PDT 2020


hokein added a comment.

In D85417#2202046 <https://reviews.llvm.org/D85417#2202046>, @ArcsinX wrote:

> In D85417#2201973 <https://reviews.llvm.org/D85417#2201973>, @hokein wrote:
>
>> Thinking more about this,
>>
>>> Inside clangd, clang-tidy checks don't see preprocessor events in the preamble.
>>> This leads to Token::PtrData == nullptr for tokens that the macro is defined to.
>>> E.g. #define SIGTERM 15:
>>>
>>> Token::Kind == tok::numeric_constant (Token::isLiteral() == true)
>>> Token::UintData == 2
>>> Token::PtrData == nullptr
>>
>> The token is in a pretty-broken state. Do you know why the UintData is set to 2, I suppose this is the length of the macro definition (`15`). If the PtrData is nullptr, I'd expect the UintData is 0.
>
> I think it's here:
>
>   Token ASTReader::ReadToken(ModuleFile &F, const RecordDataImpl &Record,
>                              unsigned &Idx) {
>     Token Tok;
>     Tok.startToken();
>     Tok.setLocation(ReadSourceLocation(F, Record, Idx));
>     Tok.setLength(Record[Idx++]);
>     if (IdentifierInfo *II = getLocalIdentifier(F, Record[Idx++]))
>       Tok.setIdentifierInfo(II);
>     Tok.setKind((tok::TokenKind)Record[Idx++]);
>     Tok.setFlag((Token::TokenFlags)Record[Idx++]);
>     return Tok;
>   }
>
> So, we set `Token::UintData` via `Token::setLength()` at preamble read, but do not set `Token::PtrData` without preprocessor events.

I see, thanks! I think this is a missing feature in AST serialization, see https://github.com/llvm/llvm-project/blob/master/clang/lib/Serialization/ASTWriter.cpp#L4260-L4261.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85417/new/

https://reviews.llvm.org/D85417



More information about the cfe-commits mailing list