1

I have an old 32-bit MFC application in C++ which was written with Visual Studio 2010. It ran without problems. Now I had to upgrade to Visual Studio 2017 and it crashes pretty often when I click on the treeview window. I have a dmp file and when I open it then I see that it crashes here:

 BOOL CObject::IsKindOf(const CRuntimeClass* pClass) const
 {
     ENSURE(this != NULL);
     // it better be in valid memory, at least for CObject size
     ASSERT(AfxIsValidAddress(this, sizeof(CObject)));

     // simple SI case
         CRuntimeClass* pClassThis = GetRuntimeClass(); //---->HERE Crash 

     ENSURE(pClassThis);
     return pClassThis->IsDerivedFrom(pClass);
 }

When I go back the call list then I end here:

//m_pTheModel is initialized with NULL
if (bValidValue == true)
    m_pTheModel = GetModel((WORD)lHint);

if (m_pTheModel == NULL || !AfxIsValidAddress(m_pTheModel, sizeof(m_pTheModel)))
{
   lock.Unlock();
   return;
}

try
{
    if ((m_pTheModel->IsKindOf(RUNTIME_CLASS(CMyClassModel))))
    ...
}
catch (...)
{
}

m_pTheModel is not NULL but when I look at the values in the debugger, for some values the memory is not readable.

What could the problem be? With the old version of visual studio I didn't have this problem. I have only recompiled this project and I had to set the target operation system to Windows XP.

The error message is “The thread tried to read from or write to a virtual address for which it does not have the appropriate access.”

I also don't understand why I can't catch this access violation with my try-catch around this.

Update: I found the reason. It was a strcpy that overwrites my pointer.

Fellnase
  • 103
  • 1
  • 9
  • `NULL` is not the only invalid address a pointer to have. Are you sure it's actually initialized? Are you sure the object points it to exists and haven't been destroyed? The fact that you have things like `this != NULL` and check that the memory the object `this` points to is valid leads me to believe your code is likely full of Undefined Behavior, any of which could cause the problem you are observing. – François Andrieux Apr 10 '19 at 14:58
  • My problem is, I haven't written this code in first place. It's a bigger project and I only should recompile it with a newer visual studio. Is there any way to check of the pointer is valid? I thought with != NULL and AfxIsValidAddress I'm on the save side. I looked thought the class, this pointer is initialized with null and is set only once in the function and I fould deletes only in the shutdown methods – Fellnase Apr 10 '19 at 15:03
  • 1
    This may be a configuration problem, since you are migrating to another version of Visual Studio. Make sure all components, including .obj files and libraries with C++ linkage (static and dynamic) were recompiled with the same version of VC++ using compatible parameters. This might also be the result of UB expressing itself differently with your newer compiler. Be sure to compile with no optimization to help spot the problem. Carefully step through the code, see at what point your debugger can no longer report on the internals of your object. – François Andrieux Apr 10 '19 at 15:10
  • I may note that this this-check is in an MFC function, CObject::IsKindOf is nothing from my selfwritten programcode. – Fellnase Apr 10 '19 at 15:10
  • What do you mean with UB Expressing? Unfortunately the problem is not happening on my developer PC when I run it with the debugger. Optimization is already disabled – Fellnase Apr 10 '19 at 15:12
  • _I also don't understand why I can't catch this access violation with my try-catch around this_ `try`/`catch` are for C++ exceptions. A Seg-Fault / Access Violation is not a C++ exception - hence it cannot be catched this way. Btw. after Access Violation it's probably the best to let that process die in peace... ;-) Found [SO: Catching access violation exceptions?](https://stackoverflow.com/a/457583/7478597) concerning this. – Scheff's Cat Apr 10 '19 at 15:15
  • 1
    Code that reaches Undefined Behavior can, by definition, exhibit any behavior. This behavior can change at any time for any reason, including changing compiler. What I mean by *"the result of UB expressing itself differently"* is that your code probably contains Undefined Behavior which use to exhibit itself as doing whatever the developer was expecting, but changing compiler has alter that behavior in such a way that it causes your program to crash. This is not a problem with changing compiler, it's a problem with the code being incorrect in the first place. – François Andrieux Apr 10 '19 at 15:17
  • Make sure that all of the dependent libraries were built with the new compiler. Binaries from Visual Studio 2010 will not be compatible. – drescherjm Apr 10 '19 at 16:27

1 Answers1

2

The use of AfxIsValidAddress is very concerning.

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

Worse, it only works in debug builds at all.

In non-debug builds, nonzero if lp is not NULL; otherwise 0.

This makes no promise that it is what you want at all. If you delete an object, the memory is likely still within the application ready to be reused, not returned to the operating system, and things like AfxIsValidAddress will return true. Worse, when you allocator does reuse that memory, it will still return true, while the pointer actually now refers to some completely different, and unknown object, leading to heap corruption.

This applies to all such similar functions, like IsBadReadPtr. Raymond Chen has a Microsoft blog post IsBadXxxPtr should really be called CrashProgramRandomly.

You will probably have to debug this in the IDE and find a specific problem, it is almost certainly a use after free issue, or a more serious issue elsewhere in the program overwriting the object. Can you produce it directly in the IDE rather than getting a dump file?

I also don't understand why I can't catch this access violation with my try-catch around this

Access violations, and such on the various platforms are not C++ exceptions. And since they generally only happen after a programs memory got corrupted, are basically impossible to recover from. There is generally some platform specific means to interact with them.

Fire Lancer
  • 29,364
  • 31
  • 116
  • 182
  • Unfortunately so far it happens only on the test system without IDE. Can I debug a release version with disabeled opimization too or do I need a debug version? – Fellnase Apr 10 '19 at 16:12
  • ***Can I debug a release version with disabeled opimization too*** Yes, debug the RelWithDebInfo configuration. Also change the settings so there is no optimization. – drescherjm Apr 10 '19 at 16:26
  • @Fellnase If you have to, you can remote debug, although experience is not perfect. This is useful if a debug build fails on the test system but not locally, debugging release crashes is still pretty difficult either way. https://learn.microsoft.com/en-us/visualstudio/debugger/remote-debugging-cpp?view=vs-2019 – Fire Lancer Apr 10 '19 at 16:50