Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serialize runtime exceptions when debug enabled and struct contains vector #49

Closed
SteveS-Serrano opened this issue May 22, 2023 · 19 comments

Comments

@SteveS-Serrano
Copy link

serializeStruct throws various errors (sometimes a "bad alloc" exception, sometimes segv, sometimes invalid pointer) if a struct contains a vector and the compiler debug options are enabled.

Compiler:

$ g++ --version
g++ (Ubuntu 11.3.0-1ubuntu1~22.04.1) 11.3.0

Compiler command line options:

-g -std=c++20     <<<<<< with -DCMAKE_BUILD_TYPE=Debug (this fails at runtime)

-O3 -DNDEBUG -std=c++20   <<<<<  with -DCMAKE_BUILD_TYPE=Release (this succeeds at runtime)

Google Test version of test code:

struct Simple
{
    std::vector<int> v;
    JS_OBJECT(JS_MEMBER(v));
};

const char expected1_compact[] = R"json({"v":[1,2,3,4,5,6,7,8,9]})json";

TEST_F(EsiJsonSerdesGTest, Struct2Json)
{
    std::vector<int> temp_vect{1,2,3,4,5,6,7,8,9};

    Simple simple;
    simple.v = temp_vect;

    std::string output = JS::serializeStruct(simple, JS::SerializerOptions(JS::SerializerOptions::Compact));
    EXPECT_STREQ(output.c_str(), expected1_compact);
}

Build/run commands:

 rm -rf build
 mkdir build
 cd build
 cmake -G Ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=./install_dir ..
 ninja
./gtest_sim --gtest_filter=EsiJsonSerdesGTest.Struct2Json
Running main() from /home/sscott/eg_link/eg_esi/apps/build/_deps/googletest-src/googletest/src/gtest_main.cc
Note: Google Test filter = EsiJsonSerdesGTest.Struct2Json
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from EsiJsonSerdesGTest
[ RUN      ] EsiJsonSerdesGTest.Struct2Json
[       OK ] EsiJsonSerdesGTest.Struct2Json (0 ms)
[----------] 1 test from EsiJsonSerdesGTest (0 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (0 ms total)
[  PASSED  ] 1 test.

cd ..
rm -rf build
mkdir build
cd build
cmake -G Ninja -DCMAKE_BUILD_TYPE=Debug -DCMAKE_INSTALL_PREFIX=./install_dir ..
ninja
./gtest_sim --gtest_filter=EsiJsonSerdesGTest.Struct2Json
Running main() from /home/sscott/eg_link/eg_esi/apps/build/_deps/googletest-src/googletest/src/gtest_main.cc
Note: Google Test filter = EsiJsonSerdesGTest.Struct2Json
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from EsiJsonSerdesGTest
[ RUN      ] EsiJsonSerdesGTest.Struct2Json
free(): invalid pointer
Aborted (core dumped)

@SteveS-Serrano
Copy link
Author

You have a great library. This library is exactly what I've been wanting for working with json in C++. It just seems like it's not quite ready to use yet. Keep working on it!

@jorgen
Copy link
Owner

jorgen commented May 22, 2023

I have tried to reproduce this on Ubuntu 22.04, but without luck. Can you tell me something about the system you are running this on. Can you run this in a debugger and inspect the callstack, or open the generated coredump? Maybe you could send me the coredump? I have tried this in the range {0..1000} but showing 25 here for readabillity.

image

@jorgen
Copy link
Owner

jorgen commented May 22, 2023

Just noticed the c++20 version. I have recompiled with c++20 with no change in the output.

@SteveS-Serrano
Copy link
Author

Sorry, I'm very confused. It is 100% reproducible on my system, fails on the first try every time. I have run it in a debugger. The errors change based on what other members are in the structure. I am using google test, not sure if that makes a difference.

@SteveS-Serrano
Copy link
Author

The way it is failing at the moment is to throw a "bad_alloc" exception. In the debugger, handle_json_escapes_out calls buffer.reserve(data.size()+10) in line 4755. data.size() returns a gigantic number for the string, so the reserve() call fails.

json_struct_dbg1
json_struct_dbg2

@jorgen
Copy link
Owner

jorgen commented May 22, 2023

There is something very odd going on. The TypeHandler for std::string is being called, but there are no string members in struct Simple! Do you have the sample Simple struct as the one in the initial example?

@SteveS-Serrano
Copy link
Author

struct Simple
{
    std::vector<int> v;
    JS_OBJECT(JS_MEMBER(v));
};

const char expected1_compact[] = R"json({"v":[1,2,3,4,5,6,7,8,9]})json";

TEST_F(EsiJsonSerdesGTest, Struct2Json)
{
    std::vector<int> temp_vect{1,2,3,4,5,6,7,8,9};

    Simple simple;
    simple.v = temp_vect;

    std::string output = JS::serializeStruct(simple, JS::SerializerOptions(JS::SerializerOptions::Compact));
    EXPECT_STREQ(output.c_str(), expected1_compact);
}

@SteveS-Serrano
Copy link
Author

As I was stepping through, I thought I saw something about a default constructor being called that created an empty string. I think it's trying to create the output string.

@jorgen
Copy link
Owner

jorgen commented May 22, 2023

oh, I think I know whats going on. Do you have another Simple struct in your executable? Try and put the entire test and definition of Simple in an anonymous namespace.

@SteveS-Serrano
Copy link
Author

Yes, a separate file has a struct named Simple:

struct Simple
{
  std::string A;
  bool b;
  int some_longer_name;
  JS_OBJECT(JS_MEMBER(A), JS_MEMBER(b), JS_MEMBER(some_longer_name));
};

@SteveS-Serrano
Copy link
Author

I changed the name of the struct from Simple to Simple2. This does indeed allow the test to pass.

struct Simple2
{
    std::vector<int> v;
    JS_OBJECT(JS_MEMBER(v));
};

const char expected1_compact[] = R"json({"v":[1,2,3,4,5,6,7,8,9]})json";

TEST_F(EsiJsonSerdesGTest, Struct2Json)
{
    std::vector<int> temp_vect{1,2,3,4,5,6,7,8,9};

    Simple2 simple;
    simple.v = temp_vect;

    std::string output = JS::serializeStruct(simple, JS::SerializerOptions(JS::SerializerOptions::Compact));
    EXPECT_STREQ(output.c_str(), expected1_compact);
    std::cout << output << std::endl;
}

@jorgen
Copy link
Owner

jorgen commented May 22, 2023

Yes, I believe you are violating the ODR. Either change the name, or put the struct definition and usage in a anonymous namespace or put the struct in a named namespace.

@SteveS-Serrano
Copy link
Author

The ODR applies to a source file, not an entire program. The struct Simple is only defined once per translation unit.

According to standard C++ (wayback machine link) : A translation unit is the basic unit of compilation in C++. It consists of the contents of a single source file,
https://stackoverflow.com/a/1106167/4541938

@SteveS-Serrano
Copy link
Author

ah, nvm. I think you are right.

@jorgen
Copy link
Owner

jorgen commented May 22, 2023

cool, is this also the problem your experiencing with the tuples?

@jorgen
Copy link
Owner

jorgen commented May 22, 2023

would be nice to have some sort of chat :)

@SteveS-Serrano
Copy link
Author

SteveS-Serrano commented May 22, 2023

cool, is this also the problem your experiencing with the tuples?

I will revisit the tuple issue. I had copied/pasted some of the test cases from the repo. I need to go back and check for duplicate structure names. I will have to recreate the tuple test - I think I inadvertently overwrote my test case file with that test case in it. I've been switching back and forth today and I lost a file.. :-(

@jorgen
Copy link
Owner

jorgen commented May 22, 2023

np. I have been able to reproduce the compile error for serializing the std::tuple. I will look at that and fix it. I'm closing this issue if its ok with you.

@SteveS-Serrano
Copy link
Author

yes. Thanks for working the issue with me.

@jorgen jorgen closed this as completed May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants