This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] libgcc/crtstuff.c: Fix incorrect alignment of entries in CRT data structures (PR target/91306)


On 8/21/19 7:34 AM, Jozef Lawrynowicz wrote:
> As described in PR target/91306, the alignment of entries in CRT data structures
> as set in libgcc/crtstuff.c can be too large for msp430-elf.
> 
> crtstuff.c assumes the correct alignment of entries with a type of (void *) or
> int32 is the "sizeof" that type.
> 
> However, for msp430-elf in the large memory model, pointer size is 4 bytes, but
> they only need to be 2-byte aligned. Likewise for int32.
> 
> This is causing problems for __frame_dummy_init_array_entry, which gets 
> inserted into .init_array.
> It is being forced into a 4 byte alignment, which can result in a gap between
> __frame_dummy_init_array_entry and the previous entry (or the start of the
> table). So when the startup code comes to run through the entries
> in .init_array, it interprets 2-bytes of padding 0s as part of a function
> address, resulting in a function call to the incorrect location.
> 
> This issue has only recently appeared because msp430-elf was just transitioned
> to use init/fini_array by default, as mandated by the mspabi.
> 
> The other CRT array entries in crtstuff.c are not used for msp430-elf, so aren't
> causing problems.
> 
> As suggested in the PR, I tried changing the alignment of the array entry from
> sizeof(func_ptr) to __alignof__(func_ptr), and this fixed the issue. In the
> attached patch I also updated the other uses of sizeof in "aligned" attributes
> to use __alignof__ instead.
> 
> I understand the alignment to sizeof(func_ptr) was originally added to fix an
> issue on mips64 (r182066
> https://gcc.gnu.org/ml/gcc-patches/2011-12/msg00393.html), however I do
> not have access to a mips64 platform for which to bootstrap on. I tried building
> a cross-compiler with the "aligned" attributes removed to see if I could
> reproduce the failure, but the build completed successfully.
> I built the mips64 cross-compiler with my patch applied and it
> also completed successfully.
> 
> I successfully regtested the patch for msp430-elf. It fixes many execution
> failures for msp430-elf/-mlarge.
> I also successfully bootstrapped and regtested x86_64-pc-linux-gnu (g++ gcc
> gfortran libatomic libgomp libitm libstdc++ objc).
> 
> According to the mips eabi (is this the right one?
> http://www.cygwin.com/ml/binutils/2003-06/msg00436.html), pointers have 64-bit
> size and alignment in 64-bit mode, so I am assuming using __alignof__ instead of
> sizeof isn't going to cause problems.
> 
> Ok for trunk?
> 
> 
> 0001-libgcc-crtstuff.c-Fix-incorrect-alignment-of-entries.patch
> 
> From 636ffa0c4f15047dc27c829d9a3c8fea9ad5d635 Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
> Date: Thu, 15 Aug 2019 14:17:25 +0100
> Subject: [PATCH] libgcc/crtstuff.c: Fix incorrect alignment of entries in CRT
>  data structures
> 
> libgcc/ChangeLog:
> 
> 2019-08-21  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> 
> 	* crtstuff.c (__CTOR_LIST__): Align to the "__alignof__" the array
> 	element type, instead of "sizeof" the element type.
> 	(__DTOR_LIST__): Likewise.
> 	(__TMC_LIST__): Likewise.
> 	(__do_global_dtors_aux_fini_array_entry): Likewise.
> 	(__frame_dummy_init_array_entry): Likewise.
> 	(__CTOR_END__): Likewise.
> 	(__DTOR_END__): Likweise.
> 	(__FRAME_END__): Likewise.
> 	(__TMC_END__): Likewise.
OK
jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]