[PATCH] D64200: [ELF] Allow placing non-string SHF_MERGE sections with different alignments into the same MergeSyntheticSection

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

In D64200#1570169 <https://reviews.llvm.org/D64200#1570169>, @MaskRay wrote:

> > That would (1) waste space and (2) create unaligned sections when tail merge (-O2) is enabled. MOVAPS on such unaligned strings can raise SIGSEGV.
>
> I haven't figured out the root cause of (2) yet.
>
> (1) justifies a change, otherwise we will get something like `a.......b.......c.......`, which wastes space.


I hadn't realised that the change would also affect SHF_STRINGS sections as well, IIRC sh_entsize means the size of the character so it can make sense for the alignment to be higher than the sh_entsize. This is bringing back memories. Many years ago we had a similar problem mixing objects from Arm's proprietary compiler and arm-none-eabi-gcc. The ARM compiler only used SHF_STRINGS with sh_align 1 with the code using byte by byte accesses to those strings. In arm-none-eabi-gcc the SHF_STRINGS has sh_align 4 and copied strings using words instead of bytes. Within the SHF_STRINGS section the strings were padded to a 4-byte boundary. On the ARM processors of the time didn't support unaligned accesses so when the strings were merged without taking into account alignment padding the resulting binary crashed.

In that particular case we permitted strings from an equal to or lower alignment to be merged, but not a higher to a lower alignment. Within that higher aligned string section we needed to maintain alignment padding to ensure each string started at an aligned boundary. This might be more difficult to do in LLD.

I think for now excluding SHF_STRINGS from different alignment merging is the right thing to do. If we want to enable it we'll need to ensure each string starts on a correctly aligned boundary to ensure correctness.


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64200/new/

https://reviews.llvm.org/D64200