Syntax errors cause crashes

[AntonC9018]

Hello.
This simple piece of code causes a crash due to an error in the provided expression. The text "Some error" never gets printed.

#include <cparse/shunting-yard.h>

int main()
{
    TokenMap tm;
    tm["x"] = 5;

    try
    {
        calculator calc;
        calc.compile("5&&&x"); // I would expect this to throw an error, or set some flag I could check
        printf("%f", calc.eval(tm).asDouble());
    } 
    catch (...)
    {
        puts("Some error");
    }
}

Changing the expression to e.g. 5*x makes it print the correct result.
This is very unfortunate, since I expect the users of my application to be notified of an error if they input an invalid expression. Is there anything I'm doing wrong or is there any other mechanism for validating the expression?

I'm compiling with gcc on 64-bit MinGW. Everything links successfully.

g++ -std=c++11 main.cpp -I../includes ../libs/cparse/core-shunting-yard.o ../libs/cparse/my-features.o

Also setting the expression to e.g. 5x does not crash the application, but doesn't throw errors either. It evaluates to x, though. However, setting it to sin(5x) does crash it. sin(5*x) works correctly.

The crash kind of looks like a stack overflow to me. Perhaps these expressions cause infinite recursion somewhere in the code?

Now, I have defined a custom function (logarithm) and a custom operator (caret for exponentiation). Those work and I don't think they have anything to do with this problem, but it's worth mentioning anyway.

[AntonC9018]

The error is for sure at calc.eval(tm) and not at asDouble(). Trying to store the result of eval() crashes the program before the result is obtained.

[AntonC9018]

This also crashes without any errors thrown.

#include <cparse/shunting-yard.h>

int main()
{
    TokenMap tm;
    tm["x"] = 2;

    try
    {
        auto result = calculator::calculate("sin(5x)", tm);
        printf("%f", result.asDouble());
    } 
    catch (...)
    {
        puts("Some error");
    }
}

[VinGarcia]

Hello @AntonC9018, thanks for reporting and also with a minimal example with the bug, I will try to understand it and then I will get back to you =].

[VinGarcia]

Uhm, ok this might be one of those pesky ones, because I was unable to reproduce the first example, so the steps I did were:

First I created a main.cpp file and cloned the master branch of the cparse project on a subdirectory cparse/.

In the main.cpp file I inserted your code:

#include <cparse/shunting-yard.h>

int main()
{
    TokenMap tm;
    tm["x"] = 5;

    try
    {
        calculator calc;
        calc.compile("5&&&x"); // I would expect this to throw an error, or set some flag I could check
        printf("%f", calc.eval(tm).asDouble());
    } 
    catch (...)
    {
        puts("Some error");
    }
}

Then I built the core-shunting-yard.o file with make release -C cparse and finally compiled and executed the program:

$ g++ -I . cparse/core-shunting-yard.o main.cpp && ./a.out
Some error

Second Problem:

I was able, however, to reproduce the incorrect compilation success you reported with the following code:

#include <cparse/shunting-yard.h>

int main()
{
    TokenMap tm;
    tm["x"] = 5;

    try
    {
        calculator calc;
        calc.compile("5x"); // Using 5x instead of 5&&&x
        printf("%f", calc.eval(tm).asDouble());
    }
    catch (...)
    {
        puts("Some error");
    }
}

It returned as if the result was the number 5.0000, so I will start by trying to understand what happened here.

[VinGarcia]

Also as a side note, the packToken type works with cout so you could do it like this:

#include <iostream>

// ...
            std::cout << calc.eval(tm) << std::endl;
// ...

Buf for debugging purposes, I am glad you used printf since it avoids implicit calls to AsDouble that would have happened using this trick

[AntonC9018]

Ok, here's what I'm doing for the first example (this time without custom features):

1. Git clone the latest code with git clone https://github.com/cparse/cparse cparse
2. Compile with mingw's make (I'm on Windows 7). mingw32-make release -C cparse
3. Compile the code below with g++ -std=c++11 cparse/core-shunting-yard.o test.cpp

// test.cpp
#include "cparse/shunting-yard.h"

int main()
{
    TokenMap tm;
    tm["x"] = 2;

    try
    {
        calculator calc("5&&&x");
        auto result = calc.eval(tm);
        printf("%f", result.asDouble());
    } 
    catch (...)
    {
        puts("Some error");
    }
}

4. Run the code with a. The program produces no output while Windows gives me a generic message that the program has stopped working.

I suppose you are on Linux. Since you're on Linux, this might be a platform-specific issue. Any clues?

[AntonC9018]

Wait, this time it's even weirder. This code also crashed for me:

#include "cparse/shunting-yard.h"

int main()
{
    TokenMap tm;
    tm["x"] = 2;

    try
    {
        calculator calc;
        calc.compile("5*x");
        auto result = calc.eval(tm);
        printf("%f", result.asDouble());
    } 
    catch (...)
    {
        puts("Some error");
    }
}

Whereas calc.compile("5"); works fine. calc.compile("5*x", tm); also crashes it.

[AntonC9018]

Ok, I guess one also has to link against builtin-features.o. If I do this, though, calc.compile("5*x"); works, but calc.compile("5&&&x"); still just crashes the application, without any output. Trying it with calc.compile("y");, while no such symbol y has been defined or given value, prints the following:

# a
terminate called after throwing an instance of 'bad_cast'
  what():  The Token is not a number!

This application has requested the Runtime to terminate it in an unusual way.
Please contact the application's support team for more information.

"Some error" is never printed.

[AntonC9018]

Thought I'd mention the compiler version too.

# g++ --version
g++ (Rev6, Built by MSYS2 project) 10.2.0

[AntonC9018]

I've also tried to catch each type of error your code ever throws individually, to no avail. This code also results in a crash, without any prints.

#include "shunting-yard.h"
#include "shunting-yard-exceptions.h"

int main()
{
    TokenMap tm;
    tm["x"] = 2;

    try
    {
        calculator calc;
        calc.compile("(");
        auto result = calc.eval(tm);
        printf("%f", result.asDouble());
    } 
    catch (const bad_cast& exc)
    {
        puts("Bad cast");
    }
    catch (const syntax_error& exc)
    {
        puts("Syntax error");
    }
    catch (const type_error& exc)
    {
        puts("Type error");
    }
    catch (const undefined_operation& exc)
    {
        puts("Undefined operation");
    }
    catch (const std::invalid_argument& invalid_argument)
    {
        puts("Invalid argument");
    }
    catch (const std::domain_error& exc)
    {
        puts("Domain error");
    }
    catch (const std::runtime_error& exc)
    {
        puts("Runtime error");
    }
}

[AntonC9018]

Also tried running the tests. I have not had experience with C++ tests, so I might be wrong. I have compiled the test-shunting-yard.cpp with g++ test-shunting-yard.cpp core-shunting-yard.o builtin-features.o catch.cpp, then ran the executable. It also simply crashes.

[AntonC9018]

The last thing I tried is compiling the entire thing, without linking to libraries. Compiling and running the code from above from scratch with g++ -std=c++11 shunting-yard.cpp packToken.cpp functions.cpp containers.cpp builtin-features.cpp test.cpp does indeed print Domain error. catch (...) also works fine with this. I guess this approach works for me.
Any ideas on why this would be happening?
Compiling without optimization flags likewise did not solve it for me.

[VinGarcia]

Hello @AntonC9018 sorry I disappeared, I had some things to do tonight, tomorrow I'll continue the investigation

[VinGarcia]

About the tests, it should be enough to run make test.

About the 5*x indeed we need the builtin-feature.o, about compiling it just with y the compilation actually works, what throws is when you try to convert it to double, since it is only an unbound variable.

One thing you could try is to to:

try
{
    // ...
{
catch (const std::exception& ex)
{
    std::cout << ex.what() << std::endl;
}

Since all exceptions inherit from std::exception this should work unless we are talking about invalid memory access, which is always random =/.

[VinGarcia]

The last thing I tried is compiling the entire thing, without linking to libraries. Compiling and running the code from above from scratch with g++ -std=c++11 shunting-yard.cpp packToken.cpp functions.cpp containers.cpp builtin-features.cpp test.cpp does indeed print Domain error. catch (...) also works fine with this. I guess this approach works for me.
Any ideas on why this would be happening?
Compiling without optimization flags likewise did not solve it for me.

This is very weird. But sadly this is normal when we are working with C++ on different platforms =/.

The g++ version I am using has an output very different from yours but it is also on the 10.2.0 version:

$ g++ --version
g++ (GCC) 10.2.0
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

[VinGarcia]

That said, if compiling including all cpp files is working for you this is great, but I have no idea of how to fix this so that future users don't have this problem. If you find out it would be great if you could submit a PR with a fix.

So, I will focus on fixing that problem where it would not throw for the expression "5x" and evaluate to 5.0000, but I have little time for making these fixes, so this might take some time, hopefully not too much.

[dimxy]

This code does print "Some error" on my MacOS:

#include <cparse/shunting-yard.h>

int main()
{
    TokenMap tm;
    tm["x"] = 5;

    try
    {
        calculator calc;
        calc.compile("5&&&x"); // I would expect this to throw an error, or set some flag I could check
        printf("%f", calc.eval(tm).asDouble());
    } 
    catch (...)
    {
        puts("Some error");
    }
}

If I remove try/catch it aborts with an unhandled exception: terminating with uncaught exception of type syntax_error: Invalid operator: &&&. So it seems cparse works correctly in this case. I think the problem with exception not caught on MINGW relates to exception handling on this specific platform. Normally catch(...) should catch all exceptions even a mem violation.
(maybe this would give a hint: https://stackoverflow.com/questions/4149118/exception-handling-doesnt-work-with-qt-on-windows)

[VinGarcia]

Interesting, but I think that the problem reported on stackoverflow is probably something different since he was able to fix it by recompiling everything from source instead of using the intermediary .o files the Makefile produce. But it might be a good place to start investigating.

[VinGarcia]

@AntonC9018 I have just submitted a commit that should fix the problem where it was not throwing for invalid expressions such as 5x:

aed8ce0

Regarding the error with throw, I don't know how to fix it since I can't reproduce it, so if the commit above solves the other problem I think we should close this ticket.

[AntonC9018]

Sure.
I ended up using a different library anyway.

[VinGarcia]

Yeah, I figured xP, anyway thanks for reporting.