[wix-devs] Debugging WIX4/Burn Unit tests

[Hoover, Jacob]

On to the next failing test…

DUtil, IniUtil, x64 mode fails in IniParse for the “setf ~PlainValue Blah…” test.

Looking at the code, it seems inside the for loop, we don’t reset the pointer for wzValueNameStart, and as such, it causes the use of a pointer that’s been free’d.

Adding the NULL allows this to succeed.

        wzValueNameStart = NULL;

        if (pi->sczValuePrefix)
        {
            wzValuePrefix = wcsstr(pi->rgsczLines[i], pi->sczValuePrefix);
            if (wzValuePrefix != NULL)
            {
               wzValueNameStart = wzValuePrefix + dwValuePrefixLength;
            }
        }
        else
        {
            wzValueNameStart = pi->rgsczLines[i];
        }



DUtil, Path2Utl, PathDirectoryContainsPath

hr = PathDirectoryContainsPath(L"C:\\Directory", L"C:\\");


if (CSTR_EQUAL != ::CompareStringW(LOCALE_NEUTRAL, NORM_IGNORECASE, sczCanonicalizedDirectory, (DWORD)cchDirectory, sczCanonicalizedPath, (DWORD)cchDirectory))

we shouldn’t be passing the same cchDirectory, the second one should indicate the length of the second string, right? (https://learn.microsoft.com/en-us/windows/win32/api/stringapiset/nf-stringapiset-comparestringw)


From: wix-devs <wix-devs-bounces at lists.wixtoolset.org> On Behalf Of Sean Hall via wix-devs
Sent: Monday, October 31, 2022 9:38 PM
To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org>
Cc: Sean Hall <r.sean.hall at gmail.com>
Subject: Re: [wix-devs] Debugging WIX4/Burn Unit tests

I think you're right. I think the test should zero out the memory first. CI
only builds in Release mode.

On Mon, Oct 31, 2022 at 9:21 PM Hoover, Jacob <Jacob.Hoover at greenheck.com<mailto:Jacob.Hoover at greenheck.com>>
wrote:

> I am beginning to think this test is invalid…
>
>
>
> void VariantBasicTest()
>
> {
>
> BURN_VARIANT expectedVariants[10];
>
> BURN_VARIANT actualVariants[10];
>
> for (DWORD i = 0; i < 10; i++)
>
> {
>
> BVariantUninitialize(expectedVariants + i);
>
> BVariantUninitialize(actualVariants + i);
>
> }
>
> …
>
>
>
> typedef struct _BURN_VARIANT
>
> {
>
> union
>
> {
>
> LONGLONG llValue;
>
> VERUTIL_VERSION* pValue;
>
> LPWSTR sczValue;
>
> };
>
> BURN_VARIANT_TYPE Type;
>
> } BURN_VARIANT;
>
>
>
>
>
> Since it’s an array of BURN_VARIANT, what ensures the memory will be zero
> allocated?
>
>
>
> I ask because using GFlags, going heap paranoid, and it only chokes on
> this test. (In theory I’ve changed nothing that should cause this.)
>
>
>
>
>
> In other words, should we add
>
>
>
> memset(expectedVariants, 0, sizeof(expectedVariants));
>
> memset(actualVariants, 0, sizeof(actualVariants));
>
>
>
> after we allocate them, or just not call
>
>
>
> for (DWORD i = 0; i < 10; i++)
>
> {
>
> BVariantUninitialize(expectedVariants + i);
>
> BVariantUninitialize(actualVariants + i);
>
> }
>
>
>
>
>
> Does CI/CD build and run both debug and release mode?
>
>
>
> *From:* wix-devs <wix-devs-bounces at lists.wixtoolset.org<mailto:wix-devs-bounces at lists.wixtoolset.org>> *On Behalf Of *Sean
> Hall via wix-devs
> *Sent:* Monday, October 31, 2022 4:33 PM
> *To:* WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org<mailto:wix-devs at lists.wixtoolset.org>>
> *Cc:* Sean Hall <r.sean.hall at gmail.com<mailto:r.sean.hall at gmail.com>>
> *Subject:* Re: [wix-devs] Debugging WIX4/Burn Unit tests
>
>
>
> Yes. It builds and runs them in x86 and x64.
>
> On Mon, Oct 31, 2022 at 4:24 PM Hoover, Jacob via wix-devs <
> wix-devs at lists.wixtoolset.org<mailto:wix-devs at lists.wixtoolset.org>> wrote:
>
> > Trying to test my changes, and I've run into an issue where the unit test
> > host process is crashing. Note this only happens on x64, and it's
> > triggering a breakpoint from
> >
> > [Fact]
> > void VariantBasicTest()
> > {
> > BURN_VARIANT expectedVariants[10];
> > BURN_VARIANT actualVariants[10];
> > for (DWORD i = 0; i < 10; i++)
> > {
> > BVariantUninitialize(expectedVariants + i); // This is the
> > line causing the issue...
> > BVariantUninitialize(actualVariants + i);
> > }
> >
> > BVariantUninitialize
> >
> ->StrSecureZeroFreeString->StrSecureZeroString->StrSize->MemSizeChecked->MemSize
> >
> > extern "C" SIZE_T DAPI MemSize(
> > __in LPCVOID pv
> > )
> > {
> > // AssertSz(vfMemInitialized, "MemInitialize() not called, this would
> > normally crash");
> > return ::HeapSize(::GetProcessHeap(), 0, pv);
> > }
> >
> > HeapSize is stating that the Heap has been corrupted.
> >
> > Does our CI pipeline build and run the x64 tests?
> >
> > Thanks,
> > Jacob
> >
> > ____________________________________________________________________
> > WiX Toolset Developer Mailing List provided by FireGiant
> > http://www.firegiant.com/<http://www.firegiant.com>
> >
> ____________________________________________________________________
> WiX Toolset Developer Mailing List provided by FireGiant
> http://www.firegiant.com/<http://www.firegiant.com/>
>
> NOTE: This email was received from an external source. Please use caution
> when opening links or attachments in the message.
>
____________________________________________________________________
WiX Toolset Developer Mailing List provided by FireGiant http://www.firegiant.com/<http://www.firegiant.com/>
NOTE: This email was received from an external source. Please use caution when opening links or attachments in the message.