Ah, right… This function mallocs, and is usually tossed in the Preprocessor Cache, but I’m doing an end-run around that this time. I’ll see if it is better to put it into the PP Cache, or just call ‘free’ on it.
Thanks! -Erich From: Kostya Serebryany [mailto:k...@google.com] Sent: Thursday, June 15, 2017 10:44 AM To: Keane, Erich <erich.ke...@intel.com> Cc: cfe-commits <cfe-commits@lists.llvm.org> Subject: Re: r305425 - [Preprocessor]Correct Macro-Arg allocation of StringifiedArguments, the bots complain about a leak in the new test code. Please fix/revert ASAP. http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/5691/steps/check-clang%20asan/logs/stdio =28905==ERROR: LeakSanitizer: detected memory leaks Direct leak of 216 byte(s) in 1 object(s) allocated from: #0 0x4eca08 in __interceptor_malloc /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:66 #1 0xefcb8f in clang::MacroArgs::create(clang::MacroInfo const*, llvm::ArrayRef<clang::Token>, bool, clang::Preprocessor&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Lex/MacroArgs.cpp:51:27 #2 0x54dc56 in (anonymous namespace)::LexerTest_DontOverallocateStringifyArgs_Test::TestBody() /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/unittests/Lex/LexerTest.cpp:405:19 #3 0x65154e in HandleExceptionsInMethodIfSupported<testing::Test, void> /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2458:12 #4 0x65154e in testing::Test::Run() /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2474 #5 0x653848 in testing::TestInfo::Run() /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2656:11 #6 0x654b86 in testing::TestCase::Run() /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2774:28 #7 0x675586 in testing::internal::UnitTestImpl::RunAllTests() /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:4649:43 #8 0x67487e in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2458:12 #9 0x67487e in testing::UnitTest::Run() /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:4257 #10 0x634bfe in RUN_ALL_TESTS /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/include/gtest/gtest.h:2233:46 #11 0x634bfe in main /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/UnitTestMain/TestMain.cpp:51 #12 0x7f016e9cb82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f) On Wed, Jun 14, 2017 at 4:09 PM, Erich Keane via cfe-commits <cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote: 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<http://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<http://tok.is>(tok::eof)) { + ArgTokens.push_back(Eof); + break; + } + if (tok.is<http://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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
