From patchwork Sat Feb 19 05:08:16 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicola Pero X-Patchwork-Id: 83685 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 8EEB3B7126 for ; Sat, 19 Feb 2011 16:08:30 +1100 (EST) Received: (qmail 13285 invoked by alias); 19 Feb 2011 05:08:29 -0000 Received: (qmail 13272 invoked by uid 22791); 19 Feb 2011 05:08:27 -0000 X-SWARE-Spam-Status: No, hits=-1.3 required=5.0 tests=AWL, BAYES_00, SARE_SUB_ENC_UTF8, TW_BJ, TW_TM, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from fencepost.gnu.org (HELO fencepost.gnu.org) (140.186.70.10) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 19 Feb 2011 05:08:20 +0000 Received: from eggs.gnu.org ([140.186.70.92]:42749) by fencepost.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1Pqf3K-00011y-Ex for gcc-patches@gnu.org; Sat, 19 Feb 2011 00:08:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pqf3J-0002IF-1d for gcc-patches@gnu.org; Sat, 19 Feb 2011 00:08:18 -0500 Received: from smtp111.iad.emailsrvr.com ([207.97.245.111]:42946) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pqf3I-0002I9-VA for gcc-patches@gnu.org; Sat, 19 Feb 2011 00:08:17 -0500 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp41.relay.iad1a.emailsrvr.com (SMTP Server) with ESMTP id 6192E29014A for ; Sat, 19 Feb 2011 00:08:16 -0500 (EST) Received: from dynamic2.wm-web.iad.mlsrvr.com (dynamic2.wm-web.iad1a.rsapps.net [192.168.2.151]) by smtp41.relay.iad1a.emailsrvr.com (SMTP Server) with ESMTP id 509B629010B for ; Sat, 19 Feb 2011 00:08:16 -0500 (EST) Received: from meta-innovation.com (localhost [127.0.0.1]) by dynamic2.wm-web.iad.mlsrvr.com (Postfix) with ESMTP id 2BB7628E8066 for ; Sat, 19 Feb 2011 00:08:16 -0500 (EST) Received: by www2.webmail.us (Authenticated sender: nicola.pero@meta-innovation.com, from: nicola.pero@meta-innovation.com) with HTTP; Sat, 19 Feb 2011 06:08:16 +0100 (CET) Date: Sat, 19 Feb 2011 06:08:16 +0100 (CET) Subject: =?UTF-8?Q?ObjC:=20fix=20PR=20objc/47784=20("Internal=20compiler=20error?= =?UTF-8?Q?=20in=20dot=20notation=20assignment=20of=20const=20value")?= From: "Nicola Pero" To: "gcc-patches@gnu.org" MIME-Version: 1.0 X-Type: plain Message-ID: <1298092096.177523530@192.168.4.58> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 207.97.245.111 X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org This patch fixes objc/47784 ("Internal compiler error in dot notation assignment of const value"). Due to an ingenuity in our implementation of the dot-syntax, x.property = y; would crash the ObjC compiler if the type of 'y' is some sort of 'const' type (the ObjC++ would just refuse to compile it with an error). This patch fixes it. It seems worthwhile to apply it to 4.6 as the bug is pretty bad; perfectly valid (and common) code would otherwise crash GCC 4.6. Ok to commit ? Thanks Index: objc/ChangeLog =================================================================== --- objc/ChangeLog (revision 170299) +++ objc/ChangeLog (working copy) @@ -1,5 +1,11 @@ 2011-01-19 Nicola Pero + PR objc/47784 + * objc-act.c (objc_maybe_build_modify_expr): If 'rhs' has side + effects, do not use a temporary variable. + +2011-01-19 Nicola Pero + * objc-act.c (objc_init, generate_struct_by_value_array): Updated comments. Index: objc/objc-act.c =================================================================== --- objc/objc-act.c (revision 170299) +++ objc/objc-act.c (working copy) @@ -1790,49 +1790,71 @@ objc_maybe_build_modify_expr (tree lhs, tree rhs) to get these to work with very little effort, we build a compound statement which does the setter call (to set the property to 'rhs'), but which can also be evaluated returning - the 'rhs'. So, we want to create the following: + the 'rhs'. If the 'rhs' has no side effects, we can simply + evaluate it twice, building - (temp = rhs; [object setProperty: temp]; temp) + ([object setProperty: rhs]; rhs) + + If it has side effects, we put it in a temporary variable first, + so we create the following: + + (temp = rhs; [object setProperty: temp]; temp) + + setter_argument is rhs in the first case, and temp in the second + case. */ - tree temp_variable_decl, bind; + tree setter_argument; + /* s1, s2 and s3 are the tree statements that we need in the compound expression. */ tree s1, s2, s3, compound_expr; + + if (TREE_SIDE_EFFECTS (rhs)) + { + tree bind; - /* TODO: If 'rhs' is a constant, we could maybe do without the - 'temp' variable ? */ + /* Declare __objc_property_temp in a local bind. */ + setter_argument = objc_create_temporary_var (TREE_TYPE (rhs), "__objc_property_temp"); + DECL_SOURCE_LOCATION (setter_argument) = input_location; + bind = build3 (BIND_EXPR, void_type_node, setter_argument, NULL, NULL); + SET_EXPR_LOCATION (bind, input_location); + TREE_SIDE_EFFECTS (bind) = 1; + add_stmt (bind); - /* Declare __objc_property_temp in a local bind. */ - temp_variable_decl = objc_create_temporary_var (TREE_TYPE (rhs), "__objc_property_temp"); - DECL_SOURCE_LOCATION (temp_variable_decl) = input_location; - bind = build3 (BIND_EXPR, void_type_node, temp_variable_decl, NULL, NULL); - SET_EXPR_LOCATION (bind, input_location); - TREE_SIDE_EFFECTS (bind) = 1; - add_stmt (bind); + /* s1: x = rhs */ + s1 = build_modify_expr (input_location, setter_argument, NULL_TREE, + NOP_EXPR, + input_location, rhs, NULL_TREE); + SET_EXPR_LOCATION (s1, input_location); + } + else + { + /* No s1. */ + setter_argument = rhs; + s1 = NULL_TREE; + } /* Now build the compound statement. */ + + /* s2: [object setProperty: x] */ + s2 = objc_build_setter_call (lhs, setter_argument); - /* s1: __objc_property_temp = rhs */ - s1 = build_modify_expr (input_location, temp_variable_decl, NULL_TREE, - NOP_EXPR, - input_location, rhs, NULL_TREE); - SET_EXPR_LOCATION (s1, input_location); - - /* s2: [object setProperty: __objc_property_temp] */ - s2 = objc_build_setter_call (lhs, temp_variable_decl); - - /* This happens if building the setter failed because the property - is readonly. */ + /* This happens if building the setter failed because the + property is readonly. */ if (s2 == error_mark_node) return error_mark_node; SET_EXPR_LOCATION (s2, input_location); - /* s3: __objc_property_temp */ - s3 = convert (TREE_TYPE (lhs), temp_variable_decl); + /* s3: x */ + s3 = convert (TREE_TYPE (lhs), setter_argument); - /* Now build the compound statement (s1, s2, s3) */ - compound_expr = build_compound_expr (input_location, build_compound_expr (input_location, s1, s2), s3); + /* Now build the compound statement (s1, s2, s3) or (s2, s3) as + appropriate. */ + if (s1) + compound_expr = build_compound_expr (input_location, build_compound_expr (input_location, s1, s2), s3); + else + compound_expr = build_compound_expr (input_location, s2, s3); /* Without this, with -Wall you get a 'valued computed is not used' every time there is a "object.property = x" where the Index: testsuite/ChangeLog =================================================================== --- testsuite/ChangeLog (revision 170297) +++ testsuite/ChangeLog (working copy) @@ -1,3 +1,9 @@ +2011-02-19 Nicola Pero + + PR objc/47784 + * objc.dg/property/dotsyntax-22.m: New. + * obj-c++.dg/property/dotsyntax-22.mm: New. + 2011-02-18 Iain Sandoe * objc/execute/exceptions/foward-1.x: New. Index: testsuite/objc.dg/property/dotsyntax-22.m =================================================================== --- testsuite/objc.dg/property/dotsyntax-22.m (revision 0) +++ testsuite/objc.dg/property/dotsyntax-22.m (revision 0) @@ -0,0 +1,19 @@ +/* PR objc/47784. This testcase used to crash the compiler. */ + +typedef struct { + float x; +} SomeType; + +@interface MyClass + +@property(assign,readwrite) SomeType position; + +@end + +void example (MyClass *x) +{ + const SomeType SomeTypeZero = {0.0f}; + + x.position= SomeTypeZero; +} + Index: testsuite/obj-c++.dg/property/dotsyntax-22.mm =================================================================== --- testsuite/obj-c++.dg/property/dotsyntax-22.mm (revision 0) +++ testsuite/obj-c++.dg/property/dotsyntax-22.mm (revision 0) @@ -0,0 +1,19 @@ +/* PR objc/47784. This testcase used to crash the compiler. */ + +typedef struct { + float x; +} SomeType; + +@interface MyClass + +@property(assign,readwrite) SomeType position; + +@end + +void example (MyClass *x) +{ + const SomeType SomeTypeZero = {0.0f}; + + x.position= SomeTypeZero; +} +