From patchwork Fri Jul 9 16:26:54 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 58411 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 08ADDB6F0E for ; Sat, 10 Jul 2010 02:27:07 +1000 (EST) Received: (qmail 24049 invoked by alias); 9 Jul 2010 16:27:04 -0000 Received: (qmail 24038 invoked by uid 22791); 9 Jul 2010 16:27:03 -0000 X-SWARE-Spam-Status: No, hits=-0.2 required=5.0 tests=AWL, BAYES_40, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, TW_PX X-Spam-Check-By: sourceware.org Received: from mail-vw0-f47.google.com (HELO mail-vw0-f47.google.com) (209.85.212.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 09 Jul 2010 16:26:56 +0000 Received: by vws3 with SMTP id 3so2653950vws.20 for ; Fri, 09 Jul 2010 09:26:54 -0700 (PDT) MIME-Version: 1.0 Received: by 10.220.49.28 with SMTP id t28mr5379746vcf.93.1278692814399; Fri, 09 Jul 2010 09:26:54 -0700 (PDT) Received: by 10.220.73.71 with HTTP; Fri, 9 Jul 2010 09:26:54 -0700 (PDT) Date: Fri, 9 Jul 2010 17:26:54 +0100 Message-ID: Subject: Prevent inlining of weak hidden symbols. From: Richard Sandiford To: gcc-patches 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 a wrong-code bug in the compilation of gPXE. gPXE has a weak symbol called pxe_menu_boot whose default implementation is defined in the same file as (one of?) the callers. However, this implementation can be overridden by earlier objects on the link line. gPXE has only a single module -- there's no symbol preemption -- so as an optimisation, it also marks every symbol as hidden. This causes GCC to think it can inline pxe_menu_boot, something it wouldn't do for default-visiblity weak symbols. The problem is that our notion of "replaceability" is based on whether the definition binds locally to the current _module_, not to the current TU. That's fine for most cases, but not for weak symbols. Bootstrapped & regression-tested on x86_64-linux-gnu. OK to install? Richard This patch fixes a wrong-code bug in the compilation of gPXE. gPXE has a weak symbol called pxe_menu_boot whose default implementation is defined in the same file as (one of?) the callers. However, this implementation can be overridden by earlier objects on the link line. gPXE has only a single module -- there's no symbol preemption -- so as an optimisation, it also marks every symbol as hidden. This causes GCC to think it can inline pxe_menu_boot, something it wouldn't do for default-visiblity weak symbols. The problem is that our notion of "replaceability" is based on whether the definition binds locally to the current _module_, not to the current TU. That's fine for most cases, but not for weak symbols. Bootstrapped & regression-tested on x86_64-linux-gnu. OK to install? Richard gcc/ * tree.h (DECL_REPLACEABLE_P): Strengthen check for weak symbols. gcc/testsuite/ * gcc.dg/attr-weak-hidden-1.c, gcc.dg/attr-weak-hidden-1a.c: New test. Index: gcc/testsuite/gcc.dg/attr-weak-hidden-1.c =================================================================== --- /dev/null 2010-06-01 09:29:56.413951209 +0100 +++ gcc/testsuite/gcc.dg/attr-weak-hidden-1.c 2010-07-09 12:58:03.000000000 +0100 @@ -0,0 +1,6 @@ +/* { dg-do run } */ +/* { dg-require-weak "" } */ +/* { dg-require-visibility "" } */ +/* { dg-options "-O2" } */ +/* { dg-additional-sources "attr-weak-hidden-1a.c" } */ +int __attribute__((weak, visibility("hidden"))) foo (void) { return 0; } Index: gcc/testsuite/gcc.dg/attr-weak-hidden-1a.c =================================================================== --- /dev/null 2010-06-01 09:29:56.413951209 +0100 +++ gcc/testsuite/gcc.dg/attr-weak-hidden-1a.c 2010-07-09 13:00:53.000000000 +0100 @@ -0,0 +1,9 @@ +void abort (void); +int __attribute__((weak, visibility("hidden"))) foo (void) { return 1; } +int +main (void) +{ + if (foo ()) + abort (); + return 0; +} Index: gcc/tree.h =================================================================== --- gcc/tree.h 2010-07-09 09:44:27.000000000 +0100 +++ gcc/tree.h 2010-07-09 12:59:26.000000000 +0100 @@ -3012,6 +3012,11 @@ #define DECL_ONE_ONLY(NODE) (DECL_COMDAT not be treated as replaceable so that use of C++ template instantiations is not penalized. + In other respects, the condition is usually equivalent to whether + the function binds to the current module (shared library or executable). + However, weak functions can always be overridden by earlier TUs + in the same module, even if they bind locally to that module. + For example, DECL_REPLACEABLE is used to determine whether or not a function (including a template instantiation) which is not explicitly declared "inline" can be inlined. If the function is @@ -3020,7 +3025,7 @@ #define DECL_ONE_ONLY(NODE) (DECL_COMDAT function that is not DECL_REPLACEABLE can be inlined, since all versions of the function will be functionally identical. */ #define DECL_REPLACEABLE_P(NODE) \ - (!DECL_COMDAT (NODE) && !targetm.binds_local_p (NODE)) + (!DECL_COMDAT (NODE) && (DECL_WEAK (NODE) || !targetm.binds_local_p (NODE))) /* The name of the object as the assembler will see it (but before any translations made by ASM_OUTPUT_LABELREF). Often this is the same