| Message ID | 20240828133033.2378322-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | tools/ocaml: Factor out compatiblity handling | expand |
On Wed, Aug 28, 2024 at 2:30 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > ... rather than having each library implement its own subset. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Christian Lindig <christian.lindig@citrix.com> > CC: David Scott <dave@recoil.org> > CC: Edwin Török <edwin.torok@cloud.com> > CC: Rob Hoes <Rob.Hoes@citrix.com> > CC: Andrii Sultanov <andrii.sultanov@cloud.com> > > Broken out of a larger series, to help Andrii with his dynlib work. > --- > tools/ocaml/libs/xc/Makefile | 2 +- > tools/ocaml/libs/xc/xenctrl_stubs.c | 13 +++---------- > tools/ocaml/libs/xen-caml-compat.h | 23 +++++++++++++++++++++++ > 3 files changed, 27 insertions(+), 11 deletions(-) > create mode 100644 tools/ocaml/libs/xen-caml-compat.h > > diff --git a/tools/ocaml/libs/xc/Makefile b/tools/ocaml/libs/xc/Makefile > index 1d9fecb06ef2..cdf4d01dac52 100644 > --- a/tools/ocaml/libs/xc/Makefile > +++ b/tools/ocaml/libs/xc/Makefile > @@ -2,7 +2,7 @@ OCAML_TOPLEVEL=$(CURDIR)/../.. > XEN_ROOT=$(OCAML_TOPLEVEL)/../.. > include $(OCAML_TOPLEVEL)/common.make > > -CFLAGS += -I../mmap $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) > +CFLAGS += -I../ -I../mmap $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) > CFLAGS += $(APPEND_CFLAGS) > OCAMLINCLUDE += -I ../mmap -I ../eventchn > > diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c > index a52908012960..c78191f95abc 100644 > --- a/tools/ocaml/libs/xc/xenctrl_stubs.c > +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c > @@ -25,6 +25,8 @@ > #include <caml/fail.h> > #include <caml/callback.h> > > +#include "xen-caml-compat.h" > + > #include <sys/mman.h> > #include <stdint.h> > #include <string.h> > @@ -37,14 +39,6 @@ > > #include "mmap_stubs.h" > > -#ifndef Val_none > -#define Val_none (Val_int(0)) > -#endif > - > -#ifndef Tag_some > -#define Tag_some 0 > -#endif > - > static inline xc_interface *xch_of_val(value v) > { > xc_interface *xch = *(xc_interface **)Data_custom_val(v); > @@ -744,8 +738,7 @@ CAMLprim value stub_xc_evtchn_status(value xch_val, value domid, value port) > Store_field(result_status, 0, Val_int(status.vcpu)); > Store_field(result_status, 1, stat); > > - result = caml_alloc_small(1, Tag_some); > - Store_field(result, 0, result_status); > + result = caml_alloc_some(result_status); > > CAMLreturn(result); > } > diff --git a/tools/ocaml/libs/xen-caml-compat.h b/tools/ocaml/libs/xen-caml-compat.h > new file mode 100644 > index 000000000000..b4a0ca4ce90c > --- /dev/null > +++ b/tools/ocaml/libs/xen-caml-compat.h > @@ -0,0 +1,23 @@ > +/* SPDX-License-Identifier: LGPL-2.1-only WITH OCaml-LGPL-linking-exception */ > +#ifndef XEN_CAML_COMPAT_H > +#define XEN_CAML_COMPAT_H > + > +#ifndef Val_none /* Option handling. Compat for Ocaml < 4.12 */ > + > +#define Val_none Val_int(0) > +#define Tag_some 0 > +#define Some_val(v) Field(v, 0) > + > +static inline value caml_alloc_some(value v) > +{ > + CAMLparam1(v); > + > + value some = caml_alloc_small(1, Tag_some); > + Store_field(some, 0, v); The compiler uses Field() rather than Store_field() here. I think using Store_field here can potentially read uninitialized data, because 'caml_alloc_small' gives you uninitialized memory that you must immediately fill with valid values. Looking at the implementation Store_field calls caml_modify which will read the old value to figure out whether it was in minor or major heap, and doing that on uninitialized data is unpredictable. We should follow what the manual says and use Field() when caml_alloc_small() is used, and use Store_field() when caml_alloc() is used, and not attempt to mix them: See https://ocaml.org/manual/5.2/intfc.html#ss:c-low-level-gc-harmony > + > + CAMLreturn(some); > +} > + > +#endif /* !Val_none */ > + > +#endif /* XEN_CAML_COMPAT_H */ > > base-commit: 75c64db3722f0245137a1e8cfd3435f4790d0fd7 > -- > 2.39.2 >
On Wed, Aug 28, 2024 at 3:15 PM Edwin Torok <edwin.torok@cloud.com> wrote: > > On Wed, Aug 28, 2024 at 2:30 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > > > ... rather than having each library implement its own subset. > > > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > > --- > > CC: Christian Lindig <christian.lindig@citrix.com> > > CC: David Scott <dave@recoil.org> > > CC: Edwin Török <edwin.torok@cloud.com> > > CC: Rob Hoes <Rob.Hoes@citrix.com> > > CC: Andrii Sultanov <andrii.sultanov@cloud.com> > > > > Broken out of a larger series, to help Andrii with his dynlib work. > > --- > > tools/ocaml/libs/xc/Makefile | 2 +- > > tools/ocaml/libs/xc/xenctrl_stubs.c | 13 +++---------- > > tools/ocaml/libs/xen-caml-compat.h | 23 +++++++++++++++++++++++ > > 3 files changed, 27 insertions(+), 11 deletions(-) > > create mode 100644 tools/ocaml/libs/xen-caml-compat.h > > > > diff --git a/tools/ocaml/libs/xc/Makefile b/tools/ocaml/libs/xc/Makefile > > index 1d9fecb06ef2..cdf4d01dac52 100644 > > --- a/tools/ocaml/libs/xc/Makefile > > +++ b/tools/ocaml/libs/xc/Makefile > > @@ -2,7 +2,7 @@ OCAML_TOPLEVEL=$(CURDIR)/../.. > > XEN_ROOT=$(OCAML_TOPLEVEL)/../.. > > include $(OCAML_TOPLEVEL)/common.make > > > > -CFLAGS += -I../mmap $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) > > +CFLAGS += -I../ -I../mmap $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) > > CFLAGS += $(APPEND_CFLAGS) > > OCAMLINCLUDE += -I ../mmap -I ../eventchn > > > > diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c > > index a52908012960..c78191f95abc 100644 > > --- a/tools/ocaml/libs/xc/xenctrl_stubs.c > > +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c > > @@ -25,6 +25,8 @@ > > #include <caml/fail.h> > > #include <caml/callback.h> > > > > +#include "xen-caml-compat.h" > > + > > #include <sys/mman.h> > > #include <stdint.h> > > #include <string.h> > > @@ -37,14 +39,6 @@ > > > > #include "mmap_stubs.h" > > > > -#ifndef Val_none > > -#define Val_none (Val_int(0)) > > -#endif > > - > > -#ifndef Tag_some > > -#define Tag_some 0 > > -#endif > > - > > static inline xc_interface *xch_of_val(value v) > > { > > xc_interface *xch = *(xc_interface **)Data_custom_val(v); > > @@ -744,8 +738,7 @@ CAMLprim value stub_xc_evtchn_status(value xch_val, value domid, value port) > > Store_field(result_status, 0, Val_int(status.vcpu)); > > Store_field(result_status, 1, stat); > > > > - result = caml_alloc_small(1, Tag_some); > > - Store_field(result, 0, result_status); > > + result = caml_alloc_some(result_status); > > > > CAMLreturn(result); > > } > > diff --git a/tools/ocaml/libs/xen-caml-compat.h b/tools/ocaml/libs/xen-caml-compat.h > > new file mode 100644 > > index 000000000000..b4a0ca4ce90c > > --- /dev/null > > +++ b/tools/ocaml/libs/xen-caml-compat.h > > @@ -0,0 +1,23 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-only WITH OCaml-LGPL-linking-exception */ > > +#ifndef XEN_CAML_COMPAT_H > > +#define XEN_CAML_COMPAT_H > > + > > +#ifndef Val_none /* Option handling. Compat for Ocaml < 4.12 */ > > + > > +#define Val_none Val_int(0) > > +#define Tag_some 0 > > +#define Some_val(v) Field(v, 0) > > + > > +static inline value caml_alloc_some(value v) > > +{ > > + CAMLparam1(v); > > + > > + value some = caml_alloc_small(1, Tag_some); > > + Store_field(some, 0, v); > > The compiler uses Field() rather than Store_field() here. > I think using Store_field here can potentially read uninitialized > data, because 'caml_alloc_small' gives you uninitialized memory > that you must immediately fill with valid values. > Looking at the implementation Store_field calls caml_modify which will > read the old value to figure out whether it was in minor or major > heap, > and doing that on uninitialized data is unpredictable. > > We should follow what the manual says and use Field() when > caml_alloc_small() is used, and use Store_field() when caml_alloc() is > used, > and not attempt to mix them: > See https://ocaml.org/manual/5.2/intfc.html#ss:c-low-level-gc-harmony Which probably means we've got a bunch of other pre-existing bugs like these that we need to fix, as otherwise we do quite a lot of operations on uninitialized data... > > > + > > + CAMLreturn(some); > > +} > > + > > +#endif /* !Val_none */ > > + > > +#endif /* XEN_CAML_COMPAT_H */ > > > > base-commit: 75c64db3722f0245137a1e8cfd3435f4790d0fd7 > > -- > > 2.39.2 > >
On 28/08/2024 3:15 pm, Edwin Torok wrote: > On Wed, Aug 28, 2024 at 2:30 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: >> ... rather than having each library implement its own subset. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> --- >> CC: Christian Lindig <christian.lindig@citrix.com> >> CC: David Scott <dave@recoil.org> >> CC: Edwin Török <edwin.torok@cloud.com> >> CC: Rob Hoes <Rob.Hoes@citrix.com> >> CC: Andrii Sultanov <andrii.sultanov@cloud.com> >> >> Broken out of a larger series, to help Andrii with his dynlib work. >> --- >> tools/ocaml/libs/xc/Makefile | 2 +- >> tools/ocaml/libs/xc/xenctrl_stubs.c | 13 +++---------- >> tools/ocaml/libs/xen-caml-compat.h | 23 +++++++++++++++++++++++ >> 3 files changed, 27 insertions(+), 11 deletions(-) >> create mode 100644 tools/ocaml/libs/xen-caml-compat.h >> >> diff --git a/tools/ocaml/libs/xc/Makefile b/tools/ocaml/libs/xc/Makefile >> index 1d9fecb06ef2..cdf4d01dac52 100644 >> --- a/tools/ocaml/libs/xc/Makefile >> +++ b/tools/ocaml/libs/xc/Makefile >> @@ -2,7 +2,7 @@ OCAML_TOPLEVEL=$(CURDIR)/../.. >> XEN_ROOT=$(OCAML_TOPLEVEL)/../.. >> include $(OCAML_TOPLEVEL)/common.make >> >> -CFLAGS += -I../mmap $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) >> +CFLAGS += -I../ -I../mmap $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) >> CFLAGS += $(APPEND_CFLAGS) >> OCAMLINCLUDE += -I ../mmap -I ../eventchn >> >> diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c >> index a52908012960..c78191f95abc 100644 >> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c >> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c >> @@ -25,6 +25,8 @@ >> #include <caml/fail.h> >> #include <caml/callback.h> >> >> +#include "xen-caml-compat.h" >> + >> #include <sys/mman.h> >> #include <stdint.h> >> #include <string.h> >> @@ -37,14 +39,6 @@ >> >> #include "mmap_stubs.h" >> >> -#ifndef Val_none >> -#define Val_none (Val_int(0)) >> -#endif >> - >> -#ifndef Tag_some >> -#define Tag_some 0 >> -#endif >> - >> static inline xc_interface *xch_of_val(value v) >> { >> xc_interface *xch = *(xc_interface **)Data_custom_val(v); >> @@ -744,8 +738,7 @@ CAMLprim value stub_xc_evtchn_status(value xch_val, value domid, value port) >> Store_field(result_status, 0, Val_int(status.vcpu)); >> Store_field(result_status, 1, stat); >> >> - result = caml_alloc_small(1, Tag_some); >> - Store_field(result, 0, result_status); >> + result = caml_alloc_some(result_status); >> >> CAMLreturn(result); >> } >> diff --git a/tools/ocaml/libs/xen-caml-compat.h b/tools/ocaml/libs/xen-caml-compat.h >> new file mode 100644 >> index 000000000000..b4a0ca4ce90c >> --- /dev/null >> +++ b/tools/ocaml/libs/xen-caml-compat.h >> @@ -0,0 +1,23 @@ >> +/* SPDX-License-Identifier: LGPL-2.1-only WITH OCaml-LGPL-linking-exception */ >> +#ifndef XEN_CAML_COMPAT_H >> +#define XEN_CAML_COMPAT_H >> + >> +#ifndef Val_none /* Option handling. Compat for Ocaml < 4.12 */ >> + >> +#define Val_none Val_int(0) >> +#define Tag_some 0 >> +#define Some_val(v) Field(v, 0) >> + >> +static inline value caml_alloc_some(value v) >> +{ >> + CAMLparam1(v); >> + >> + value some = caml_alloc_small(1, Tag_some); >> + Store_field(some, 0, v); > The compiler uses Field() rather than Store_field() here. > I think using Store_field here can potentially read uninitialized > data, because 'caml_alloc_small' gives you uninitialized memory > that you must immediately fill with valid values. > Looking at the implementation Store_field calls caml_modify which will > read the old value to figure out whether it was in minor or major > heap, > and doing that on uninitialized data is unpredictable. > > We should follow what the manual says and use Field() when > caml_alloc_small() is used, and use Store_field() when caml_alloc() is > used, > and not attempt to mix them: > See https://ocaml.org/manual/5.2/intfc.html#ss:c-low-level-gc-harmony Lovely, this got changed in Ocaml with no information or justification... https://github.com/ocaml/ocaml/pull/9819 I'll resync this locally, but I shaltn't repost. ~Andrew
On Wed, Aug 28, 2024 at 3:20 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > On 28/08/2024 3:15 pm, Edwin Torok wrote: > > On Wed, Aug 28, 2024 at 2:30 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > >> ... rather than having each library implement its own subset. > >> > >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > >> --- > >> CC: Christian Lindig <christian.lindig@citrix.com> > >> CC: David Scott <dave@recoil.org> > >> CC: Edwin Török <edwin.torok@cloud.com> > >> CC: Rob Hoes <Rob.Hoes@citrix.com> > >> CC: Andrii Sultanov <andrii.sultanov@cloud.com> > >> > >> Broken out of a larger series, to help Andrii with his dynlib work. > >> --- > >> tools/ocaml/libs/xc/Makefile | 2 +- > >> tools/ocaml/libs/xc/xenctrl_stubs.c | 13 +++---------- > >> tools/ocaml/libs/xen-caml-compat.h | 23 +++++++++++++++++++++++ > >> 3 files changed, 27 insertions(+), 11 deletions(-) > >> create mode 100644 tools/ocaml/libs/xen-caml-compat.h > >> > >> diff --git a/tools/ocaml/libs/xc/Makefile b/tools/ocaml/libs/xc/Makefile > >> index 1d9fecb06ef2..cdf4d01dac52 100644 > >> --- a/tools/ocaml/libs/xc/Makefile > >> +++ b/tools/ocaml/libs/xc/Makefile > >> @@ -2,7 +2,7 @@ OCAML_TOPLEVEL=$(CURDIR)/../.. > >> XEN_ROOT=$(OCAML_TOPLEVEL)/../.. > >> include $(OCAML_TOPLEVEL)/common.make > >> > >> -CFLAGS += -I../mmap $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) > >> +CFLAGS += -I../ -I../mmap $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) > >> CFLAGS += $(APPEND_CFLAGS) > >> OCAMLINCLUDE += -I ../mmap -I ../eventchn > >> > >> diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c > >> index a52908012960..c78191f95abc 100644 > >> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c > >> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c > >> @@ -25,6 +25,8 @@ > >> #include <caml/fail.h> > >> #include <caml/callback.h> > >> > >> +#include "xen-caml-compat.h" > >> + > >> #include <sys/mman.h> > >> #include <stdint.h> > >> #include <string.h> > >> @@ -37,14 +39,6 @@ > >> > >> #include "mmap_stubs.h" > >> > >> -#ifndef Val_none > >> -#define Val_none (Val_int(0)) > >> -#endif > >> - > >> -#ifndef Tag_some > >> -#define Tag_some 0 > >> -#endif > >> - > >> static inline xc_interface *xch_of_val(value v) > >> { > >> xc_interface *xch = *(xc_interface **)Data_custom_val(v); > >> @@ -744,8 +738,7 @@ CAMLprim value stub_xc_evtchn_status(value xch_val, value domid, value port) > >> Store_field(result_status, 0, Val_int(status.vcpu)); > >> Store_field(result_status, 1, stat); > >> > >> - result = caml_alloc_small(1, Tag_some); > >> - Store_field(result, 0, result_status); > >> + result = caml_alloc_some(result_status); > >> > >> CAMLreturn(result); > >> } > >> diff --git a/tools/ocaml/libs/xen-caml-compat.h b/tools/ocaml/libs/xen-caml-compat.h > >> new file mode 100644 > >> index 000000000000..b4a0ca4ce90c > >> --- /dev/null > >> +++ b/tools/ocaml/libs/xen-caml-compat.h > >> @@ -0,0 +1,23 @@ > >> +/* SPDX-License-Identifier: LGPL-2.1-only WITH OCaml-LGPL-linking-exception */ > >> +#ifndef XEN_CAML_COMPAT_H > >> +#define XEN_CAML_COMPAT_H > >> + > >> +#ifndef Val_none /* Option handling. Compat for Ocaml < 4.12 */ > >> + > >> +#define Val_none Val_int(0) > >> +#define Tag_some 0 > >> +#define Some_val(v) Field(v, 0) > >> + > >> +static inline value caml_alloc_some(value v) > >> +{ > >> + CAMLparam1(v); > >> + > >> + value some = caml_alloc_small(1, Tag_some); > >> + Store_field(some, 0, v); > > The compiler uses Field() rather than Store_field() here. > > I think using Store_field here can potentially read uninitialized > > data, because 'caml_alloc_small' gives you uninitialized memory > > that you must immediately fill with valid values. > > Looking at the implementation Store_field calls caml_modify which will > > read the old value to figure out whether it was in minor or major > > heap, > > and doing that on uninitialized data is unpredictable. > > > > We should follow what the manual says and use Field() when > > caml_alloc_small() is used, and use Store_field() when caml_alloc() is > > used, > > and not attempt to mix them: > > See https://ocaml.org/manual/5.2/intfc.html#ss:c-low-level-gc-harmony > > Lovely, this got changed in Ocaml with no information or justification... > > https://github.com/ocaml/ocaml/pull/9819 > Looking at the code more it only dereferences the old value if Is_young returns false. And Is_young is done as a pointer comparison. Newly allocated values will live in the young (minor) heap by default and you're not allowed to have a GC run between caml_alloc_small and Store_field anyway. So in practice Store_field is probably OK, but is on very dangerous ground as it relies on an implementation detail. > I'll resync this locally, but I shaltn't repost. > > ~Andrew
> On 28 Aug 2024, at 14:30, Andrew Cooper <Andrew.Cooper3@citrix.com> wrote: > > ... rather than having each library implement its own subset. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Christian Lindig <christian.lindig@citrix.com> > CC: David Scott <dave@recoil.org> > CC: Edwin Török <edwin.torok@cloud.com> > CC: Rob Hoes <Rob.Hoes@citrix.com> > CC: Andrii Sultanov <andrii.sultanov@cloud.com> > > Broken out of a larger series, to help Andrii with his dynlib work. > --- > tools/ocaml/libs/xc/Makefile | 2 +- > tools/ocaml/libs/xc/xenctrl_stubs.c | 13 +++---------- > tools/ocaml/libs/xen-caml-compat.h | 23 +++++++++++++++++++++++ > 3 files changed, 27 insertions(+), 11 deletions(-) > create mode 100644 tools/ocaml/libs/xen-caml-compat.h > > diff --git a/tools/ocaml/libs/xc/Makefile b/tools/ocaml/libs/xc/Makefile > index 1d9fecb06ef2..cdf4d01dac52 100644 > --- a/tools/ocaml/libs/xc/Makefile > +++ b/tools/ocaml/libs/xc/Makefile > @@ -2,7 +2,7 @@ OCAML_TOPLEVEL=$(CURDIR)/../.. > XEN_ROOT=$(OCAML_TOPLEVEL)/../.. > include $(OCAML_TOPLEVEL)/common.make > > -CFLAGS += -I../mmap $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) > +CFLAGS += -I../ -I../mmap $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) > CFLAGS += $(APPEND_CFLAGS) > OCAMLINCLUDE += -I ../mmap -I ../eventchn > > diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c > index a52908012960..c78191f95abc 100644 > --- a/tools/ocaml/libs/xc/xenctrl_stubs.c > +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c > @@ -25,6 +25,8 @@ > #include <caml/fail.h> > #include <caml/callback.h> > > +#include "xen-caml-compat.h" > + > #include <sys/mman.h> > #include <stdint.h> > #include <string.h> > @@ -37,14 +39,6 @@ > > #include "mmap_stubs.h" > > -#ifndef Val_none > -#define Val_none (Val_int(0)) > -#endif > - > -#ifndef Tag_some > -#define Tag_some 0 > -#endif > - > static inline xc_interface *xch_of_val(value v) > { > xc_interface *xch = *(xc_interface **)Data_custom_val(v); > @@ -744,8 +738,7 @@ CAMLprim value stub_xc_evtchn_status(value xch_val, value domid, value port) > Store_field(result_status, 0, Val_int(status.vcpu)); > Store_field(result_status, 1, stat); > > - result = caml_alloc_small(1, Tag_some); > - Store_field(result, 0, result_status); > + result = caml_alloc_some(result_status); > > CAMLreturn(result); > } > diff --git a/tools/ocaml/libs/xen-caml-compat.h b/tools/ocaml/libs/xen-caml-compat.h > new file mode 100644 > index 000000000000..b4a0ca4ce90c > --- /dev/null > +++ b/tools/ocaml/libs/xen-caml-compat.h > @@ -0,0 +1,23 @@ > +/* SPDX-License-Identifier: LGPL-2.1-only WITH OCaml-LGPL-linking-exception */ > +#ifndef XEN_CAML_COMPAT_H > +#define XEN_CAML_COMPAT_H > + > +#ifndef Val_none /* Option handling. Compat for Ocaml < 4.12 */ > + > +#define Val_none Val_int(0) > +#define Tag_some 0 > +#define Some_val(v) Field(v, 0) > + > +static inline value caml_alloc_some(value v) > +{ > + CAMLparam1(v); > + > + value some = caml_alloc_small(1, Tag_some); > + Store_field(some, 0, v); > + > + CAMLreturn(some); > +} > + > +#endif /* !Val_none */ > + > +#endif /* XEN_CAML_COMPAT_H */ > > base-commit: 75c64db3722f0245137a1e8cfd3435f4790d0fd7 > -- > 2.39.2 > Acked-by: Christian Lindig <christian.lindig@cloud.com>
diff --git a/tools/ocaml/libs/xc/Makefile b/tools/ocaml/libs/xc/Makefile index 1d9fecb06ef2..cdf4d01dac52 100644 --- a/tools/ocaml/libs/xc/Makefile +++ b/tools/ocaml/libs/xc/Makefile @@ -2,7 +2,7 @@ OCAML_TOPLEVEL=$(CURDIR)/../.. XEN_ROOT=$(OCAML_TOPLEVEL)/../.. include $(OCAML_TOPLEVEL)/common.make -CFLAGS += -I../mmap $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) +CFLAGS += -I../ -I../mmap $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) CFLAGS += $(APPEND_CFLAGS) OCAMLINCLUDE += -I ../mmap -I ../eventchn diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c index a52908012960..c78191f95abc 100644 --- a/tools/ocaml/libs/xc/xenctrl_stubs.c +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c @@ -25,6 +25,8 @@ #include <caml/fail.h> #include <caml/callback.h> +#include "xen-caml-compat.h" + #include <sys/mman.h> #include <stdint.h> #include <string.h> @@ -37,14 +39,6 @@ #include "mmap_stubs.h" -#ifndef Val_none -#define Val_none (Val_int(0)) -#endif - -#ifndef Tag_some -#define Tag_some 0 -#endif - static inline xc_interface *xch_of_val(value v) { xc_interface *xch = *(xc_interface **)Data_custom_val(v); @@ -744,8 +738,7 @@ CAMLprim value stub_xc_evtchn_status(value xch_val, value domid, value port) Store_field(result_status, 0, Val_int(status.vcpu)); Store_field(result_status, 1, stat); - result = caml_alloc_small(1, Tag_some); - Store_field(result, 0, result_status); + result = caml_alloc_some(result_status); CAMLreturn(result); } diff --git a/tools/ocaml/libs/xen-caml-compat.h b/tools/ocaml/libs/xen-caml-compat.h new file mode 100644 index 000000000000..b4a0ca4ce90c --- /dev/null +++ b/tools/ocaml/libs/xen-caml-compat.h @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: LGPL-2.1-only WITH OCaml-LGPL-linking-exception */ +#ifndef XEN_CAML_COMPAT_H +#define XEN_CAML_COMPAT_H + +#ifndef Val_none /* Option handling. Compat for Ocaml < 4.12 */ + +#define Val_none Val_int(0) +#define Tag_some 0 +#define Some_val(v) Field(v, 0) + +static inline value caml_alloc_some(value v) +{ + CAMLparam1(v); + + value some = caml_alloc_small(1, Tag_some); + Store_field(some, 0, v); + + CAMLreturn(some); +} + +#endif /* !Val_none */ + +#endif /* XEN_CAML_COMPAT_H */
... rather than having each library implement its own subset. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Christian Lindig <christian.lindig@citrix.com> CC: David Scott <dave@recoil.org> CC: Edwin Török <edwin.torok@cloud.com> CC: Rob Hoes <Rob.Hoes@citrix.com> CC: Andrii Sultanov <andrii.sultanov@cloud.com> Broken out of a larger series, to help Andrii with his dynlib work. --- tools/ocaml/libs/xc/Makefile | 2 +- tools/ocaml/libs/xc/xenctrl_stubs.c | 13 +++---------- tools/ocaml/libs/xen-caml-compat.h | 23 +++++++++++++++++++++++ 3 files changed, 27 insertions(+), 11 deletions(-) create mode 100644 tools/ocaml/libs/xen-caml-compat.h base-commit: 75c64db3722f0245137a1e8cfd3435f4790d0fd7