Bug#918376: nsis 3.03-2: FTBFS, alignment problem

[Thomas Gaugler]

Thanks for your detailed bug report.

I tried to reproduce the bug via the Disco Dingo Ubuntu/arm64 cloud 
image running inside the QEMU emulator [1]. I did not succeed in 
triggering the bug with this setup.


I concluded from your debug output that the icon group boundary is not 
aligned to the nearest multiple of a double word.


The size of the icon group is calculated as the following:
   size_t group_size = sizeof(IconGroupHeader) // header
 + order.size() * SIZEOF_RSRC_ICON_GROUP_ENTRY; // entries

sizeof(IconGroupHeader) = 6
SIZEOF_RSRC_ICON_GROUP_ENTRY = 14

group_size is a multiple of a double word for uneven numbers of 
order.size() but not if order.size() is an even number.


For illustration group_size for the order.size() values of 1 and 2:
group_size = 6 + 1 * 14 = 20 (double word aligned)
group_size = 6 + 2 * 14 = 34 (not double word aligned)

Therefore I expect something like the attached patch 
(fix-icongroup-alignment.patch) would be necessary. I need to verify 
with the
Microsoft Portable Executable and Common Object File Format 
Specification [2] and upstream.


[1] https://wiki.ubuntu.com/ARM64/QEMU
[2] http://www.microsoft.com/whdc/system/platform/firmware/PECOFF.mspx
Align icon group boundary to the nearest multiple of a double word.
--- a/Source/icon.cpp
+++ b/Source/icon.cpp
@@ -341,8 +341,12 @@
   // calculate size
   size_t group_size = sizeof(IconGroupHeader) // header
 + order.size() * SIZEOF_RSRC_ICON_GROUP_ENTRY; // entries
+  size_t padding = group_size % sizeof(DWORD);
+  if (padding > 0) {
+padding = sizeof(DWORD) - padding;
+  }
 
-  data_size = group_size // group header
+  data_size = group_size + padding // group header
 + sizeof(DWORD) * 2 // offset and size of group header
 + (sizeof(DWORD) * 2) * icon2.size() // offset and size per entry
 + sizeof(DWORD); // terminator
@@ -358,13 +362,16 @@
   LPBYTE seeker = uninst_data;
 
   // fill group header
-  *(LPDWORD) seeker = FIX_ENDIAN_INT32((UINT32)group_size);
+  *(LPDWORD) seeker = FIX_ENDIAN_INT32((UINT32)(group_size + padding));
   seeker += sizeof(DWORD);
   *(LPDWORD) seeker = 0;
   seeker += sizeof(DWORD);
 
   memcpy(seeker, group, group_size);
   seeker += group_size;
+  for (;padding > 0; padding--) {
+*(seeker++) = '\0';
+  }
 
   // fill entries
   for (i = 0; i < icon2.size(); i++)

Bug#918376: nsis 3.03-2: FTBFS, alignment problem

[Steve McIntyre]

Source: nsis
Version: 3.03-2
Severity: important
Tags: ftbfs
User: debian-...@lists.debian.org
Usertags: alignment

Hi!

I've been doing a full rebuild of the Debian archive, building all
source packages targeting armel and armhf using arm64 hardware. We are
planning in future to move all of our 32-bit armel/armhf builds to
using arm64 machines, so this rebuild is to identify packages that
might have problems with this configuration.

A feature of the arm64 kernel is that it does *not* support fixing up
code with broken alignment, so code that might have built and run OK
on our older armel/armhf build machines due to kernel fixups will now
fail.

When building your package, I've found a bus error (aka alignment
fault). The full log is online at

  
https://www.einval.com/debian/arm/rebuild-logs/armhf/FAIL/nsis_3.03-2_armhf.log

for reference

I've done a quick bit of debugging to find the source of the
bug. Here's a gdb stacktrace and variable printout to demonstrate the
problem.

(sid-armhf)steve@mjolnir:~/debian/build/nsis/nsis-3.03$ gdb 
build/test/usr/bin/makensis build/test/usr/share/doc/nsis/Examples/core
...
Reading symbols from build/test/usr/bin/makensis...done.
[New LWP 14637]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/arm-linux-gnueabihf/libthread_db.so.1".
Core was generated by 
`/home/steve/debian/build/nsis/nsis-3.03/build/test/usr/bin/makensis 
/home/steve'.
Program terminated with signal SIGBUS, Bus error.
#0  0x009ca7a8 in generate_uninstall_icon_data (icon1=..., icon2=..., 
data_size=@0xffc4cf28: 1102) at Source/icon.cpp:373
373 DWORD size = FIX_ENDIAN_INT32(icon->meta.dwRawSize);
(gdb) bt
#0  0x009ca7a8 in generate_uninstall_icon_data (icon1=std::vector of length 2, 
capacity 2 = {...}, icon2=std::vector of length 2, capacity 2 = {...}, 
data_size=@0xffc4cf28: 1102) at Source/icon.cpp:373
#1  0x009c4e36 in CEXEBuild::uninstall_generate (this=this@entry=0xffc38218) at 
/usr/include/c++/8/ext/new_allocator.h:99
#2  0x009c5610 in CEXEBuild::write_output (this=0xffc38218) at 
Source/build.cpp:2639
#3  0x009ce7d4 in makensismain (argc=3, argv=0x13e5058) at 
Source/makenssi.cpp:669
#4  0x009bb970 in wmain (argv=0x13e5058, argc=3) at Source/makenssi.cpp:695
#5  main (argc=3, argv=) at Source/makenssi.cpp:719
(gdb) list
368
369   // fill entries
370   for (i = 0; i < icon2.size(); i++)
371   {
372 Icon* icon = &icon2[order[i].index2];
373 DWORD size = FIX_ENDIAN_INT32(icon->meta.dwRawSize);
374
375 *(LPDWORD) seeker = FIX_ENDIAN_INT32(size);
376 seeker += sizeof(DWORD);
377 *(LPDWORD) seeker = 0;
(gdb) p i
$1 = 0
(gdb) p order[i]
$2 = {index1 = 1, index2 = 1, size = 744, size_index = 0}
(gdb) p *icon2
No symbol "operator*" in current context.
(gdb) p icon2
$3 = std::vector of length 2, capacity 2 = {{index = 0, meta = {bWidth = 16 
'\020', bHeight = 16 '\020', bPaletteEntries = 16 '\020', bReserved = 0 '\000', 
wPlanes = 0, 
  wBitsPerPixel = 0, dwRawSize = 296}, data = 0x1473530 "("}, {index = 1, 
meta = {bWidth = 32 ' ', bHeight = 32 ' ', bPaletteEntries = 16 '\020', 
bReserved = 0 '\000', 
  wPlanes = 0, wBitsPerPixel = 0, dwRawSize = 744}, data = 0x142b070 "("}}
(gdb) p order[i].index2
$4 = 1
(gdb) p icon2[1]
$5 = {index = 1, meta = {bWidth = 32 ' ', bHeight = 32 ' ', bPaletteEntries = 
16 '\020', bReserved = 0 '\000', wPlanes = 0, wBitsPerPixel = 0, dwRawSize = 
744}, 
  data = 0x142b070 "("}
(gdb) p &icon2[1]
$6 = (Icon *) 0x1496c1c
(gdb) p &icon2[1].meta
$7 = (IconGroupEntry *) 0x1496c20
(gdb) p &icon2[1].meta.dwRawSize
$8 = (DWORD *) 0x1496c28
(gdb) p seeker
$9 = (LPBYTE) 0x1494bfa ""
(gdb) p *seeker
$10 = 0 '\000'

I think gdb is maybe mis-identifying the exact source line for the
crash. Line 373 looks OK, with all the reading aligned correctly.
However, in line 375 the pointer "seeker" is clearly not safely
aligned for storing a DWORD.

-- System Information:
Debian Release: 9.6
  APT prefers stable-updates
  APT policy: (500, 'stable-updates'), (500, 'stable-debug'), (500, 'stable')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 4.9.0-8-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8), 
LANGUAGE=en_GB.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)