jorgen / json_struct

json_struct is a single header only C++ library for parsing JSON directly to C++ structs and vice versa
Other
422 stars 57 forks source link

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

Closed SteveS-Serrano closed 1 year ago

SteveS-Serrano commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

SteveS-Serrano commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago
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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

ah, nvm. I think you are right.

jorgen commented 1 year ago

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

jorgen commented 1 year ago

would be nice to have some sort of chat :)

SteveS-Serrano commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

yes. Thanks for working the issue with me.