PL_retry() and non-positive values

The documentation for PL_retry() says “handle is a signed value two bits smaller than a pointer, i.e., 30 or 62 bits (two bits are used for status indication)”.

However, if the handle is 0 or negative, PL_retry() seems to be treated like PL_succeed. Is this a bug or incorrect documentation?

Following is a simple example. When the code has PL_retry(0), bagof(V,pcre:test1(V),Vs) gives Vs=['FIRST'] but with PL_retry(1), it gives Vs=['FIRST', 'SECOND'].

static foreign_t
test1(term_t choice, control_t handle)
{ switch( PL_foreign_control(handle) )
  { case PL_FIRST_CALL:
      if ( !PL_unify_atom_chars(choice, "FIRST") )
        PL_fail;
      PL_retry(0); /* <<== does what I expect with PL_retry(1); */
    case PL_REDO:
      if ( !PL_unify_atom_chars(choice, "SECOND") )
        PL_fail;
      PL_succeed;
    case PL_PRUNED:
      PL_succeed;
    default:
      assert(0);
  }
}

Also, negative numbers are treated as unsigned, contrary to the documentation. Here’s a demonstration:

#include <stdio.h>
static foreign_t
test1(term_t choice, control_t handle)
{ char buf[100];
  switch( PL_foreign_control(handle) )
  { case PL_FIRST_CALL:
      if ( !PL_unify_atom_chars(choice, "FIRST") )
        PL_fail;
      PL_retry(-1); /* <<== does what I expect with PL_retry(1); */
    case PL_REDO:
      sprintf(buf, "SECOND(0x%08lx)", PL_foreign_context(handle));
      if ( !PL_unify_atom_chars(choice, buf) )
        PL_fail;
      PL_succeed;
    case PL_PRUNED:
      PL_succeed;
    default:
      assert(0);
  }
}

which produces:

?- bagof(V, test1(V), Vs).
Vs = ['FIRST', 'SECOND(0x3fffffffffffffff)'].

Also, I tried changing the uintptr_t to intptr_t in pl-builtin.h but it gave the same result (and, of course, wouldn’t have solved the problem with zero).

--- a/src/pl-builtin.h
+++ b/src/pl-builtin.h
@@ -531,7 +531,7 @@ typedef enum
 } frg_code;
 
 struct foreign_context
-{ uintptr_t		context;	/* context value */
+{ intptr_t		context;	/* context value */
   frg_code		control;	/* FRG_* action */
   struct PL_local_data *engine;		/* invoking engine */
   struct definition    *predicate;	/* called Prolog predicate */
@@ -543,7 +543,7 @@ struct foreign_context
 #define REDO_INT	0x01		/* Returned an integer */
 #define YIELD_PTR	0x02		/* Returned a pointer */
 
-#define ForeignRedoIntVal(v)	(((uintptr_t)(v)<<FRG_REDO_BITS)|REDO_INT)
+#define ForeignRedoIntVal(v)	(((intptr_t)(v)<<FRG_REDO_BITS)|REDO_INT)
 #define ForeignRedoPtrVal(v)	(((uintptr_t)(v))|REDO_PTR)
 #define ForeignYieldPtrVal(v)	(((uintptr_t)(v))|YIELD_PTR)
 

To get the original value that was given to PL_retry(), I can do something like this:

intptr_t = PL_foreign_context(handle) << 2 >> 2;

Is that what’s meant by “signed value two bits smaller than a pointer”?

Even so, that still leaves the problem of PL_retry(0) being treated as PL_succeed.

Thanks. Got broken over time :frowning: Retrying with an integer is not very popular. Should be fixed again. This was my test:

#include <SWI-Prolog.h>
#include <SWI-Stream.h>
#include <assert.h>

static foreign_t
test1(term_t start, control_t handle)
{ long i;

  switch( PL_foreign_control(handle) )
  { case PL_FIRST_CALL:
      if ( !PL_get_long_ex(start, &i) )
	return FALSE;

      PL_retry(i);
    case PL_REDO:
      i = PL_foreign_context(handle);
      Sdprintf("Got %ld\n", i);
      PL_retry(i+1);
    case PL_PRUNED:
      PL_succeed;
    default:
      assert(0);
  }
}

install_t
install_fint()
{ PL_register_foreign("test", 1, test1, PL_FA_NONDETERMINISTIC);
}

Doesn’t work for me … has the change been pushed?

Oops. Pushed now. Sorry.

I’ve written a small regression test, to avoid this happening again (because my foreign code only exercises one of the two errors) but I can’t figure out how to add it to the core tests – presumably, I’d add a directory to swipl-devel/src/Test (plus a suitable add_subirectory in src/CmakeLists.txt) but I don’t know how to compile the C code and make it available to the test. Is there an example anywhere that I can use as a model? (src/Tests/Foreign/hello.c doesn’t seem to be used in the build.)

As is, there are no unit tests for the foreign interface. It is probably not that hard to add such tests using cmake/ctest. For the core C API that isn’t as bad as it may sound as this interface is used throughout the core system as well as in the default packages and most bugs will thus surface somewhere in the existing test suite. Non-determinism based on integers appears an exception. Possibly it was never a good idea :slight_smile: Also the C++ interface has no coverage in the default system.

More tests are always welcome!

1 Like

I’ve put together DOC: Added to "foreign" section by kamahen · Pull Request #977 · SWI-Prolog/swipl-devel · GitHub , which also contains some documentation updates based on emails and discussions.

However, I don’t know how to compile the foreign test code under cmake, so - when you have time - please tell me what to do with src/Tests/foreign2/CMakeLists.txt .

(As for C++ test cases – I’ll leave that for someone else)

As I think of it, the simplest way to add tests for the ffi stuff is probably to define it as a package. The package infrastructure has everything in place for building and testing ffi components. The C++ interface is already a package. That should make it fairly easy to add tests for C++ loadable modules. Testing embedding is a bit more CMake tweaking. I think I’m not completely against adding plain C tests to the C++ package as well. After all, the C++ layer is just a small layer around the C API.

As suggested, I’ve moved the tests to package-cpp: TEST: Added foreign language interface regression tests by kamahen · Pull Request #9 · SWI-Prolog/packages-cpp · GitHub