Merge pull request #5246 from bclothier/MiscBugFixes

Prevent crashes on 64-bit VBA using COM projects

Author: retailcoder

Rubberduck.CodeAnalysis/Inspections/Concrete/ImplicitDefaultMemberAccessInspection.cs

@@ -45,6 +45,7 @@ namespace Rubberduck.Inspections.Concrete
     ///     arg.ConnectionString = bar
     /// End Sub
     /// ]]>
+    /// </example>
     public sealed class ImplicitDefaultMemberAccessInspection : IdentifierReferenceInspectionBase
     {
         public ImplicitDefaultMemberAccessInspection(RubberduckParserState state)

Rubberduck.CodeAnalysis/Inspections/Concrete/ImplicitUnboundDefaultMemberAccessInspection.cs

@@ -24,6 +24,7 @@ namespace Rubberduck.Inspections.Concrete
     ///     bar = arg
     /// End Sub
     /// ]]>
+    /// </example>
     /// <example hasresult="true">
     /// <![CDATA[
     /// Public Sub DoSomething(ByVal arg As Object)
@@ -39,6 +40,7 @@ namespace Rubberduck.Inspections.Concrete
     ///     bar = arg.SomeValueReturningMember
     /// End Sub
     /// ]]>
+    /// </example>
     /// <example hasresult="false">
     /// <![CDATA[
     /// Public Sub DoSomething(ByVal arg As Object)

Rubberduck.JunkDrawer/Hacks/VarDescExtensions.cs

@@ -0,0 +1,28 @@
+using System.Runtime.InteropServices.ComTypes;
+
+namespace Rubberduck.JunkDrawer.Hacks
+{
+    public static class VarDescExtensions
+    {
+        /// <remarks>
+        /// Use only with VBA-supplied <see cref="ITypeInfo"/> which may return a <see cref="VARDESC"/> that do not conform to 
+        /// the MS-OAUT in describing the constants. See section 2.2.43 at: https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-oaut/ae7791d2-4399-4dff-b7c6-b0d4f3dce982
+        /// 
+        /// To further complicate the situation, on 64-bit platform, the <see cref="VARDESC.DESCUNION.oInst"/> is a 32-bit integer whereas
+        /// the <see cref="VARDESC.DESCUNION.lpvarValue"/> is a pointer. On 32-bit platform, the sizes of 2 members are exactly same so no
+        /// problem. But on 64-bit platform, setting the <c>oInst</c>to 0 does not necessarily zero-initialize the entire region. Thus, the 
+        /// upper 32-bit part of the <c>lpvarValue</c> can contain garbage which will confound the simple null pointer check. Thus to guard 
+        /// against this, we will check the <c>oInst</c> value to see if it's zero. 
+        /// 
+        /// There is a small but non-zero chance that there might be a valid pointer that happens to be only in high half of the address...
+        /// in that case, it'll be wrong but since VBA is always writing <see cref="VARKIND.VAR_STATIC"/> to the <see cref="VARDESC.varkind"/>
+        /// field, we're kind of stuck...
+        /// </remarks>
+        /// <param name="varDesc">The <see cref="VARDESC"/> from a VBA <see cref="ITypeInfo"/></param>
+        /// <returns>True if this is most likely a constant. False when it's definitely not.</returns>
+        public static bool IsValidVBAConstant(this VARDESC varDesc)
+        {
+            return varDesc.varkind == VARKIND.VAR_STATIC && varDesc.desc.oInst != 0;
+        }
+    }
+}

Rubberduck.Parsing/ComReflection/ComModule.cs

@@ -11,6 +11,8 @@
 using TYPEATTR = System.Runtime.InteropServices.ComTypes.TYPEATTR;
 using VARDESC = System.Runtime.InteropServices.ComTypes.VARDESC;
 using CALLCONV = System.Runtime.InteropServices.ComTypes.CALLCONV;
+using Rubberduck.VBEditor.ComManagement.TypeLibs.Abstract;
+using Rubberduck.JunkDrawer.Hacks;
 
 namespace Rubberduck.Parsing.ComReflection
 {
@@ -58,7 +60,15 @@ private void GetComFields(ITypeInfo info, TYPEATTR attrib)
                     info.GetNames(desc.memid, names, names.Length, out int length);
                     Debug.Assert(length == 1);
 
-                    var type = desc.desc.lpvarValue == IntPtr.Zero ? DeclarationType.Variable : DeclarationType.Constant;
+                    DeclarationType type;
+                    if(info is ITypeInfoWrapper wrapped && wrapped.HasVBEExtensions)
+                    {
+                        type = desc.IsValidVBAConstant() ? DeclarationType.Constant : DeclarationType.Variable;
+                    }
+                    else
+                    {
+                        type = desc.varkind == VARKIND.VAR_CONST ? DeclarationType.Constant : DeclarationType.Variable;
+                    }
 
                     _fields.Add(new ComField(this, info, names[0], desc, index, type));
                 }

Rubberduck.VBEEditor/ComManagement/TypeLibs/TypeInfoConstantsCollection.cs

@@ -1,6 +1,6 @@
-using System;
-using System.Collections.Generic;
+using System.Collections.Generic;
 using System.Runtime.InteropServices.ComTypes;
+using Rubberduck.JunkDrawer.Hacks;
 using Rubberduck.VBEditor.ComManagement.TypeLibs.Abstract;
 using Rubberduck.VBEditor.ComManagement.TypeLibs.Unmanaged;
 using TYPEATTR = System.Runtime.InteropServices.ComTypes.TYPEATTR;
@@ -45,9 +45,8 @@ public TypeInfoConstantsCollection(ITypeInfo parent, TYPEATTR attributes) :
                 parent.GetVarDesc(i, out var ppVarDesc);
                 var varDesc = StructHelper.ReadStructureUnsafe<VARDESC>(ppVarDesc);
 
-                // VBA constants are "static".... go figure. We can still infer it is a 
-                // constant rather than a field by checking the lpvarValue
-                if (varDesc.varkind == VARKIND.VAR_STATIC && varDesc.desc.lpvarValue != IntPtr.Zero)
+                // VBA constants are "static".... go figure.
+                if (varDesc.IsValidVBAConstant())
                 {
                     _mapper.Add(_mapper.Count, i);
                 }

Rubberduck.VBEEditor/Rubberduck.VBEditor.csproj

@@ -22,6 +22,7 @@
     <Reference Include="System.Windows.Forms" />
   </ItemGroup>
   <ItemGroup>
+    <ProjectReference Include="..\Rubberduck.JunkDrawer\Rubberduck.JunkDrawer.csproj" />
     <ProjectReference Include="..\Rubberduck.Resources\Rubberduck.Resources.csproj" />
   </ItemGroup>
   <ItemGroup>