fixes issue 891 (#893)

CONTRIBUTING.md

@@ -32,6 +32,17 @@ We discourage the following types of contributions:
 
 In short, most code changes should either bring new features or better performance. We want to avoid unmotivated code changes.
 
+
+Specific rules
+----------
+
+We have few hard rules, but we have some:
+
+- Printing to standard output or standard error (`stderr`, `stdout`, `std::cerr`, `std::cout`) in the core library is forbidden. This follows from the [Writing R Extensions](https://cran.r-project.org/doc/manuals/R-exts.html) manual which states that "Compiled code should not write to stdout or stderr".
+- Calls to `abort()` are forbidden in the core library. This follows from the [Writing R Extensions](https://cran.r-project.org/doc/manuals/R-exts.html) manual which states that "Under no circumstances should your compiled code ever call abort or exit".
+
+Tools, tests and benchmarks are not held to these same strict rules.
+
 General Guidelines
 ----------
 
@@ -43,7 +54,6 @@ Contributors are encouraged to :
 - Tools may report "problems" with the code, but we never delegate programming to tools: if there is a problem with the code, we need to understand it. Thus we will not "fix" code merely to please a static analyzer if we do not understand.
 - Provide tests for any new feature. We will not merge a new feature without tests.
 
-
 Pull Requests
 --------------
 

include/simdjson/inline/element.h

@@ -293,7 +293,7 @@ inline std::ostream& operator<<(std::ostream& out, element_type type) {
     case element_type::NULL_VALUE:
       return out << "null";
     default:
-      abort();
+      return out << "unexpected content!!!"; // abort() usage is forbidden in the library
   }
 }
 
@@ -405,7 +405,7 @@ inline std::ostream& minify<dom::element>::print(std::ostream& out) {
     case tape_type::END_ARRAY:
     case tape_type::END_OBJECT:
     case tape_type::ROOT:
-      abort();
+      out << "unexpected content!!!"; // abort() usage is forbidden in the library
     }
     iter.json_index++;
     after_value = true;

include/simdjson/inline/parsedjson_iterator.h

@@ -220,7 +220,7 @@ dom::parser::Iterator::Iterator(const dom::parser &pj) noexcept(false)
 #if SIMDJSON_EXCEPTIONS
   if (!pj.valid) { throw simdjson_error(pj.error); }
 #else
-  if (!pj.valid) { abort(); }
+  if (!pj.valid) { return; } //  abort() usage is forbidden in the library
 #endif
 
   max_depth = pj.max_depth();

singleheader/amalgamate_demo.cpp

@@ -1,4 +1,4 @@
-/* auto-generated on Tue May 19 14:39:19 PDT 2020. Do not edit! */
+/* auto-generated on Wed May 20 10:23:07 EDT 2020. Do not edit! */
 
 #include <iostream>
 #include "simdjson.h"

singleheader/simdjson.cpp

@@ -1,4 +1,4 @@
-/* auto-generated on Tue May 19 14:39:19 PDT 2020. Do not edit! */
+/* auto-generated on Wed May 20 10:23:07 EDT 2020. Do not edit! */
 /* begin file src/simdjson.cpp */
 #include "simdjson.h"
 
@@ -569,7 +569,7 @@ const implementation *available_implementation_list::detect_best_supported() con
     uint32_t required_instruction_sets = impl->required_instruction_sets();
     if ((supported_instruction_sets & required_instruction_sets) == required_instruction_sets) { return impl; }
   }
-  return &unsupported_singleton;
+  return &unsupported_singleton; // this should never happen?
 }
 
 const implementation *detect_best_supported_implementation_on_first_use::set_best() const noexcept {
@@ -580,11 +580,12 @@ const implementation *detect_best_supported_implementation_on_first_use::set_bes
 
   if (force_implementation_name) {
     auto force_implementation = available_implementations[force_implementation_name];
-    if (!force_implementation) {
+    if (force_implementation) {
-      fprintf(stderr, "SIMDJSON_FORCE_IMPLEMENTATION environment variable set to '%s', which is not a supported implementation name!\n", force_implementation_name);
+      return active_implementation = force_implementation;
-      abort();
+    } else {
+      // Note: abort() and stderr usage within the library is forbidden.
+      return active_implementation = &unsupported_singleton;
     }
-    return active_implementation = force_implementation;
   }
   return active_implementation = available_implementations.detect_best_supported();
 }

singleheader/simdjson.h

@@ -1,4 +1,4 @@
-/* auto-generated on Tue May 19 14:39:19 PDT 2020. Do not edit! */
+/* auto-generated on Wed May 20 10:23:07 EDT 2020. Do not edit! */
 /* begin file include/simdjson.h */
 #ifndef SIMDJSON_H
 #define SIMDJSON_H
@@ -5552,7 +5552,7 @@ inline std::ostream& operator<<(std::ostream& out, element_type type) {
     case element_type::NULL_VALUE:
       return out << "null";
     default:
-      abort();
+      return out << "unexpected content!!!"; // abort() usage is forbidden in the library
   }
 }
 
@@ -5664,7 +5664,7 @@ inline std::ostream& minify<dom::element>::print(std::ostream& out) {
     case tape_type::END_ARRAY:
     case tape_type::END_OBJECT:
     case tape_type::ROOT:
-      abort();
+      out << "unexpected content!!!"; // abort() usage is forbidden in the library
     }
     iter.json_index++;
     after_value = true;
@@ -6461,7 +6461,7 @@ dom::parser::Iterator::Iterator(const dom::parser &pj) noexcept(false)
 #if SIMDJSON_EXCEPTIONS
   if (!pj.valid) { throw simdjson_error(pj.error); }
 #else
-  if (!pj.valid) { abort(); }
+  if (!pj.valid) { return; } //  abort() usage is forbidden in the library
 #endif
 
   max_depth = pj.max_depth();

src/CMakeLists.txt

@@ -45,6 +45,34 @@ endif()
 target_link_libraries(simdjson PUBLIC simdjson-headers simdjson-flags) # Only expose the headers, not sources
 target_link_libraries(simdjson PRIVATE simdjson-source simdjson-internal-flags)
 
+##
+## In systems like R, libraries must not use stderr or abort to be acceptable.
+## Thus we make it a hard rule that one is not allowed to call abort or stderr.
+## The sanitized builds are allowed to abort.
+##
+if(NOT SIMDJSON_SANITIZE)
+  find_program(GREP grep)
+  find_program(NM nm)
+  if((NOT GREP) OR (NOT NM))
+    message("grep and nm are unavailable on this system.")
+  else()
+    add_test(
+      NAME "avoid_abort"
+      COMMAND sh -c "${NM}  $<TARGET_FILE_NAME:simdjson> |  ${GREP} abort || exit 0  && exit 1"
+      WORKING_DIRECTORY ${PROJECT_BINARY_DIR}
+    )
+    add_test(
+      NAME "avoid_stdout"
+      COMMAND sh -c "${NM}  $<TARGET_FILE_NAME:simdjson> |  ${GREP} stdout || exit 0 && exit 1"
+      WORKING_DIRECTORY ${PROJECT_BINARY_DIR}
+    )
+    add_test(
+      NAME "avoid_stderr"
+      COMMAND sh -c "${NM}  $<TARGET_FILE_NAME:simdjson> |  ${GREP} stderr || exit 0 && exit 1"
+      WORKING_DIRECTORY ${PROJECT_BINARY_DIR}
+    )
+  endif()
+endif()
 
 if(NOT MSVC)
   ## We output the library at the root of the current directory where cmake is invoked

src/implementation.cpp

@@ -118,7 +118,7 @@ const implementation *available_implementation_list::detect_best_supported() con
     uint32_t required_instruction_sets = impl->required_instruction_sets();
     if ((supported_instruction_sets & required_instruction_sets) == required_instruction_sets) { return impl; }
   }
-  return &unsupported_singleton;
+  return &unsupported_singleton; // this should never happen?
 }
 
 const implementation *detect_best_supported_implementation_on_first_use::set_best() const noexcept {
@@ -129,11 +129,12 @@ const implementation *detect_best_supported_implementation_on_first_use::set_bes
 
   if (force_implementation_name) {
     auto force_implementation = available_implementations[force_implementation_name];
-    if (!force_implementation) {
+    if (force_implementation) {
-      fprintf(stderr, "SIMDJSON_FORCE_IMPLEMENTATION environment variable set to '%s', which is not a supported implementation name!\n", force_implementation_name);
+      return active_implementation = force_implementation;
-      abort();
+    } else {
+      // Note: abort() and stderr usage within the library is forbidden.
+      return active_implementation = &unsupported_singleton;
     }
-    return active_implementation = force_implementation;
   }
   return active_implementation = available_implementations.detect_best_supported();
 }