1

I'm looking for a double check on my understanding. I ran across code of this form:

#define BUFLEN_256 256

int main()
{

    const char* charPtr = "";
    
    if (true /* some real test here */)
    {
        char buf[BUFLEN_256] = { 0 };
        snprintf(buf, BUFLEN_256, "Some string goes here..");
        charPtr = buf;
    }
    std::cout << charPtr << std::endl;  // Is accessing charPtr technically dangerous here? 

}

My immediate thought was bug, the stack memory assigned to buf[] is no longer guaranteed to belong to the array once you exit the if(){}. But the code builds and runs without problem, and in double checking myself I got confused. I'm not good at assembly, but if I'm reading it correctly it does not appear that the stack pointer is reset after leaving the curly braces. Can someone double check me on that and chime in as to whether this code is technically valid? Here is the code with the assembly (built with Visual Studio 2019). My thought is this code is not OK, but I've been wrong on odd issues before.

#define BUFLEN_256 256

int main()
{
00DA25C0  push        ebp  
00DA25C1  mov         ebp,esp  
00DA25C3  sub         esp,1D8h  
00DA25C9  push        ebx  
00DA25CA  push        esi  
00DA25CB  push        edi  
00DA25CC  lea         edi,[ebp-1D8h]  
00DA25D2  mov         ecx,76h  
00DA25D7  mov         eax,0CCCCCCCCh  
00DA25DC  rep stos    dword ptr es:[edi]  
00DA25DE  mov         eax,dword ptr [__security_cookie (0DAC004h)]  
00DA25E3  xor         eax,ebp  
00DA25E5  mov         dword ptr [ebp-4],eax  
00DA25E8  mov         ecx,offset _1FACD15F_scratch@cpp (0DAF029h)  
00DA25ED  call        @__CheckForDebuggerJustMyCode@4 (0DA138Eh)  

    const char* charPtr = "";
00DA25F2  mov         dword ptr [charPtr],offset string "" (0DA9B30h)  
    
    if (true /* some real test here */)
00DA25F9  mov         eax,1  
00DA25FE  test        eax,eax  
00DA2600  je          main+7Ah (0DA263Ah)  
    {
        char buf[BUFLEN_256] = { 0 };
00DA2602  push        100h  
00DA2607  push        0  
00DA2609  lea         eax,[ebp-114h]  
00DA260F  push        eax  
00DA2610  call        _memset (0DA1186h)  
00DA2615  add         esp,0Ch  
        snprintf(buf, BUFLEN_256, "Some string goes here..");
00DA2618  push        offset string "Some string goes here.." (0DA9BB8h)  
00DA261D  push        100h  
00DA2622  lea         eax,[ebp-114h]  
00DA2628  push        eax  
00DA2629  call        _snprintf (0DA1267h)  
00DA262E  add         esp,0Ch  
        charPtr = buf;
00DA2631  lea         eax,[ebp-114h]  
00DA2637  mov         dword ptr [charPtr],eax  
    }
    std::cout << charPtr << std::endl;
00DA263A  mov         esi,esp  
00DA263C  push        offset std::endl<char,std::char_traits<char> > (0DA103Ch)  
00DA2641  mov         eax,dword ptr [charPtr]  
00DA2644  push        eax  
00DA2645  mov         ecx,dword ptr [__imp_std::cout (0DAD0D4h)]  
00DA264B  push        ecx  
00DA264C  call        std::operator<<<std::char_traits<char> > (0DA11AEh)  
00DA2651  add         esp,8  
00DA2654  mov         ecx,eax  
00DA2656  call        dword ptr [__imp_std::basic_ostream<char,std::char_traits<char> >::operator<< (0DAD0A0h)]  
00DA265C  cmp         esi,esp  
00DA265E  call        __RTC_CheckEsp (0DA129Eh)  

}
00DA2663  xor         eax,eax  
00DA2665  push        edx  
00DA2666  mov         ecx,ebp  
00DA2668  push        eax  
00DA2669  lea         edx,ds:[0DA2694h]  
00DA266F  call        @_RTC_CheckStackVars@8 (0DA1235h)  
00DA2674  pop         eax  
00DA2675  pop         edx  
00DA2676  pop         edi  
00DA2677  pop         esi  
00DA2678  pop         ebx  
00DA2679  mov         ecx,dword ptr [ebp-4]  
00DA267C  xor         ecx,ebp  
00DA267E  call        @__security_check_cookie@4 (0DA1181h)  
00DA2683  add         esp,1D8h  
00DA2689  cmp         ebp,esp  
00DA268B  call        __RTC_CheckEsp (0DA129Eh)  
00DA2690  mov         esp,ebp  
00DA2692  pop         ebp  
00DA2693  ret  
00DA2694  add         dword ptr [eax],eax  
00DA2696  add         byte ptr [eax],al  
00DA2698  pushfd  
00DA2699  fiadd       dword ptr es:[eax]  
00DA269C  in          al,dx  
00DA269D  ?? ?????? 
00DA269E  ?? ?????? 

}
Matt
  • 391
  • 5
  • 15

3 Answers3

3

What you're seeing is 'undefined' behavior. Stack memory is typically allocated all in one go at the start. So when a variable goes out-of-scope on the stack, that memory becomes available for re-use. Since you're not overwriting the stack with anything after the if statement, the data previously stored there is still intact. If you were to allocate additional memory/data to the stack after the if statement, you'd see a much different result.

See this post here: What happens when a variable goes out of scope?

Edit: To elaborate and demonstrate this, consider the following modification of your code (Compiled on VS2019 v142 x64):

#include <iostream>

#define BUFLEN_256 256

int main()
{
    
    char* charPtr;
    char other_buf[BUFLEN_256] = { 0 };
    char* charPtr2 = other_buf;

    if (true /* some real test here */)
    {
        char buf[BUFLEN_256] = { 0 };
        snprintf(buf, BUFLEN_256, "Some string goes here..");
        charPtr = buf;
    }
    std::cout << charPtr << std::endl;

    for (int n = 0; n < 3000; ++n)
    {
        *charPtr2 = 'a';
        charPtr2++;
    }

    std::cout << charPtr << std::endl;
}

Output

Some string goes here..
Some string goes haaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaca

Of course, keeping in mind that every compiler handles optimizations differently, and this may or may not happen in every case. That is why the behavior is 'undefined'. This example is more-so demonstrating overrunning the stack intentionally (buffer-overrun), but it illustrates the same effect. I'd produce a more direct example of legitimate cases where this could happen, but ironically undefined behavior is difficult to intentionally reproduce.

Abstract
  • 985
  • 2
  • 8
  • 18
3

My immediate thought was bug, the stack memory assigned to buf[] is no longer guaranteed to belong to the array once you exit the if(){}.

That is correct.

But the code builds and runs without problem

Undefined Behavior. In the cout << charPtr statement, charPtr is a dangling pointer to invalid memory. Whether or not the memory has been physically freed is irrelevent. The memory has gone out of scope.

I'm not good at assembly, but if I'm reading it correctly it does not appear that the stack pointer is reset after leaving the curly braces.

That is correct.

The memory for the array is being pre-allocated at the top of the stack frame when the function is entered (as part of the sub esp, 1D8h instruction), and then gets released during cleanup of the stack frame when the function exits (as part of the add esp, 1D8h instruction).

As you can see, when the if is entered, the very first thing it does is to call _memset() to zero out an array which already exists at [ebp-114h].

But that is an implementation detail, don't rely on that.

Can someone double check me on that and chime in as to whether this code is technically valid?

It is not.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
1

Yes, accessing charPtr in this way is undefined behaviour - and hence dangerous - because buf goes out of scope at the closing brace.

In practise, the code may work (or appear to work) because the memory used for buf is not re-used immediately but you should not, of course, rely on that. Whoever wrote this code made a mistake.

Paul Sanders
  • 24,133
  • 4
  • 26
  • 48