Ann: SWI-Prolog 9.1.17

Releases
1

Dear SWI-Prolog user,

SWI-Prolog 9.1.17 is ready for download. This update fixes a a few
bugs, makes some non-terminating (in reasonable time) predicates
interruptable, improves stream handling from C++ and notably pushes
the Python Janus interface to a new level. Following discussions on
the performance of arithmetic expression evaluation, this is improved
in some places. That may result in a significant performance
improvement in some applications. This version brings some small
improvements to the VM performance.

The Janus Python interface is still in flux. Experience with the MIDI
interface using the Python mido package added thread support to
Janus. This should allow using Janus with e.g., HTTP server
frameworks as well. Following -and actually anticipating a bit on- the
ongoing meetings on Janus, we now implement two styles for calling
Prolog from Python and renamed the existing. The old names are
available for backward compatibility. Now, we have four functions
expression two types of parameter passing and det vs nondet. Please
checkout SWI-Prolog -- Calling Prolog from Python
The proposed new functions are:

  • janus.query_once(query:string, input:dict) ā†’ output:dict
    Was janus.once()
  • janus.apply_once(module:string, predicate:string, input ā€¦) ā†’ output
    New, e.g. janus.apply_once("user", "plus", 1, 2) ā†’ 3
  • janus.query(query:string, input:dict) ā†’ iterator ā†’ output:dict
    Was janus.Query()
  • janus.apply(module:string, predicate:string, input ā€¦) ā†’ iterator ā†’ output
    New, e.g. [*janus.apply("user", "between", 1, 6)] ā†’ [1,2,3,4,5,6]

We had a long debate on the names of these. If someone has a better idea,
please share.

It is adviced to stick with the old names for this release. The new
names should be final by the next release.

Enjoy --- Jan

SWI-Prolog Changelog since version 9.1.16

  • MODIFIED: Make the Prolog flag float_overflow to affect read_term/3.
    With this flag setting, too large floats are read as infinity rather
    than raising a syntax error.

  • MODIFIED: Issue a warning on suspect autoloading The autoloader now
    issues a warning if it loads a library that contains global goal
    or term expansion rules. Expansion typically improves performance
    or otherwise enhances the functionality of some of the predicates
    defined in the module.

  • FIXED: Preserve table properties on reconsult. Reported by Jan Burse.

  • CLEANUP: Removed unfinished and not maintained packages/cpproxy

  • FIXED: PL_thread_attach_engine()/PL_thread_destroy_engine() on main
    thread The main thread did not set the open_count to 1, causing
    hthe attach/destroy cycle in the main thread to kill the main thread.

  • DOC: read_string/3 [no ci]

  • DOC: clarify some details of stream error handling

  • TEST: Enable rational number tests when using LibBF.

  • TEST: Test suite for bomb DoS vulnerabilities.

  • SECURITY: Allow interrupts in the compiler This avoids using the
    ā€œbombā€ DoS attack using assertz/1 and friends.

  • SECURITY: Allow interrupts in term_variables/2 and numbervars/4
    variants Some of these variant goals require processing the tree
    such that shared terms are walked normally. Such goals can take
    exponential time. This patch allows processing interrupts.

    Without this, notably services such as SWISH that allow users to run
    arbitrary Prolog goals become subject to DoS attacks.

  • ENHANCED: Speedup arithmetic comparision in non-O mode.

Package cpp

  • ENHANCED: PlStream error handling + wrapper methods (and remove
    PlAcquireStream)

Package http

  • ENHANCED: Pass number reading exceptions rather than generating
    invalid_number.

Package ltx2htm

  • FIXED: Various math symbols Unicode translation

Package plunit

  • ENHANCED: If --on-warning=status or --on-error=status is active,
    consider such messages testv failures When running tests using
    --on-error=status, printing an error message that does not cause
    a test to fail silently ignores the message while Prolog exits with
    status 1. That is hard to debug. Now, if these flags are present,
    tests that emit warnings or errors are considered failed and thus
    the test is printed.

Package ssl

  • FIXED: Avoid leaking file handle in test

  • PORT: Handle the zlib OpenSSL dependency correctly on Windows.

Package swipy

  • FIXED: Avoid crash in callback when finalizing.

  • DOCS: Documentation updates Also enhanced py_all_lib_dir/0-2

  • MODIFIED: Renamed janus.Query() into janus.query().

  • MODIFIED: Renamed janus.once() to janus.query_once() Old function is
    still available as deprecated function. As this is under discussion,
    it might be wise to stick to once() until the next release.

  • ADDED: apply() functional style iterator Also makes the benchmarks
    look nice and flexible to handle.

  • ADDED: janus.apply1() As agreed in the Oct 13 Janus meeting. The name
    is still tentative.

  • ENHANCED: Allow initializing Python in a thread and terminate that
    thread.

  • FIXED: Allow clean exit from py_shell/0 using quit()

  • ENHANCED: Allow Python threading to continue when inside a Prolog
    thread.

  • ADDED: py_gil_owner/1.

  • ADDED: py_pp/1-3: option nl(Bool) to add a newline. Previous versions
    did not add a newline. Now we do by default, while adding nl(false)
    provides the old behavior.

  • FIXED: Multithread support Improve interaction with the Python GIL.
    Claim the GIL for printing Python object references from Prolog
    (<py_>(Ref)).

  • ADDED: Partial attempt to get rid of Python cleanly at shutdown.

  • FIXED: Avoid main thread keeping the GIL This allows creating Python
    threads and have these making callbacks.

  • ADDED: Allow calling Prolog from multiple Python threads.

  • PORT: Only use Python3.dll if it really exists.

  • FIXED: Wrong order in Python initialization for old Python versions.

2 Likes
2

In R-hub, there are a number of containers for compiling R packages, so I tried to compile rswipl there. One of them complains:

[ 90 pct.] Building CXX object packages/cpp/CMakeFiles/plugin_test_cpp.dir/test_cpp.cpp.o
In file included from /tmp/RtmpMFpcB2/R.INSTALL1e405bca50aa/rswipl/src/swipl-devel/packages/cpp/test_cpp.cpp:63:
/tmp/RtmpMFpcB2/R.INSTALL1e405bca50aa/rswipl/src/swipl-devel/packages/cpp/SWI-cpp2.h:1790:3: warning: C++ designated initializers only available with ā€˜-std=c++20ā€™ or ā€˜-std=gnu++20ā€™ [-Wc++20-extensions]
 | { .magic = PL_BLOB_MAGIC, \
 | ^
/tmp/RtmpMFpcB2/R.INSTALL1e405bca50aa/rswipl/src/swipl-devel/packages/cpp/test_cpp.cpp:1304:28: note: in expansion of macro ā€˜PL_BLOB_DEFINITIONā€™
3

I think for @peter.ludemann :slight_smile: I donā€™t think demanding c++20 is a good idea. c++17 is probably already going to be a problem for some installations.

4

Hmmm ā€¦ I thought I was using C++17 ā€“ a quick check of the documentation shows that ā€œdesignated initializersā€ are indeed a C++20 feature.

The CMakeLists.txt file sets ā€œrequires C++20ā€ for MSVC, C++17 otherwise - @jan is there possibly a bug in how what gets passed to the C++ compiler?

This is easily fixed; I have a bit of a busy day today, but Iā€™ll try to push a PR soon.

5

Good question. After rm packages/cpp/test_cpp.so and ninja -v we see the output below. There is no standard flag. I wonder why not.

[11/13] : && /bin/ccache /bin/c++ -fPIC    -shared  -o packages/cpp/test_cpp.so packages/cpp/CMakeFiles/plugin_test_cpp.dir/test_cpp.cpp.o packages/cpp/CMakeFiles/plugin_test_cpp.dir/SWI-cpp2.cpp.o   && :

But then, we find on CMAKE_CXX_KNOWN_FEATURES ā€” CMake 3.28.0-rc5 Documentation this:

Note: If the compilerā€™s default standard level is at least that of the requested feature, CMake may omit the -std= flag.

We need a way to specify the max :frowning:

6

It appears that designated initializers are a C99 feature, and are used in a few places in the code (pl-stream.c, pl-prof.c, pcre4pl.c). I presume that these arenā€™t causing any problems.

(Itā€™s interesting that a C feature didnā€™t get added to C++ for 20 years.)

7

Pushed a50a29fc3a11ff1b7194a4a1b339920c64851024 to packages-cpp that adds

if(CMAKE_COMPILER_IS_GNUCC)
  target_compile_options(plugin_test_cpp PRIVATE -std=c++17 -Wpedantic)
endif()

Now I get the warnings :slight_smile: It is remarkable hard to tell Cmake to be specific. Anything else I tried was simply ignored ā€¦ Anyway, it is good enough if we force strict compliance for our main development platform.

Didnā€™t update the main project, so you must pull the submodule.
Could you fix the real code, @peter.ludemann ?

1 Like
8

Is there a negative features test? If so, we could have a CheckC++20.cpp file that has a ā€œdesignated initializerā€ ā€“ but this feels a bit fragile. Also does -std=c++17 mean that it must be exactly C++17 and canā€™t be C++20? (My guess is that it does, because some features would be deprecated in C++20 but not in C++17)
I tried various ā€œdumpā€ commands with g++ but none of them seemed to output the C++ version.

9

This seems to work for getting the C++ standard thatā€™s supported with GNU C++; I donā€™t know about other compilers (I called the test file cv.cpp):

#include <iostream>
int main() {
  std::cout << __cplusplus << std::endl;
  return 0;
}

$ g++ --version
g++ (GCC) 13.2.0
Copyright (C) 2023 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ g++ cv.cpp && ./a.out
201703
$ g++ -std=c++17 cv.cpp && ./a.out
201703
$ g++ -std=c++20 cv.cpp && ./a.out
202002
2 Likes
10

One more little thing, if you donā€™t mind: UBSAN also complains about some cast from nullptr to PlAtom* in PlTerm::arity().

swipl-devel/packages/cpp/SWI-cpp2.h:456:105: runtime error: upcast of null pointer of type 'PlAtom'
    #0 0x7f4c68ca18a5 in PlTerm::get_name_arity(PlAtom*, unsigned long*) const /data/gannet/ripley/R/packages/tests-clang-SAN/rswipl/src/swipl-devel/packages/cpp/SWI-cpp2.h:456:111
    #1 0x7f4c68ca18a5 in PlTerm::arity() const /data/gannet/ripley/R/packages/tests-clang-SAN/rswipl/src/swipl-devel/packages/cpp/SWI-cpp2.cpp:484:8

More details here: https://www.stats.ox.ac.uk/pub/bdr/memtests/clang-UBSAN/rswipl/rswipl-Ex.Rout

I can silence the error message by redefining arity() like this (itā€™s not pretty, but a workaround):

size_t
PlTerm::arity() const
{ atom_t name;
  size_t arity;
  if ( Plx_get_name_arity(C_, &name, &arity) )
    return arity;
  throw PlTypeError("compound", *this);
}

I am wondering, however, if we should not have a proper C function PL_get_arity()ā€”instead of PL_get_name_arity() with name set to NULL.

11

I leave the C++ to @peter.ludemann . Too hard for me.

I donā€™t see a reason for that. It is a perfectly fine C API, in most cases you need both and if you need to get one, youā€™ve done all (if you need the name) or most (if you need the arity for small arities) of the work. Getting both of the separately is more work because you have to do the TLS involving fetching and dereferencing of the term handle twice. Keeping the interface small has a value in itself.

12

Does changing SWI-cpp2.h:456 as in the following diff fix the problem? (replacing &name->C_ by name ? &name->C : nullptr)

diff --git a/SWI-cpp2.h b/SWI-cpp2.h
index bf8ae3a..4fb1744 100644
--- a/SWI-cpp2.h
+++ b/SWI-cpp2.h
@@ -453,7 +453,7 @@ public:
   [[nodiscard]] bool get_pointer(void **ptr) const { return Plx_get_pointer(C_, ptr); }
   [[nodiscard]] bool get_float(double *f) const { return  Plx_get_float(C_, f); }
   [[nodiscard]] bool get_functor(PlFunctor *f) const { return Plx_get_functor(C_, &f->C_); }
-  [[nodiscard]] bool get_name_arity(PlAtom *name, size_t *arity) const { return Plx_get_name_arity(C_, &name->C_, arity);  }
+  [[nodiscard]] bool get_name_arity(PlAtom *name, size_t *arity) const { return Plx_get_name_arity(C_, name ? &name->C_ : nullptr, arity);  }
   [[nodiscard]] bool get_compound_name_arity(PlAtom *name, size_t *arity) const { return Plx_get_compound_name_arity(C_, &name->C_, arity); }
   [[nodiscard]] bool get_module(PlModule *module) const { return Plx_get_module(C_, &module->C_); }
   [[nodiscard]] bool get_arg(size_t index, PlTerm a) const { return Plx_get_arg(index, C_, a.C_); }

That seems to be a subtle bug that Iā€™ve let through ā€¦ Iā€™ll have to do a global edit to SWI-cpp2.h to fix it everywhere (the final fix will probably different from the quick fix I gave above).

13

Yes, it does. [and the correction makes sense indeed]

Thank you.

14

My use case was a loop such as for(i=1; i<=arity(); i++) {ā€¦}. But I see your point, I should rather fetch the arity once and store it in a local variable.

15

I see. Considering what is behind, the compiler canā€™t figure out it can fetch this just once. I think GCC allows to declare a function to be pure, in the sense that its output purely depends on the input arguments. That is not true though. You can rebind the term_t, causing fetching the arity to produce different results.

16

This is something different from declaring a method as const? (e.g. size_t PlTerm::arity() const { ... }, similar to PlFunctor::arity())

Thereā€™s Range-based for loop (since C++11) - cppreference.com , which Iā€™ve never used; and I donā€™t know how much work it would take to make it work with PlTerm::arity(). (Thereā€™s also a ā€œforeachā€ loop and std::for_each.)

17

Iā€™ve created a PR with a more comprehensive fix. It might break some of your code ā€“ the C_ field is now private, so it should be accessed by the unwrap() method. (For a quick fix, you can redefine PL_WRAPPED_VISIBILITY to public).

18

I donā€™t know. const typically means the data is not modified. Note that this particular case is still dubious. Use this and the whole thing falls apart if the compiler assumes the result of arity() doesnā€™t change.

for(size_t i=1; i<=arity(t); i++)
{ ...
  PL_put_term(t, ...)
}
19

I think that similar cases exist when using ā€œconst pointersā€ ā€“ the compiler is allowed to assume strict aliasing, for example. As term_t is essentially a pointer, it can be abused in ways similar to ā€œconst pointersā€. (Informally, C++ has a notion of ā€œlogical constā€; this allows, for example, an object to contain a cache that is mutable but the object itself is otherwise immutable; again, care must be taken that the compiler doesnā€™t make unwarranted assumptions.)

20

Interesting. So, possibly using arity(t) in the loop could work with appropriate definitions of the interface functions? Basically, the PL_get* functions to not modify the term they work on (they may modify other term handles, e.g., PL_get_arg() does not modify the compound it is working on, but does assign the argument handle). The PL_put* does modify the principal term and PL_unify* may instantiate the principal term. Typically that should also be considered modification from the C++ aspect.

Iā€™m not sure how useful it is. Looping over arguments is a typical case. The C API however defines PL_get_name_arity() which performs the compound check fills variables with the name and arity. So, in C api invites to use something like this:

atom_t name;
size_t arity;

if ( PL_get_name_arity(term, &name, &arity) )
{ for(size_t i=1; i<=arity; i++)
    ...

While the C++ API invites code like below. This may be one of the few places where it matters though. And, this is anyway not a good idea from the point of view of performance as each operation on a term using the PL_* functions first has to find the real term, which implies accessing thead local storage to the find the involved Prolog engine, translate the handle to the actual memory location and follow the Prolog reference chain. For most of these PL_* functions that find information about the term, these are the costly steps. Good news that, although less performant, there are not that many scenarios where the difference will be noticeable :slight_smile:

if ( term.is_compound() )
{ for(size_t i=1; i<=term.arity(); i++)