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

Strange behaviour when using std::sort on std::vector<json> #3080

Closed
1 of 5 tasks
crabster opened this issue Oct 14, 2021 · 5 comments
Closed
1 of 5 tasks

Strange behaviour when using std::sort on std::vector<json> #3080

crabster opened this issue Oct 14, 2021 · 5 comments
Labels
solution: invalid the issue is not related to the library solution: proposed fix a fix for the issue has been proposed and waits for confirmation

Comments

@crabster
Copy link

Hi, I'm having a hard time finding the root cause of the behaviour specified below.

What is the issue you have?

When trying to sort std::vector<nlohmann::json> via custom comparator, parameters of the comparator are not elements of the said std::vector. The values of the j1 and j2 from the comparator

[](const nlohmann::json& j1, const nlohmann::json& j2) {
    return j1.at("key") <= j2.at("key");
})

should be objects, but the exception thrown during the std::sort in the working example says something else:

terminate called after throwing an instance of 'nlohmann::detail::type_error'
  what():  [json.exception.type_error.304] cannot use at() with null

The strange thing is when I change the variable jsons_count to 16 or variable value to i in the working example, the code runs successfully.

I ran the code with valgrind and ended up with:

==403499== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)
...
==403912== 1 errors in context 1 of 3:
==403912== Use of uninitialised value of size 8
==403912==    at 0x40A50D: nlohmann::basic_json<std::map, std::vector, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char> > >::type_name() const (.conan/data/nlohmann_json/3.10.3/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/include/nlohmann/json.hpp:0)
==403912==    by 0x40F788: nlohmann::basic_json<std::map, std::vector, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char> > >::at(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const (.conan/data/nlohmann_json/3.10.3/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/include/nlohmann/json.hpp:3652)
...
==403636== Conditional jump or move depends on uninitialised value(s)
==403636==    at 0x40A4FC: nlohmann::basic_json<std::map, std::vector, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char> > >::type_name() const (.conan/data/nlohmann_json/3.10.3/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/include/nlohmann/json.hpp:7178)
...
==403636== 1 errors in context 3 of 3:
==403636== Conditional jump or move depends on uninitialised value(s)
==403636==    at 0x40F60C: nlohmann::basic_json<std::map, std::vector, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char> > >::at(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const (.conan/data/nlohmann_json/3.10.3/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/include/nlohmann/json.hpp:3638)

which looks like some of the comparator's parameters could have uninitialized m_type (https://github.com/nlohmann/json/blob/v3.9.1/include/nlohmann/json.hpp#L6924) and possibly might not be initialized at all.

Could you please look into it? Thank you in advance.

Please describe the steps to reproduce the issue.

Compile and run the working code example below.

Can you provide a small but working code example?

#include "nlohmann/json.hpp"
                                                                                                      
int main() {
    std::vector<nlohmann::json> jsons;
    int jsons_count = 17;
    for (int i = 0; i < jsons_count; ++i) {
        int value = 0;
        nlohmann::json j;
        j["key"] = value;
        jsons.push_back(j);
    }

    std::sort(jsons.begin(), jsons.end(),
            [](const nlohmann::json& j1, const nlohmann::json& j2) {
                return j1.at("key") <= j2.at("key");
            });
}

What is the expected behavior?

The code should run successfully without any exception thrown.

And what is the actual behavior instead?

The code throws exception during std::sort.

Which compiler and operating system are you using?

  • Compiler: gcc 10.3.0
  • Operating system: Ubuntu 21.04

Which version of the library did you use?

  • latest release version 3.10.3
  • other release - please state the version: ___
  • the develop branch

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

  • yes
  • no - please copy/paste the error message below
@nlohmann
Copy link
Owner

I can confirm the issue. I addition, there seem to be also an issue when JSON_DIAGNOSTICS.

@nlohmann
Copy link
Owner

Interestingly, the error disappears when each object stores a different integer. I further could reproduce it with jsons_count=7.

@gregmarr
Copy link
Contributor

gregmarr commented Oct 14, 2021

You have an error in your comparator:

https://en.cppreference.com/w/cpp/algorithm/sort

comparison function object (i.e. an object that satisfies the requirements of Compare) which returns ​true if the first argument is less than (i.e. is ordered before) the second.

You are using <= when it requires <.

@nlohmann
Copy link
Owner

Good catch, as always @gregmarr. I haven't read the fine print...

@nlohmann nlohmann added solution: invalid the issue is not related to the library solution: proposed fix a fix for the issue has been proposed and waits for confirmation and removed kind: bug confirmed labels Oct 14, 2021
@crabster
Copy link
Author

Thank you guys and sorry for bothering you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solution: invalid the issue is not related to the library solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

No branches or pull requests

3 participants