On Mar 30, 2012, at 3:25 PM, John McCall wrote: > On Mar 30, 2012, at 2:45 PM, jahanian wrote: >> On Mar 30, 2012, at 2:29 PM, John McCall wrote: >>> Author: rjmccall >>> Date: Fri Mar 30 16:29:05 2012 >>> New Revision: 153778 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=153778&view=rev >>> Log: >>> Fix a pair of invalidation bugs when emitting protocol definitions >>> in the fragile and non-fragile Mac ObjC runtimes. No useful test >>> case. Fixes rdar://problem/11072576. >>> >>> Modified: >>> cfe/trunk/lib/CodeGen/CGObjCMac.cpp >>> >>> Modified: cfe/trunk/lib/CodeGen/CGObjCMac.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCMac.cpp?rev=153778&r1=153777&r2=153778&view=diff >>> ============================================================================== >>> --- cfe/trunk/lib/CodeGen/CGObjCMac.cpp (original) >>> +++ cfe/trunk/lib/CodeGen/CGObjCMac.cpp Fri Mar 30 16:29:05 2012 >>> @@ -1914,7 +1914,7 @@ >>> See EmitProtocolExtension(). >>> */ >>> llvm::Constant *CGObjCMac::GetOrEmitProtocol(const ObjCProtocolDecl *PD) { >>> - llvm::GlobalVariable *&Entry = Protocols[PD->getIdentifier()]; >>> + llvm::GlobalVariable *Entry = Protocols[PD->getIdentifier()]; >>> >>> // Early exit if a defining object has already been generated. >>> if (Entry && Entry->hasInitializer()) >>> @@ -1997,6 +1997,8 @@ >>> Entry->setSection("__OBJC,__protocol,regular,no_dead_strip"); >>> // FIXME: Is this necessary? Why only for protocol? >>> Entry->setAlignment(4); >>> + >>> + Protocols[PD->getIdentifier()] = Entry; >> >> A test case would have been nice. It uclear to me what is being fixed any >> why above assignment is necessary. >> A brief description of the problem and the fix is the next best thing. > > Sorry, I thought "invalidation bug" covered it. The code was keeping > a reference into the Protocols table across the emission of the protocol > body. Unfortunately, the emission of the protocol may require the > emission of references to other protocols, which in turn may require > modifying the Protocols table and therefore invalidating the original > reference.
Got it. Thanks much for the explanation. - Fariborz > > The test case is basically to make a ton of protocols that imply other > protocols and hope they trigger the right circumstances to crash the > compiler. I generally don't think that's a very useful kind of test to > keep around, since it's a memory bug and can be easily masked by > changes in the data structure implementation. > > John. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
