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. 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
