rubberduck-vba / Rubberduck

Every programmer needs a rubberduck. COM add-in for the VBA & VB6 IDE (VBE).
https://rubberduckvba.com
GNU General Public License v3.0
1.91k stars 299 forks source link

MS Access : Crash upon Inspecting #3883

Closed FuSoftware closed 6 years ago

FuSoftware commented 6 years ago

As discussed in the chat, one of my ACCDB files (and its sibling files with only a few differences) consistently crash when performing the RD scan. Here is the trace log if you can find what's wrong.

I haven't been able to reproduce the issue, starting with a new ACCDB so far

RubberduckLog.txt

FuSoftware commented 6 years ago

After deleting a few forms the problem got moved

RubberduckLog.txt

FuSoftware commented 6 years ago

In my file, deleting all forms+modules but this makes the error still happen (deleting this form makes the parsing work).

I created a full new ACCDB file, put only the form in it, and RD still crashes upon Inspecting. So that form code is the culprit it seems.

Form.txt

FuSoftware commented 6 years ago

Narrowed down to that specific function. I still have to try with a previous version of RD to see if it's linked to the latest merge regarding Select Case.

Installing the 3021 and starting RD, the code successfully parses, so it definitely is linked to the 3038 update

Function.txt RubberduckLog.txt

Vogel612 commented 6 years ago

Copying over the function for other people to use:

Option Compare Database
Option Explicit

Sub AddVariable(stName As String, stValue As String, wdoc As Object)
    On Error Resume Next
    wdoc.Variables.Add Name:=stName, Value:=stValue
    Select Case err.Number
        Case "5903"
            wdoc.Variables(stName).Value = stValue
        Case Else
            Exit Sub
    End Select
End Sub
Vogel612 commented 6 years ago

Norepro on MSAccess 2016 from the branch of my PR. I'll attempt to repro from next

FuSoftware commented 6 years ago

Most likely has to do with the recent Select Case changes. 3021 works fine, but 3038 and 3039 both crash upon inspecting

Vogel612 commented 6 years ago

I don't see that behavior... then again I don't see the Select Case inspection in my compiled binary .... lemme dig a bit deeper...

FuSoftware commented 6 years ago

I'm currently re-installing RD 3039 to try to make a proper sample file

FuSoftware commented 6 years ago

Well, I can't reproduce the bug on 3039 here, idk why. The only difference is that it is an Access 2013 64 bits, compared to the computer I had earlier, which had Access 2016 32 bits

Vogel612 commented 6 years ago

Okay, just repro'd this. We have a StackOverflowException on the following line in Declaration:

public bool AsTypeIsBaseType => string.IsNullOrWhiteSpace(AsTypeName) || SymbolList.BaseTypes.Contains(AsTypeName.ToUpperInvariant());

the problem seems to be that the BaseType list does not include all basetypes or that resolution thereof is incorrect. The source of the overflow lies within the newly introduced ParseTreeValueVisitor's GetBaseTypeForDeclaration method:

        private static string GetBaseTypeForDeclaration(Declaration declaration)
        {
            if (!declaration.AsTypeIsBaseType)
            {
                return GetBaseTypeForDeclaration(declaration.AsTypeDeclaration);
            }
            return declaration.AsTypeName;
        }
retailcoder commented 6 years ago

@Vogel612 can you repro in a unit test? (might be hard, IIRC ErrObject is hacked-up with a custom type loader)

Vogel612 commented 6 years ago

Current Suspect: ErrObject. We may have needed to hack that one in to get the resolver to work with it correctly. But as of now I can only see it in the serialized declarations for ADODB 6.1 😕

We may need to make the resolver a bit smarter...

BZngr commented 6 years ago

I've got a resolution nearly ready. The Recursive call to private static string GetBaseTypeForDeclaration(Declaration declaration) is where the problem starts, but there is a bit more to resolve as well.