bitwiseworks / qtwebengine-chromium-os2

Port of Chromium and related tools to OS/2
9 stars 2 forks source link

Fix assembly to use 32-bit code and data segments #15

Closed dmik closed 4 years ago

dmik commented 4 years ago

Chromium uses YASM to compile its assembly code. On OS/2 we don't have a working YASM port and use it predecessor NASM. NASM seems to emit 16-bit code segments for OBJ format by default and this format doesn't have predefined section names (like .text and .bss in other formats). As a result, all assembly goes to 16-bit segments. From the WL map file the last three segments are 16-bit and they end up in two 16-bit objects in the DLL:

TEXT32                 CODE           CGROUP         0001:00000000   0832a750
________TEXT           CODE           CGROUP         0001:0832a750   00000000
DATA32                 DATA           DGROUP         0002:00000000   01fb60f8
...
text                                  AUTO           0003:00000000   00004b5d
rodata                                AUTO           0003:00004b60   00000100
__NASMDEFSEG                          AUTO           0004:00000000   00000024

This is essentially not correct because these segments are used from 32-bit code w/o any thunks etc so that everything breaks due to the word size mismatch (2 bytes vs 4 bytes). I had a really nice crash while working on https://github.com/bitwiseworks/qtwebengine-os2/issues/4:

______________________________________________________________________

 Exception C0000005 - Access Violation
______________________________________________________________________

 Process:  D:\CODING\QT5\QT5-DEV-BUILD\QTWEBENGINE\TESTS\AUTO\CORE\QWEBENGINECOOKIESTORE\DEBUG\TST_QWEBENGINECOOKIESTORE.EXE (08/09/2020 13:12:17 448,947)
 PID:      1E2 (482)
 TID:      0D (13)
 Priority: 200

 Address:  005B:17CA0008 (0004:00000008)
 Cause:    Attempted to read from 0000C1C4
           (not a valid address)
 Code:     failing instruction can not be disassembled

Only low 16-bits of the 32-bit register were used in some MOV nstruction, hence the weird address 0000C1C4. (The 16-bit code was generated from https://github.com/bitwiseworks/qtwebengine-chromium-os2/blob/master/chromium/third_party/blink/renderer/platform/heap/asm/SaveRegisters_x86.asm and used in garbage collection logic).

The fix is rather easy (see https://nasm.us/doc/nasmdoc8.html#section-8.4 that describes NASM directives specific to OBJ format). Use:

  segment TEXT32 CLASS=CODE USE32

for code and

  segment DATA32 CLASS=DATA USE32

for data.

Note that segment names (TEXT32 and DATA32) are actually irrelevant since the Watcom Linker will join them with its main 32-bit code and data segments anyway (due to the correct CLASS specification I suppose).

dryeo commented 4 years ago

On 08/09/20 11:31 AM, Dmitriy Kuminov wrote:

|segment TEXT32 CLASS=CODE USE32 |

for code and

|segment DATA32 CLASS=DATA USE32 |

for data.

Seems to me at one point we needed more like, |segment TEXT32 public CLASS=CODE USE32 class=CODE| |segment DATA32 public CLASS=DATA USE32 class=DATA| To prevent extra segments being created. Something to be checked in the map file. Might also need an align=16 after the TEXT32, DATA32 as well.

dmik commented 4 years ago

Almost the same as I came up with, good to know. Although PUBLIC IMHO is not needed (assumed by default?) as segments get combined w/o it pretty well. It can be checked in the map file (above is an excerpt from it BTW) and also seen in the object list of the binary file (DLL or EXE) with any LX header viewer (I use BIEW for quick looking). Re ALIGN, yes, might be needed sometimes.