From patchwork Wed Nov 20 23:59:08 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sandra Loosemore X-Patchwork-Id: 1198584 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-514259-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="qDrutNze"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 47JKRj272rz9s7T for ; Thu, 21 Nov 2019 10:59:27 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=xQOGahLjqUv/VDuDUDg46xYe7PC/O5uMD2qipMxYh2WX8i0gTg SQEvL9PbKSYzzRaKVEOMn1+wxsbNncYgYGssvIGE6Cw1+gblFdRKWWMKZWhS1VCN CJtZMJITs/p0uGDbmAU5Tm2vBbd/ukN/eevWQXKJR+gr4hV4grH+c8joA= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type; s= default; bh=7v2F8tuTY2xw01w5Tk2l+QOoTbE=; b=qDrutNzepAODCePB6iZf rV0KwGcMQQggcMIV0R2KNIJAqqnpwqukmPWglS17q9rEW9pWuL3/kalGeQrelZRi eQKSh1QGWHIcDAF4CYiU1E2BVSDC7UeuxRRZ4EC+8wTzx72DvBBY90SZQoXFBk3K DZ0qtgH269FeVc/sVZU3jaI= Received: (qmail 21526 invoked by alias); 20 Nov 2019 23:59:19 -0000 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 Received: (qmail 21509 invoked by uid 89); 20 Nov 2019 23:59:19 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS autolearn=ham version=3.3.1 spammy=symptom, TREE_READONLY, tree_readonly X-HELO: esa3.mentor.iphmx.com Received: from esa3.mentor.iphmx.com (HELO esa3.mentor.iphmx.com) (68.232.137.180) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 20 Nov 2019 23:59:16 +0000 IronPort-SDR: Ik2JnsgaV163YUl4+YSwNycP2Ty2HimPNyOcg8ARK5nWg++R/6Dj6mk5JvJvqodOvhDYuDmw4B 7bGgalvBy2q5RiCBR2aspgLCba64z3qqZ1lkkYloc8WTnG6Lrm0/PWKP/uKwSN7aRdMUS1G4Aw Xe/AifjQ5kffUua77TcwxqaEtQq6SYu3zP+jFN+1Za3rgtfKlG3mn+31ce7TMViyyqxFmPVdmx r2oWgFBIzkaoC0yHOONYhq+q3QB5nPVnWEr8C22AXwfr2t9B4D9JQJyV7MbDZauBLiiYAbfSFd Cd8= Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa3.mentor.iphmx.com with ESMTP; 20 Nov 2019 15:59:14 -0800 IronPort-SDR: u2kcAvEmht71fuuJREGMsFkPKXdKpEgMc6RylQOcWPJviml/cKzGV/vAWCwwdUXx19ar9+8U46 eGgRSMMIFXeuqYI+6K4BV9wfO59FnI9jslhIDX1uaWEgZt4t1EgbZaG5XKi91G/8Fd6Rljjf7w qom6gv2pnqyIF3h7wIuwkcHQ8P/zf/y9pMxLokOgX9dT3NSRHGbYHimWiGMWvOt5bGEi3zbw4t m5zXw4aBDmWpYt9RF3BmQ1HV021e4StwaIGvjT2hyCxNw0ErssILBj3vbF1kDnFdyP+I9waVG1 BQE= To: "gcc-patches@gcc.gnu.org" From: Sandra Loosemore Subject: [PATCH] Don't put objects with flexible array members in small data Message-ID: Date: Wed, 20 Nov 2019 16:59:08 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 This patch is for PR target/92499, a bug reported by GLIBC maintainers against nios2 target. The initial failure symptom was a linker error resulting from use of GP-relative addressing on an object that wasn't allocated in the small data section. It turns out that the real problem is that the TARGET_IN_SMALL_DATA_P hook uses the function int_size_in_bytes to check whether an object is "small", and this function treats flexible array members as having size 0. All backends except for tic6x that implement TARGET_IN_SMALL_DATA_P use a similar implementation of the hook that has the same problem. The solution here is to add a new function that returns -1 as the size of any type with a flexible array member, and adjust all the affected backends to use it. I swiped the predicate for identifying these types from the C front end. I regression-tested this on nios2-elf and nios2-linux-gnu. I'm not set up to test any of the other affected back ends, but the code change is trivial and identical to what I did for nios2. OK to commit? I'd like to backport this patch to all active branches as well, once it is in on trunk. -Sandra Index: gcc/c/c-decl.c =================================================================== --- gcc/c/c-decl.c (revision 278366) +++ gcc/c/c-decl.c (working copy) @@ -5667,39 +5667,6 @@ check_compound_literal_type (location_t "defining a type in a compound literal is invalid in C++"); } -/* Determine whether TYPE is a structure with a flexible array member, - or a union containing such a structure (possibly recursively). */ - -static bool -flexible_array_type_p (tree type) -{ - tree x; - switch (TREE_CODE (type)) - { - case RECORD_TYPE: - x = TYPE_FIELDS (type); - if (x == NULL_TREE) - return false; - while (DECL_CHAIN (x) != NULL_TREE) - x = DECL_CHAIN (x); - if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE - && TYPE_SIZE (TREE_TYPE (x)) == NULL_TREE - && TYPE_DOMAIN (TREE_TYPE (x)) != NULL_TREE - && TYPE_MAX_VALUE (TYPE_DOMAIN (TREE_TYPE (x))) == NULL_TREE) - return true; - return false; - case UNION_TYPE: - for (x = TYPE_FIELDS (type); x != NULL_TREE; x = DECL_CHAIN (x)) - { - if (flexible_array_type_p (TREE_TYPE (x))) - return true; - } - return false; - default: - return false; - } -} - /* Performs sanity checks on the TYPE and WIDTH of the bit-field NAME, replacing with appropriate values if they are invalid. */ Index: gcc/config/alpha/alpha.c =================================================================== --- gcc/config/alpha/alpha.c (revision 278366) +++ gcc/config/alpha/alpha.c (working copy) @@ -796,7 +796,7 @@ alpha_in_small_data_p (const_tree exp) } else { - HOST_WIDE_INT size = int_size_in_bytes (TREE_TYPE (exp)); + HOST_WIDE_INT size = fixed_int_size_in_bytes (TREE_TYPE (exp)); /* If this is an incomplete type with size 0, then we can't put it in sdata because it might be too big when completed. */ Index: gcc/config/arc/arc.c =================================================================== --- gcc/config/arc/arc.c (revision 278366) +++ gcc/config/arc/arc.c (working copy) @@ -8643,7 +8643,7 @@ arc_in_small_data_p (const_tree decl) section. */ else if (TREE_PUBLIC (decl)) { - size = int_size_in_bytes (TREE_TYPE (decl)); + size = fixed_int_size_in_bytes (TREE_TYPE (decl)); return (size > 0 && size <= g_switch_value); } return false; Index: gcc/config/frv/frv.c =================================================================== --- gcc/config/frv/frv.c (revision 278366) +++ gcc/config/frv/frv.c (working copy) @@ -9319,7 +9319,7 @@ frv_in_small_data_p (const_tree decl) return false; } - size = int_size_in_bytes (TREE_TYPE (decl)); + size = fixed_int_size_in_bytes (TREE_TYPE (decl)); if (size > 0 && size <= g_switch_value) return true; Index: gcc/config/ia64/ia64.c =================================================================== --- gcc/config/ia64/ia64.c (revision 278366) +++ gcc/config/ia64/ia64.c (working copy) @@ -10016,7 +10016,7 @@ ia64_in_small_data_p (const_tree exp) } else { - HOST_WIDE_INT size = int_size_in_bytes (TREE_TYPE (exp)); + HOST_WIDE_INT size = fixed_int_size_in_bytes (TREE_TYPE (exp)); /* If this is an incomplete type with size 0, then we can't put it in sdata because it might be too big when completed. */ Index: gcc/config/lm32/lm32.c =================================================================== --- gcc/config/lm32/lm32.c (revision 278366) +++ gcc/config/lm32/lm32.c (working copy) @@ -795,7 +795,7 @@ lm32_in_small_data_p (const_tree exp) } else { - HOST_WIDE_INT size = int_size_in_bytes (TREE_TYPE (exp)); + HOST_WIDE_INT size = fixed_int_size_in_bytes (TREE_TYPE (exp)); /* If this is an incomplete type with size 0, then we can't put it in sdata because it might be too big when completed. */ Index: gcc/config/m32r/m32r.c =================================================================== --- gcc/config/m32r/m32r.c (revision 278366) +++ gcc/config/m32r/m32r.c (working copy) @@ -513,7 +513,7 @@ m32r_in_small_data_p (const_tree decl) { if (! TREE_READONLY (decl) && ! TARGET_SDATA_NONE) { - int size = int_size_in_bytes (TREE_TYPE (decl)); + int size = fixed_int_size_in_bytes (TREE_TYPE (decl)); if (size > 0 && size <= g_switch_value) return true; Index: gcc/config/microblaze/microblaze.c =================================================================== --- gcc/config/microblaze/microblaze.c (revision 278366) +++ gcc/config/microblaze/microblaze.c (working copy) @@ -3236,7 +3236,7 @@ microblaze_elf_in_small_data_p (const_tr return true; } - size = int_size_in_bytes (TREE_TYPE (decl)); + size = fixed_int_size_in_bytes (TREE_TYPE (decl)); return (size > 0 && size <= microblaze_section_threshold); } Index: gcc/config/mips/mips.c =================================================================== --- gcc/config/mips/mips.c (revision 278366) +++ gcc/config/mips/mips.c (working copy) @@ -9399,7 +9399,7 @@ mips_in_small_data_p (const_tree decl) /* We have traditionally not treated zero-sized objects as small data, so this is now effectively part of the ABI. */ - size = int_size_in_bytes (TREE_TYPE (decl)); + size = fixed_int_size_in_bytes (TREE_TYPE (decl)); return size > 0 && size <= mips_small_data_threshold; } Index: gcc/config/nios2/nios2.c =================================================================== --- gcc/config/nios2/nios2.c (revision 278366) +++ gcc/config/nios2/nios2.c (working copy) @@ -2375,7 +2375,7 @@ nios2_in_small_data_p (const_tree exp) } else { - HOST_WIDE_INT size = int_size_in_bytes (TREE_TYPE (exp)); + HOST_WIDE_INT size = fixed_int_size_in_bytes (TREE_TYPE (exp)); /* If this is an incomplete type with size 0, then we can't put it in sdata because it might be too big when completed. */ Index: gcc/config/riscv/riscv.c =================================================================== --- gcc/config/riscv/riscv.c (revision 278366) +++ gcc/config/riscv/riscv.c (working copy) @@ -3377,7 +3377,7 @@ riscv_in_small_data_p (const_tree x) return strcmp (sec, ".sdata") == 0 || strcmp (sec, ".sbss") == 0; } - return riscv_size_ok_for_small_data_p (int_size_in_bytes (TREE_TYPE (x))); + return riscv_size_ok_for_small_data_p (fixed_int_size_in_bytes (TREE_TYPE (x))); } /* Switch to the appropriate section for output of DECL. */ Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 278366) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -19442,7 +19442,7 @@ rs6000_elf_in_small_data_p (const_tree d && !rs6000_readonly_in_sdata) return false; - HOST_WIDE_INT size = int_size_in_bytes (TREE_TYPE (decl)); + HOST_WIDE_INT size = fixed_int_size_in_bytes (TREE_TYPE (decl)); if (size > 0 && size <= g_switch_value Index: gcc/config/rx/rx.c =================================================================== --- gcc/config/rx/rx.c (revision 278366) +++ gcc/config/rx/rx.c (working copy) @@ -2289,7 +2289,7 @@ rx_in_small_data (const_tree decl) if (section) return (strcmp (section, "D_2") == 0) || (strcmp (section, "B_2") == 0); - size = int_size_in_bytes (TREE_TYPE (decl)); + size = fixed_int_size_in_bytes (TREE_TYPE (decl)); return (size > 0) && (size <= rx_small_data_limit); } Index: gcc/testsuite/gcc.dg/pr92499.c =================================================================== --- gcc/testsuite/gcc.dg/pr92499.c (nonexistent) +++ gcc/testsuite/gcc.dg/pr92499.c (working copy) @@ -0,0 +1,67 @@ +/* PR target/92499 */ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +enum { size = 100 }; + +struct flexible +{ + int length; + int data[]; +}; + +struct inflexible +{ + int length; + int data[size]; +}; + +static struct flexible flexible = + { + .data = { [size - 1] = 0, } + }; + +static struct flexible flexible_nonzero = + { + .length = size, + .data = { [size - 1] = 0, } + }; + +static struct inflexible inflexible = + { + .data = { [size - 1] = 0, } + }; + +struct flexible * +get_flexible (void) +{ + return &flexible; +} + +struct flexible * +get_flexible_nonzero (void) +{ + return &flexible_nonzero; +} + +struct inflexible * +get_inflexible (void) +{ + return &inflexible; +} + +/* Check that variables containing flexible array elements are not placed + in small data. Almost all targets that support small data sections use + the canonical names for them. */ +/* { dg-final { scan-assembler-not "\\.sdata" } } */ +/* { dg-final { scan-assembler-not "\\.sbss" } } */ +/* { dg-final { scan-assembler-not "\\.section D_2" { target rx*-*-* } } } */ +/* { dg-final { scan-assembler-not "\\.section B_2" { target rx*-*-* } } } */ + +/* Also check that GP-relative addressing is not used to reference any + of the variables, on targets that support that form of addressing for + small data. Since the assembler syntax varies on the targets that support + this, we need to list the targets individually. This list is not a + complete set. */ +/* { dg-final { scan-assembler-not "%gp_rel\(.*flexible.*\)" { target mips*-*-* } } } */ +/* { dg-final { scan-assembler-not "%gprel\(.*flexible.*\)" { target nios2*-*-* } } } */ Index: gcc/tree.c =================================================================== --- gcc/tree.c (revision 278366) +++ gcc/tree.c (working copy) @@ -3266,7 +3266,10 @@ size_in_bytes_loc (location_t loc, const } /* Return the size of TYPE (in bytes) as a wide integer - or return -1 if the size can vary or is larger than an integer. */ + or return -1 if the size can vary or is larger than an integer. + Note that this function returns the non-variable size for objects + containing flexible array members (like the C sizeof operator + does) instead of -1; see fixed_int_size_in_bytes below. */ HOST_WIDE_INT int_size_in_bytes (const_tree type) @@ -15000,6 +15003,54 @@ default_is_empty_record (const_tree type return default_is_empty_type (TYPE_MAIN_VARIANT (type)); } +/* Determine whether TYPE is a structure with a flexible array member, + or a union containing such a structure (possibly recursively). */ + +bool +flexible_array_type_p (const_tree type) +{ + tree x, last; + switch (TREE_CODE (type)) + { + case RECORD_TYPE: + last = NULL_TREE; + for (x = TYPE_FIELDS (type); x != NULL_TREE; x = DECL_CHAIN (x)) + if (TREE_CODE (x) == FIELD_DECL) + last = x; + if (last == NULL_TREE) + return false; + if (TREE_CODE (TREE_TYPE (last)) == ARRAY_TYPE + && TYPE_SIZE (TREE_TYPE (last)) == NULL_TREE + && TYPE_DOMAIN (TREE_TYPE (last)) != NULL_TREE + && TYPE_MAX_VALUE (TYPE_DOMAIN (TREE_TYPE (last))) == NULL_TREE) + return true; + return false; + case UNION_TYPE: + for (x = TYPE_FIELDS (type); x != NULL_TREE; x = DECL_CHAIN (x)) + { + if (TREE_CODE (x) == FIELD_DECL + && flexible_array_type_p (TREE_TYPE (x))) + return true; + } + return false; + default: + return false; + } +} + +/* Like int_size_in_bytes, but also return -1 for flexible array + types, so that it only returns a positive value for types with a + fixed size. */ + +HOST_WIDE_INT +fixed_int_size_in_bytes (const_tree type) +{ + if (flexible_array_type_p (type)) + return -1; + else + return int_size_in_bytes (type); +} + /* Like int_size_in_bytes, but handle empty records specially. */ HOST_WIDE_INT Index: gcc/tree.h =================================================================== --- gcc/tree.h (revision 278366) +++ gcc/tree.h (working copy) @@ -6077,6 +6077,8 @@ extern void gt_pch_nx (tree &, gt_pointe extern bool nonnull_arg_p (const_tree); extern bool default_is_empty_record (const_tree); +extern bool flexible_array_type_p (const_tree); +extern HOST_WIDE_INT fixed_int_size_in_bytes (const_tree); extern HOST_WIDE_INT arg_int_size_in_bytes (const_tree); extern tree arg_size_in_bytes (const_tree); extern bool expr_type_first_operand_type_p (tree_code);