Skip to content

JSON cmdline interface #4585

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions regression/cbmc/json-interface1/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#include <assert.h>

void foo(int x)
{
for(int i = 0; i < 5; ++i)
{
if(x)
assert(0);
assert(0);
}
assert(0);
}
12 changes: 12 additions & 0 deletions regression/cbmc/json-interface1/test.desc
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
CORE broken-smt-backend
--json-interface
< test.json
activate-multi-line-match
^EXIT=10$
^SIGNAL=0$
Not unwinding loop foo\.0 iteration 3 file main\.c line 5 function foo thread 0
\{\n\s*"description": "assertion 0",\n\s*"property": "foo\.assertion\.(1|3)",\n\s*"status": "SUCCESS"\n\s*\}
\{\n\s*"description": "assertion 0",\n\s*"property": "foo\.assertion\.(1|3)",\n\s*"status": "FAILURE",\n\s*"trace": \[
VERIFICATION FAILED
--
"property": "foo\.assertion\.2"
15 changes: 15 additions & 0 deletions regression/cbmc/json-interface1/test.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"arguments": [
"main.c"
],
"options": {
"function": "foo",
"unwind": 3,
"property": [
"foo.assertion.1",
"foo.assertion.3"
],
"trace": true,
"show-properties": false
}
}
4 changes: 4 additions & 0 deletions src/cbmc/cbmc_parse_options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ Author: Daniel Kroening, [email protected]

#include <langapi/mode.h>

#include <json/json_interface.h>
#include <xmllang/xml_interface.h>

#include "c_test_input_generator.h"
Expand All @@ -90,6 +91,7 @@ cbmc_parse_optionst::cbmc_parse_optionst(int argc, const char **argv)
argv,
std::string("CBMC ") + CBMC_VERSION)
{
json_interface(cmdline, ui_message_handler);
xml_interface(cmdline, ui_message_handler);
}

Expand All @@ -103,6 +105,7 @@ ::cbmc_parse_optionst::cbmc_parse_optionst(
argv,
std::string("CBMC ") + CBMC_VERSION)
{
json_interface(cmdline, ui_message_handler);
xml_interface(cmdline, ui_message_handler);
}

Expand Down Expand Up @@ -1021,6 +1024,7 @@ void cbmc_parse_optionst::help()
" --xml-ui use XML-formatted output\n"
" --xml-interface bi-directional XML interface\n"
" --json-ui use JSON-formatted output\n"
" --json-interface bi-directional JSON interface\n"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually bi-directional?! Doesn't it only become "bi"directional when using both --json-interface and --json-ui? (The same is of course true for XML.)

Copy link
Member Author

@peterschrammel peterschrammel Apr 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering about that and have now more investigated in the XML case (see comments there). The original intention was that --xml-interface subsumes --xml-ui.
Actually, the current behaviour was broken because the UI was set before the option could have been parsed from stdin, which seems difficult to fix without splitting the parse_optionst constructor into two phases and adding a set_ui to ui_message_handlert (rather no).
Also, the question is what happens with errors during parsing the options - they should already be output in a proper format. Thus, I'd rather stick with the original intention of 'bi-directional',

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I fully understand - I agree that --{json,xml}-interface should subsume --{json,xml}-ui - but I don't think that was actually work as such? If things can be made to work as such then we'd have the desired bi-directional interface.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--json-interface now implies --json-ui, and --xml-interface now implies --xml-ui.

but I don't think that was actually work as such?

Not sure, I understand...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me retry, my apologies for completely broken English:

  1. A "bi"-directional interface is about input and output.
  2. --{json,xml}-ui set up the output side of an interface.
  3. The comment claims that --{json,xml}-interface configure a bi-directional interface.
  4. The actual implementation, however, only reads input when using --{json,xml}-interface.
  5. One possible fix would be that upon seeing --{json,xml}-interface we also set the option for --{json,xml}-ui.
  6. This might not actually work as it could be too late, in which case a more extensive change would be required. If this is the case, a simple fix would be to change the comment such as not to promise a bi-directional interface. Users wanting to use a bi-directional interface would then need to provide both --{json,xml}-interface and --{json,xml}-ui.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. yes
  2. yes
  3. yes
  4. yes
  5. is being fixed here (was broken)
  6. not required because 5 works.

// NOLINTNEXTLINE(whitespace/line_length)
HELP_VALIDATE
HELP_GOTO_TRACE
Expand Down
4 changes: 3 additions & 1 deletion src/cbmc/cbmc_parse_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ class optionst;
"(object-bits):" \
OPT_GOTO_CHECK \
"(no-assertions)(no-assumptions)" \
"(xml-ui)(xml-interface)(json-ui)" \
"(xml-ui)(xml-interface)" \
"(json-interface)" \
"(json-ui)" \
"(smt1)(smt2)(fpa)(cvc3)(cvc4)(boolector)(yices)(z3)(mathsat)" \
"(cprover-smt2)" \
"(no-sat-preprocessor)" \
Expand Down
1 change: 1 addition & 0 deletions src/cbmc/module_dependencies.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ goto-instrument
goto-programs
goto-symex
jsil
json
json-symtab-language
langapi # should go away
linking
Expand Down
3 changes: 2 additions & 1 deletion src/json/Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
SRC = json_lex.yy.cpp \
SRC = json_interface.cpp \
json_lex.yy.cpp \
json_parser.cpp \
json_y.tab.cpp \
# Empty last line
Expand Down
120 changes: 120 additions & 0 deletions src/json/json_interface.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
/*******************************************************************\

Module: JSON Commandline Interface

Author: Peter Schrammel

\*******************************************************************/

/// \file
/// JSON Commandline Interface

#include "json_interface.h"

#include <util/cmdline.h>
#include <util/exception_utils.h>
#include <util/json.h>
#include <util/message.h>

#include "json_parser.h"

#include <iostream>

/// Parse commandline options from \p json into \p cmdline
static void get_json_options(const json_objectt &json, cmdlinet &cmdline)
{
const jsont &arguments = json["arguments"];
if(!arguments.is_array())
{
throw invalid_command_line_argument_exceptiont(
"array expected", "'arguments'");
}

for(const auto &argument : to_json_array(arguments))
{
if(!argument.is_string())
{
throw invalid_command_line_argument_exceptiont(
"string expected", "argument");
}

cmdline.args.push_back(argument.value);
}

const jsont &options = json["options"];
if(!options.is_object())
{
throw invalid_command_line_argument_exceptiont(
"array expected", "'options'");
}

for(const auto &option_pair : to_json_object(options))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit pick: add a blank line before this one to be consistent with the code a few lines above.

{
if(option_pair.second.is_string() || option_pair.second.is_number())
{
// e.g. --option x
cmdline.set(option_pair.first, option_pair.second.value);
}
else if(option_pair.second.is_boolean())
{
// e.g. --flag
if(option_pair.second.is_true())
cmdline.set(option_pair.first);
}
else if(option_pair.second.is_array())
{
// e.g. --option x --option y
for(const auto &element : to_json_array(option_pair.second))
{
if(element.is_string())
cmdline.set(option_pair.first, element.value);
else
{
throw invalid_command_line_argument_exceptiont(
"string expected", option_pair.first);
}
}
}
else
{
throw invalid_command_line_argument_exceptiont(
"unrecognized commandline option format",
option_pair.first,
"Boolean, string, number, or string array expected");
}
}
}

void json_interface(cmdlinet &cmdline, message_handlert &message_handler)
{
if(cmdline.isset("json-interface"))
{
jsont json;

parse_json(std::cin, "", message_handler, json);

try
{
if(!json.is_object())
{
throw invalid_command_line_argument_exceptiont(
"JSON object expected at top-level", "command-line JSON input");
}

get_json_options(to_json_object(json), cmdline);

// Add this so that it gets propagated into optionst;
// the ui_message_handlert::uit has already been set on the basis
// of the json-interface flag.
cmdline.set("json-ui");
}
catch(const invalid_command_line_argument_exceptiont &e)
{
messaget log(message_handler);
log.error() << e.what() << messaget::eom;

// make sure we fail with a usage error
cmdline.clear();
}
}
}
40 changes: 40 additions & 0 deletions src/json/json_interface.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*******************************************************************\

Module: JSON Commandline Interface

Author: Peter Schrammel

\*******************************************************************/

/// \file
/// JSON Commandline Interface

#ifndef CPROVER_JSON_JSON_INTERFACE_H
#define CPROVER_JSON_JSON_INTERFACE_H

class cmdlinet;
class message_handlert;

/// Parses the JSON-formatted command line from stdin
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be rather important to document the expected JSON syntax! Otherwise you're bound to trial and error...

///
/// Example:
/// \code{.json}
/// {
/// "arguments": [
/// "main.c"
/// ],
/// "options": {
/// "function": "foo",
/// "unwind": 3,
/// "property": [
/// "foo.assertion.1",
/// "foo.assertion.3"
/// ],
/// "trace": true,
/// "show-properties": false
/// }
/// }
/// \endcode
void json_interface(cmdlinet &, message_handlert &);

#endif // CPROVER_JSON_JSON_INTERFACE_H
5 changes: 5 additions & 0 deletions src/util/json.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ class jsont
return kind==kindt::J_ARRAY;
}

bool is_boolean() const
{
return kind == kindt::J_TRUE || kind == kindt::J_FALSE;
}

bool is_true() const
{
return kind==kindt::J_TRUE;
Expand Down
4 changes: 3 additions & 1 deletion src/util/ui_message.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ ui_message_handlert::ui_message_handlert(
nullptr,
cmdline.isset("xml-ui") || cmdline.isset("xml-interface")
? uit::XML_UI
: cmdline.isset("json-ui") ? uit::JSON_UI : uit::PLAIN,
: cmdline.isset("json-ui") || cmdline.isset("json-interface")
? uit::JSON_UI
: uit::PLAIN,
program,
cmdline.isset("flush"),
cmdline.isset("timestamp") ? cmdline.get_value("timestamp") == "monotonic"
Expand Down