GetTextMetricsW causing memory corruption

I have been converting our ANSI product to UNICODE, and today during testing I was getting “Run-Time Check Failure #2 – Stack around the variable ‘tm’ was corrupted.” error messages.

Turned out it was this code:

	TEXTMETRICW tm;
	GetTextMetricsW( hDC, &tm ) ;

I isolated the code into a test function, which then became two functions:

void TestFuncA(HDC hDC)
{
	TEXTMETRICA tm;
	void* p = &tm;
	memset(&tm, -1, sizeof(tm) );
	GetTextMetricsA( hDC, &tm ) ;
}

void TestFuncW(HDC hDC)
{
	TEXTMETRICW tm;
	void* p = &tm;
	memset(&tm, 0, sizeof(tm) );
	GetTextMetricsW( hDC, &tm ) ;
}

For TestFuncW the sizeof(tm) is 57 bytes, yet GetTextMetricsW is over writing 60, as seen by the Memory debugging window, whereas for TestFuncA sizeof(tm) is only 53 bytes, and GetTextMetricsA overwrites only 53 bytes.

I then created a brand new Win32 Windows C++ application, and placed my test code into this, and strangely enough, for TestFuncW the sizeof(tm) is 60 bytes, yet GetTextMetricsW is over writing 60, and for TestFuncA sizeof(tm) is only 56 bytes, and GetTextMetricsA overwrites only 53 bytes.

Hazarding a guess, “that it is the fact the original product as Struct Member Alignment (same as #pragma pack() ) set to 1 Byte (/Zp1)”, I changed this from the default to 1, and tada!

Now my new project crashes also.

Lesson for others, avoid Struct Member Alignment across full projects.

Lesson for me, I now have weeks of work to unravel the effects/impact of why it was set to 1 in the first place (old legacy code and lazy developers)

SafeArrayTypeMismatchException

We extended our legacy C++ DCOM application last week, and when the developer wrote the C#.Net end to call the new method, we were getting the error:

System.Runtime.InteropServices.SafeArrayTypeMismatchException

The developer that had added the method stated it work, and pointed to his Delphi test app that worked happily.

Reviewing the method code it all looked fine. Single stepping through the code there where no problems.

The help for this exception says:

The exception thrown when the type of the incoming SAFEARRAY does not match the type specified in the managed signature.

But as we are dynamically calling the method like this:

objAddType = Type.GetTypeFromProgID("DCOM_OBJECT_NAME.COMPANY_CLASS_A");
objAdd = Activator.CreateInstance(objAddType);

object[] input = {};

object result = objAddType.InvokeMember("MethodName", BindingFlags.InvokeMethod, null, objAdd, input);

There was no was no signature.

This is for this Method:

VARIANT CompanyClassAObject::MethodName(void)
{
    VARIANT vaResult;
    VariantInit(&vaResult);

    SAFEARRAY* sfa = GetSafeArray();

    vaResult.vt = VT_UI1 | VT_ARRAY;
    vaResult.parray = sfa;

    return vaResult;
}

with the body of GetSafeArray looked like:

    SAFEARRAYBOUND sfabounds[1];
    sfabounds[0].lLbound = 0;
    sfabounds[0].cElements = bytes_needed;
    SAFEARRAY *sfa = SafeArrayCreate(VT_I1, 1, sfabounds);
    if(sfa)
    {
        void *data;
        if(SafeArrayAccessData(sfa, &data) == S_OK)
        {
            memcpy(data, buffer, bytes_needed);
            SafeArrayUnaccessData(sfa);

            delete buffer;
            return sfa;
        }
    }

The problem ended up being that the SafeArray is created as VT_I1 type but when it is put into the variant type it was typed as VT_UI1.  So the .Net runtime was picking up the conflict and correctly complaining, and once you know what the error is, the exception message make sense.  Funny that!

Setting the SafeArrayCreate to use VT_UI1 and every thing worked.

Evils of typedefs and legacy code

Our legacy C++ codebase once was a C codebase.  Way back then, there was no standard boolean type.  So in the wisdom of the day a type was declared

typedef int tBool;

The major problem with tBool’s is that over time enums, #define or magic constants get assigned to them, or they can be compared to said enums, #defines or magic constants, as they are all int’s.

Which works, but years later *somebody* came along and replaced the tBool for bool, and we started getting issues.

Now as in a lot of large codebases there tends to be a large amount of ignored warnings, and not enough daylight hours to remove them all.

So given that, and that your using Visual Studio 2005, here are the ones you should find and remove.

#define NON_BOOL_DEFINE 42
bool boolValue;

boolValue = NON_BOOL_DEFINE;

Warning C4305: ‘=’ : truncation from ‘int’ to ‘bool’

if( boolValue == NON_BOOL_DEFINE )

Warning C4806: ‘==’ : unsafe operation: no value of type ‘bool’ promoted to type ‘int’ can equal the given constant

Another hint your boolean types are being used wrong is this warning

Warning C4800: ‘const int’ : forcing value to bool ‘true’ or ‘false’ (performance warning)

#pragma pack()

I have just spent the last hour bashing my head, trying to workout why some new C++ code (to my project) was crashing.

I was constructing one class, and it was building some worker classes and crashing in the std::vector code when the worker class was being destroyed.

It just wasn’t making sense.

Why would code that’s been running for another team for over a year, blow-up when I use it?

I turned to a co-worker, described what was happening and said “it’s like some memory corruption or packing issue

And it was a structure packing alignment issue. The imported project had default packing (8), and my really old solution uses packing (1), so somewhere the default class functions where getting generated with different opinions from where member’s were….

#pragma pack(push)
#pragma pack(4)
#pragma pack(pop)

to the rescue, and low and behold the problems gone. I hate this type of issue. Importing existing C++ projects happens so infrequently that this issue is not on the must check list. Where-as I was convinced I had fluffed up the usage of the new code….

Lint, the best thing for your C/C++ code

I was first introduced to PC-Lint at my old company, after complaining about code style and the state of the software after having warnings turned off for years to a co-worker.  I then spent a few months evaluating the software and removing bugs from our system before getting the sign off to purchase licenses for the team.

The best part was finding odd-case errors, fixing them, and later reviewing customer crashes and observing, Oh I’ve fixed that already. Nothing better to say to the management types who love to rush products out the door.

Anyway in our current C++ code base I have been removing large numbers of warnings (and solved some corner case bugs), and now have team buy-in. It’s the most beautiful thing to see the warning count drop, and read check-in comments about warning removal.

On Friday I pulled out the trusty PC-Lint again, but was not sure how best to run this beast of a tool against a Visual Studio project/solution. Enter Visual Lint, a fantastic integration piece of software.

I can see a few more purchase orders in the near future.