[PATCH] D91540: [preprocessor] Assertions on the inferrable null pointers in Preprocessor befere dereference

[Ella Ma via Phabricator via cfe-commits]

OikawaKirie created this revision.
OikawaKirie added reviewers: dnsampaio, rsmith, lattner, ilya-biryukov.
OikawaKirie added a project: clang.
Herald added a subscriber: cfe-commits.
OikawaKirie requested review of this revision.

This patch asserts on some smart pointers that can be statically inferred to be null pointers. For a smart pointer, when its nullability is checked, and the null branch can reach a dereference, we think it is a potential null pointer dereference. In this patch, I added an assertion before each reported dereference.

- clang/lib/Lex/PPDirectives.cpp:441, inferred from the loop body: e.g. line 481, check and continue
- clang/lib/Lex/PPDirectives.cpp:1385, inferred from line 911: check `CurLexer`
- clang/lib/Lex/PPDirectives.cpp:1723, inferred from line 911: check `CurLexer`
- clang/lib/Lex/PPDirectives.cpp:2540, inferred from line 911: check `CurLexer`
- clang/lib/Lex/PPLexerChange.cpp:320, inferred from line 311: check `CurLexer`
- clang/lib/Lex/Preprocessor.cpp:1226, inferred from recomputeCurLexerKind line 383: check `CurLexer`

Besides, the analyzer also reports the dereference in function SkipTokensWhileUsingPCH and Lex. I think these two reported dereference cannot actually happen, but I also add the asserts to make sure the value is correct.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91540

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPLexerChange.cpp
  clang/lib/Lex/Preprocessor.cpp


Index: clang/lib/Lex/Preprocessor.cpp
===================================================================
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -637,9 +637,12 @@
         (CurLexer && CurLexer->getFileID() == getPredefinesFileID());
     switch (CurLexerKind) {
     case CLK_Lexer:
+      assert(CurLexer && "CurLexer is empty while CurLexerKind is CLK_Lexer.");
       CurLexer->Lex(Tok);
      break;
     case CLK_TokenLexer:
+      assert(CurTokenLexer &&
+          "CurTokenLexer is empty while CurLexerKind is CLK_TokenLexer.");
       CurTokenLexer->Lex(Tok);
       break;
     case CLK_CachingLexer:
@@ -895,9 +898,12 @@
   do {
     switch (CurLexerKind) {
     case CLK_Lexer:
+      assert(CurLexer && "CurLexer is empty while CurLexerKind is CLK_Lexer.");
       ReturnedToken = CurLexer->Lex(Result);
       break;
     case CLK_TokenLexer:
+      assert(CurTokenLexer &&
+          "CurTokenLexer is empty while CurLexerKind is CLK_TokenLexer.");
       ReturnedToken = CurTokenLexer->Lex(Result);
       break;
     case CLK_CachingLexer:
@@ -1217,6 +1223,7 @@
       assert(TheModuleLoader.HadFatalFailure &&
              "This should be an early exit only to a fatal error");
       Result.setKind(tok::eof);
+      assert(CurLexer);
       CurLexer->cutOffLexing();
       EnterTokens(Suffix);
       return true;
Index: clang/lib/Lex/PPLexerChange.cpp
===================================================================
--- clang/lib/Lex/PPLexerChange.cpp
+++ clang/lib/Lex/PPLexerChange.cpp
@@ -317,6 +317,7 @@
     Module *M = LeaveSubmodule(/*ForPragma*/true);
 
     Result.startToken();
+    assert(CurLexer && "Got EOF but no current lexer set!");
     const char *EndPos = getCurLexerEndPos();
     CurLexer->BufferPtr = EndPos;
     CurLexer->FormTokenWithChars(Result, EndPos, tok::annot_module_end);
Index: clang/lib/Lex/PPDirectives.cpp
===================================================================
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -438,6 +438,7 @@
   }
   SourceLocation endLoc;
   while (true) {
+    assert(CurLexer);
     CurLexer->Lex(Tok);
 
     if (Tok.is(tok::code_completion)) {
@@ -1381,6 +1382,8 @@
 ///
 void Preprocessor::HandleUserDiagnosticDirective(Token &Tok,
                                                  bool isWarning) {
+  assert(CurLexer);
+
   // Read the rest of the line raw.  We do this because we don't want macros
   // to be expanded and we don't require that the tokens be valid preprocessing
   // tokens.  For example, this is allowed: "#warning `   'foo".  GCC does
@@ -1717,6 +1720,7 @@
            "This should be an early exit only to a fatal error");
     TheModuleLoader.HadFatalFailure = true;
     IncludeTok.setKind(tok::eof);
+    assert(CurLexer);
     CurLexer->cutOffLexing();
     return;
   }
@@ -2533,6 +2537,7 @@
   // Ensure we consume the rest of the macro body if errors occur.
   auto _ = llvm::make_scope_exit([&]() {
     // The flag indicates if we are still waiting for 'eod'.
+    assert(CurLexer);
     if (CurLexer->ParsingPreprocessorDirective)
       DiscardUntilEndOfDirective();
   });


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D91540.305491.patch
Type: text/x-patch
Size: 3170 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201116/5066a919/attachment.bin>