Erratic behaviour when raising exception from foreign code

Help!
1

I am yet again stuck with a problem that pops up sometimes but quite often nevertheless. I suspect I am misusing the foreign language interface so this might be just a code review.

What I am doing wrong is probably how I deal with options that I have declared in PL_option_t but were not provided by the Prolog call.

I’m using: SWI-Prolog version

threaded, 64 bits, version 9.3.7-8-gc292c73b1

I want the code to: parse an option list in a foreign function implemented in C using PL_option_t and PL_scan_options(). It kinda works, most of the time.

But what I’m getting is:

Here are three consecutive executions of the same code on my command line:

First run

$ make test
swipl -g run_tests -t halt swiplite.pl
[10/10] sqlite3:bad_mode ..
ERROR: Received fatal signal 11 (segv)
Time: Tue May 28 18:09:28 2024
Inferences: 557649
Thread: 1 (main)
C-stack trace labeled "crash":
  [0] save_backtrace() at :? [0x77e61794174e]
  [1] sigCrashHandler() at :? [0x77e617941839]
  [2] __sigaction() at ??:? [0x77e617655ae0]
  [3] PL_is_variable___LD() at :? [0x77e61790a3fc]
  [4] PL_error() at :? [0x77e617851835]
  [5] PL_domain_error() at ??:? [0x77e617852fa9]
  [6] swiplite(pl_sqlite3_open+0x18b) [0x77e617bb937b]
  [7] PL_next_solution___LD() at :? [0x77e617840e94]
  [8] callProlog() at :? [0x77e617889b62]
  [9] pl_with_output_to2_va() at pl-file.c:? [0x77e617921636]
  [10] PL_next_solution___LD() at :? [0x77e6178410a9]
  [11] callProlog() at :? [0x77e617889b62]
  [12] pl_with_mutex() at :? [0x77e6178fe019]
  [13] PL_next_solution___LD() at :? [0x77e617840f10]
  [14] query_loop() at :? [0x77e6178895eb]
  [15] prologToplevel() at :? [0x77e617889f4d]
  [16] swipl(+0x1096) [0x5f5f1e819096]
  [17] __libc_init_first() at ??:? [0x77e61763ec88]
  [18] __libc_start_main() at ??:? [0x77e61763ed4c]
  [19] swipl(+0x10e5) [0x5f5f1e8190e5]


PROLOG STACK:
make: *** [Makefile:12: test] Segmentation fault (core dumped)

Second run

$ make test
swipl -g run_tests -t halt swiplite.pl
[10/10] sqlite3:bad_mode .................................................... **FAILED (0.000 sec)
ERROR: /home/boris/code/swiplite/swiplite.pl:53:
ERROR:     test sqlite3:bad_mode: wrong error
ERROR:     Expected: error(domain_error(A,B),C)
ERROR:     Got:      error(instantiation_error,context(swiplite:sqlite3_open/3,D))
ERROR: 1 test failed
% 9 tests passed
% Test run completed in 0.060 seconds (0.059 cpu)
ERROR: -g run_tests: false
make: *** [Makefile:12: test] Error 1

Third run

$ make test
swipl -g run_tests -t halt swiplite.pl
[10/10] sqlite3:bad_mode ...................................................... passed (0.000 sec)
% All 10 tests passed in 0.060 seconds (0.059 cpu)

I have not changed anything at all between the runs, they are some seconds apart. I was not able to reproduce it with a smaller example :frowning: but I am attaching the one C file and Prolog file that do that. Discourse wouldn’t let me attach the Makefile so here it is:

CPPFLAGS += -I/your/own/path/to/include
CFLAGS += -fpic -Wall -Wextra -O2
LDFLAGS += -shared
LDLIBS += -l sqlite3

obj = swiplite

$(obj): $(obj).o

.PHONY: test
test: $(obj)
	swipl -g run_tests -t halt $(obj).pl

.PHONY: clean
clean:
	-rm $(obj).o
	-rm $(obj)

swiplite.c (2.8 KB)
swiplite.pl (1.7 KB)

PS: if anyone is wondering, I ended up here because I thought it was gonna be quite easy :smiley:

2

Might I suggest using the C++ interface rather than C? The C++ interface has built-in checking for errors (doesn’t require checking the return code from every PL_*() call), so it’s less error-prone.

A small style point: instead of if (TRUE != PL_get_chars(...)) you can write if (!PL_get_chars(...).

I noticed that you’re not always checking the return code of the call to PL_atom_nchars() at line 35 and others; if that raises an error (which will set its return code to false), you should immediately return false to Prolog.

3

You are quite close :slight_smile: I think @peter.ludemann is right that the C++ interface is easier. Also note that there is also "prosqlite" pack for SWI-Prolog.

The real mistake is probably

return PL_domain_error("mode(read|write|create)", mode);

mode must be a term_t, but it is an atom_t. As both are eventually integers, the compiler won’t tell you :frowning: That is a common issue when using PL_scan_options() when you want an additional check on the value. You need something like this …

term_t ex;
return ( (ex = PL_new_term_ref()) &&
         PL_put_atom(ex, mode) &&
         PL_domain_error("mode", ex) );

Most of the other PL_*_error() calls suffer from similar problems. Note that the integer (atom_t) is taken as a stack offset when interpreted as term_t. If you are too far of you get a SEGV. Otherwise you get some random data from the stack that may or may not be valid.

4

That’s a mistake I’ve made a number of times. We’ve discussed preventing this by wrapping atom_t and term_t as C structs, but it was unclear if the compilers would generate as good code. Also, it’d be quite a bit of work (I experimented with this change after the third time I made this mistake).

The C++ interface prevents making the mistake of using an atom instead of a term. (It’s not entirely free of gotchas, but it has fewer of them than the C interface.)

5

Yes, thank you, this was it. I thought about this for a second but then it compiled and the thought seized.

Yes, and there is also the ODBC library. I wanted to see if I can do something interesting with the compiled statements that sqlite provides. I will report if this gets anywhere at all.

Yes, and I would maybe do it, but I have barely any hands-on experience with C++ and I want to tackle the novelties one by one :slight_smile: I hope I get to it. And thank you for the suggestion to not compare to TRUE, I was too lazy to figure out if TRUE and FALSE and 1 and 0…

6

C++ can be treated as a “better C than C”; that is, you could write mostly in a C style (although you might need to know something about how exceptions work) … I could probably quickly rewrite your code in C++ if you wish (but not sure about any testing if it requires anything more than installing libsqlite3-dev).

PS: Both C and C++ treat 0 or null-ptr as false and anything else is true.

7

I considered several times to just pick up the defines from sqlite and drop them in the Prolog wrapper of the sqlite bindings; then parse the options in Prolog and give the flags as an int. This creates yet another mixed layer though :-/

8

I would gladly take your offer but maybe it is a bit preliminary. And, most of the real fun is figuring out how to do it, right? On the other hand, there is very little code right now, so little work for you and easy to figure out for me. Here is what I have after I fixed the C code.

swiplite.c (3.1 KB)

I should put it on github eventually.

9

Here’s a very quick conversion of your code to C++ (untested). This reminded me that I was going to improve the interface to PL_scan_options(). Also, I think some of the calls to PL_() and Plx_() functions in the C++ code can be improved (e.g., the PlEx<bool>(dbname.get_chars(...)) can probably be replaced by something simpler).

You can diff this with your original C code to see how the C APIs are converted to C++.

If you want a better conversion, just ask; and I’ll try to use more “idiomatic” parts of the C++ API. Also, it’s probably better to make the sqlite3* value into a blob; the C++ API is much easier for this (I can do this also, if you wish).

10

3 posts were split to a new topic: A safer C API