| Message ID | d6b98ca75017bf4aa72a69468321705263e66f30.1729170005.git.javi.merino@cloud.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | CI: Upgrade the Fedora container and fix build failure | expand |
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@cloud.com> 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
On 17/10/2024 6:47 pm, Andrew Cooper wrote: > 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@cloud.com> > 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. And https://github.com/ocaml/ocaml/pull/13561 to update Ocaml's documentation on the matter. ~Andrew
> On 17 Oct 2024, at 18:47, Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > > 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. Acked-by: Christian Lindig <christian.lindig@cloud.com> I like the effort to remove warnings. — C
On Thu, Oct 17, 2024 at 06:47:44PM +0100, Andrew Cooper wrote: > 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@cloud.com> > > 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__) > Right! I actually tried put a (void)caml__frame to silence the warning, but I didn't think of looking for a macro that would do exactly that. Do I have to resend the series with your patch, or can you commit it directly? Cheers, Javi
On 18/10/2024 9:50 am, Javi Merino wrote: > On Thu, Oct 17, 2024 at 06:47:44PM +0100, Andrew Cooper wrote: >> 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@cloud.com> >> 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__) >> > Right! I actually tried put a (void)caml__frame to silence the > warning, but I didn't think of looking for a macro that would do > exactly that. :) It's Ocaml. I had a hunch there'd be something. > Do I have to resend the series with your patch, or can you commit it > directly? I'll just fold it on commit. No point spending more time on this. In the meantime, Ocaml upstream have already merged my docs PR about this. ~Andrew
diff --git a/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/domain_getinfo_stubs_v1.c b/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/domain_getinfo_stubs_v1.c index 7be386f4d4c2..df2b3c74c727 100644 --- a/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/domain_getinfo_stubs_v1.c +++ b/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/domain_getinfo_stubs_v1.c @@ -42,6 +42,8 @@ static struct custom_operations xsd_glue_xenctrl_ops = { .compare_ext = custom_compare_ext_default, /* Can't compare */ }; +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-variable" static void Noreturn xsd_glue_failwith( xc_interface *xch, const char *func, unsigned int line) { @@ -70,6 +72,7 @@ static void Noreturn xsd_glue_failwith( caml_raise_with_arg(*caml_named_value("xsg.error_v1"), msg); } +#pragma GCC diagnostic pop #define xsd_glue_failwith(xch) xsd_glue_failwith(xch, __func__, __LINE__) CAMLprim value stub_xsd_glue_xc_interface_open(value unit)
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@cloud.com> --- While it is not ideal, this is the most effective option I have found that avoids treating warnings as errors. I can add a comment above the pragma explaining why if we decide that this is the way forward. .../domain_getinfo_plugin_v1/domain_getinfo_stubs_v1.c | 3 +++ 1 file changed, 3 insertions(+)