Re: [XEN PATCH v1 2/3] ocaml/libs: Fill build failure due to unused variable in ocaml macro

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 17/10/2024 5:20 pm, Javi Merino wrote:
> On Fedora 40, the build fails with:
>
>     gcc -m64 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall 
> -Wstrict-prototypes -Wno-unused-but-set-variable -Wno-unused-local-typedefs   
> -Werror -O2 -fomit-frame-pointer 
> -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MP -MF 
> .domain_getinfo_stubs_v1.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE   
> -fPIC -I/usr/lib64/ocaml -I 
> /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs -I 
> /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs/xsd_glue
>  
> -I/build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include
>  -D__XEN_TOOLS__ 
> -I/build/tools/ocaml/libs\/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include
>   -c -o domain_getinfo_stubs_v1.o domain_getinfo_stubs_v1.c
>     In file included from domain_getinfo_stubs_v1.c:10:
>     domain_getinfo_stubs_v1.c: In function 'xsd_glue_failwith':
>     /usr/lib64/ocaml/caml/memory.h:275:29: error: unused variable 
> 'caml__frame' [-Werror=unused-variable]
>       275 |   struct caml__roots_block *caml__frame = *caml_local_roots_ptr
>           |                             ^~~~~~~~~~~
>     domain_getinfo_stubs_v1.c:48:9: note: in expansion of macro 'CAMLparam0'
>       48 |         CAMLparam0();
>          |         ^~~~~~~~~~
>     cc1: all warnings being treated as errors
>
> The CAMLparam0 macro is defined in /usr/lib64/ocaml/caml/memory.h:255 as:
>
>     #define CAMLparam0()                                                    \
>       struct caml__roots_block** caml_local_roots_ptr =                     \
>         (DO_CHECK_CAML_STATE ? Caml_check_caml_state() : (void)0,           \
>          &CAML_LOCAL_ROOTS);                                                \
>       struct caml__roots_block *caml__frame = *caml_local_roots_ptr
>
> We can't modify the macro.  Mark the xsd_glue_failwith() function with
> ignore "-Wunused-variable" to prevent gcc from failing the build due
> to the unused variable.
>
> Fixes: a6576011a4d2 ("ocaml/libs: Implement a dynamically-loaded plugin for 
> Xenctrl.domain_getinfo")
> Signed-off-by: Javi Merino <javi.merino@xxxxxxxxx>

That's horrible...

I note the Ocaml manual even says:

"Some C compilers give bogus warnings about unused variables
caml__dummy_xxx at each use of CAMLparam and CAMLlocal. You should
ignore them."  which a brave claim to make...


The problem with pragma gcc is that we support Clang too.

Having had a play, this works too

@@ -45,7 +45,9 @@ static struct custom_operations xsd_glue_xenctrl_ops = {
 static void Noreturn xsd_glue_failwith(
        xc_interface *xch, const char *func, unsigned int line)
 {
+#define caml__frame __attribute__((unused)) caml__frame
        CAMLparam0();
+#undef caml__frame
        CAMLlocal1(msg);
        const xc_error *error = xch ? xc_get_last_error(xch) : NULL;
        char *str = NULL;

and is rather more selective.


However, looking through other bits of memory.h, there's this gem:

#define CAMLnoreturn ((void) caml__frame)

which has existed since db3745919 "suppression des warnings "unused
variable" de gcc" in 2004.

So, I think this is a better fix:

@@ -69,6 +69,7 @@ static void Noreturn xsd_glue_failwith(
        free(str);
 
        caml_raise_with_arg(*caml_named_value("xsg.error_v1"), msg);
+       CAMLnoreturn;
 }
 #define xsd_glue_failwith(xch) xsd_glue_failwith(xch, __func__, __LINE__)
 
and F40 seem happy with the result.

~Andrew