Some cleanup for restrict function pointer by name feature

danpoe · danpoe · commit f55f636d4a45 · 2020-03-16T16:09:49.000Z
diff --git a/src/goto-instrument/goto_instrument_parse_options.cpp b/src/goto-instrument/goto_instrument_parse_options.cpp
@@ -1038,18 +1038,15 @@ void goto_instrument_parse_optionst::instrument_goto_program()
     parse_function_pointer_restriction_options_from_cmdline(cmdline, options);
 
     if(
-      !options.get_list_option(RESTRICT_FUNCTION_POINTER_OPT).empty() ||
-      !options.get_list_option(RESTRICT_FUNCTION_POINTER_BY_NAME_OPT).empty() ||
-      !options.get_list_option(RESTRICT_FUNCTION_POINTER_FROM_FILE_OPT).empty())
+      options.is_set(RESTRICT_FUNCTION_POINTER_OPT) ||
+      options.is_set(RESTRICT_FUNCTION_POINTER_BY_NAME_OPT) ||
+      options.is_set(RESTRICT_FUNCTION_POINTER_FROM_FILE_OPT))
     {
       label_function_pointer_call_sites(goto_model);
 
-      auto const by_name_restrictions =
-        get_function_pointer_by_name_restrictions(goto_model, options);
-
       const auto function_pointer_restrictions =
-        by_name_restrictions.merge(function_pointer_restrictionst::from_options(
-          options, log.get_message_handler()));
+        function_pointer_restrictionst::from_options(
+          options, goto_model, log.get_message_handler());
 
       restrict_function_pointers(goto_model, function_pointer_restrictions);
     }
diff --git a/src/goto-programs/restrict_function_pointers.cpp b/src/goto-programs/restrict_function_pointers.cpp
@@ -215,15 +215,26 @@ void parse_function_pointer_restriction_options_from_cmdline(
   const cmdlinet &cmdline,
   optionst &options)
 {
-  options.set_option(
-    RESTRICT_FUNCTION_POINTER_OPT,
-    cmdline.get_values(RESTRICT_FUNCTION_POINTER_OPT));
-  options.set_option(
-    RESTRICT_FUNCTION_POINTER_FROM_FILE_OPT,
-    cmdline.get_values(RESTRICT_FUNCTION_POINTER_FROM_FILE_OPT));
-  options.set_option(
-    RESTRICT_FUNCTION_POINTER_BY_NAME_OPT,
-    cmdline.get_values(RESTRICT_FUNCTION_POINTER_BY_NAME_OPT));
+  if(cmdline.isset(RESTRICT_FUNCTION_POINTER_OPT))
+  {
+    options.set_option(
+      RESTRICT_FUNCTION_POINTER_OPT,
+      cmdline.get_values(RESTRICT_FUNCTION_POINTER_OPT));
+  }
+
+  if(cmdline.isset(RESTRICT_FUNCTION_POINTER_FROM_FILE_OPT))
+  {
+    options.set_option(
+      RESTRICT_FUNCTION_POINTER_FROM_FILE_OPT,
+      cmdline.get_values(RESTRICT_FUNCTION_POINTER_FROM_FILE_OPT));
+  }
+
+  if(cmdline.isset(RESTRICT_FUNCTION_POINTER_BY_NAME_OPT))
+  {
+    options.set_option(
+      RESTRICT_FUNCTION_POINTER_BY_NAME_OPT,
+      cmdline.get_values(RESTRICT_FUNCTION_POINTER_BY_NAME_OPT));
+  }
 }
 
 function_pointer_restrictionst::restrictionst
@@ -249,17 +260,17 @@ function_pointer_restrictionst::merge_function_pointer_restrictions(
 }
 
 function_pointer_restrictionst::restrictionst
-parse_function_pointer_restrictions_from_command_line(
+function_pointer_restrictionst::parse_function_pointer_restrictions(
   const std::list<std::string> &restriction_opts,
-  const std::string &option_name)
+  const std::string &option)
 {
   auto function_pointer_restrictions =
     function_pointer_restrictionst::restrictionst{};
 
   for(const std::string &restriction_opt : restriction_opts)
   {
     const auto restriction =
-      parse_function_pointer_restriction(restriction_opt);
+      parse_function_pointer_restriction(restriction_opt, option);
 
     const bool inserted = function_pointer_restrictions
                             .emplace(restriction.first, restriction.second)
@@ -270,23 +281,34 @@ parse_function_pointer_restrictions_from_command_line(
       throw invalid_command_line_argument_exceptiont{
         "function pointer restriction for `" + id2string(restriction.first) +
           "' was specified twice",
-        "--" RESTRICT_FUNCTION_POINTER_OPT};
+        option};
     }
   }
 
   return function_pointer_restrictions;
 }
 
+
+function_pointer_restrictionst::restrictionst
+function_pointer_restrictionst::
+parse_function_pointer_restrictions_from_command_line(
+  const std::list<std::string> &restriction_opts)
+{
+  return parse_function_pointer_restrictions(
+    restriction_opts,
+    "--" RESTRICT_FUNCTION_POINTER_OPT);
+}
+
 function_pointer_restrictionst::restrictionst
 function_pointer_restrictionst::parse_function_pointer_restrictions_from_file(
   const std::list<std::string> &filenames,
   message_handlert &message_handler)
 {
   auto merged_restrictions = function_pointer_restrictionst::restrictionst{};
+
   for(auto const &filename : filenames)
   {
-    auto const restrictions =
-      function_pointer_restrictionst::read_from_file(filename, message_handler);
+    auto const restrictions = read_from_file(filename, message_handler);
 
     merged_restrictions = merge_function_pointer_restrictions(
       std::move(merged_restrictions), restrictions.restrictions);
@@ -297,7 +319,8 @@ function_pointer_restrictionst::parse_function_pointer_restrictions_from_file(
 
 function_pointer_restrictionst::restrictiont
 function_pointer_restrictionst::parse_function_pointer_restriction(
-  const std::string &restriction_opt)
+  const std::string &restriction_opt,
+  const std::string &option)
 {
   // the format for restrictions is <pointer_name>/<target[,more_targets]*>
   // exactly one pointer and at least one target
@@ -310,23 +333,23 @@ function_pointer_restrictionst::parse_function_pointer_restriction(
   {
     throw invalid_command_line_argument_exceptiont{
       "couldn't find '/' in `" + restriction_opt + "'",
-      "--" RESTRICT_FUNCTION_POINTER_OPT,
+      option,
       restriction_format_message};
   }
 
   if(pointer_name_end == restriction_opt.size())
   {
     throw invalid_command_line_argument_exceptiont{
       "couldn't find names of targets after '/' in `" + restriction_opt + "'",
-      "--" RESTRICT_FUNCTION_POINTER_OPT,
+      option,
       restriction_format_message};
   }
 
   if(pointer_name_end == 0)
   {
     throw invalid_command_line_argument_exceptiont{
       "couldn't find target name before '/' in `" + restriction_opt + "'",
-      "--" RESTRICT_FUNCTION_POINTER_OPT};
+      option};
   }
 
   auto const pointer_name = restriction_opt.substr(0, pointer_name_end);
@@ -338,7 +361,7 @@ function_pointer_restrictionst::parse_function_pointer_restriction(
   {
     throw invalid_command_line_argument_exceptiont{
       "missing target list for function pointer restriction " + pointer_name,
-      "--" RESTRICT_FUNCTION_POINTER_OPT,
+      option,
       restriction_format_message};
   }
 
@@ -351,7 +374,7 @@ function_pointer_restrictionst::parse_function_pointer_restriction(
     {
       throw invalid_command_line_argument_exceptiont(
         "leading or trailing comma in restrictions for `" + pointer_name + "'",
-        "--" RESTRICT_FUNCTION_POINTER_OPT,
+        option,
         restriction_format_message);
     }
   }
@@ -361,18 +384,27 @@ function_pointer_restrictionst::parse_function_pointer_restriction(
 
 function_pointer_restrictionst function_pointer_restrictionst::from_options(
   const optionst &options,
+  const goto_modelt &goto_model,
   message_handlert &message_handler)
 {
   auto const restriction_opts =
     options.get_list_option(RESTRICT_FUNCTION_POINTER_OPT);
   auto const commandline_restrictions =
-    parse_function_pointer_restrictions_from_command_line(
-      restriction_opts, RESTRICT_FUNCTION_POINTER_OPT);
+    parse_function_pointer_restrictions_from_command_line(restriction_opts);
+
+  auto const restriction_file_opts =
+    options.get_list_option(RESTRICT_FUNCTION_POINTER_FROM_FILE_OPT);
   auto const file_restrictions = parse_function_pointer_restrictions_from_file(
-    options.get_list_option(RESTRICT_FUNCTION_POINTER_FROM_FILE_OPT),
-    message_handler);
+    restriction_file_opts, message_handler);
+
+  auto const restriction_name_opts =
+    options.get_list_option(RESTRICT_FUNCTION_POINTER_BY_NAME_OPT);
+  auto const name_restrictions = get_function_pointer_by_name_restrictions(
+    restriction_name_opts, goto_model);
+
   return {merge_function_pointer_restrictions(
-    std::move(file_restrictions), commandline_restrictions)};
+    commandline_restrictions,
+    merge_function_pointer_restrictions(file_restrictions, name_restrictions))};
 }
 
 function_pointer_restrictionst
@@ -465,21 +497,16 @@ void function_pointer_restrictionst::write_to_file(
 
   function_pointer_restrictions_json.output(outFile);
 }
-function_pointer_restrictionst function_pointer_restrictionst::merge(
-  const function_pointer_restrictionst &other) const
-{
-  return function_pointer_restrictionst{
-    merge_function_pointer_restrictions(restrictions, other.restrictions)};
-}
 
-function_pointer_restrictionst get_function_pointer_by_name_restrictions(
-  const goto_modelt &goto_model,
-  const optionst &options)
+function_pointer_restrictionst::restrictionst
+function_pointer_restrictionst::get_function_pointer_by_name_restrictions(
+  const std::list<std::string> &restriction_name_opts,
+  const goto_modelt &goto_model)
 {
   function_pointer_restrictionst::restrictionst by_name_restrictions =
-    parse_function_pointer_restrictions_from_command_line(
-      options.get_list_option(RESTRICT_FUNCTION_POINTER_BY_NAME_OPT),
-      RESTRICT_FUNCTION_POINTER_BY_NAME_OPT);
+    parse_function_pointer_restrictions(
+      restriction_name_opts, RESTRICT_FUNCTION_POINTER_BY_NAME_OPT);
+
   function_pointer_restrictionst::restrictionst restrictions;
   for(auto const &goto_function : goto_model.goto_functions.function_map)
   {
@@ -520,5 +547,6 @@ function_pointer_restrictionst get_function_pointer_by_name_restrictions(
         }
       });
   }
-  return function_pointer_restrictionst{restrictions};
+
+  return restrictions;
 }
diff --git a/src/goto-programs/restrict_function_pointers.h b/src/goto-programs/restrict_function_pointers.h
@@ -66,12 +66,11 @@ class function_pointer_restrictionst
 
   const restrictionst restrictions;
 
-  /// parse function pointer restrictions from command line
-  ///
-  /// Note: These are are only syntactically checked at this stage,
-  ///       because type checking them requires a goto_modelt
-  static function_pointer_restrictionst
-  from_options(const optionst &options, message_handlert &message_handler);
+  /// Parse function pointer restrictions from command line
+  static function_pointer_restrictionst from_options(
+    const optionst &options,
+    const goto_modelt &goto_model,
+    message_handlert &message_handler);
 
   jsont to_json() const;
   static function_pointer_restrictionst from_json(const jsont &json);
@@ -82,9 +81,6 @@ class function_pointer_restrictionst
 
   void write_to_file(const std::string &filename) const;
 
-  function_pointer_restrictionst
-  merge(const function_pointer_restrictionst &other) const;
-
 protected:
   static restrictionst merge_function_pointer_restrictions(
     restrictionst lhs,
@@ -97,13 +93,18 @@ class function_pointer_restrictionst
     const std::list<std::string> &filenames,
     message_handlert &message_handler);
 
-  static restrictiont
-  parse_function_pointer_restriction(const std::string &restriction_opt);
-};
+  static restrictionst parse_function_pointer_restrictions(
+    const std::list<std::string> &restriction_opts,
+    const std::string &option);
 
-function_pointer_restrictionst get_function_pointer_by_name_restrictions(
-  const goto_modelt &goto_model,
-  const optionst &options);
+  static restrictiont parse_function_pointer_restriction(
+    const std::string &restriction_opt,
+    const std::string &option);
+
+  static restrictionst get_function_pointer_by_name_restrictions(
+    const std::list<std::string> &restriction_name_opts,
+    const goto_modelt &goto_model);
+};
 
 /// Apply function pointer restrictions to a goto_model. Each restriction is a
 /// mapping from a pointer name to a set of possible targets. Replace calls of
diff --git a/unit/goto-programs/restrict_function_pointers.cpp b/unit/goto-programs/restrict_function_pointers.cpp
@@ -23,43 +23,47 @@ void restriction_parsing_test()
 {
   {
     const auto res =
-      fp_restrictionst::parse_function_pointer_restriction("func1/func2");
+      fp_restrictionst::parse_function_pointer_restriction(
+        "func1/func2", "test");
     REQUIRE(res.first == "func1");
     REQUIRE(res.second.size() == 1);
     REQUIRE(res.second.find("func2") != res.second.end());
   }
 
   {
     const auto res =
-      fp_restrictionst::parse_function_pointer_restriction("func1/func2,func3");
+      fp_restrictionst::parse_function_pointer_restriction(
+        "func1/func2,func3", "test");
     REQUIRE(res.first == "func1");
     REQUIRE(res.second.size() == 2);
     REQUIRE(res.second.find("func2") != res.second.end());
     REQUIRE(res.second.find("func3") != res.second.end());
   }
 
   REQUIRE_THROWS_AS(
-    fp_restrictionst::parse_function_pointer_restriction("func"),
+    fp_restrictionst::parse_function_pointer_restriction("func", "test"),
     invalid_command_line_argument_exceptiont);
 
   REQUIRE_THROWS_AS(
-    fp_restrictionst::parse_function_pointer_restriction("/func"),
+    fp_restrictionst::parse_function_pointer_restriction("/func", "test"),
     invalid_command_line_argument_exceptiont);
 
   REQUIRE_THROWS_AS(
-    fp_restrictionst::parse_function_pointer_restriction("func/"),
+    fp_restrictionst::parse_function_pointer_restriction("func/", "test"),
     invalid_command_line_argument_exceptiont);
 
   REQUIRE_THROWS_AS(
-    fp_restrictionst::parse_function_pointer_restriction("func/,"),
+    fp_restrictionst::parse_function_pointer_restriction("func/,", "test"),
     invalid_command_line_argument_exceptiont);
 
   REQUIRE_THROWS_AS(
-    fp_restrictionst::parse_function_pointer_restriction("func1/func2,"),
+    fp_restrictionst::parse_function_pointer_restriction(
+      "func1/func2,", "test"),
     invalid_command_line_argument_exceptiont);
 
   REQUIRE_THROWS_AS(
-    fp_restrictionst::parse_function_pointer_restriction("func1/,func2"),
+    fp_restrictionst::parse_function_pointer_restriction(
+      "func1/,func2", "test"),
     invalid_command_line_argument_exceptiont);
 }
 
