From patchwork Wed Dec 29 15:48:38 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicola Pero X-Patchwork-Id: 76926 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 62158B7063 for ; Thu, 30 Dec 2010 02:48:53 +1100 (EST) Received: (qmail 29574 invoked by alias); 29 Dec 2010 15:48:52 -0000 Received: (qmail 29561 invoked by uid 22791); 29 Dec 2010 15:48:50 -0000 X-SWARE-Spam-Status: No, hits=-1.3 required=5.0 tests=AWL, BAYES_00, 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; Wed, 29 Dec 2010 15:48:45 +0000 Received: from eggs.gnu.org ([140.186.70.92]:43229) by fencepost.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.69) (envelope-from ) id 1PXyGW-000511-F9 for gcc-patches@gnu.org; Wed, 29 Dec 2010 10:48:40 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PXyGX-0002tl-N9 for gcc-patches@gnu.org; Wed, 29 Dec 2010 10:48:43 -0500 Received: from smtp151.iad.emailsrvr.com ([207.97.245.151]:38685) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PXyGX-0002tR-KE for gcc-patches@gnu.org; Wed, 29 Dec 2010 10:48:41 -0500 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp35.relay.iad1a.emailsrvr.com (SMTP Server) with ESMTP id AF9D02C89EB for ; Wed, 29 Dec 2010 10:48:40 -0500 (EST) Received: by smtp35.relay.iad1a.emailsrvr.com (Authenticated sender: nicola.pero-AT-meta-innovation.com) with ESMTPA id 13EE82C8A29 for ; Wed, 29 Dec 2010 10:48:39 -0500 (EST) Message-Id: <53C686BE-5660-4B67-A873-EE9FB7D8212D@meta-innovation.com> From: Nicola Pero To: gcc-patches@gnu.org Mime-Version: 1.0 (Apple Message framework v936) Subject: Fix for PR objc/47118 ("ICE on incorrect parameter to @synchronized()") Date: Wed, 29 Dec 2010 16:48:38 +0100 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) 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 PR objc/47118. The problem occurs when use @synchronized() with an invalid argument that is not an Objective-C object. GCC from trunk wasn't properly checking and sanitizing the argument; moreover, some invalid arguments would confuse it to the point of crashing. This patch adds the checks, prints a nice error message if the argument is invalid, and deals with errors gracefully. A full testcase is included, testing lots of cases. Ok to commit ? Thanks + @synchronized () /* { dg-error "expected" } */ + { dummy++; } + + @synchronized (int) /* { dg-error "expected" } */ + { dummy++; } + + return dummy; +} Index: objc/objc-act.c =================================================================== --- objc/objc-act.c (revision 168314) +++ objc/objc-act.c (working copy) @@ -5627,28 +5627,59 @@ objc_build_throw_stmt (location_t loc, t } tree -objc_build_synchronized (location_t start_locus, tree mutex, tree body) +objc_build_synchronized (location_t start_locus, tree object_expr, tree body) { - tree args, call; + /* object_expr should never be NULL; but in case it is, convert it to + error_mark_node. */ + if (object_expr == NULL) + object_expr = error_mark_node; - /* First lock the mutex. */ - mutex = save_expr (mutex); - args = tree_cons (NULL, mutex, NULL); - call = build_function_call (input_location, - objc_sync_enter_decl, args); - SET_EXPR_LOCATION (call, start_locus); - add_stmt (call); - - /* Build the mutex unlock. */ - args = tree_cons (NULL, mutex, NULL); - call = build_function_call (input_location, - objc_sync_exit_decl, args); - SET_EXPR_LOCATION (call, input_location); - - /* Put the that and the body in a TRY_FINALLY. */ - objc_begin_try_stmt (start_locus, body); - objc_build_finally_clause (input_location, call); - return objc_finish_try_stmt (); + /* Validate object_expr. If not valid, set it to error_mark_node. */ + if (object_expr != error_mark_node) + { + if (!objc_type_valid_for_messaging (TREE_TYPE (object_expr), true)) + { + error_at (start_locus, "%<@synchronized%> argument is not an object"); + object_expr = error_mark_node; + } + } + + if (object_expr == error_mark_node) + { + /* If we found an error, we simply ignore the '@synchronized'. + Compile the body so we can keep going with minimal + casualties. */ + return add_stmt (body); + } + else + { + tree call; + tree args; + + /* objc_sync_enter (object_expr); */ + object_expr = save_expr (object_expr); + args = tree_cons (NULL, object_expr, NULL); + call = build_function_call (input_location, + objc_sync_enter_decl, args); + SET_EXPR_LOCATION (call, start_locus); + add_stmt (call); + + /* Build "objc_sync_exit (object_expr);" but do not add it yet; + it goes inside the @finalize() clause. */ + args = tree_cons (NULL, object_expr, NULL); + call = build_function_call (input_location, + objc_sync_exit_decl, args); + SET_EXPR_LOCATION (call, input_location); + + /* @try { body; } */ + objc_begin_try_stmt (start_locus, body); + + /* @finally { objc_sync_exit (object_expr); } */ + objc_build_finally_clause (input_location, call); + + /* End of try statement. */ + return objc_finish_try_stmt (); + } } ^L Index: objc/ChangeLog =================================================================== --- objc/ChangeLog (revision 168314) +++ objc/ChangeLog (working copy) @@ -1,3 +1,11 @@ +2010-12-29 Nicola Pero + + PR objc/47118 + * objc-act.c (objc_build_synchronized): Check the argument of + @synchronized and emit an appropriate error if it is not a valid + Objective-C object. Deal gracefully with that case. Updated + comments and variable names. + 2010-12-28 Nicola Pero PR objc/47076 Index: testsuite/ChangeLog =================================================================== --- testsuite/ChangeLog (revision 168314) +++ testsuite/ChangeLog (working copy) @@ -1,3 +1,9 @@ +2010-12-29 Nicola Pero + + PR objc/47118 + * objc.dg/sync-3.m: New. + * obj-c++.dg/sync-3.mm: New. + 2010-12-28 Jason Merrill PR c++/47068 Index: testsuite/objc.dg/sync-3.m =================================================================== --- testsuite/objc.dg/sync-3.m (revision 0) +++ testsuite/objc.dg/sync-3.m (revision 0) @@ -0,0 +1,128 @@ +/* Contributed by Nicola Pero , December 2010. */ +/* { dg-options "-fobjc-exceptions" } */ +/* { dg-do compile } */ + +/* Test that the compiler is checking the argument of @synchronized(), + and produce errors when invalid types are used. */ + +#include + +@interface MyObject +{ + Class isa; +} +@end + +@implementation MyObject +@end + +@protocol MyProtocol; + +typedef MyObject MyObjectTypedef; +typedef MyObject *MyObjectPtrTypedef; +typedef int intTypedef; + +typedef struct { float x; float y; } point, *point_ptr; + +int test (id object) +{ + int dummy = 0; + + { + int x; + @synchronized (x) /* { dg-error ".@synchronized. argument is not an object" } */ + { dummy++; } + } + + { + intTypedef x; + @synchronized (x) /* { dg-error ".@synchronized. argument is not an object" } */ + { dummy++; } + } + + { + int *x; + @synchronized (x) /* { dg-error ".@synchronized. argument is not an object" } */ + { dummy++; } + } + + { + point x; + @synchronized (x) /* { dg-error ".@synchronized. argument is not an object" } */ + { dummy++; } + } + + { + point_ptr x; + @synchronized (x) /* { dg-error ".@synchronized. argument is not an object" } */ + { dummy++; } + } + + { + id x; + @synchronized (x) /* Ok */ + { dummy++; } + } + + { + id x; + @synchronized (x) /* Ok */ + { dummy++; } + } + + { + MyObject *x; + @synchronized (x) /* Ok */ + { dummy++; } + } + + { + MyObject *x; + @synchronized (x) /* Ok */ + { dummy++; } + } + + { + static MyObject *x; + @synchronized (x) /* Ok */ + { dummy++; } + } + + { + MyObjectTypedef *x; + @synchronized (x) /* Ok */ + { dummy++; } + } + + { + MyObjectTypedef *x; + @synchronized (x) /* Ok */ + { dummy++; } + } + + { + MyObjectPtrTypedef x; + @synchronized (x) /* Ok */ + { dummy++; } + } + + { + Class x; + @synchronized (x) /* Ok */ + { dummy++; } + } + + @synchronized (1) /* { dg-error ".@synchronized. argument is not an object" } */ + { dummy++; } + + @synchronized ("Test") /* { dg-error ".@synchronized. argument is not an object" } */ + { dummy++; } + + @synchronized () /* { dg-error "expected expression" } */ + { dummy++; } + + @synchronized (int) /* { dg-error "expected expression" } */ + { dummy++; } + + return dummy; +} Index: testsuite/obj-c++.dg/sync-3.mm =================================================================== --- testsuite/obj-c++.dg/sync-3.mm (revision 0) +++ testsuite/obj-c++.dg/sync-3.mm (revision 0) @@ -0,0 +1,128 @@ +/* Contributed by Nicola Pero , December 2010. */ +/* { dg-options "-fobjc-exceptions" } */ +/* { dg-do compile } */ + +/* Test that the compiler is checking the argument of @synchronized(), + and produce errors when invalid types are used. */ + +#include + +@interface MyObject +{ + Class isa; +} +@end + +@implementation MyObject +@end + +@protocol MyProtocol; + +typedef MyObject MyObjectTypedef; +typedef MyObject *MyObjectPtrTypedef; +typedef int intTypedef; + +typedef struct { float x; float y; } point, *point_ptr; + +int test (id object) +{ + int dummy = 0; + + { + int x; + @synchronized (x) /* { dg-error ".@synchronized. argument is not an object" } */ + { dummy++; } + } + + { + intTypedef x; + @synchronized (x) /* { dg-error ".@synchronized. argument is not an object" } */ + { dummy++; } + } + + { + int *x; + @synchronized (x) /* { dg-error ".@synchronized. argument is not an object" } */ + { dummy++; } + } + + { + point x; + @synchronized (x) /* { dg-error ".@synchronized. argument is not an object" } */ + { dummy++; } + } + + { + point_ptr x; + @synchronized (x) /* { dg-error ".@synchronized. argument is not an object" } */ + { dummy++; } + } + + { + id x; + @synchronized (x) /* Ok */ + { dummy++; } + } + + { + id x; + @synchronized (x) /* Ok */ + { dummy++; } + } + + { + MyObject *x; + @synchronized (x) /* Ok */ + { dummy++; } + } + + { + MyObject *x; + @synchronized (x) /* Ok */ + { dummy++; } + } + + { + static MyObject *x; + @synchronized (x) /* Ok */ + { dummy++; } + } + + { + MyObjectTypedef *x; + @synchronized (x) /* Ok */ + { dummy++; } + } + + { + MyObjectTypedef *x; + @synchronized (x) /* Ok */ + { dummy++; } + } + + { + MyObjectPtrTypedef x; + @synchronized (x) /* Ok */ + { dummy++; } + } + + { + Class x; + @synchronized (x) /* Ok */ + { dummy++; } + } + + @synchronized (1) /* { dg-error ".@synchronized. argument is not an object" } */ + { dummy++; } + + @synchronized ("Test") /* { dg-error ".@synchronized. argument is not an object" } */ + { dummy++; } +