<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Consolas;
panose-1:2 11 6 9 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman",serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
pre
{mso-style-priority:99;
mso-style-link:"HTML Preformatted Char";
margin:0in;
margin-bottom:.0001pt;
font-size:10.0pt;
font-family:"Courier New";}
span.HTMLPreformattedChar
{mso-style-name:"HTML Preformatted Char";
mso-style-priority:99;
mso-style-link:"HTML Preformatted";
font-family:Consolas;}
span.gmail-stdout
{mso-style-name:gmail-stdout;}
span.EmailStyle20
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">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.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Thanks!<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">-Erich<o:p></o:p></span></p>
<p class="MsoNormal"><a name="_MailEndCompose"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></a></p>
<p class="MsoNormal"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> Kostya Serebryany [mailto:kcc@google.com]
<br>
<b>Sent:</b> Thursday, June 15, 2017 10:44 AM<br>
<b>To:</b> Keane, Erich <erich.keane@intel.com><br>
<b>Cc:</b> cfe-commits <cfe-commits@lists.llvm.org><br>
<b>Subject:</b> Re: r305425 - [Preprocessor]Correct Macro-Arg allocation of StringifiedArguments,<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">the bots complain about a leak in the new test code. <o:p></o:p></p>
<div>
<p class="MsoNormal">Please fix/revert ASAP. <o:p></o:p></p>
<div>
<div>
<p class="MsoNormal"><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><o:p></o:p></p>
</div>
<div>
<pre><span class="gmail-stdout"><span style="font-size:13.5pt;color:black">=28905==ERROR: LeakSanitizer: detected memory leaks<o:p></o:p></span></span></pre>
<pre><span class="gmail-stdout"><span style="font-size:13.5pt;color:black"><o:p> </o:p></span></span></pre>
<pre><span class="gmail-stdout"><span style="font-size:13.5pt;color:black">Direct leak of 216 byte(s) in 1 object(s) allocated from:<o:p></o:p></span></span></pre>
<pre><span class="gmail-stdout"><span style="font-size:13.5pt;color:black"> #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<o:p></o:p></span></span></pre>
<pre><span class="gmail-stdout"><span style="font-size:13.5pt;color:black"> #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<o:p></o:p></span></span></pre>
<pre><span class="gmail-stdout"><span style="font-size:13.5pt;color:black"> #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<o:p></o:p></span></span></pre>
<pre><span class="gmail-stdout"><span style="font-size:13.5pt;color:black"> #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<o:p></o:p></span></span></pre>
<pre><span class="gmail-stdout"><span style="font-size:13.5pt;color:black"> #4 0x65154e in testing::Test::Run() /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2474<o:p></o:p></span></span></pre>
<pre><span class="gmail-stdout"><span style="font-size:13.5pt;color:black"> #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<o:p></o:p></span></span></pre>
<pre><span class="gmail-stdout"><span style="font-size:13.5pt;color:black"> #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<o:p></o:p></span></span></pre>
<pre><span class="gmail-stdout"><span style="font-size:13.5pt;color:black"> #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<o:p></o:p></span></span></pre>
<pre><span class="gmail-stdout"><span style="font-size:13.5pt;color:black"> #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<o:p></o:p></span></span></pre>
<pre><span class="gmail-stdout"><span style="font-size:13.5pt;color:black"> #9 0x67487e in testing::UnitTest::Run() /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:4257<o:p></o:p></span></span></pre>
<pre><span class="gmail-stdout"><span style="font-size:13.5pt;color:black"> #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<o:p></o:p></span></span></pre>
<pre><span class="gmail-stdout"><span style="font-size:13.5pt;color:black"> #11 0x634bfe in main /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/UnitTestMain/TestMain.cpp:51<o:p></o:p></span></span></pre>
<pre><span class="gmail-stdout"><span style="font-size:13.5pt;color:black"> #12 0x7f016e9cb82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)<o:p></o:p></span></span></pre>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">On Wed, Jun 14, 2017 at 4:09 PM, Erich Keane via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<o:p></o:p></p>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal">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" target="_blank">
http://llvm.org/viewvc/llvm-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" target="_blank">
https://reviews.llvm.org/D32046</a><br>
<br>
Modified:<br>
cfe/trunk/include/clang/Lex/MacroArgs.h<br>
cfe/trunk/lib/Lex/MacroArgs.cpp<br>
cfe/trunk/unittests/Lex/LexerTest.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Lex/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" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/MacroArgs.h?rev=305425&r1=305424&r2=305425&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Lex/MacroArgs.h (original)<br>
+++ cfe/trunk/include/clang/Lex/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.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" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/MacroArgs.cpp?rev=305425&r1=305424&r2=305425&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Lex/MacroArgs.cpp (original)<br>
+++ cfe/trunk/lib/Lex/MacroArgs.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(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(), VarargsElided);<br>
+ new (Result)<br>
+ MacroArgs(UnexpArgTokens.size(), 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(getNumArguments());<br>
- memset((void*)&StringifiedArgs[0], 0,<br>
- sizeof(StringifiedArgs[0])*getNumArguments());<br>
- }<br>
+ assert(ArgNo < getNumMacroArguments() && "Invalid argument number!");<br>
+ if (StringifiedArgs.empty())<br>
+ StringifiedArgs.resize(getNumMacroArguments(), {});<br>
+<br>
if (StringifiedArgs[ArgNo].isNot(tok::string_literal))<br>
StringifiedArgs[ArgNo] = StringifyArgument(getUnexpArgument(ArgNo), PP,<br>
/*Charify=*/false,<br>
<br>
Modified: cfe/trunk/unittests/Lex/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" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/LexerTest.cpp?rev=305425&r1=305424&r2=305425&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/Lex/LexerTest.cpp (original)<br>
+++ cfe/trunk/unittests/Lex/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/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/PreprocessorOptions.h"<br>
@@ -41,26 +43,33 @@ protected:<br>
Target = TargetInfo::CreateTargetInfo(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::MemoryBuffer> Buf =<br>
llvm::MemoryBuffer::getMemBuffer(Source);<br>
SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));<br>
<br>
- TrivialModuleLoader ModLoader;<br>
MemoryBufferCache PCMCache;<br>
HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,<br>
Diags, LangOpts, Target.get());<br>
- Preprocessor PP(std::make_shared<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<Preprocessor>(<br>
+ std::make_shared<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" 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.getFileIDSize(SourceMgr.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->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" target="_blank">tok.is</a>(tok::eof)) {<br>
+ ArgTokens.push_back(Eof);<br>
+ break;<br>
+ }<br>
+ if (<a href="http://tok.is" 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\\\"\"", 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->getStringifiedArgument(3, *PP, {}, {}),<br>
+ "Invalid argument number!");<br>
+#endif<br>
+}<br>
+<br>
} // anonymous namespace<br>
<br>
<br>
_______________________________________________<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" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><o:p></o:p></p>
</blockquote>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</body>
</html>