r305425 - [Preprocessor]Correct Macro-Arg allocation of StringifiedArguments,
Kostya Serebryany via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 15 10:43:53 PDT 2017
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 at 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(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
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170615/601beca2/attachment-0001.html>
More information about the cfe-commits
mailing list