<div dir="ltr">the bots complain about a leak in the new test code. <div>Please fix/revert ASAP. <br><div><div><a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/5691/steps/check-clang%20asan/logs/stdio">http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/5691/steps/check-clang%20asan/logs/stdio</a><br></div><div><pre style="font-family:"Courier New",courier,monotype,monospace;color:rgb(0,0,0);font-size:medium"><span class="gmail-stdout">=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)
</span></pre></div><div><span class="gmail-stdout"><br></span></div></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jun 14, 2017 at 4:09 PM, Erich Keane via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: erichkeane<br>
Date: Wed Jun 14 18:09:01 2017<br>
New Revision: 305425<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=305425&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=305425&view=rev</a><br>
Log:<br>
[Preprocessor]Correct Macro-Arg allocation of StringifiedArguments,<br>
correct getNumArguments<br>
<br>
StringifiedArguments is allocated (resized) based on the size the<br>
getNumArguments function. However, this function ACTUALLY currently<br>
returns the amount of total UnexpArgTokens which is minimum the same as<br>
the new implementation of getNumMacroArguments, since empty/omitted arguments<br>
result in 1 UnexpArgToken, and included ones at minimum include 2<br>
(1 for the arg itself, 1 for eof).<br>
<br>
This patch renames the otherwise unused getNumArguments to be more clear<br>
that it is the number of arguments that the Macro expects, and thus the maximum<br>
number that can be stringified. This patch also replaces the explicit memset<br>
(which results in value instantiation of the new tokens, PLUS clearing the<br>
memory) with brace initialization.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D32046" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D32046</a><br>
<br>
Modified:<br>
cfe/trunk/include/clang/Lex/<wbr>MacroArgs.h<br>
cfe/trunk/lib/Lex/MacroArgs.<wbr>cpp<br>
cfe/trunk/unittests/Lex/<wbr>LexerTest.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Lex/<wbr>MacroArgs.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/MacroArgs.h?rev=305425&r1=305424&r2=305425&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/include/<wbr>clang/Lex/MacroArgs.h?rev=<wbr>305425&r1=305424&r2=305425&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/include/clang/Lex/<wbr>MacroArgs.h (original)<br>
+++ cfe/trunk/include/clang/Lex/<wbr>MacroArgs.h Wed Jun 14 18:09:01 2017<br>
@@ -53,9 +53,12 @@ class MacroArgs {<br>
/// Preprocessor owns which we use to avoid thrashing malloc/free.<br>
MacroArgs *ArgCache;<br>
<br>
- MacroArgs(unsigned NumToks, bool varargsElided)<br>
- : NumUnexpArgTokens(NumToks), VarargsElided(varargsElided),<br>
- ArgCache(nullptr) {}<br>
+ /// MacroArgs - The number of arguments the invoked macro expects.<br>
+ unsigned NumMacroArgs;<br>
+<br>
+ MacroArgs(unsigned NumToks, bool varargsElided, unsigned MacroArgs)<br>
+ : NumUnexpArgTokens(NumToks), VarargsElided(varargsElided),<br>
+ ArgCache(nullptr), NumMacroArgs(MacroArgs) {}<br>
~MacroArgs() = default;<br>
<br>
public:<br>
@@ -94,10 +97,9 @@ public:<br>
SourceLocation ExpansionLocStart,<br>
SourceLocation ExpansionLocEnd);<br>
<br>
- /// getNumArguments - Return the number of arguments passed into this macro<br>
- /// invocation.<br>
- unsigned getNumArguments() const { return NumUnexpArgTokens; }<br>
-<br>
+ /// getNumMacroArguments - Return the number of arguments the invoked macro<br>
+ /// expects.<br>
+ unsigned getNumMacroArguments() const { return NumMacroArgs; }<br>
<br>
/// isVarargsElidedUse - Return true if this is a C99 style varargs macro<br>
/// invocation and there was no argument specified for the "..." argument. If<br>
<br>
Modified: cfe/trunk/lib/Lex/MacroArgs.<wbr>cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/MacroArgs.cpp?rev=305425&r1=305424&r2=305425&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/Lex/<wbr>MacroArgs.cpp?rev=305425&r1=<wbr>305424&r2=305425&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Lex/MacroArgs.<wbr>cpp (original)<br>
+++ cfe/trunk/lib/Lex/MacroArgs.<wbr>cpp Wed Jun 14 18:09:01 2017<br>
@@ -44,20 +44,22 @@ MacroArgs *MacroArgs::create(const Macro<br>
// Otherwise, use the best fit.<br>
ClosestMatch = (*Entry)->NumUnexpArgTokens;<br>
}<br>
-<br>
+<br>
MacroArgs *Result;<br>
if (!ResultEnt) {<br>
// Allocate memory for a MacroArgs object with the lexer tokens at the end.<br>
- Result = (MacroArgs*)malloc(sizeof(<wbr>MacroArgs) +<br>
- UnexpArgTokens.size() * sizeof(Token));<br>
+ Result = (MacroArgs *)malloc(sizeof(MacroArgs) +<br>
+ UnexpArgTokens.size() * sizeof(Token));<br>
// Construct the MacroArgs object.<br>
- new (Result) MacroArgs(UnexpArgTokens.size(<wbr>), VarargsElided);<br>
+ new (Result)<br>
+ MacroArgs(UnexpArgTokens.size(<wbr>), VarargsElided, MI->getNumArgs());<br>
} else {<br>
Result = *ResultEnt;<br>
// Unlink this node from the preprocessors singly linked list.<br>
*ResultEnt = Result->ArgCache;<br>
Result->NumUnexpArgTokens = UnexpArgTokens.size();<br>
Result->VarargsElided = VarargsElided;<br>
+ Result->NumMacroArgs = MI->getNumArgs();<br>
}<br>
<br>
// Copy the actual unexpanded tokens to immediately after the result ptr.<br>
@@ -298,12 +300,10 @@ const Token &MacroArgs::getStringifiedAr<br>
Preprocessor &PP,<br>
SourceLocation ExpansionLocStart,<br>
SourceLocation ExpansionLocEnd) {<br>
- assert(ArgNo < NumUnexpArgTokens && "Invalid argument number!");<br>
- if (StringifiedArgs.empty()) {<br>
- StringifiedArgs.resize(<wbr>getNumArguments());<br>
- memset((void*)&<wbr>StringifiedArgs[0], 0,<br>
- sizeof(StringifiedArgs[0])*<wbr>getNumArguments());<br>
- }<br>
+ assert(ArgNo < getNumMacroArguments() && "Invalid argument number!");<br>
+ if (StringifiedArgs.empty())<br>
+ StringifiedArgs.resize(<wbr>getNumMacroArguments(), {});<br>
+<br>
if (StringifiedArgs[ArgNo].isNot(<wbr>tok::string_literal))<br>
StringifiedArgs[ArgNo] = StringifyArgument(<wbr>getUnexpArgument(ArgNo), PP,<br>
/*Charify=*/false,<br>
<br>
Modified: cfe/trunk/unittests/Lex/<wbr>LexerTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/LexerTest.cpp?rev=305425&r1=305424&r2=305425&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/unittests/<wbr>Lex/LexerTest.cpp?rev=305425&<wbr>r1=305424&r2=305425&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/unittests/Lex/<wbr>LexerTest.cpp (original)<br>
+++ cfe/trunk/unittests/Lex/<wbr>LexerTest.cpp Wed Jun 14 18:09:01 2017<br>
@@ -18,6 +18,8 @@<br>
#include "clang/Basic/TargetOptions.h"<br>
#include "clang/Lex/HeaderSearch.h"<br>
#include "clang/Lex/<wbr>HeaderSearchOptions.h"<br>
+#include "clang/Lex/MacroArgs.h"<br>
+#include "clang/Lex/MacroInfo.h"<br>
#include "clang/Lex/ModuleLoader.h"<br>
#include "clang/Lex/Preprocessor.h"<br>
#include "clang/Lex/<wbr>PreprocessorOptions.h"<br>
@@ -41,26 +43,33 @@ protected:<br>
Target = TargetInfo::CreateTargetInfo(<wbr>Diags, TargetOpts);<br>
}<br>
<br>
- std::vector<Token> Lex(StringRef Source) {<br>
+ std::unique_ptr<Preprocessor> CreatePP(StringRef Source,<br>
+ TrivialModuleLoader &ModLoader) {<br>
std::unique_ptr<llvm::<wbr>MemoryBuffer> Buf =<br>
llvm::MemoryBuffer::<wbr>getMemBuffer(Source);<br>
SourceMgr.setMainFileID(<wbr>SourceMgr.createFileID(std::<wbr>move(Buf)));<br>
<br>
- TrivialModuleLoader ModLoader;<br>
MemoryBufferCache PCMCache;<br>
HeaderSearch HeaderInfo(std::make_shared<<wbr>HeaderSearchOptions>(), SourceMgr,<br>
Diags, LangOpts, Target.get());<br>
- Preprocessor PP(std::make_shared<<wbr>PreprocessorOptions>(), Diags, LangOpts,<br>
- SourceMgr, PCMCache, HeaderInfo, ModLoader,<br>
- /*IILookup =*/nullptr,<br>
- /*OwnsHeaderSearch =*/false);<br>
- PP.Initialize(*Target);<br>
- PP.EnterMainSourceFile();<br>
+ std::unique_ptr<Preprocessor> PP = llvm::make_unique<<wbr>Preprocessor>(<br>
+ std::make_shared<<wbr>PreprocessorOptions>(), Diags, LangOpts, SourceMgr,<br>
+ PCMCache, HeaderInfo, ModLoader,<br>
+ /*IILookup =*/nullptr,<br>
+ /*OwnsHeaderSearch =*/false);<br>
+ PP->Initialize(*Target);<br>
+ PP->EnterMainSourceFile();<br>
+ return PP;<br>
+ }<br>
+<br>
+ std::vector<Token> Lex(StringRef Source) {<br>
+ TrivialModuleLoader ModLoader;<br>
+ auto PP = CreatePP(Source, ModLoader);<br>
<br>
std::vector<Token> toks;<br>
while (1) {<br>
Token tok;<br>
- PP.Lex(tok);<br>
+ PP->Lex(tok);<br>
if (<a href="http://tok.is" rel="noreferrer" target="_blank">tok.is</a>(tok::eof))<br>
break;<br>
toks.push_back(tok);<br>
@@ -365,4 +374,48 @@ TEST_F(LexerTest, DontMergeMacroArgsFrom<br>
EXPECT_EQ(SourceMgr.<wbr>getFileIDSize(SourceMgr.<wbr>getFileID(helper1ArgLoc)), 8U);<br>
}<br>
<br>
+TEST_F(LexerTest, DontOverallocateStringifyArgs) {<br>
+ TrivialModuleLoader ModLoader;<br>
+ auto PP = CreatePP("\"StrArg\", 5, 'C'", ModLoader);<br>
+<br>
+ llvm::BumpPtrAllocator Allocator;<br>
+ std::array<IdentifierInfo *, 3> ArgList;<br>
+ MacroInfo *MI = PP->AllocateMacroInfo({});<br>
+ MI->setIsFunctionLike();<br>
+ MI->setArgumentList(ArgList, Allocator);<br>
+ EXPECT_EQ(3, MI->getNumArgs());<br>
+ EXPECT_TRUE(MI-><wbr>isFunctionLike());<br>
+<br>
+ Token Eof;<br>
+ Eof.setKind(tok::eof);<br>
+ std::vector<Token> ArgTokens;<br>
+ while (1) {<br>
+ Token tok;<br>
+ PP->Lex(tok);<br>
+ if (<a href="http://tok.is" rel="noreferrer" target="_blank">tok.is</a>(tok::eof)) {<br>
+ ArgTokens.push_back(Eof);<br>
+ break;<br>
+ }<br>
+ if (<a href="http://tok.is" rel="noreferrer" target="_blank">tok.is</a>(tok::comma))<br>
+ ArgTokens.push_back(Eof);<br>
+ else<br>
+ ArgTokens.push_back(tok);<br>
+ }<br>
+<br>
+ MacroArgs *MA = MacroArgs::create(MI, ArgTokens, false, *PP);<br>
+ Token Result = MA->getStringifiedArgument(0, *PP, {}, {});<br>
+ EXPECT_EQ(tok::string_literal, Result.getKind());<br>
+ EXPECT_STREQ("\"\\\"StrArg\\\"<wbr>\"", Result.getLiteralData());<br>
+ Result = MA->getStringifiedArgument(1, *PP, {}, {});<br>
+ EXPECT_EQ(tok::string_literal, Result.getKind());<br>
+ EXPECT_STREQ("\"5\"", Result.getLiteralData());<br>
+ Result = MA->getStringifiedArgument(2, *PP, {}, {});<br>
+ EXPECT_EQ(tok::string_literal, Result.getKind());<br>
+ EXPECT_STREQ("\"'C'\"", Result.getLiteralData());<br>
+#if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST<br>
+ EXPECT_DEATH(MA-><wbr>getStringifiedArgument(3, *PP, {}, {}),<br>
+ "Invalid argument number!");<br>
+#endif<br>
+}<br>
+<br>
} // anonymous namespace<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>