Hi Richard,
Thanks for the fix!
Unfortunately it doesn't help for cling case. I implement a
removeMacro routine using ReleaseMacroInfo. ReleaseMacroInfo allowed me
to implement efficiently the removal of a macro instead of dragging a
long def undef chains, for example.
IIUC it allowed some memory reduction in some cases for clang, too.
Is there any chance to keep the ReleaseMacroInfo upstream?
Vassil
On 08/04/2014 01:50 AM, Richard Smith wrote:
Fixed in a much more simple way in r214675. Thanks for reporting!
On Sun, Aug 3, 2014 at 11:52 AM, Vassil Vassilev <vvasi...@cern.ch
<mailto:vvasi...@cern.ch>> wrote:
I will try just one more time and then shut up :)
diff --git a/lib/Lex/PPDirectives.cpp b/lib/Lex/PPDirectives.cpp
index 5f38387..000ea7a 100644
--- a/lib/Lex/PPDirectives.cpp
+++ b/lib/Lex/PPDirectives.cpp
@@ -94,6 +94,19 @@
Preprocessor::AllocateVisibilityMacroDirective(SourceLocation Loc,
/// error in the macro definition.
void Preprocessor::ReleaseMacroInfo(MacroInfo *MI) {
// Don't try to reuse the storage; this only happens on error
paths.
+
+ // If this is on the macro info chain, avoid double deletion on
teardown.
+ MacroInfoChain *current = MIChainHead;
+ while (MacroInfoChain *I = current) {
+ if (&(I->MI) == MI) {
+ I->Next = (I->Next) ? I->Next->Next : 0;
+ if (I == MIChainHead)
+ MIChainHead = I->Next;
+ break;
+ }
+ current = I->Next;
+ }
+
MI->~MacroInfo();
}
On 03/08/14 20:47, Vassil Vassilev wrote:
Hi,
Sorry overlooked, thanks for pointing it out!
I hope this is what we want.
Vassil
diff --git a/lib/Lex/PPDirectives.cpp b/lib/Lex/PPDirectives.cpp
index 5f38387..000ea7a 100644
--- a/lib/Lex/PPDirectives.cpp
+++ b/lib/Lex/PPDirectives.cpp
@@ -94,6 +94,19 @@
Preprocessor::AllocateVisibilityMacroDirective(SourceLocation Loc,
/// error in the macro definition.
void Preprocessor::ReleaseMacroInfo(MacroInfo *MI) {
// Don't try to reuse the storage; this only happens on error
paths.
+
+ // If this is on the macro info chain, avoid double deletion
on teardown.
+ MacroInfoChain *current = MIChainHead;
+ while (MacroInfoChain *I = current) {
+ if (&(I->MI) == MI) {
+ I->Next = (I->Next) ? I->Next->Next : 0;
+ if (I == MIChainHead)
+ MIChainHead = I;
+ break;
+ }
+ current = I->Next;
+ }
+
MI->~MacroInfo();
}
On 03/08/14 20:28, Yaron Keren wrote:
Hi,
MIChainHead is a pointer to the head of a linked list
of MacroInfoChain nodes, each containing a MacroInfo
and MacroInfoChain*.
Why does the while loop modify MIChainHead on every iteration?
MIChainHead should be modified only if it points to the node
containing the removed MacroInfo MI. In all other cases it
should not change.
As it is now, the loop will always terminate with MIChainHead ==
nullptr.
Yaron
2014-08-03 21:10 GMT+03:00 Vassil Vassilev <vvasi...@cern.ch
<mailto:vvasi...@cern.ch>>:
Hi Yaron,
Yes I meant double destruction.
Vassil
On 03/08/14 20:08, Yaron Keren wrote:
Hi Vassil,
Do you mean double destruction (not deletion) of MacroInfo
first time in ReleaseMacroInfo and the second time in
~Preprocessor via ~MacroInfoChain?
while (MacroInfoChain *I = MIChainHead) {
MIChainHead = I->Next;
I->~MacroInfoChain();
}
or something else?
Yaron
2014-08-02 23:05 GMT+03:00 Vassil Vassilev
<vvasi...@cern.ch <mailto:vvasi...@cern.ch>>:
Hi,
In cases where ReleaseMacroInfo gets called and it
doesn't cleanup the Preprocessor's MIChainHead can lead
to double deletion. I am sending the patch that fixes
the problem for me.
Vassil
diff --git a/lib/Lex/PPDirectives.cpp
b/lib/Lex/PPDirectives.cpp
index 5f38387..1a9b5eb 100644
--- a/lib/Lex/PPDirectives.cpp
+++ b/lib/Lex/PPDirectives.cpp
@@ -94,6 +94,14 @@
Preprocessor::AllocateVisibilityMacroDirective(SourceLocation
Loc,
/// error in the macro definition.
void Preprocessor::ReleaseMacroInfo(MacroInfo *MI) {
// Don't try to reuse the storage; this only happens
on error paths.
+
+ // If this is on the macro info chain, avoid double
deletion on teardown.
+ while (MacroInfoChain *I = MIChainHead) {
+ if (&(I->MI) == MI)
+ I->Next = (I->Next) ? I->Next->Next : 0;
+ MIChainHead = I->Next;
+ }
+
MI->~MacroInfo();
}
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu <mailto:cfe-commits@cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu <mailto:cfe-commits@cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu <mailto:cfe-commits@cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
