From patchwork Wed Dec 28 17:39:57 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Michael Meissner X-Patchwork-Id: 133452 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 E9900B6FBE for ; Thu, 29 Dec 2011 04:40:59 +1100 (EST) Received: (qmail 2458 invoked by alias); 28 Dec 2011 17:40:56 -0000 Received: (qmail 2446 invoked by uid 22791); 28 Dec 2011 17:40:54 -0000 X-SWARE-Spam-Status: No, hits=-1.5 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from e36.co.us.ibm.com (HELO e36.co.us.ibm.com) (32.97.110.154) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 28 Dec 2011 17:40:40 +0000 Received: from /spool/local by e36.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 28 Dec 2011 10:40:37 -0700 Received: from d03relay04.boulder.ibm.com (9.17.195.106) by e36.co.us.ibm.com (192.168.1.136) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Wed, 28 Dec 2011 10:40:01 -0700 Received: from d03av06.boulder.ibm.com (d03av06.boulder.ibm.com [9.17.195.245]) by d03relay04.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id pBSHe01a131032 for ; Wed, 28 Dec 2011 10:40:00 -0700 Received: from d03av06.boulder.ibm.com (loopback [127.0.0.1]) by d03av06.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id pBSHe0DQ001484 for ; Wed, 28 Dec 2011 10:40:00 -0700 Received: from ibm-tiger.the-meissners.org ([9.33.37.223]) by d03av06.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id pBSHdxKE001405; Wed, 28 Dec 2011 10:39:59 -0700 Received: by ibm-tiger.the-meissners.org (Postfix, from userid 500) id 06B74444B5; Wed, 28 Dec 2011 12:39:57 -0500 (EST) Date: Wed, 28 Dec 2011 12:39:57 -0500 From: Michael Meissner To: Chung-Lin Tang , gcc-patches , David Edelsohn , Richard Henderson Subject: Re: [PATCH] PowerPC section type conflict (created PR 51623) Message-ID: <20111228173957.GA23383@ibm-tiger.the-meissners.org> Mail-Followup-To: Michael Meissner , Chung-Lin Tang , gcc-patches , David Edelsohn , Richard Henderson References: <4EEB2886.6040108@codesourcery.com> <4EEBC463.1090802@redhat.com> <4EED89EF.1050207@codesourcery.com> <4EEE3C76.3010204@redhat.com> <4EEF5C1F.2020107@codesourcery.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4EEF5C1F.2020107@codesourcery.com> User-Agent: Mutt/1.5.20 (2009-12-10) x-cbid: 11122817-3352-0000-0000-0000019B466A 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 On Mon, Dec 19, 2011 at 11:45:35PM +0800, Chung-Lin Tang wrote: > On 2011/12/19 上午 03:18, Richard Henderson wrote: > > On 12/17/2011 10:36 PM, Chung-Lin Tang wrote: > >> I don't think it's that kind of problem; the powerpc backend uses > >> unlikely_text_section_p(), which compares the passed in argument section > >> and the value of function_section_1(current_function_decl,true). > > > > I think this might be the real bug, or something related. > > > >> Since current_function_decl is NULL at assembly phase, it retrieves > >> ".text.unlikely" to test for equality. It's the retrieving/lookup that > >> fails here, because the default looked-up section flags set when decl == > >> NULL does not really seem to make sense (adds SECTION_WRITE). > > > > current_function_decl is only null when we're not inside a function. > > > > One possible fix is to test for current_function_section inside > > unlikely_text_section_p. However, I think that begs the question > > of what in the world is actually going on in rs6000_assemble_integer. > > Why are we testing for emitting data in text sections? > > I think I sort of mis-represented the context here; this was not really > during the assembly phase of a function, but already in > toplev.c:output_object_blocks(). > > I've created a bugzilla PR for this, with a testcase from U-boot, and a > minimal testcase: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51623 This one line patch fixes the problem by using a different test than unlikely_text_section_p, which assumes it is called within a function context. I bootstrapped it, and there were no regressions. I have added the test case from the PR so it doesn't come back. Is it ok to apply? It is also a bug in GCC 4.6, and I will backport the patch to that branch as well. FWIW, I wrote -mrelocatable around 1990 or so to for a specific Cygnus customer that needed to have pseudo shared libraries in embedded code, as long as they were willing to live with various restrictions. At the time, the Linux shared library code was non-existant, and this was a quick hack. In the nature of all quick hacks, eventually things change in the machine independent code layer, and it has to be revisited. In hindsight, it would have been better if the Linux shared library code was operational, and that there was a non-GPL dynamic linker written to handle the relocations, rather than having this quick hack. The check for unlikely text was added in 2004 by Caroline Tice of Apple, and it is curious that they didn't add a check for it being in a hot function as well as a cold function. I also suspect the check would not work as well if -ffunctions-section was used. The point of the check is not to add to the fixup table pointers that are stored in the read-only text section (which would cause a segfault at runtime, but it would leave a pointer that is not fixed up when the program starts). It was modified in 2005 by Richard Sandiford in a global change in how sections are dealt with, and modified by Alan Modra in 2006. [gcc] 2011-12-27 Michael Meissner PR target/51623 * config/rs6000/rs6000.c (rs6000_assemble_integer): Don't call unlikely_text_section_p. Instead check for being in a code section. [gcc/testsuite] 2011-12-27 Michael Meissner PR target/51623 * gcc.target/powerpc/pr51623.c: New file. Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 182694) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -15461,7 +15461,7 @@ rs6000_assemble_integer (rtx x, unsigned if (TARGET_RELOCATABLE && in_section != toc_section && in_section != text_section - && !unlikely_text_section_p (in_section) + && (in_section && (in_section->common.flags & SECTION_CODE)) == 0 && !recurse && GET_CODE (x) != CONST_INT && GET_CODE (x) != CONST_DOUBLE Index: gcc/testsuite/gcc.target/powerpc/pr51623.c =================================================================== --- gcc/testsuite/gcc.target/powerpc/pr51623.c (revision 0) +++ gcc/testsuite/gcc.target/powerpc/pr51623.c (revision 0) @@ -0,0 +1,123 @@ +/* PR target/51623 */ +/* { dg-do compile { target { { powerpc*-*-linux* && ilp32 } || { powerpc-*-eabi* } } } } */ +/* { dg-options "-mrelocatable -ffreestanding" } */ + +/* This generated an error, since the compiler was calling + unlikely_text_section_p in a context where it wasn't valid. */ + +typedef long long loff_t; +typedef unsigned size_t; + + +struct mtd_info { + unsigned writesize; + unsigned oobsize; + const char *name; +}; + +extern int strcmp(const char *,const char *); +extern char * strchr(const char *,int); + +struct cmd_tbl_s { + char *name; +}; + + +int printf(const char *fmt, ...) __attribute__ ((format (__printf__, 1, 2))); +void* malloc(size_t); +void free(void*); + + +extern int nand_curr_device; +extern struct mtd_info nand_info[]; + +static int nand_dump(struct mtd_info *nand, unsigned long off, int only_oob) +{ + int i; + unsigned char *datbuf, *oobbuf, *p; + + datbuf = malloc(nand->writesize + nand->oobsize); + oobbuf = malloc(nand->oobsize); + off &= ~(nand->writesize - 1); + + printf("Page %08lx dump:\n", off); + i = nand->writesize >> 4; + p = datbuf; + + while (i--) { + if (!only_oob) + printf("\t%02x %02x %02x %02x %02x %02x %02x %02x" + " %02x %02x %02x %02x %02x %02x %02x %02x\n", + p[0], p[1], p[2], p[3], p[4], p[5], p[6], p[7], + p[8], p[9], p[10], p[11], p[12], p[13], p[14], + p[15]); + p += 16; + } + + i = nand->oobsize >> 3; + free(datbuf); + free(oobbuf); + + return 0; +} + +int do_nand(struct cmd_tbl_s * cmdtp, int flag, int argc, char *argv[]) +{ + int dev; + unsigned long off; + char *cmd, *s; + struct mtd_info *nand; + + if (argc < 2) + goto usage; + + cmd = argv[1]; + + if (strcmp(cmd, "info") == 0) { + putc('\n'); + return 0; + } + + if (strcmp(cmd, "device") == 0) { + if (argc < 3) { + putc('\n'); + } + dev = (int)simple_strtoul(argv[2], ((void *)0), 10); + nand_curr_device = dev; + return 0; + } + + if (strcmp(cmd, "bad") != 0 && strcmp(cmd, "erase") != 0 ) + goto usage; + + if (nand_curr_device < 0 ) { + return 1; + } + nand = &nand_info[nand_curr_device]; + + if (strcmp(cmd, "erase") == 0 || strcmp(cmd, "scrub") == 0) { + int clean = argc > 2 && !strcmp("clean", argv[2]); + int scrub = !strcmp(cmd, "scrub"); + return 0; + } + + if (strncmp(cmd, "dump", 4) == 0) { + if (argc < 3) + goto usage; + + s = strchr(cmd, '.'); + off = (int)simple_strtoul(argv[2], ((void *)0), 16); + + if (s != ((void *)0) && strcmp(s, ".oob") == 0) + nand_dump(nand, off, 1); + else + nand_dump(nand, off, 0); + + return 0; + } +usage: + cmd_usage(cmdtp); + return 1; +} + +void *ptr = do_nand;