[cfe-commits] r95189 - in /cfe/trunk: include/clang/AST/ExprObjC.h lib/AST/Expr.cpp lib/CodeGen/CGExprConstant.cpp lib/CodeGen/CGObjC.cpp lib/CodeGen/CGObjCGNU.cpp lib/CodeGen/CGObjCMac.cpp lib/CodeGen/CGObjCRuntime.h lib/Sema/SemaExprObjC.cpp
Daniel Dunbar
daniel at zuster.org
Wed Feb 3 12:12:06 PST 2010
Hi David.
This is *totally* unacceptable.
You cannot just commit a patch which substantially breaks working code
without notice, without explanation, without coordinating with other
developers. This code works, it is continuously tested, it is used to
compile substantial amounts of code, and many people use it to build
their projects.
Your commit does not even add a test case.
Reverted in r95244, please submit as a patch with explanation if you
want someone to work with you to bring this code in, in a way that
doesn't break the NeXT runtime.
- Daniel
On Tue, Feb 2, 2010 at 6:09 PM, David Chisnall <csdavec at swan.ac.uk> wrote:
> Author: theraven
> Date: Tue Feb 2 20:09:30 2010
> New Revision: 95189
>
> URL: http://llvm.org/viewvc/llvm-project?rev=95189&view=rev
> Log:
> Numerous changes to selector handling:
>
> - Don't use GlobalAliases with non-0 GEPs (GNU runtime) - this was unsupported and LLVM will be generating errors if you do it soon. This also simplifies the code generated by the GNU runtime a bit.
>
> - Make GetSelector() return a constant (GNU runtime), not a load of a store of a constant.
>
> - Recognise @selector() expressions as valid static initialisers (as GCC does).
>
> - Add methods to GCObjCRuntime to emit selectors as constants (needed for using @selector() expressions as constants. These need implementing for the Mac runtimes - I couldn't figure out how to do this, they seem to require a load.
>
> - Store an ObjCMethodDecl in an ObjCSelectorExpr so that we can get at the type information for the selector. This is needed for generating typed selectors from @selector() expressions (as GCC does). Ideally, this information should be stored in the Selector, but that would be an invasive change. We should eventually add checks for common uses of @selector() expressions. Possibly adding an attribute that can be applied to method args providing the types of a selector so, for example, you'd do something like this:
>
> - (id)performSelector: __attribute__((selector_types(id, SEL, id)))(SEL)
> withObject: (id)object;
>
> Then, any @selector() expressions passed to the method will be check to ensure that it conforms to this signature. We do this at run time on the GNU runtime already, but it would be nice to do it at compile time on all runtimes.
>
> - Made @selector() expressions emit type info if available and the runtime supports it.
>
> Someone more familiar with the Mac runtime needs to implement the GetConstantSelector() function in CGObjCMac. This currently just assert()s.
>
>
> Modified:
> cfe/trunk/include/clang/AST/ExprObjC.h
> cfe/trunk/lib/AST/Expr.cpp
> cfe/trunk/lib/CodeGen/CGExprConstant.cpp
> cfe/trunk/lib/CodeGen/CGObjC.cpp
> cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
> cfe/trunk/lib/CodeGen/CGObjCMac.cpp
> cfe/trunk/lib/CodeGen/CGObjCRuntime.h
> cfe/trunk/lib/Sema/SemaExprObjC.cpp
>
> Modified: cfe/trunk/include/clang/AST/ExprObjC.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprObjC.h?rev=95189&r1=95188&r2=95189&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/AST/ExprObjC.h (original)
> +++ cfe/trunk/include/clang/AST/ExprObjC.h Tue Feb 2 20:09:30 2010
> @@ -96,18 +96,22 @@
> /// ObjCSelectorExpr used for @selector in Objective-C.
> class ObjCSelectorExpr : public Expr {
> Selector SelName;
> + ObjCMethodDecl *Method;
> SourceLocation AtLoc, RParenLoc;
> public:
> ObjCSelectorExpr(QualType T, Selector selInfo,
> SourceLocation at, SourceLocation rp)
> - : Expr(ObjCSelectorExprClass, T, false, false), SelName(selInfo), AtLoc(at),
> - RParenLoc(rp){}
> + : Expr(ObjCSelectorExprClass, T, false, false), SelName(selInfo), Method(0),
> + AtLoc(at), RParenLoc(rp){}
> explicit ObjCSelectorExpr(EmptyShell Empty)
> : Expr(ObjCSelectorExprClass, Empty) {}
>
> Selector getSelector() const { return SelName; }
> void setSelector(Selector S) { SelName = S; }
>
> + ObjCMethodDecl *getMethodDecl() const { return Method; }
> + void setMethodDecl(ObjCMethodDecl *M) { Method = M; }
> +
> SourceLocation getAtLoc() const { return AtLoc; }
> SourceLocation getRParenLoc() const { return RParenLoc; }
> void setAtLoc(SourceLocation L) { AtLoc = L; }
>
> Modified: cfe/trunk/lib/AST/Expr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=95189&r1=95188&r2=95189&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/AST/Expr.cpp (original)
> +++ cfe/trunk/lib/AST/Expr.cpp Tue Feb 2 20:09:30 2010
> @@ -1454,6 +1454,7 @@
> case StringLiteralClass:
> case ObjCStringLiteralClass:
> case ObjCEncodeExprClass:
> + case ObjCSelectorExprClass:
> return true;
> case CompoundLiteralExprClass: {
> // This handles gcc's extension that allows global initializers like
>
> Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprConstant.cpp?rev=95189&r1=95188&r2=95189&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Tue Feb 2 20:09:30 2010
> @@ -701,6 +701,14 @@
> CGM.GetStringForStringLiteral(E), false);
> }
>
> + llvm::Constant *VisitObjCSelectorExpr(const ObjCSelectorExpr *E) {
> + ObjCMethodDecl *OMD = E->getMethodDecl();
> + if (OMD)
> + return CGM.getObjCRuntime().GetConstantTypedSelector(OMD);
> + else
> + return CGM.getObjCRuntime().GetConstantSelector(E->getSelector());
> + }
> +
> llvm::Constant *VisitObjCEncodeExpr(ObjCEncodeExpr *E) {
> // This must be an @encode initializing an array in a static initializer.
> // Don't emit it as the address of the string, emit the string data itself
>
> Modified: cfe/trunk/lib/CodeGen/CGObjC.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?rev=95189&r1=95188&r2=95189&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGObjC.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGObjC.cpp Tue Feb 2 20:09:30 2010
> @@ -38,7 +38,11 @@
> // Note that this implementation allows for non-constant strings to be passed
> // as arguments to @selector(). Currently, the only thing preventing this
> // behaviour is the type checking in the front end.
> - return CGM.getObjCRuntime().GetSelector(Builder, E->getSelector());
> + ObjCMethodDecl *OMD = E->getMethodDecl();
> + if (OMD)
> + return CGM.getObjCRuntime().GetSelector(Builder, OMD);
> + else
> + return CGM.getObjCRuntime().GetSelector(Builder, E->getSelector());
> }
>
> llvm::Value *CodeGenFunction::EmitObjCProtocolExpr(const ObjCProtocolExpr *E) {
>
> Modified: cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCGNU.cpp?rev=95189&r1=95188&r2=95189&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGObjCGNU.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGObjCGNU.cpp Tue Feb 2 20:09:30 2010
> @@ -146,9 +146,17 @@
> const ObjCMethodDecl *Method);
> virtual llvm::Value *GetClass(CGBuilderTy &Builder,
> const ObjCInterfaceDecl *OID);
> - virtual llvm::Value *GetSelector(CGBuilderTy &Builder, Selector Sel);
> - virtual llvm::Value *GetSelector(CGBuilderTy &Builder, const ObjCMethodDecl
> - *Method);
> + virtual llvm::Constant *GetConstantSelector(Selector Sel);
> + virtual llvm::Constant *GetConstantTypedSelector(
> + const ObjCMethodDecl *Method);
> + llvm::Value *GetSelector(CGBuilderTy &Builder,
> + Selector Sel) {
> + return cast<llvm::Constant>((GetConstantSelector(Sel)));
> + }
> + llvm::Value *GetSelector(CGBuilderTy &Builder,
> + const ObjCMethodDecl *Method) {
> + return cast<llvm::Constant>(GetConstantTypedSelector(Method));
> + }
>
> virtual llvm::Function *GenerateMethod(const ObjCMethodDecl *OMD,
> const ObjCContainerDecl *CD);
> @@ -287,18 +295,18 @@
> return Builder.CreateCall(ClassLookupFn, ClassName);
> }
>
> -llvm::Value *CGObjCGNU::GetSelector(CGBuilderTy &Builder, Selector Sel) {
> +llvm::Constant *CGObjCGNU::GetConstantSelector(Selector Sel) {
> llvm::GlobalAlias *&US = UntypedSelectors[Sel.getAsString()];
> if (US == 0)
> - US = new llvm::GlobalAlias(llvm::PointerType::getUnqual(SelectorTy),
> + US = new llvm::GlobalAlias(SelectorTy,
> llvm::GlobalValue::PrivateLinkage,
> ".objc_untyped_selector_alias"+Sel.getAsString(),
> NULL, &TheModule);
>
> - return Builder.CreateLoad(US);
> + return US;
> }
>
> -llvm::Value *CGObjCGNU::GetSelector(CGBuilderTy &Builder, const ObjCMethodDecl
> +llvm::Constant *CGObjCGNU::GetConstantTypedSelector(const ObjCMethodDecl
> *Method) {
>
> std::string SelName = Method->getSelector().getAsString();
> @@ -310,17 +318,17 @@
>
> // If it's already cached, return it.
> if (TypedSelectors[Selector]) {
> - return Builder.CreateLoad(TypedSelectors[Selector]);
> + return TypedSelectors[Selector];
> }
>
> // If it isn't, cache it.
> llvm::GlobalAlias *Sel = new llvm::GlobalAlias(
> - llvm::PointerType::getUnqual(SelectorTy),
> + SelectorTy,
> llvm::GlobalValue::PrivateLinkage, ".objc_selector_alias" + SelName,
> NULL, &TheModule);
> TypedSelectors[Selector] = Sel;
>
> - return Builder.CreateLoad(Sel);
> + return Sel;
> }
>
> llvm::Constant *CGObjCGNU::MakeConstantString(const std::string &Str,
> @@ -1461,40 +1469,43 @@
>
> // Now that all of the static selectors exist, create pointers to them.
> int index = 0;
> + llvm::SmallVector<std::pair<llvm::GlobalAlias*,llvm::Value*>, 16> selectors;
> for (std::map<TypedSelector, llvm::GlobalAlias*>::iterator
> iter=TypedSelectors.begin(), iterEnd =TypedSelectors.end();
> iter != iterEnd; ++iter) {
> llvm::Constant *Idxs[] = {Zeros[0],
> llvm::ConstantInt::get(llvm::Type::getInt32Ty(VMContext), index++), Zeros[0]};
> - llvm::Constant *SelPtr = new llvm::GlobalVariable(TheModule, SelStructPtrTy,
> - true, llvm::GlobalValue::InternalLinkage,
> - llvm::ConstantExpr::getGetElementPtr(SelectorList, Idxs, 2),
> - ".objc_sel_ptr");
> + llvm::Constant *SelPtr =
> + llvm::ConstantExpr::getGetElementPtr(SelectorList, Idxs, 2);
> // If selectors are defined as an opaque type, cast the pointer to this
> // type.
> if (isSelOpaque) {
> - SelPtr = llvm::ConstantExpr::getBitCast(SelPtr,
> - llvm::PointerType::getUnqual(SelectorTy));
> + SelPtr = llvm::ConstantExpr::getBitCast(SelPtr,SelectorTy);
> }
> - (*iter).second->setAliasee(SelPtr);
> + selectors.push_back(
> + std::pair<llvm::GlobalAlias*,llvm::Value*>((*iter).second, SelPtr));
> }
> for (llvm::StringMap<llvm::GlobalAlias*>::iterator
> iter=UntypedSelectors.begin(), iterEnd = UntypedSelectors.end();
> iter != iterEnd; iter++) {
> llvm::Constant *Idxs[] = {Zeros[0],
> llvm::ConstantInt::get(llvm::Type::getInt32Ty(VMContext), index++), Zeros[0]};
> - llvm::Constant *SelPtr = new llvm::GlobalVariable
> - (TheModule, SelStructPtrTy,
> - true, llvm::GlobalValue::InternalLinkage,
> - llvm::ConstantExpr::getGetElementPtr(SelectorList, Idxs, 2),
> - ".objc_sel_ptr");
> + llvm::Constant *SelPtr =
> + llvm::ConstantExpr::getGetElementPtr(SelectorList, Idxs, 2);
> // If selectors are defined as an opaque type, cast the pointer to this
> // type.
> if (isSelOpaque) {
> - SelPtr = llvm::ConstantExpr::getBitCast(SelPtr,
> - llvm::PointerType::getUnqual(SelectorTy));
> + SelPtr = llvm::ConstantExpr::getBitCast(SelPtr, SelectorTy);
> }
> - (*iter).second->setAliasee(SelPtr);
> + selectors.push_back(
> + std::pair<llvm::GlobalAlias*,llvm::Value*>((*iter).second, SelPtr));
> + }
> + for (llvm::SmallVectorImpl<std::pair<
> + llvm::GlobalAlias*,llvm::Value*> >::iterator
> + iter=selectors.begin(), iterEnd =selectors.end();
> + iter != iterEnd; ++iter) {
> + iter->first->replaceAllUsesWith(iter->second);
> + iter->first->eraseFromParent();
> }
> // Number of classes defined.
> Elements.push_back(llvm::ConstantInt::get(llvm::Type::getInt16Ty(VMContext),
>
> Modified: cfe/trunk/lib/CodeGen/CGObjCMac.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCMac.cpp?rev=95189&r1=95188&r2=95189&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGObjCMac.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGObjCMac.cpp Tue Feb 2 20:09:30 2010
> @@ -953,6 +953,14 @@
> CGObjCCommonMac(CodeGen::CodeGenModule &cgm) :
> CGM(cgm), VMContext(cgm.getLLVMContext()) { }
>
> + virtual llvm::Constant *GetConstantSelector(Selector Sel) {
> + assert(0 && "Constant Selectors are not yet supported on the Mac runtimes");
> + return 0;
> + }
> + virtual llvm::Constant *GetConstantTypedSelector(
> + const ObjCMethodDecl *Method) {
> + return GetConstantSelector(Method->getSelector());
> + }
> virtual llvm::Constant *GenerateConstantString(const StringLiteral *SL);
>
> virtual llvm::Function *GenerateMethod(const ObjCMethodDecl *OMD,
>
> Modified: cfe/trunk/lib/CodeGen/CGObjCRuntime.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCRuntime.h?rev=95189&r1=95188&r2=95189&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGObjCRuntime.h (original)
> +++ cfe/trunk/lib/CodeGen/CGObjCRuntime.h Tue Feb 2 20:09:30 2010
> @@ -95,6 +95,12 @@
> /// this compilation unit with the runtime library.
> virtual llvm::Function *ModuleInitFunction() = 0;
>
> + virtual llvm::Constant *GetConstantSelector(Selector Sel) = 0;
> +
> + /// Get a typed selector.
> + virtual llvm::Constant *GetConstantTypedSelector(
> + const ObjCMethodDecl *Method) = 0;
> +
> /// Get a selector for the specified name and type values. The
> /// return value should have the LLVM type for pointer-to
> /// ASTContext::getObjCSelType().
>
> Modified: cfe/trunk/lib/Sema/SemaExprObjC.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=95189&r1=95188&r2=95189&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExprObjC.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Tue Feb 2 20:09:30 2010
> @@ -140,7 +140,20 @@
> Diag(SelLoc, diag::warn_undeclared_selector) << Sel;
>
> QualType Ty = Context.getObjCSelType();
> - return new (Context) ObjCSelectorExpr(Ty, Sel, AtLoc, RParenLoc);
> + ObjCSelectorExpr *E =
> + new (Context) ObjCSelectorExpr(Ty, Sel, AtLoc, RParenLoc);
> + // Make sure that we have seen this selector. There are lots of checks we
> + // should be doing on this selector. For example, when this is passed as the
> + // second argument to objc_msgSend() on the Mac runtime, or as the selector
> + // argument to the -performSelector:. We can do these checks at run time
> + // with the GNU runtimes, but the Apple runtimes let you sneak stack
> + // corruption in easily by passing the wrong selector to these functions if
> + // there is no static checking.
> + //
> + // Only log a warning on the GNU runtime.
> + E->setMethodDecl(LookupInstanceMethodInGlobalPool(Sel,
> + SourceRange(LParenLoc, LParenLoc), !LangOpts.NeXTRuntime));
> + return E;
> }
>
> Sema::ExprResult Sema::ParseObjCProtocolExpression(IdentifierInfo *ProtocolId,
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
More information about the cfe-commits
mailing list