Segfault on nested parsing #778
Comments
|
I cannot reproduce this with the following code running Xcode 9.0 with Address Sanitizer and Undefined Behavior Sanitizer: #include "json.hpp"
#include <iostream>
#include <unordered_map>
using json = nlohmann::json;
struct Foo { int x; bool y; };
void from_json(const json& j, Foo& f) {
f.x = j.at("x").get<int>();
try {
f.y = j.at("y");
} catch(...) {
f.y = false;
}
}
int main()
{
auto j = json::parse(R"(
{ "m": {"one": {"x": 1},
"two": {"x": 2, "y": false }},
"s": "arg"}
)");
std::unordered_map<std::string, Foo> m = j.at("m");
for (auto el : m)
{
std::cout << el.first << " -> (" << el.second.x << ","
<< std::boolalpha << el.second.y << ")" << std::endl;
}
}Maybe there is something wrong with your |
|
Sorry, why the try/catch around y? If you remove that, does it make a difference? I am generating the from_json based on reflection, but I'll try to write it out explicitly and see if I can reproduce. |
|
In your example, the object at key |
|
Okay, here's what I have now: Once again, this segfaults for me. This is run in the context of a gtest suite but it's hard to believe that could be making the difference. And since your library is header only, we couldnt' have built it incorrectly locally. |
|
Below is the backtrace (collapsed cause its huge). Backtrace |
|
As @nlohmann said above, the object at key |
|
@gregmarr An exception would not cause a segfault running inside of a gtest. I've also just experience this in an executable I wrote where everything was wrapped in a |
|
Are you saying that gtest should be catching that |
|
@gregmarr gtest tests catch everything, yes; it would be bad if a single exception in a single test segfaulted and stopped the rest of the tests from running, right? And I also tested it in an application. I can test it in a simple main as well, just a little more involved for me, but I'll try that now. |
|
@gregmarr this program segfaults, even though it clearly shouldn't: Interestingly I tried with gcc (6.3, 5.4) and clang (3.7), it doesn't segfault with clang. |
|
Can you try replacing |
|
If I change it to auto then it does work. I'm not sure what the type of |
|
It could be that there's two exceptions getting thrown. That would force a std::terminate even with a outer try-catch. I haven't been able to find where that could happen in the example, but just throwing the idea out there in case someone else can? |
|
Can someone maybe try to use the exact code as me, and gcc, and try to reproduce? |
|
There's an abort in the stdlib: The type of |
|
So there's this function in the call stack: So it's trying to construct the std::unordered_map<std::string, Blub> using this constructor: with |
|
I'll bet what's needed here is a proper |
This is the |
|
While this does solve the problem, this is boilerplate to put on the user
and this wouldn't be a good choice for the library. Incomplete json dict
trying to get serialized to a type is a very common failure mode, and
exception is way to handle failures, so the library should default to
cleanly propagating that exception.
What's not clear is whether an input iterator is allowed to throw. If it
is, this is just a standard library bug. If not, json should first
construct a vector (with reserve and push back), and then pass move
iterators from the vector into the map. Minimally more allocations for much
more correct behavior.
…On Oct 11, 2017 7:48 PM, "gregmarr" ***@***.***> wrote:
I'll bet what's needed here is a proper from_json function for
std::unordered_map<std::string, Blub>.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#778 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AJ-QCPYcIIXTr7_VpomUR4U5ddWqirpjks5srVPVgaJpZM4P11Py>
.
|
|
Can you try with this function: |
|
I didn't, but I think it's pretty clear that won't cause the problem. However, I did manage to write up the following json agnostic example: https://wandbox.org/permlink/G5u9xZWCvsQLeRvz. This segfaults in gcc6.3 but not in 7 series. My readings on the matter seem to indicate that this is a standard library bug; its not safe to assume that iterators can't throw. I'm going to do one more test to make sure that json does the expected thing with gcc7 and then I think we can probably close it as an upstream issue. |
|
This tells me that this is exactly the problem. We KNOW that the |
|
A correction on my previous comment, it's actually gcc 8 seemingly that fixes this bug. Is it possible to easily access json on wandbox somehow? Or if someone here has local access to gcc 8, maybe they could test? @gregmarr I agree it's a valid workaround, that is clear. But that's what it is: a workaround. Writing At any rate it seems unlikely that there's still an issue present since with clang code works as expected, and gcc8 is a pain to try and test with, so I'll close this for now. |
|
This has nothing to do with |
|
@gregmarr No data will be missed, it throws an exception. That's the whole point, that's the error handling mechanism. That exception will bubble up. If you simply try to de-serialize a What's more, there isn't really any other way to handle errors. |
|
Ah, so you're saying that all the fields are in fact required, and you're not expecting to handle the case you've given. I thought you actually wanted to read the sample file. Thanks for the clarification. |
It's a bit difficult to provide a full repro here as it would mean tearing out a bunch of code, but the gist of the bug is as follows:
The last line should throw IMHO, but in fact it segfaults. I think I'm on version 2.1.1.
The text was updated successfully, but these errors were encountered: