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

Mixing different JSON_DIAGNOSTICS settings in separately compiled units leads to core #3360

Closed
puffetto opened this issue Feb 28, 2022 · 7 comments · Fixed by #3378 or #3590
Closed
Assignees
Labels
documentation solution: wontfix the issue will not be fixed (either it is impossible or deemed out of scope)

Comments

@puffetto
Copy link
Contributor

puffetto commented Feb 28, 2022

What is the issue you have?

If I compile a simple program using the library with JSON_DIAGNOSTICS=1 and link it with another object compiled with JSON_DIAGNOSTICS=0 this leads to a coredump (SIGSEGV).

Please describe the steps to reproduce the issue.

Consider this simple code:

blackye@antani:~/prcd cat programs/crashme.cpp 
#define JSON_DIAGNOSTICS 1

#include <iostream>
#include <string>
#include "json.hpp"

int main (int argc, char * const argv[]) {
    std::string s ="{\"foo\":{\"bar\":[\"baz\"]}}";
    nlohmann::json j = nlohmann::json::parse(s);
    std::cout << j.dump() << std::endl;
    return 0;
}

It compiles and runs fine:

blackye@antani:~/prcd c++ -std=c++20 -Wall -O0 -g -I/usr/local/include -Iinclude -Iimports -o bin/crashme programs/crashme.cpp 
blackye@antani:~/prcd ./bin/crashme 
{"foo":{"bar":["baz"]}}
blackye@antani:~/prcd 

But if I link together with this other code (no calls performed from one to the other, but the library contains a. singleton which is executed at startup):


blackye@antani:~/prcd cat include/config.h 
#ifndef PRCD_CONFIG_H
#define PRCD_CONFIG_H

#include <string>
#include "json.hpp"

struct upstream {
    std::string uri, domain, user, key;
    NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(upstream, uri, domain, user, key);
};
struct core {
    std::string bind;
    int port=0, servers=0, threads=0;
    NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(core, bind, port, servers, threads);
};
struct configuration {
    core server;
    std::map<std::string, int> keys;
    std::map<std::string, std::string> files;
    std::map<std::string, upstream> upstreams;
    NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(configuration, server, keys, files, upstreams);
};

configuration loadconf();

inline const configuration& conf() {
    static configuration c = loadconf();
    return c;
};

inline const upstream& slave(std::string name){
    static upstream none;
    if(conf().upstreams.contains(name))
        return conf().upstreams.at(name);
    return none;
}

#endif //PRCD_CONFIG_H
blackye@antani:~/prcd cat libsource/config.cpp 

#include "config.h"
#include <iostream>
#include <fstream>

configuration loadconf() {
    std::ifstream i("prcd.json");
    nlohmann::json j;
    i >> j;
    configuration res = j;
    return res;
};
blackye@antani:~/prcd 

It cores:

blackye@antani:~/prcd c++ -std=c++20 -Wall -O0 -g -I/usr/local/include -Iinclude -Iimports -o bin/crashme lib/config.o programs/crashme.cpp 
blackye@antani:~/prcd ./bin/crashme 
Segmentation fault (core dumped)
blackye@antani:~/prcd lldb bin/crashme 
(lldb) target create "bin/crashme"
Current executable set to '/home/blackye/prcd/bin/crashme' (x86_64).
(lldb) r
Process 38838 launching
Process 38838 launched: '/home/blackye/prcd/bin/crashme' (x86_64)
Process 38838 stopped
* thread #1, name = 'crashme', stop reason = signal SIGSEGV: invalid address (fault address: 0x8002aa008)
    frame #0: 0x000000000025a73f crashme`nlohmann::detail::serializer<nlohmann::basic_json<std::__1::map, std::__1::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, bool, long, unsigned long, double, std::__1::allocator, nlohmann::adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char> > > >::dump(this=0x00007fffffffe6d0, val=0x00000008002aa008, pretty_print=false, ensure_ascii=false, indent_step=0, current_indent=0) at json.hpp:16155:21
   16152	              const unsigned int indent_step,
   16153	              const unsigned int current_indent = 0)
   16154	    {
-> 16155	        switch (val.m_type)
   16156	        {
   16157	            case value_t::object:
   16158	            {
(lldb) bt
* thread #1, name = 'crashme', stop reason = signal SIGSEGV: invalid address (fault address: 0x8002aa008)
  * frame #0: 0x000000000025a73f crashme`nlohmann::detail::serializer<nlohmann::basic_json<std::__1::map, std::__1::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, bool, long, unsigned long, double, std::__1::allocator, nlohmann::adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char> > > >::dump(this=0x00007fffffffe6d0, val=0x00000008002aa008, pretty_print=false, ensure_ascii=false, indent_step=0, current_indent=0) at json.hpp:16155:21
    frame #1: 0x000000000025b31f crashme`nlohmann::detail::serializer<nlohmann::basic_json<std::__1::map, std::__1::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, bool, long, unsigned long, double, std::__1::allocator, nlohmann::adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char> > > >::dump(this=0x00007fffffffe6d0, val=0x000000080080e088, pretty_print=false, ensure_ascii=false, indent_step=0, current_indent=0) at json.hpp:16275:25
    frame #2: 0x000000000025aec5 crashme`nlohmann::detail::serializer<nlohmann::basic_json<std::__1::map, std::__1::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, bool, long, unsigned long, double, std::__1::allocator, nlohmann::adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char> > > >::dump(this=0x00007fffffffe6d0, val=0x000000080080e038, pretty_print=false, ensure_ascii=false, indent_step=0, current_indent=0) at json.hpp:16222:21
    frame #3: 0x000000000025aec5 crashme`nlohmann::detail::serializer<nlohmann::basic_json<std::__1::map, std::__1::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, bool, long, unsigned long, double, std::__1::allocator, nlohmann::adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char> > > >::dump(this=0x00007fffffffe6d0, val=0x00007fffffffea00, pretty_print=false, ensure_ascii=false, indent_step=0, current_indent=0) at json.hpp:16222:21
    frame #4: 0x000000000025132d crashme`nlohmann::basic_json<std::__1::map, std::__1::vector, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, bool, long, unsigned long, double, std::__1::allocator, nlohmann::adl_serializer, std::__1::vector<unsigned char, std::__1::allocator<unsigned char> > >::dump(this=0x00007fffffffea00, indent=-1, indent_char=' ', ensure_ascii=false, error_handler=strict) const at json.hpp:18458:15
    frame #5: 0x0000000000250ed1 crashme`main(argc=1, argv=0x00007fffffffeaa8) at crashme.cpp:10:20
    frame #6: 0x0000000000222930 crashme`_start(ap=<unavailable>, cleanup=<unavailable>) at crt1.c:76:7
(lldb) quit
Quitting LLDB will kill one or more processes. Do you really want to proceed: [Y/n] 
blackye@antani:~/prcd 

Can you provide a small but working code example?

See above.

What is the expected behavior?

See above.
I suppose some internal ABI changes shape when JSON_DIAGNOSTICS=1 and this makes differently behaving code to have the same symbol/signature for the linker.

And what is the actual behavior instead?

See above.

Which compiler and operating system are you using?

Tested both on macOS Monterey 12.2.1 [compiler: Apple clang version 13.0.0 (clang-1300.0.29.30)] and FreeBSD 12.2-RELEASE-p4 [compiler: FreeBSD clang version 10.0.1 (git@github.com:llvm/llvm-project.git llvmorg-10.0.1-0-gef32c611aa2)]

Which version of the library did you use?

  • [X ] latest release version 3.10.5; json_all.hpp copied into local path as imports/json.hpp

If you experience a compilation error: can you compile and run the unit tests?

Not pertinent.

@puffetto
Copy link
Contributor Author

As an additional info: the code does not SEGV anymore if I get rid of the array:

blackye@antani:~/prcd diff programs/crashme.cpp programs/crashmenot.cpp 
8c8
<     std::string s ="{\"foo\":{\"bar\":[\"baz\"]}}";
---
>     std::string s ="{\"foo\":{\"bar\":\"baz\"}}";
blackye@antani:~/prcd c++ -std=c++20 -Wall -O0 -g -I/usr/local/include -Iinclude -Iimports -o bin/crashmenot lib/config.o programs/crashmenot.cpp 
blackye@antani:~/prcd ./bin/crashmenot 
{"foo":{"bar":"baz"}}
blackye@antani:~/prcd 

@puffetto
Copy link
Contributor Author

A workaround I am using is setting JSON_DIAGNOSTICS only globally in the Makefile, still I think it should be considered a bug.
Maybe renaming the conflicting methods and exception with a #define under an #if JSON_DIAGNOSTICS ?

@nlohmann
Copy link
Owner

Mixing binaries compiled with and without diagnostics is not expected to work as the latter introduces a new data member, so objects are not compatible. If you want to use diagnostics, you have to use them everywhere by defining JSON_DIAGNOSTICS globally.

@nlohmann nlohmann added solution: wontfix the issue will not be fixed (either it is impossible or deemed out of scope) and removed kind: bug labels Feb 28, 2022
@gregmarr
Copy link
Contributor

What you are trying to do is a violation of the C++ "One Definition Rule" (ODR). This means that any type must have the same definition everywhere that it is used in a single program. If you violate this, then your program behavior is undefined, but the compiler is not required to provide you with a diagnostic (error/warning).

https://en.cppreference.com/w/cpp/language/definition

@nlohmann MSVC provides tooling to help catch this, might be worth looking into.

https://devblogs.microsoft.com/oldnewthing/20160803-00/?p=94015

@puffetto
Copy link
Contributor Author

puffetto commented Mar 6, 2022

Mixing binaries compiled with and without diagnostics is not expected to work as the latter introduces a new data member, so objects are not compatible. If you want to use diagnostics, you have to use them everywhere by defining JSON_DIAGNOSTICS globally.

Thanks for your reply, basically I had said the same thing stating that the workaround is to define JSON_DIAGNOSTICS only globally.

I honestly would not expect this behaviour from an header-only library, but I understand it is a design chioice that is up to you.

I would add a note to the documentation to make this more clear, issued a pull request about it.

Thank you and all the best,

A.

@nlohmann
Copy link
Owner

nlohmann commented Mar 6, 2022

On https://json.nlohmann.me/home/exceptions/#extended-diagnostic-messages, it states:

As this global context comes at the price of storing one additional pointer per JSON value and runtime overhead to maintain the parent relation, extended diagnostics are disabled by default. They can, however, be enabled by defining the preprocessor symbol JSON_DIAGNOSTICS to 1 before including json.hpp.

Maybe this should be more prominent, but it clearly states that it adds a member to the class.

@nlohmann nlohmann linked a pull request Mar 7, 2022 that will close this issue
4 tasks
@nlohmann nlohmann added this to the Release 3.10.6 milestone Mar 7, 2022
@nlohmann nlohmann self-assigned this Mar 7, 2022
nlohmann pushed a commit that referenced this issue Mar 7, 2022
@puffetto
Copy link
Contributor Author

Cool, I see it has been fixed in 3.11.*!
Thanks,
A.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation solution: wontfix the issue will not be fixed (either it is impossible or deemed out of scope)
Projects
None yet
3 participants