Use /safeseh to compile x86 MASM files

[mikedn]

Fixes #2245

I haven't actually removed /SAFESEH:NO yet as that may conflict with #2170. If it turns out that ARM64 doesn't need /SAFESEH:NO then it can be entirely removed in that PR without breaking the x86 build.

[jkotas]

Thank you for jumping on it!

ARM64 doesn't need /SAFESEH:NO

I am pretty sure it should not be need it. You can remove it now.

Could you please also add src\dlls\mscoree\i386\handlers.asm to the build? It may be nice to move this file to vm\i386 as part of doing this - it would be more logical place for it, and it would save us from replicating asm building boiler plate code in yet another CMake file.

handlers.asm is necessary to tell the OS about the fs:0 handlers used by the VM. The exception handling would be broken without it. The OS would treat these fs:0 handlers as malicious (ie introduced by a hack - stack buffer overrun) and kill the process immediately whenever it would see one of them.

[mikedn]

I am pretty sure it should not be need it. You can remove it now.

Done.

Could you please also add src\dlls\mscoree\i386\handlers.asm to the build? It may be nice to move this file to vm\i386 as part of doing this - it would be more logical place for it, and it would save us from replicating asm building boiler plate code in yet another CMake file.

I tried to move handlers.asm (and included it in makefiles) but it doesn't work. It builds fine but those functions don't show up in the SEH table of coreclr.dll. They show up if I copy the .safeseh directives to an existing file - asmhelpers.asm for example. I suspect that this happens because handlers.obj doesn't define any symbols and the linker simply ignores it (there's probably a LNK4221 somewhere but I didn't look for it, too many of them).

IMO moving those .safeseh directives to asmhelpers.asm is preferable to keeping the file in the wrong place and duplicating the CMake ASM goo in mscoree. Opinions?

[jkotas]

moving those .safeseh directives to asmhelpers.asm

Sounds good to me.

[mikedn]

Done. Here's an excerpt from dumpbin /loadconfig coreclr.dll output:

    Safe Exception Handler Table

          Address
          --------
          10205360  _COMPlusFrameHandler
          102056E0  _COMPlusFrameHandlerRevCom
          10205730  _COMPlusNestedExceptionHandler
          102059A0  _FastNExportExceptHandler
          10205A00  _UMThunkPrestubHandler
          10CA0BDD  __except_handler4

[benpye]

607978a has a commit message that indicates the inverse of the changes made.

[mikedn]

607978a has a commit message that indicates the inverse of the changes made.

Thanks, the message is indeed wrong. This will likely get squashed so it will disappear, waiting to see if @jkotas is ok with the deletion in the last commit.

[mikedn]

Deleted. I take it that mirroring is smart enough to deal with this :)

[jkotas]

This is part of the file that has just macros and forward declarations that do not generate any code. It would look better to have it later in the file - e.g. before CallRtlUnwind may be a good place.

[jkotas]

LGTM otherwise. Could you please squash it into fewer commits so it is ready to merge?