AfxIsValidAddress (and Others) Don’t Work as Advertised

MFC exposes a some memory debugging facilities such as AfxIsValidAddress, which (for debug builds) supposedly -

Tests any memory address to ensure that it is contained entirely within the program’s memory space.

Or does it?   AfxIsValidAddress only delegates the call to the undocumented ATL::AtlIsValidAddress, which reads:

// Verify that a pointer points to valid memory
inline BOOL AtlIsValidAddress(const void* p,
                  size_t nBytes, BOOL bReadWrite = TRUE)
{
      (bReadWrite);
      (nBytes);
      return (p != NULL);
}

The first two lines are just no-ops to silence compiler warnings about unused parameters. The promised verification that p is-contained-entirely-within-the-program’s-memory-space, amounts to the test that p is not null.

AfxIsValidString identically does not hold to its word and tests only if the input string is non-null:

inline BOOL AtlIsValidString(LPCSTR psz,
                          size_t nMaxLength = UINT_MAX)
{
      (nMaxLength);
      return (psz != NULL);
}

AfxAssertValidObject  is undocumented, and runs a bunch of redundant AfxIsValidAddress-es. CObject::AssertValid is documented (‘performs a validity check on this object by checking its internal state’!!), and is just as disappointing:

void CObject::AssertValid() const
{
     ASSERT(this != NULL);
}

The reason is probably that somewhere along the way MS realized there is no sane way to keep such promises.

There are several Win32 API similar in functionality: IsBadWritePtr, IsBadHugeWritePtr, IsBadReadPtr, IsBadHugeReadPtr, IsBadCodePtr, IsBadStringPtr. It has been known since at least 2004 that these functions are broken beyond repair and should never be used. The almighty Raymond Chen and Larry Osterman both discuss the reasons in detail, so just a short rehash: IsBad*Ptr all work by accessing the tested address and catching any thrown exceptions. Problem is that a certain few of these access violations (namely, those on stack guard pages) should never be caught – the OS uses them to properly enlarge thread stacks.  In the words of Michael Howard, who first realized this (or so I think – both Larry and Raymond attribute the CrashMyApplication nicknames to him):

You should also not catch all exceptions, but only types that you know about. Catching all exceptions is just as bad as using IsBad*Ptr.

I’m guessing that AfxIsValidAddress – and sisters – worked the same way, until someone realized they too were probably generating more debugging effort than they were saving. However, while the Win32 guys decided to leave their API semantics as are and clearly document them as obsolete and dangerous, the MFC guys turned their API into no-ops and did cleanup work that cannot be described as anything but sloppy. Not only is the documentation wrong, source comments are too:

// AfxIsValidAddress() returns TRUE if the passed parameter points
// to at least nBytes of accessible memory. If bReadWrite is TRUE,
// the memory must be writeable; if bReadWrite is FALSE, the memory
// may be const.

BOOL AFXAPI AfxIsValidAddress( ...

– and even the MFC sources themselves still contain many naive uses of these no-op tests (search around, you’ll find plenty).

Bonus: What If You Really Have To Test Memory for Validity?

This is after all not so far fetched. Years ago I had to work around a bug in nVidia GPU drivers, where occasionally some API (namely IDirect3DVolumeTexture9::LockBox, applied on a huuuuuuuge texture) returned success but gave a bogus memory buffer. The workaround was to VirtualQuery the address, and if not accessible – retry the process on a smaller volume texture. It went something like -

...
D3DLOCKED_BOX box;
HRESULT res = pVolumeTexture->LockBox(0, &box, NULL,
                                     D3DLOCK_DISCARD);
_ASSERT(res==D3D_OK  && box.pBits);
// this should have sufficed, but...

MEMORY_BASIC_INFORMATION meminf;
VirtualQuery(box.pBits, &meminf, sizeof(meminf));

BOOL  bOk = (meminf.State == MEM_COMMIT)  &&
      ( 0 !=  (meminf.Protect &
      ( PAGE_READWRITE | PAGE_WRITECOPY | PAGE_EXECUTE_READWRITE));

if (!bOk)
 //fallback
...

Of course VirtualQuery is innately sluggish, so this is completely unacceptable as mere parameter validation. Use this only when you know you have to, not just to be on the safe side.

Child Breakpoints in Visual Studio

You often see in the breakpoints window that certain breakpoints are expandable:

bpwin1

These are called child breakpoints, and are a strong contender to the title of most poorly documented feature of VS.  According to MSDN, child breakpoints occur -

…when you set a breakpoint in a location such as an overloaded function…

…when you attach to more than  one instance of the same program.

And I’m not sure what they mean by either. AFAIK, a single debugger cannot attach to more than one instance of a program, and different overloads of the same functions reside in different code lines – so they cannot possibly count for child breakpoints.

It’s reasonable to assume that child breakpoints are generated whenever a breakpoint is set in a line of code  that is compiled into multiple binary locations. I’m aware of two such scenarios – but there may be more:

  1. Templated function/method instantiated with different arguments,
  2. A function in a static library linked into multiple executables (exe, dll) in a single solution.

Perhaps by ‘overloaded function’ MSDN means (1) and by ‘more than one instance of the same program’ they mean (2)?  I sure hope not.  Also, the ever generous John Robbins confirmed the 1st case by email.

It’s good to be aware of this feature because it does have debugging performance implications – In a large project such that I work on, setting a breakpoint in (e.g.) smart pointer code results in spreading INT 3 instructions in thousands of instantiations – occasionally causing full minutes of IDE stall. There are often better alternatives – maybe more on that in a future post.

To make things even more interesting, in VS2005 this feature was badly bugged: more often than not a breakpoint was split into children for no apparent reason. This seems to be fixed in VS2010.