jmespath::search() segfault on valid query #343
Comments
mtmorgan
changed the title
|
Thanks for the detailed report. I've added your test case to the JMESPath test suite, but was unable to reproduce the [
{
"collection": "foo",
"dataset": ["bar"],
"tissue": []
},
{
"collection": "foo1",
"dataset": ["bar1"],
"tissue": []
}
]I don't think the |
|
Thanks, i am able to run the examples on the jsoncons/jmespath page and have generally been using your library over the last several days without running into problems. Also, I can run my example on other platforms, so I guess this is something unique to my OS or configuration. I'll try to narrow this down but any suggestions welcome. |
|
Okay, if you can make the example the simplest possible that still produces the |
|
I meant for the 'tissue/tissues' in the data and query to agree; the segfault occurs whether they do or not but the following has both as 'tissue' A single object in the outer array if there are two object And one weird observation: #include <jsoncons/json.hpp>
#include <jsoncons_ext/jmespath/jmespath.hpp>
using jsoncons::json;
namespace jmespath = jsoncons::jmespath;
int main()
{
std::string data;
std::string path;
json j, result;
path = R"(
[].{
collection: name,
dataset: datasets[].name,
tissue: datasets[].tissue[*].label
}
)";
data = R"(
[{
"name" : "foo",
"datasets" : [
{ "name" : "bar", "tissue" : [ { "label" : "baz" }] }
]
}]
)";
j = json::parse(data);
result = jmespath::search(j, path);
std::cout << pretty_print(result) << "\n\n";
data = R"(
[{
"name" : "foo",
"datasets" : [
{ "name" : "bar", "tissue" : [ { "label" : "baz" }] }
]
},
{
"name" : "foo1",
"datasets" : [
{ "name" : "bar1", "tissue" : [ { "label" : "baz1" }] }
]
}]
)";
j = json::parse(data);
result = jmespath::search(j, path);
std::cout << pretty_print(result) << "\n\n";
return 0;
}succeeds (correct answer; no segfault)! |
Maybe not that weird. The stack trace does suggest an order of destruction issue, and these changes could affect that. Could you do the following experiment? Declare data and path as before, I don't think they matter. Then try these combinations separately: Thanks, |
|
Either order worked, including on repeated compilations / runs. I tried to work back from the code in my second comment #343 (comment) to the code in my first #343 (comment). I removed the first query, and moved the data outside main. #include <jsoncons/json.hpp>
#include <jsoncons_ext/jmespath/jmespath.hpp>
using jsoncons::json;
namespace jmespath = jsoncons::jmespath;
std::string data = R"(
[{
"name" : "foo",
"datasets" : [
{ "name" : "bar", "tissue" : [ { "label" : "baz" }] }
]
},
{
"name" : "foo1",
"datasets" : [
{ "name" : "bar1", "tissue" : [ { "label" : "baz1" }] }
]
}]
)";
int main()
{
std::string path;
json j, result;
path = R"(
[].{
collection: name,
dataset: datasets[].name,
tissue: datasets[].tissue[*].label
}
)";
j = json::parse(data);
result = jmespath::search(j, path);
std::cout << pretty_print(result) << "\n\n";
return 0;
}and everything is fine. I don't like the data indentation and clean it up as std::string data = R"(
[{
"name" : "foo",
"datasets" : [
{ "name" : "bar", "tissue" : [ { "label" : "baz" }] }
]
},
{
"name" : "foo1",
"datasets" : [
{ "name" : "bar1", "tissue" : [ { "label" : "baz1" }] }
]
}]
)";and I am back to consistent segfaults. Also, until this session I had thought this was deterministic -- either a segfault or not. But it seemed like I hit configurations (whether or not the three lines were commented out?) where it seemed like there were different behaviors for the same input. Honestly though I wasn't able to keep results straight. |
|
... I realize this is a little quixotic without reproducibility on your end, so thanks for the patience. I'll confirm that the segfault is not consistent -- no segfaults this morning. I ran the segfaulting code from the previous comment under valgrind and offer the following but with caveats
|
|
If the criterion for minimal reproducible example is generation of valgrind warnings, then we have query with nested #include <jsoncons/json.hpp>
#include <jsoncons_ext/jmespath/jmespath.hpp>
using jsoncons::json;
namespace jmespath = jsoncons::jmespath;
std::string data = R"([{ "datasets" : [] }])";
int main()
{
std::string path = R"([].{ dataset: datasets[]})";
json j = json::parse(data);
json result = jmespath::search(j, path);
return 0;
}The valgrind trace suggests that the error occurs when |
|
If I make this change jsoncons master$ git diff
diff --git a/include/jsoncons/basic_json.hpp b/include/jsoncons/basic_json.hpp
index 30e63dc75..df311fd7e 100644
--- a/include/jsoncons/basic_json.hpp
+++ b/include/jsoncons/basic_json.hpp
@@ -880,7 +880,7 @@ namespace jsoncons {
void destroy() noexcept
{
object_allocator alloc(ptr_->get_allocator());
- std::allocator_traits<object_allocator>::destroy(alloc, type_traits::to_plain_pointer(ptr_));
+ // std::allocator_traits<object_allocator>::destroy(alloc, type_traits::to_plain_pointer(ptr_));
std::allocator_traits<object_allocator>::deallocate(alloc, ptr_,1);
}
};my problems (and the valgrind invalid reads) go away, but I don't have a deep enough understanding of C++ or jsoncons to know what the right solution is... Probably this introduces a memory leak... |
|
Yes, removing that line would cause a memory leak. The issue has to be elsewhere. The valgrind results and the stack trace you initially reported are consistent, the issue is happening in these lines of code dynamic_resources<Json,JsonReference> dynamic_storage;
return deep_copy(*evaluate_tokens(doc, output_stack_, dynamic_storage, ec));in the destructor of I need to think about what could possible go wrong there. Daniel |
|
When you have a moment, could you do the following experiment, with the code from master, and your minimal example: #include <jsoncons/json.hpp>
#include <jsoncons_ext/jmespath/jmespath.hpp>
using jsoncons::json;
namespace jmespath = jsoncons::jmespath;
std::string data = R"([{ "datasets" : [] }])";
int main()
{
std::string path = R"([].{ dataset: datasets[]})";
json j = json::parse(data);
json result = jmespath::search(j, path);
return 0;
}In the include file and line 647, like this ~json_object() noexcept
{
// destroy();
}See if you still have valgrind warnings. If this compiles without warnings, uncomment line 199 and try again. Thanks, |
|
I guess that's line 109, and not 199. valgrind does not produce warnings when 109 & 647 are commented out, and does not produce warnings when only 647 is commented out. |
|
Right 109 :-) Can you now take the latest code from master and see if you still get valgrind warnings. Thanks, |
|
The latest code from master does not generate valgrind warnings! |
|
Okay, one last test, on line 1222 of {
Json tempitem;
tempitem.swap(kv.value());
temp.emplace_back(std::move(tempitem));
//temp.emplace_back(std::move(kv.value()));
}Could you change that to {
//Json tempitem;
//tempitem.swap(kv.value());
//temp.emplace_back(std::move(tempitem));
temp.emplace_back(std::move(kv.value()));
}(comment out the first three lines, and uncomment the last) and run the test again? Thanks, |
|
Still no valgrind warnings, with those changes in place. |
|
Good! I think this fixes the issue, I can tell where the issue was arising, and how to avoid it, but I still don't understand why. Those "destroy" functions exist to flatten json array and object structures before their elements are destroyed, for deeply nested structures, it isn't safe to destroy them recursively. As such, they only need to be applied to elements that are non-empty array and object types, and the original code was applying them unnecessarily to one other kind of value (which is used by jmespath.) However, that should have been a no-op, as it appears to have been in all other environments besides your's. Daniel |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
For this json
[ { "name" : "foo", "datasets" : [ { "name" : "bar", "tissues" : [ { "label" : "baz" }] } ] }, { "name" : "foo1", "datasets" : [ { "name" : "bar1", "tissues" : [ { "label" : "baz1" }] } ] } ]the JMESpath query
should return a valid object, instead it segfaults with
Enumerate the steps to reproduce the bug
Here's a small program demonstrating this
compiled and executed as
Under lldb
What compiler, architecture, and operating system?
What jsoncons library version?
The text was updated successfully, but these errors were encountered: