sheredom / json.h

🗄️ single header json parser for C and C++
The Unlicense
698 stars 77 forks source link

json.h under TrustInSoft CI #74

Closed jakub-zwolakowski closed 3 years ago

jakub-zwolakowski commented 3 years ago

Hi,

I initiated the configuration of json.h on the new tool TrustInSoft CI. It's a source code analyzer, which analyzes execution paths (usually unit tests in your repo) to detect Undefined Behaviors along the way. Coverage includes all the defects that sanitizers ASAN, UBSan and MSan find, plus a large number of other (usually subtle) defects.

I've set up TrustInSoft CI on your test suite located in the test directory (using test/main.cpp as the entry point) and I've added tests from the JSON Parsing Test Suite (318 parsing tests + 22 transform tests = 340 tests total). Each test was run 4 times to emulate 4 different architectures: x86 32-bit, x86 64-bit, PPC 32-bit, and PPC 64-bit. You can check the results here.

The tool has proved the absence of Undefined Behaviors on all the analyzed execution paths. Since it relies on formal methods, it is able to detect the most subtle Undefined Behaviors, so these results are particularly impressive!

On the other hand I have noticed that 14 test cases from the JSON Parsing Test Suite give unexpected results: invalid JSON is parsed successfully.

Would you mind taking a look?

As I've set up TrustInSoft CI on my fork, one next step would be that you try it out in a continuous integration mode.

May I ask if this is something that you'd be interested in?

The setup consists in writing a configuration file. I can put my initial setup in a PR if you wish me to.

Thanks!

sheredom commented 3 years ago

Firstly - this is awesome! Thanks for doing these checks 😄!

So I can take a look at the unexpected results from the JSON Parsing Test Suite, I'll put it on the list of work.

I cannot add TrustInSoft CI because it asks for permissions I cannot give (access to organizations that I honestly can't see why it needs tbh).

jakub-zwolakowski commented 3 years ago

I cannot add TrustInSoft CI because it asks for permissions I cannot give (access to organizations that I honestly can't see why it needs tbh).

Hmm, I think that you should not be required to give access to your organization(s) to TIS-CI. Access to your personal account where the json.h project is should be enough. Could you please try to give only these permissions and tell me whether it works?

jakub-zwolakowski commented 3 years ago

Hi again!

Have you checked if you are able to proceed with TrustInSoft CI without giving the access to your organizations?

If doesn't work, and this problem is stopping you from testing the tool, it's a valuable feedback. Please let me know and we'll do our best to fix it :)

sheredom commented 3 years ago

Haven't had a chance yet to test it again sorry!

sheredom commented 3 years ago

Hey there.

So I will not be adding TrustInSoft to my CI - its unclear to me from the outset the pricing strategy for it, and honestly I just don't want to maintain yet another CI on my project.

I did take the opportunity to run the JSONTestSuite tests against my library and I've fixed a bunch of bugs, and I've also added santizers more generally to my testing to help catch these issues in the future.

Thanks for the interest in my library!