Bug#764432: FTBFS on sparc, segfault in TestAlignment

[James Clarke]

Control: tags -1 patch

On Wed, Oct 08, 2014 at 03:10:57AM +0000, Mike Gabriel wrote:
> Package: freerdp
> Version: 1.1.0~git20140921.1.440916e+dfsg1-2
> Severity: important
>
> Hi Aurelien, hi other porters,
>
> The freerdp version 1.1.0~git20140921.1.440916e+dfsg1-2 still shows FTBFS
> symptoms on Sparc architecture.
>
> """
> [...]
>
> 99% tests passed, 1 tests failed out of 89
>
> Total Test time (real) =   3.07 sec
>
> The following tests FAILED:
> Errors while running CTest
> 	 16 - TestAlignment (SEGFAULT)
> make[1]: *** [test] Error 8
> Makefile:140: recipe for target 'test' failed
> make[1]: Leaving directory
> '/«BUILDDIR»/freerdp-1.1.0~git20140921.1.440916e+dfsg1/obj-sparc-linux-gnu'
> make: *** [debian/stamp-makefile-check] Error 2
> dpkg-buildpackage: error: debian/rules build-arch gave error exit status 2
> /usr/share/cdbs/1/class/makefile.mk:67: recipe for target
> 'debian/stamp-makefile-check' failed
> """
>
> Any help to dig this out is appreciated...
>
> Thanks,
> Mike
>
>
> --
>
> DAS-NETZWERKTEAM
> mike gabriel, herweg 7, 24357 fleckeby
> fon: +49 (1520) 1976 148
>
> GnuPG Key ID 0x25771B31
> mail: mike.gabriel at das-netzwerkteam.de, http://das-netzwerkteam.de
>
> freeBusy:
> https://mail.das-netzwerkteam.de/freebusy/m.gabriel%40das-netzwerkteam.de.xfb

I had a look at it today. It isn't actually segfaulting, it's getting a
bus error, but somewhere in the testing framework this gets treated the
same as a segfault. The problem lies because the _aligned_offset_malloc
code doesn't ensure that the *header* is aligned, and it then tries to
dereference an unaligned header pointer, if a non-zero offset is given
(well, if you give offset as a multiple of 8, that's fine, since it
won't break the alignment). The attached patch ensures that the header
is always pointer-size-aligned. In fact, since Debian uses gcc, it could
use __alignof__(struct _aligned_meminfo) instead of sizeof(void *), but
that's not portable.

Regards,
James
-------------- next part --------------
Description: Ensure the _aligned_meminfo pointer itself is sufficiently aligned
Author: James Clarke <jrtc27 at jrtc27.com>

--- a/winpr/libwinpr/crt/alignment.c
+++ b/winpr/libwinpr/crt/alignment.c
@@ -73,15 +73,20 @@ void* _aligned_offset_malloc(size_t size
 	if (alignment < sizeof(void*))
 		alignment = sizeof(void*);
 
-	/* malloc size + alignment to make sure we can align afterwards */
-	tmpptr = malloc(size + alignment + sizeof(struct _aligned_meminfo));
+	/* malloc size + alignment to make sure we can align afterwards.
+	 * Include an extra sizeof(void*) to ensure there's always space to align
+	 * ameminfo downwards, in case malloc doesn't align to sizeof(void*). This
+	 * could be dropped if there was a portable way to get alignof(struct
+	 * _aligned_meminfo), but instead we have to overestimate with
+	 * sizeof(void*). */
+	tmpptr = malloc(size + alignment + sizeof(struct _aligned_meminfo) + sizeof(void*));
 	if (!tmpptr)
 		return NULL;
 
 
-	memptr = (void *)((((size_t)((PBYTE)tmpptr + alignment + offset + sizeof(struct _aligned_meminfo)) & ~(alignment - 1)) - offset));
+	memptr = (void *)((((size_t)((PBYTE)tmpptr + alignment + offset + sizeof(struct _aligned_meminfo) + sizeof(void*)) & ~(alignment - 1)) - offset));
 
-	ameminfo = (struct _aligned_meminfo *) (((size_t)((PBYTE)memptr - sizeof(struct _aligned_meminfo))));
+	ameminfo = (struct _aligned_meminfo *) (((size_t)((PBYTE)memptr - sizeof(struct _aligned_meminfo))) & ~(sizeof(void*)-1));
 	ameminfo->base_addr = tmpptr;
 	ameminfo->size = size;
 
@@ -107,7 +112,7 @@ void* _aligned_offset_realloc(void* memb
 	if (!newmem)
 		return NULL;
 
-	ameminfo = (struct _aligned_meminfo *) (((size_t)((PBYTE)memblock - sizeof(struct _aligned_meminfo))));
+	ameminfo = (struct _aligned_meminfo *) (((size_t)((PBYTE)memblock - sizeof(struct _aligned_meminfo))) & ~(sizeof(void*)-1));
 	memcpy(newmem, memblock, ameminfo->size);
 	_aligned_free(memblock);
 	return newmem;
@@ -129,7 +134,7 @@ void _aligned_free(void* memblock)
 	if (!memblock)
 		return;
 
-	ameminfo = (struct _aligned_meminfo *) (((size_t)((PBYTE)memblock - sizeof(struct _aligned_meminfo))));
+	ameminfo = (struct _aligned_meminfo *) (((size_t)((PBYTE)memblock - sizeof(struct _aligned_meminfo))) & ~(sizeof(void*)-1));
 
 	free(ameminfo->base_addr);
 }