r305425 - [Preprocessor]Correct Macro-Arg allocation of StringifiedArguments,

Erich Keane via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 14 16:09:01 PDT 2017


Author: erichkeane
Date: Wed Jun 14 18:09:01 2017
New Revision: 305425

URL: http://llvm.org/viewvc/llvm-project?rev=305425&view=rev
Log:
[Preprocessor]Correct Macro-Arg allocation of StringifiedArguments, 
correct getNumArguments

StringifiedArguments is allocated (resized) based on the size the 
getNumArguments function. However, this function ACTUALLY currently 
returns the amount of total UnexpArgTokens which is minimum the same as 
the new implementation of getNumMacroArguments, since empty/omitted arguments 
result in 1 UnexpArgToken, and included ones at minimum include 2 
(1 for the arg itself, 1 for eof).

This patch renames the otherwise unused getNumArguments to be more clear 
that it is the number of arguments that the Macro expects, and thus the maximum 
number that can be stringified. This patch also replaces the explicit memset 
(which results in value instantiation of the new tokens, PLUS clearing the 
memory) with brace initialization.

Differential Revision: https://reviews.llvm.org/D32046

Modified:
    cfe/trunk/include/clang/Lex/MacroArgs.h
    cfe/trunk/lib/Lex/MacroArgs.cpp
    cfe/trunk/unittests/Lex/LexerTest.cpp

Modified: cfe/trunk/include/clang/Lex/MacroArgs.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/MacroArgs.h?rev=305425&r1=305424&r2=305425&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/MacroArgs.h (original)
+++ cfe/trunk/include/clang/Lex/MacroArgs.h Wed Jun 14 18:09:01 2017
@@ -53,9 +53,12 @@ class MacroArgs {
   /// Preprocessor owns which we use to avoid thrashing malloc/free.
   MacroArgs *ArgCache;
 
-  MacroArgs(unsigned NumToks, bool varargsElided)
-    : NumUnexpArgTokens(NumToks), VarargsElided(varargsElided),
-      ArgCache(nullptr) {}
+  /// MacroArgs - The number of arguments the invoked macro expects.
+  unsigned NumMacroArgs;
+
+  MacroArgs(unsigned NumToks, bool varargsElided, unsigned MacroArgs)
+      : NumUnexpArgTokens(NumToks), VarargsElided(varargsElided),
+        ArgCache(nullptr), NumMacroArgs(MacroArgs) {}
   ~MacroArgs() = default;
 
 public:
@@ -94,10 +97,9 @@ public:
                                       SourceLocation ExpansionLocStart,
                                       SourceLocation ExpansionLocEnd);
 
-  /// getNumArguments - Return the number of arguments passed into this macro
-  /// invocation.
-  unsigned getNumArguments() const { return NumUnexpArgTokens; }
-
+  /// getNumMacroArguments - Return the number of arguments the invoked macro
+  /// expects.
+  unsigned getNumMacroArguments() const { return NumMacroArgs; }
 
   /// isVarargsElidedUse - Return true if this is a C99 style varargs macro
   /// invocation and there was no argument specified for the "..." argument.  If

Modified: cfe/trunk/lib/Lex/MacroArgs.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/MacroArgs.cpp?rev=305425&r1=305424&r2=305425&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/MacroArgs.cpp (original)
+++ cfe/trunk/lib/Lex/MacroArgs.cpp Wed Jun 14 18:09:01 2017
@@ -44,20 +44,22 @@ MacroArgs *MacroArgs::create(const Macro
       // Otherwise, use the best fit.
       ClosestMatch = (*Entry)->NumUnexpArgTokens;
     }
-  
+
   MacroArgs *Result;
   if (!ResultEnt) {
     // Allocate memory for a MacroArgs object with the lexer tokens at the end.
-    Result = (MacroArgs*)malloc(sizeof(MacroArgs) + 
-                                UnexpArgTokens.size() * sizeof(Token));
+    Result = (MacroArgs *)malloc(sizeof(MacroArgs) +
+                                 UnexpArgTokens.size() * sizeof(Token));
     // Construct the MacroArgs object.
-    new (Result) MacroArgs(UnexpArgTokens.size(), VarargsElided);
+    new (Result)
+        MacroArgs(UnexpArgTokens.size(), VarargsElided, MI->getNumArgs());
   } else {
     Result = *ResultEnt;
     // Unlink this node from the preprocessors singly linked list.
     *ResultEnt = Result->ArgCache;
     Result->NumUnexpArgTokens = UnexpArgTokens.size();
     Result->VarargsElided = VarargsElided;
+    Result->NumMacroArgs = MI->getNumArgs();
   }
 
   // Copy the actual unexpanded tokens to immediately after the result ptr.
@@ -298,12 +300,10 @@ const Token &MacroArgs::getStringifiedAr
                                                Preprocessor &PP,
                                                SourceLocation ExpansionLocStart,
                                                SourceLocation ExpansionLocEnd) {
-  assert(ArgNo < NumUnexpArgTokens && "Invalid argument number!");
-  if (StringifiedArgs.empty()) {
-    StringifiedArgs.resize(getNumArguments());
-    memset((void*)&StringifiedArgs[0], 0,
-           sizeof(StringifiedArgs[0])*getNumArguments());
-  }
+  assert(ArgNo < getNumMacroArguments() && "Invalid argument number!");
+  if (StringifiedArgs.empty())
+    StringifiedArgs.resize(getNumMacroArguments(), {});
+
   if (StringifiedArgs[ArgNo].isNot(tok::string_literal))
     StringifiedArgs[ArgNo] = StringifyArgument(getUnexpArgument(ArgNo), PP,
                                                /*Charify=*/false,

Modified: cfe/trunk/unittests/Lex/LexerTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/LexerTest.cpp?rev=305425&r1=305424&r2=305425&view=diff
==============================================================================
--- cfe/trunk/unittests/Lex/LexerTest.cpp (original)
+++ cfe/trunk/unittests/Lex/LexerTest.cpp Wed Jun 14 18:09:01 2017
@@ -18,6 +18,8 @@
 #include "clang/Basic/TargetOptions.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/HeaderSearchOptions.h"
+#include "clang/Lex/MacroArgs.h"
+#include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/ModuleLoader.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
@@ -41,26 +43,33 @@ protected:
     Target = TargetInfo::CreateTargetInfo(Diags, TargetOpts);
   }
 
-  std::vector<Token> Lex(StringRef Source) {
+  std::unique_ptr<Preprocessor> CreatePP(StringRef Source,
+                                         TrivialModuleLoader &ModLoader) {
     std::unique_ptr<llvm::MemoryBuffer> Buf =
         llvm::MemoryBuffer::getMemBuffer(Source);
     SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));
 
-    TrivialModuleLoader ModLoader;
     MemoryBufferCache PCMCache;
     HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
                             Diags, LangOpts, Target.get());
-    Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
-                    SourceMgr, PCMCache, HeaderInfo, ModLoader,
-                    /*IILookup =*/nullptr,
-                    /*OwnsHeaderSearch =*/false);
-    PP.Initialize(*Target);
-    PP.EnterMainSourceFile();
+    std::unique_ptr<Preprocessor> PP = llvm::make_unique<Preprocessor>(
+        std::make_shared<PreprocessorOptions>(), Diags, LangOpts, SourceMgr,
+        PCMCache, HeaderInfo, ModLoader,
+        /*IILookup =*/nullptr,
+        /*OwnsHeaderSearch =*/false);
+    PP->Initialize(*Target);
+    PP->EnterMainSourceFile();
+    return PP;
+  }
+
+  std::vector<Token> Lex(StringRef Source) {
+    TrivialModuleLoader ModLoader;
+    auto PP = CreatePP(Source, ModLoader);
 
     std::vector<Token> toks;
     while (1) {
       Token tok;
-      PP.Lex(tok);
+      PP->Lex(tok);
       if (tok.is(tok::eof))
         break;
       toks.push_back(tok);
@@ -365,4 +374,48 @@ TEST_F(LexerTest, DontMergeMacroArgsFrom
   EXPECT_EQ(SourceMgr.getFileIDSize(SourceMgr.getFileID(helper1ArgLoc)), 8U);
 }
 
+TEST_F(LexerTest, DontOverallocateStringifyArgs) {
+  TrivialModuleLoader ModLoader;
+  auto PP = CreatePP("\"StrArg\", 5, 'C'", ModLoader);
+
+  llvm::BumpPtrAllocator Allocator;
+  std::array<IdentifierInfo *, 3> ArgList;
+  MacroInfo *MI = PP->AllocateMacroInfo({});
+  MI->setIsFunctionLike();
+  MI->setArgumentList(ArgList, Allocator);
+  EXPECT_EQ(3, MI->getNumArgs());
+  EXPECT_TRUE(MI->isFunctionLike());
+
+  Token Eof;
+  Eof.setKind(tok::eof);
+  std::vector<Token> ArgTokens;
+  while (1) {
+    Token tok;
+    PP->Lex(tok);
+    if (tok.is(tok::eof)) {
+      ArgTokens.push_back(Eof);
+      break;
+    }
+    if (tok.is(tok::comma))
+      ArgTokens.push_back(Eof);
+    else
+      ArgTokens.push_back(tok);
+  }
+
+  MacroArgs *MA = MacroArgs::create(MI, ArgTokens, false, *PP);
+  Token Result = MA->getStringifiedArgument(0, *PP, {}, {});
+  EXPECT_EQ(tok::string_literal, Result.getKind());
+  EXPECT_STREQ("\"\\\"StrArg\\\"\"", Result.getLiteralData());
+  Result = MA->getStringifiedArgument(1, *PP, {}, {});
+  EXPECT_EQ(tok::string_literal, Result.getKind());
+  EXPECT_STREQ("\"5\"", Result.getLiteralData());
+  Result = MA->getStringifiedArgument(2, *PP, {}, {});
+  EXPECT_EQ(tok::string_literal, Result.getKind());
+  EXPECT_STREQ("\"'C'\"", Result.getLiteralData());
+#if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST
+  EXPECT_DEATH(MA->getStringifiedArgument(3, *PP, {}, {}),
+               "Invalid argument number!");
+#endif
+}
+
 } // anonymous namespace




More information about the cfe-commits mailing list