Re: [XEN PATCH] xen/types: address Rule 10.1 for macro BITS_TO_LONGS

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 08/09/2023 21:37, Stefano Stabellini wrote:
> On Fri, 8 Sep 2023, Nicola Vetrini wrote:
> > On 08/09/2023 13:59, Jan Beulich wrote:
> > > On 08.09.2023 13:57, Jan Beulich wrote:
> > > > On 08.09.2023 10:48, Nicola Vetrini wrote:
> > > > > There is a build error due to -Werror because of a pointer comparison at
> > > > > line 469 of common/numa.c:
> > > > > i = min(PADDR_BITS, BITS_PER_LONG - 1);
> > > > > where
> > > > > #define PADDR_BITS              52
> > > > >
> > > > > I guess PADDR_BITS can become unsigned or gain a cast
> > > >
> > > > While generally converting constants to unsigned comes with a certain
> > > > risk, I think for this (and its siblings) this ought to be okay. As to
> > > > the alternative of a cast - before considering that, please consider
> > > > e.g. adding 0u (as we do elsewhere in the code base to deal with such
> > > > cases).
> > >
> > > And just after sending I realized that this would still be disliked by
> > > Misra's type system. (Much like then aiui the 1 above will need to
> > > become 1u. Which is all pretty horrible.)
> > >
> > > Jan
> >
> > I have a proposal: in our tests we enabled an ECLAIR configuration 
> > that allows
> > to bypass the
> > constraint imposed by Rule 10.4 that warrants the 1U iff the value is 
> > constant
> > and both types
> > can represent it correctly (in this case BITS_PER_LONG -1). This would 
> > allow
> > using the proposed
> > solution and documenting why it's ok not to respect R10.4. What do you 
> > think?
> I think that would be OK. I think we would want to document this in
> rules.rst. Please send a patch.
I checked: that configuration is already enabled in current staging, so 
perhaps the only
action in that regard would be to send a patch documenting it in 
rules.rst.
I just noticed one further issue with making BYTES_PER_LONG unsigned, in 
that causes
several instances of (1U << 3) to appear inside the file 
xen/arch/x86/xen.lds
produced by the build, which in turn causes ld to fail on that 'U'. For 
reference, the version of ld used by the build is the following:
GNU ld (GNU Binutils for Ubuntu) 2.38

The following is a snippet of the output:

       . = ALIGN((1 << 12));
       __ro_after_init_end = .;
       __start_bug_frames = .;
       *(.bug_frames.0)
       __stop_bug_frames_0 = .;
       *(.bug_frames.1)
       __stop_bug_frames_1 = .;
       *(.bug_frames.2)
       __stop_bug_frames_2 = .;
       *(.bug_frames.3)
       __stop_bug_frames_3 = .;
       *(.rodata)
       *(.rodata.*)
       *(.data.rel.ro)
       *(.data.rel.ro.*)
       . = ALIGN((1U << 3)); __start_vpci_array = .; 
*(SORT(.data.vpci.*)) __end_vpci_array = .;
  } :text
  .note.gnu.build-id : AT(ADDR(".note.gnu.build-id") - (((((((261 >> 8) 
* 0xffff000000000000) | (261 << 39))) + ((1 << 39) / 2)) + (64 << 30)) + 
(1 << 30))) {
       __note_gnu_build_id_start = .;
       *(.note.gnu.build-id)
       __note_gnu_build_id_end = .;


--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)