[PATCH] Inconsistency in Preprocessor::ReleaseMacroInfo(MacroInfo *MI)

[Vassil Vassilev]

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 <vvasilev at cern.ch 
> <mailto:vvasilev at 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 <vvasilev at cern.ch
>>     <mailto:vvasilev at 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 at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>>         http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140803/d6784c57/attachment.html>