From patchwork Thu Nov 17 19:15:51 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Maciej W. Rozycki" X-Patchwork-Id: 17550 Received: (qmail 71857 invoked by alias); 17 Nov 2016 19:16:18 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 71841 invoked by uid 89); 17 Nov 2016 19:16:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.9 required=5.0 tests=AWL, BAYES_00, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=Meanwhile, diagnosing, $25, 7.1 X-HELO: mailapp01.imgtec.com Date: Thu, 17 Nov 2016 19:15:51 +0000 From: "Maciej W. Rozycki" To: CC: "Joseph S. Myers" , Matthew Fortune Subject: [PATCH] MIPS: Add `.insn' to ensure a text label is defined as code not data Message-ID: User-Agent: Alpine 2.20.17 (DEB 179 2016-10-28) MIME-Version: 1.0 Avoid a build error with microMIPS compilation and recent versions of GAS which complain if a branch targets a label which is marked as data rather than microMIPS code: ../sysdeps/mips/mips32/crti.S: Assembler messages: ../sysdeps/mips/mips32/crti.S:72: Error: branch to a symbol in another ISA mode make[2]: *** [.../csu/crti.o] Error 1 as commit 9d862524f6ae ("MIPS: Verify the ISA mode and alignment of branch and jump targets") closed a hole in branch processing, making relocation calculation respect the ISA mode of the symbol referred. This allowed diagnosing the situation where an attempt is made to pass control from code assembled for one ISA mode to code assembled for a different ISA mode and either relaxing the branch to a cross-mode jump or if that is not possible, then reporting this as an error rather than letting such code build and then fail unpredictably at the run time. This however requires the correct annotation of branch targets as code, because the ISA mode is not relevant for data symbols and is therefore not recorded for them. The `.insn' pseudo-op is used for this purpose and has been supported by GAS since: Wed Feb 12 14:36:29 1997 Ian Lance Taylor * config/tc-mips.c (mips_pseudo_table): Add "insn". (s_insn): New static function. * doc/c-mips.texi: Document .insn. so there has been no reason to avoid it where required. More recently this pseudo-op has been documented, by the microMIPS architecture specification[1][2], as required for the correct interpretation of any code label which is not followed by an actual instruction in an assembly source. Use it in our crti.S files then, to mark that the trailing label there with no instructions following is indeed not a code bug and the branch is legitimate. References: [1] "MIPS Architecture for Programmers, Volume II-B: The microMIPS32 Instruction Set", MIPS Technologies, Inc., Document Number: MD00582, Revision 5.04, January 15, 2014, Section 7.1 "Assembly-Level Compatibility", p. 533 [2] "MIPS Architecture for Programmers, Volume II-B: The microMIPS64 Instruction Set", MIPS Technologies, Inc., Document Number: MD00594, Revision 5.04, January 15, 2014, Section 8.1 "Assembly-Level Compatibility", p. 623 2016-11-17 Matthew Fortune Maciej W. Rozycki * sysdeps/mips/mips32/crti.S (_init): Add `.insn' pseudo-op at `.Lno_weak_fn' label. * sysdeps/mips/mips64/n32/crti.S (_init): Likewise. * sysdeps/mips/mips64/n64/crti.S (_init): Likewise. --- This should qualify as obvious, however for good measure I pushed it through full regression testing with the `mips-mti-linux-gnu' target and big-endian regular MIPS o32 and n64 multilibs as well as little-endian o32 MIPS16 and microMIPS multilibs, all with binutils 2.27 predating the extra branch/jump checks (or otherwise glibc wouldn't build without this patch for the microMIPS multilib). While working on this change I've noticed the R_MIPS_JALR regular MIPS relocations, used unconditionally even with microMIPS compilations where R_MICROMIPS_JALR relocations (if any) need to be used instead. Support for R_MICROMIPS_JALR handling is incomplete in the BFD linker meaning these relocs are always ignored, however this may change sometime and we ought to have correct code, so I'll send a separate change to correct it, once it has passed regression testing with a similar set of multilibs. Meanwhile OK to apply this change? Maciej glibc-umips-insn.diff Index: glibc/sysdeps/mips/mips32/crti.S =================================================================== --- glibc.orig/sysdeps/mips/mips32/crti.S 2016-11-17 12:38:44.330014101 +0000 +++ glibc/sysdeps/mips/mips32/crti.S 2016-11-17 15:28:32.203511190 +0000 @@ -74,6 +74,7 @@ .reloc 1f,R_MIPS_JALR,PREINIT_FUNCTION 1: jalr $25 .Lno_weak_fn: + .insn #else lw $25,%got(PREINIT_FUNCTION)($28) .reloc 1f,R_MIPS_JALR,PREINIT_FUNCTION Index: glibc/sysdeps/mips/mips64/n32/crti.S =================================================================== --- glibc.orig/sysdeps/mips/mips64/n32/crti.S 2016-11-17 15:30:23.000000000 +0000 +++ glibc/sysdeps/mips/mips64/n32/crti.S 2016-11-17 15:30:48.510029889 +0000 @@ -74,6 +74,7 @@ .reloc 1f,R_MIPS_JALR,PREINIT_FUNCTION 1: jalr $25 .Lno_weak_fn: + .insn #else lw $25,%got_disp(PREINIT_FUNCTION)($28) .reloc 1f,R_MIPS_JALR,PREINIT_FUNCTION Index: glibc/sysdeps/mips/mips64/n64/crti.S =================================================================== --- glibc.orig/sysdeps/mips/mips64/n64/crti.S 2016-11-17 15:30:30.000000000 +0000 +++ glibc/sysdeps/mips/mips64/n64/crti.S 2016-11-17 15:30:56.644097664 +0000 @@ -74,6 +74,7 @@ .reloc 1f,R_MIPS_JALR,PREINIT_FUNCTION 1: jalr $25 .Lno_weak_fn: + .insn #else ld $25,%got_disp(PREINIT_FUNCTION)($28) .reloc 1f,R_MIPS_JALR,PREINIT_FUNCTION