BUGS: get_lasterrors and set_lasterrors

[ladislav-zezula]

1. The intrinsic function __readeflags() used in get_lasterrors() returns 64-bit value on 64-bit build. It is necessary to change lasterror_t::Eflags to DWORD_PTR

2. What is the meaning of the condition at line 372? It causes the problem described below

3. The code created by conditional __writeeflags() at line 372 in set_lasterrors seems to invoke error in compiler.

How to reproduce the compiler error:

1. Build capemon in Visual Studio 2019, Platform Toolset 142, Release version X64 (141 builds it just fine).
2. Set breakpoint to New_NtOpenFile
3. Run a sample until it stops in New_NtOpenFile:

.text:00000001800A5280                 sub     rsp, 70h
.text:00000001800A5284                 mov     rsi, r9         ; RSI = IoStatusBlock
.text:00000001800A5287                 mov     rbx, r8         ; RBX = ObjectAttributes
.text:00000001800A528A                 mov     ebp, edx        ; EBP = DesiredAccess
.text:00000001800A528C                 mov     rdi, rcx        ; RDI = Pointer to FileHandle
.text:00000001800A528F                 int     3               ; Trap to Debugger (I added this here to stop in debugger)
.text:00000001800A5290                 mov     rcx, r8         ; obj
.text:00000001800A5293                 call    check_for_logging_resumption ; This call doesn't preserve RBX properly
.text:00000001800A5298                 mov     rcx, rbx        ; obj
.text:00000001800A529B                 call    is_protected_objattr ; This call crashes
.text:00000001800A52A0                 test    al, al
.text:00000001800A52A2                 jz      short loc_1800A52AE
.text:00000001800A52A4                 mov     eax, 0C0000022h
.text:00000001800A52A9                 jmp     loc_1800A53B7

4. Step over the check_for_logging_resumption() function
5. Notice that RBX is lost during the step oper operation.

I experimented a bit. The V142 platform toolset makes functions get_lasterrors and set_lasterrors inline. As consequence, there are "pushfq - pop" constructs (created by __readeflags() and __writeeflags()) all over the place. This operation causes RSP to be copied into R11 at the beginning of check_for_logging_resumption. RBX is stored to stack cell referenced by R11+imm, but it's restored from different stack cell referenced by RSP+imm.

TLDR: I found that preventing both get_lasterrors and set_lasterrors from being inlined (via void declspec (noinline) get_lasterrors(lasterror_t *errors) will make the bug go away.

[kevoreilly]

Thank you for pointing out these issues! It is much appreciated.

Issue 1 fixed in be0d4d5

Regarding the VS2019 compiler error, as far as I can tell this is not an issue with VS2017. Therefore, this is duly noted and will be useful if and when it's decided to move to VS2019.

[ladislav-zezula]

Regarding the VS2019 compiler error, as far as I can tell this is not an issue with VS201

I can confirm this. I also noted it in the description ("141 builds it just fine"). At the moment, I am trying to make PoC of bad compiler behavior in order to send it to the compiler guys, hoping they will fix it.

[michaelweiser]

I am also using VS2019 and had run into an initial issue of Winword not starting up on either Windows 7 or Windows 10 with my builds of capemon_x64.dll while working with Kev's (even before they run into #12 on Win 10). I was able to work around that by disabling whole program optimization in the bson and capemon projects. I can confirm that preventing {g,s}et_lasterrors from being inlined using __declspec(noinline) solves the same issue. This might be a hint at what inlining defaults have changed from v141 to v142. My workaround as diff:

diff --git a/bson/bson.vcxproj b/bson/bson.vcxproj
index b841018..627d2c9 100644
--- a/bson/bson.vcxproj
+++ b/bson/bson.vcxproj
@@ -40,7 +40,7 @@
     <ConfigurationType>StaticLibrary</ConfigurationType>
     <UseDebugLibraries>false</UseDebugLibraries>
     <CharacterSet>MultiByte</CharacterSet>
-    <WholeProgramOptimization>true</WholeProgramOptimization>
+    <WholeProgramOptimization>false</WholeProgramOptimization>
     <PlatformToolset>v142</PlatformToolset>
   </PropertyGroup>
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'" Label="Configuration">
@@ -48,7 +48,7 @@
     <UseDebugLibraries>false</UseDebugLibraries>
     <PlatformToolset>v142</PlatformToolset>
     <CharacterSet>MultiByte</CharacterSet>
-    <WholeProgramOptimization>true</WholeProgramOptimization>
+    <WholeProgramOptimization>false</WholeProgramOptimization>
   </PropertyGroup>
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.props" />
   <ImportGroup Label="ExtensionSettings">
diff --git a/capemon.vcxproj b/capemon.vcxproj
index 91dcb0c..188ec6e 100644
--- a/capemon.vcxproj
+++ b/capemon.vcxproj
@@ -43,7 +43,7 @@
     <ConfigurationType>DynamicLibrary</ConfigurationType>
     <UseDebugLibraries>false</UseDebugLibraries>
     <CharacterSet>MultiByte</CharacterSet>
-    <WholeProgramOptimization>true</WholeProgramOptimization>
+    <WholeProgramOptimization>false</WholeProgramOptimization>
     <UseOfMfc>false</UseOfMfc>
     <PlatformToolset>v142</PlatformToolset>
   </PropertyGroup>
@@ -51,7 +51,7 @@
     <ConfigurationType>DynamicLibrary</ConfigurationType>
     <UseDebugLibraries>false</UseDebugLibraries>
     <CharacterSet>NotSet</CharacterSet>
-    <WholeProgramOptimization>true</WholeProgramOptimization>
+    <WholeProgramOptimization>false</WholeProgramOptimization>
     <UseOfMfc>false</UseOfMfc>
     <PlatformToolset>v142</PlatformToolset>
   </PropertyGroup>

[ladislav-zezula]

Thanks Michael. I made a minimalistic proof-of-concept of the compiler bug and trying to reach Microsoft people.
Feel free to look: CompilerError_Toolset142.zip

Yes, I did the same observation - as soon as you remove inlining on the (s|g)et_lasterrors, it goes away. It also goes away if you remove the condition at line 372.