Hex-rays decompiler incorrect logic

Question

I'm practicing reverse engineering in IDA and I created an example application in Visual C++ to practice working with classes/structs and the decompiler output seems to be incorrect - I would like to know if it's possible to fix this to get a closer to correct decompiled result or whether this is simply a limitation of the decompiler.

#include <iostream>
#include "exstruct.cpp"

int main()
{
    int x, y;
    std::cin >> x;
    std::cin >> y;
    calculator c(x, y);
    std::cout << c.multiply() << "\r\n";
}

I compiled with the Visual C++ compiler with optimizations turned off and after defining the functions and data structures, the hex-rays decompiler spits out this:

int __cdecl main(int argc, const char **argv, const char **envp)
{
  char *newline; // ST04_4
  int cout; // eax
  Calculator calculator; // [esp+4h] [ebp-18h]
  int x; // [esp+10h] [ebp-Ch]
  int y; // [esp+14h] [ebp-8h]

  std::basic_istream<char,std::char_traits<char>>::operator>>(std::cin, &x, calculator.x);
  std::basic_istream<char,std::char_traits<char>>::operator>>(std::cin, &y, calculator.y);
  Calculator_constructor(&calculator, x, y);
  calculator.x = (int)new_line_string;
  newline = (char *)Calculator_multiply(&calculator);
  cout = std::basic_ostream<char,std::char_traits<char>>::operator<<(std::cout);
  printf(cout, (int)newline);
  return 0;
}

It all looks pretty good up until it assigns the new line string \r\n to calculator.x and then the result of the multiplication to the newline variable, which is wrong for obvious reasons.

I've reviewed the assembly and this is simply not what happens. A snippet of the assembly below:

.text:00701088                 lea     ecx, [ebp+calculator] ; this
.text:0070108B                 call    Calculator_constructor
.text:00701090                 push    offset new_line_string ; this
.text:00701095                 lea     ecx, [ebp+calculator] ; this
.text:00701098                 call    Calculator_multiply
.text:0070109D                 push    eax
.text:0070109E                 mov     ecx, ds:?cout@std@@3V?$basic_ostream@DU?$char_traits@D@std@@@1@A ; std::basic_ostream<char,std::char_traits<char>> std::cout
.text:007010A4                 call    ds:??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QAEAAV01@H@Z ; std::basic_ostream<char,std::char_traits<char>>::operator<<(int)
.text:007010AA                 push    eax
.text:007010AB                 call    printf

It looks to me like the decompiler is getting confused because the pointer to \r\n literal push happens before the multiply call, making it look like an argument where it actually is not.

Is there anyway I can fix this?

Full assembly is here.

exstruct.cpp as text:

class calculator
{
public:
    int x;
    int y;
    int z;

    calculator(int x, int y)
    {
        this->x = x;
        this->y = y;
        this->z = x + y;
    }

    int multiply()
    {
        return this->x * this->y;
    }
};

Constructor & Multiply Source & Decompiled Source:

.

Notes:

• Hex-Rays Decompiler v7.0.0.170914
• I manually increased the size of the function to 1C as it wasn't originally detecting newline as a field
• I manually defined the location of \r\n in memory as a string

Answer 1

I was able to work this one out.

The decompiler was not detecting the correct call parameter counts for the >> and << operators of cin and cout.

For example:

int __thiscall std__basic_istream_char_std__char_traits_char____operator__(_DWORD, _DWORD, _DWORD)

Is the detected signature of the >> operator.

However, reviewing the C++ reference, it should be something more like this:

int __thiscall std__basic_istream_char_std__char_traits_char____operator__(void *, int *)

Where the first parameter is the cin object and the second is a pointer to the integer output. Because these functions use the __thiscall convention if the parameter count is wrong then the decompiler assumes that the function has modified the stack pointer by the incorrect amount and this leads to all sorts of issues.