From patchwork Wed May 4 15:37:05 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Bennett X-Patchwork-Id: 618512 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3r0Mc26Ffjz9s9x for ; Thu, 5 May 2016 01:37:22 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=ZWJ5DqT8; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:content-type :content-transfer-encoding:mime-version; q=dns; s=default; b=ZaZ NsaDYv/wa/cIpuzZejLVjoa1ayXLl4zBhMqjDxRumDXnP4yb6P82x4EHgzxE137y AjD0otk9Y41Hv48kuIw8y7tCJUpm5R24hF2ds0sTyUqvSKDYkkiB5pKjY09PY6M4 UvksCKkH4yxmNjjUUCHOsOharHJNJjnGT2pRXFs0= 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:from :to:cc:subject:date:message-id:content-type :content-transfer-encoding:mime-version; s=default; bh=visgd5WkV y6u6kdX3P7GmeZbBvk=; b=ZWJ5DqT8i3f7aNBCrZxU5IbMDI2HFCPe5youdO7up 92ZYNetFBoEkXDcDUolNMGgBZ/okTeYJ4IGSTFyO9lgowkPsKPPb2FscjF0LrpgK JLSmU03AzSargQ99z7yU5+rzEdYiQ/4VxE46zxTWpC1pLvD0VnUW0rgabK1cQ8xK qk= Received: (qmail 40361 invoked by alias); 4 May 2016 15:37:14 -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 40351 invoked by uid 89); 4 May 2016 15:37:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.7 required=5.0 tests=AWL, BAYES_05, KAM_STOCKGEN, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=hs, HIGH, hl, $31 X-HELO: mailapp01.imgtec.com Received: from mailapp01.imgtec.com (HELO mailapp01.imgtec.com) (195.59.15.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 04 May 2016 15:37:11 +0000 Received: from hhmail02.hh.imgtec.org (unknown [10.100.10.20]) by Websense Email with ESMTPS id 82162BD4D9893 for ; Wed, 4 May 2016 16:37:03 +0100 (IST) Received: from LEMAIL01.le.imgtec.org (192.168.152.62) by hhmail02.hh.imgtec.org (10.100.10.20) with Microsoft SMTP Server (TLS) id 14.3.266.1; Wed, 4 May 2016 16:37:07 +0100 Received: from LEMAIL01.le.imgtec.org ([fe80::5ae:ee16:f4b9:cda9]) by LEMAIL01.le.imgtec.org ([fe80::5ae:ee16:f4b9:cda9%17]) with mapi id 14.03.0266.001; Wed, 4 May 2016 16:37:06 +0100 From: Andrew Bennett To: "gcc-patches@gcc.gnu.org" CC: Matthew Fortune Subject: [PATCH] MIPS: Ensure that lo_sums do not contain an unaligned symbol Date: Wed, 4 May 2016 15:37:05 +0000 Message-ID: <0DA23CC379F5F945ACB41CF394B982776A69BD79@LEMAIL01.le.imgtec.org> MIME-Version: 1.0 X-IsSubscribed: yes Hi, In MIPS (and similarly for other RISC architectures) to load an absolute address of an object requires a two instruction sequence: one instruction to load the high part of the object's address, and one instruction to load the low part of the object's address. Typically the result from the calculation of the high part of the address will only be used by one instruction to load the low part of the address. However, in certain situations (for example when loading or storing double word values) the result of computing the high part of the address can be used by multiple instructions to load the low parts of an address at different offsets. Lets show this with an example C program. struct { short s; unsigned long long l; } h; void foo (void) { h.l = 0; } When this is compiled for MIPS it produces the following assembly: lui $2,%hi(h+8) sw $0,%lo(h+8)($2) jr $31 sw $0,%lo(h+12)($2) ... .globl h .section .bss,"aw",@nobits .align 3 .type h, @object .size h, 16 h: .space 16 Notice here that the high part of the address of object h is loaded into register $2, and this is then used as part of the low part calculation by two the sw instructions which each have different offsets. In MIPS the value of a low part calculation is treated as a signed value. It is therefore valid to use the result of a high part calculation with multiple low part calculations containing different offsets so long as when adding the result of the high part to the each of the sign extended low parts we get valid addresses. However, if we add the packed attribute to the h structure, the fields of the structure will not be naturally aligned and we can break the previous condition. Lets explain this in more detail with the following C program. struct __attribute__((packed)) { short s; unsigned long long l; } h; void foo (void) { h.l = 0; } When this is compiled for MIPS it produces the following assembly: lui $2,%hi(h) addiu $4,$2,%lo(h+2) addiu $3,$2,%lo(h+6) swl $0,3($4) swr $0,%lo(h+2)($2) swl $0,3($3) jr $31 swr $0,%lo(h+6)($2) ... .globl h .section .bss,"aw",@nobits .align 2 .type h, @object .size h, 10 h: .space 10 There are two things to highlight here. Firstly the alignment of the h structure has decreased from 8 bytes to 4 bytes. Secondly we have a low part calculation which adds an offset of 6 to the address of the h structure which is greater than its alignment. When the MIPS linker resolves a HI relocation (i.e. %hi(h)) it finds the next LO relocation (i.e. %lo(h+2)) in the relocation table and using the information from both of these relocations it computes the object's address and extracts its high part. Then, when the MIPS linker resolves a LO relocation it adds the offset to the object's address and then extracts the low part. Lets assume that object h has an address of 0x80007ffc. When the MIPS linker resolves the value of the HI relocation for object h, it will also use the value of the LO relocation for object h with an offset of 2. The high part value is therefore: HIGH (0x80007ffc + 2) = HIGH (0x80007ffe) = 0x8000 Then the MIPS linker resolves the value of LO relocation for object h with an offset of 2: LO (0x80007ffc + 2) = LO (0x80007ffe) = 0x7ffe Finally the MIPS linker resolves the value of the LO relocation for object h with an offset of 6: LO (0x80007ffc + 6) = LO (0x80008002) = 0x8002 In MIPS the value of a LO relocation is treated as a signed value, so when the program is run the address of h+6 will be 0x7fff8002 when it should be 0x80008002. To fix this issue I have changed the mips_split_symbol function in the case when it generates a set of instructions to compute the high and low part of a symbol's address. If the symbol is unaligned the low-part calculation is forced into a register. I have also added a condition into mips_classify_address that prevents lo_sums from being used if they contain an unaligned symbol. This stops GCC from trying to merge the result of a lo_sum that is currently in register back into an address calculation. I have tested the patch on the mips-mti-elf toolchain and there have been no regressions. The patch and ChangeLog are below. Ok to commit? Many thanks, Andrew gcc/ * config/mips/mips.c (mips_valid_lo_sum_p): New function. (mips_classify_address): Call mips_valid_lo_sum_p to check if we have a valid lo_sum. (mips_split_symbol): Force the lo_sum to a register if mips_valid_lo_sum_p is false. testsuite/ * gcc.target/mips/hi-lo-reloc-offset1: New test. * gcc.target/mips/hi-lo-reloc-offset2: New test. * gcc.target/mips/hi-lo-reloc-offset3: New test. * gcc.target/mips/hi-lo-reloc-offset4: New test. * gcc.target/mips/hi-lo-reloc-offset5: New test. * gcc.target/mips/hi-lo-reloc-offset6: New test. * gcc.target/mips/hi-lo-reloc-offset7: New test. * gcc.target/mips/hi-lo-reloc-offset8: New test. * gcc.target/mips/hi-lo-reloc-offset9: New test. * gcc.target/mips/hi-lo-reloc-offset10: New test. * gcc.target/mips/hi-lo-reloc-offset11: New test. diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index 6cdda3b..f07e433 100644 --- a/gcc/config/mips/mips.c +++ b/gcc/config/mips/mips.c @@ -2354,6 +2354,38 @@ mips_valid_lo_sum_p (enum mips_symbol_type symbol_type, machine_mode mode) return true; } +/* Return true if X in LO_SUM (REG, X) is a valid. */ + +bool +mips_valid_lo_sum_lo_part_p (machine_mode mode, rtx reg, rtx x) +{ + rtx symbol = NULL; + + if (mips_abi != ABI_32) + return true; + + if (mode == BLKmode) + return true; + + if (reg && REG_P (reg) && REGNO (reg) == GLOBAL_POINTER_REGNUM) + return true; + + if (GET_CODE (x) == CONST + && GET_CODE (XEXP (x, 0)) == PLUS + && GET_CODE (XEXP (XEXP (x, 0), 0)) == SYMBOL_REF) + symbol = XEXP (XEXP (x, 0), 0); + else if (GET_CODE (x) == SYMBOL_REF) + symbol = x; + + if (symbol + && SYMBOL_REF_DECL (symbol) + && (GET_MODE_ALIGNMENT (mode) / BITS_PER_UNIT) > + (DECL_ALIGN_UNIT (SYMBOL_REF_DECL (symbol)))) + return false; + + return true; +} + /* Return true if X is a valid address for machine mode MODE. If it is, fill in INFO appropriately. STRICT_P is true if REG_OK_STRICT is in effect. */ @@ -2394,7 +2426,8 @@ mips_classify_address (struct mips_address_info *info, rtx x, info->symbol_type = mips_classify_symbolic_expression (info->offset, SYMBOL_CONTEXT_MEM); return (mips_valid_base_register_p (info->reg, mode, strict_p) - && mips_valid_lo_sum_p (info->symbol_type, mode)); + && mips_valid_lo_sum_p (info->symbol_type, mode) + && mips_valid_lo_sum_lo_part_p (mode, info->reg, info->offset)); case CONST_INT: /* Small-integer addresses don't occur very often, but they @@ -3143,6 +3176,8 @@ mips_split_symbol (rtx temp, rtx addr, machine_mode mode, rtx *low_out) high = gen_rtx_HIGH (Pmode, copy_rtx (addr)); high = mips_force_temporary (temp, high); *low_out = gen_rtx_LO_SUM (Pmode, high, addr); + if (!mips_valid_lo_sum_lo_part_p (mode, NULL, addr)) + *low_out = mips_force_temporary (temp, *low_out); break; } return true; diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset1.c b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset1.c new file mode 100644 index 0000000..dc81fd9 --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset1.c @@ -0,0 +1,14 @@ +/* { dg-options "-mcode-readable=no -mno-gpopt -mabi=32" } */ +/* { dg-final { scan-assembler-not "%lo\\(h\\+6\\)" } } */ + +struct __attribute__((packed)) +{ + short s; + unsigned long long l; +} h; + +void foo (void) +{ + h.l = 0; +} + diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset10.c b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset10.c new file mode 100644 index 0000000..da80b67 --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset10.c @@ -0,0 +1,19 @@ +/* { dg-options "-mcode-readable=no -mno-gpopt -mabi=32 -mfpxx isa=2" } */ +/* { dg-final { scan-assembler "%lo\\(d\\+4\\)" } } */ +/* { dg-final { scan-assembler "%lo\\(f\\)" } } */ +/* { dg-final { scan-assembler-not "\tsw\t\\\$\[0-9\],%lo\\(a_f\\)" } } */ +/* { dg-final { scan-assembler-not "\tsw\t\\\$\[0-9\],%lo\\(a_d\\)" } } */ + +double d; +double __attribute__((aligned(1))) a_d; +float f; +float __attribute__((aligned(1))) a_f; + +void foo (void) +{ + d = 0; + f = 0; + a_d = 0; + a_f = 0; +} + diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset11.c b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset11.c new file mode 100644 index 0000000..b2679f9 --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset11.c @@ -0,0 +1,17 @@ +/* { dg-options "-mcode-readable=no -mno-gpopt -mabi=32" } */ +/* { dg-final { scan-assembler-not "%lo\\(h\\+\[1-9\]\\)" } } */ + +struct __attribute__((packed)) +{ + char c; + short s; + int i; +} h; + +void foo (void) +{ + h.c = 0; + h.s = 0; + h.i = 0; +} + diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset2.c b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset2.c new file mode 100644 index 0000000..1b9a0b1 --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset2.c @@ -0,0 +1,13 @@ +/* { dg-options "-mcode-readable=no -mno-gpopt -mabi=32" } */ +/* { dg-final { scan-assembler-not "%lo\\(a_l\\+4\\)" } } */ +/* { dg-final { scan-assembler "%lo\\(l\\+4\\)" } } */ + +unsigned long long l; +unsigned long long __attribute__((aligned(1))) a_l; + +void foo (void) +{ + l = 0; + a_l = 0; +} + diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset3.c b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset3.c new file mode 100644 index 0000000..a01c1d8 --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset3.c @@ -0,0 +1,15 @@ +/* { dg-options "-mcode-readable=no -mno-gpopt -mabi=32" } */ +/* { dg-final { scan-assembler-not "%lo\\(a_l\\+4\\)" } } */ +/* { dg-final { scan-assembler "%lo\\(l\\+4\\)" } } */ + +long long l; +double d; + +long long __attribute__((aligned(1))) a_l; +double __attribute__((aligned(1))) a_d; + +void foo (void) +{ + d = (double) l; + a_d = (double) a_l; +} diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset4.c b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset4.c new file mode 100644 index 0000000..496c078 --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset4.c @@ -0,0 +1,16 @@ +/* { dg-options "-mcode-readable=no -mno-gpopt -mabi=32" } */ +/* { dg-final { scan-assembler-not "%lo\\(h\\+\[1-9\]\\)" } } */ + +struct __attribute__((packed)) +{ + short s; + double d; + float f; +} h; + +void foo (void) +{ + h.d = 0; + h.f = 0; +} + diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset5.c b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset5.c new file mode 100644 index 0000000..828c328 --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset5.c @@ -0,0 +1,20 @@ +/* { dg-options "-mcode-readable=no -mno-gpopt -mabi=32" } */ +/* { dg-final { scan-assembler "%lo\\(d\\+4\\)" } } */ +/* { dg-final { scan-assembler "\tsw\t\\\$\[0-9\],%lo\\(f\\)" } } */ +/* { dg-final { scan-assembler-not "%lo\\(a_d\\+4\\)" } } */ +/* { dg-final { scan-assembler-not "\tsw\t\\\$\[0-9\],%lo\\(a_f\\)" } } */ + +double d; +double __attribute__((aligned(1))) a_d; + +float f; +float __attribute__((aligned(1))) a_f; + +void foo (void) +{ + d = 0; + f = 0; + a_d = 0; + a_f = 0; +} + diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset6.c b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset6.c new file mode 100644 index 0000000..2eb8f9a --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset6.c @@ -0,0 +1,25 @@ +/* { dg-options "-mcode-readable=no -mno-gpopt -mabi=32" } */ +/* { dg-final { scan-assembler "\tsb\t\\\$\[0-9\],%lo\\(c\\)" } } */ +/* { dg-final { scan-assembler "\tsh\t\\\$\[0-9\],%lo\\(s\\)" } } */ +/* { dg-final { scan-assembler "\tsw\t\\\$\[0-9\],%lo\\(i\\)" } } */ +/* { dg-final { scan-assembler "\tsb\t\\\$\[0-9\],%lo\\(a_c\\)" } } */ +/* { dg-final { scan-assembler-not "\tsh\t\\\$\[0-9\],%lo\\(a_s\\)" } } */ +/* { dg-final { scan-assembler-not "\tsw\t\\\$\[0-9\],%lo\\(a_i\\)" } } */ + +char __attribute__((aligned(1))) a_c; +short __attribute__((aligned(1))) a_s; +int __attribute__((aligned(1))) a_i; +char c; +short s; +int i; + +void foo (void) +{ + c = 0; + s = 0; + i = 0; + a_c = 0; + a_s = 0; + a_i = 0; +} + diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset7.c b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset7.c new file mode 100644 index 0000000..8f348fb --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset7.c @@ -0,0 +1,16 @@ +/* { dg-options "-mcode-readable=no -mabi=32 -mfp32 isa=1" } */ +/* { dg-final { scan-assembler-not "%lo\\(h\\+\[1-9\]\\)" } } */ + +struct __attribute__((packed)) +{ + short s; + double d; + float f; +} h; + +void foo (void) +{ + h.d = 0; + h.f = 0; +} + diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset8.c b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset8.c new file mode 100644 index 0000000..9070b53 --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset8.c @@ -0,0 +1,20 @@ +/* { dg-options "-mcode-readable=no -mno-gpopt -mabi=32 -mfp32 isa=1" } */ +/* { dg-final { scan-assembler "%lo\\(d\\+4\\)" } } */ +/* { dg-final { scan-assembler "\tsw\t\\\$\[0-9\],%lo\\(f\\)" } } */ +/* { dg-final { scan-assembler-not "\tsw\t\\\$\[0-9\],%lo\\(a_d\\)" } } */ +/* { dg-final { scan-assembler-not "\tsw\t\\\$\[0-9\],%lo\\(a_f\\)" } } */ + +double d; +double __attribute__((aligned(1))) a_d; + +float f; +float __attribute__((aligned(1))) a_f; + +void foo (void) +{ + d = 0; + f = 0; + a_d = 0; + a_f = 0; +} + diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset9.c b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset9.c new file mode 100644 index 0000000..ee11366 --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset9.c @@ -0,0 +1,16 @@ +/* { dg-options "-mcode-readable=no -mabi=32 -mfpxx isa=2" } */ +/* { dg-final { scan-assembler-not "%lo\\(h\\+\[1-9\]\\)" } } */ + +struct __attribute__((packed)) +{ + short s; + double d; + float f; +} h; + +void foo (void) +{ + h.d = 0; + h.f = 0; +} +