Bug 1392147

Created attachment 1218481 [details]
Proposed fix

The attached patch fixed the problem for me, but maybe it's not correct (I don't know much about OCAML).

From the OCAML documentation (e.g. [1]):
> 19.5.1  Simple interface
> ...
> Rule 1
> ...
> In particular, CAMLlocal and CAMLxparam can only be called after CAMLparam.

In e.g. raise_brlapi_error and raise_brlapi_exception the CAMLlocal1 and CAMLlocal2 were used without CAMLparam.

The proposed patch seems to at least fix the compilation problem.

[1] http://caml.inria.fr/pub/docs/manual-ocaml-4.04/intfc.html

The patch is nearly but not quite right.

You mustn't pass non-"value" type arguments to CAMLparamX.  If
you try that then the garbage collector may crash when it tries
to follow those non-value pointers and interpret them as OCaml
heap objects.  So the second instance must use CAMLparam0() as well.

(In reply to Richard W.M. Jones from comment #2)
> The patch is nearly but not quite right.
> 
> You mustn't pass non-"value" type arguments to CAMLparamX.  If
> you try that then the garbage collector may crash when it tries
> to follow those non-value pointers and interpret them as OCaml
> heap objects.  So the second instance must use CAMLparam0() as well.

Thanks for info, I doubt the patch was correct :)

Created attachment 1218489 [details]
Proposed fix

Jon, do you want to handle this bugzilla? I can do it myself as co-maintainer.

Patch looks good to me.  I'll add it and do the build as I
am coordinating everything in the f26-ocaml side tag.

Fix included in:
http://koji.fedoraproject.org/koji/taskinfo?taskID=16351380
brltty-5.4-4.fc26

Please send the patch upstream.

(In reply to Richard W.M. Jones from comment #7)
> Fix included in:
> http://koji.fedoraproject.org/koji/taskinfo?taskID=16351380
> brltty-5.4-4.fc26
> 
> Please send the patch upstream.

Reassigning to myself, I will manage it with upstream.

Patch sent upstream.

Richard, upstream encountered the following warnings with OCaml-4.00 and the patch applied:

> ./brlapi_stubs.c: In function ‘raise_brlapi_error’:
> ./brlapi_stubs.c:147:29: warning: unused variable ‘caml__frame’
> [-Wunused-variable]
> ./brlapi_stubs.c: In function ‘raise_brlapi_exception’:
> ./brlapi_stubs.c:161:29: warning: unused variable ‘caml__frame’
> [-Wunused-variable]

This doesn't occur with OCaml-4.04. Is it bug? Is there a way how to workaround it?

It does occur with OCaml 4.04.0, it is a bug, and it is fixed by your patch
which I have now included in the package.

(In reply to Richard W.M. Jones from comment #11)
> It does occur with OCaml 4.04.0, it is a bug, and it is fixed by your patch
> which I have now included in the package.

Sure, I wonder whether there is a way how to workaround the warnings with older OCaml. Brltty upstream is curious. He needs to support multiple versions of OCaml in brltty.

Interesting ...  I believe this was NOT an error in older versions
of OCaml.  I'm not sure it's an error even in OCaml 4.04.

There has been a change to the CAMLxparam<N> macros in the header,
but at the moment I'm unclear why it causes this new warning.

Perhaps it's newer GCC being cleverer about finding unused variables.

FYI upstream fix:
https://github.com/brltty/brltty/pull/81

It seems upstream is happy, so it's resolved for me. Thanks Richard.

Thanks for liaising with upstream on this issue.

I used upstream patch in rawhide (just commited, not built) as according to upstream using CAMLparam0() without CAMLreturn0() can lead to runtime problems.