[PATCH u-boot 14/39] lib: crc32: make the crc_table variable non-const

[Marek Behun]

On Sun, 7 Mar 2021 13:31:11 +0100
Pali Rohár <pali at kernel.org> wrote:

> On Sunday 07 March 2021 13:26:36 Marek Behun wrote:
> > On Sun, 7 Mar 2021 06:02:16 +0100
> > Marek Vasut <marex at denx.de> wrote:
> >   
> > > On 3/7/21 5:58 AM, Marek Behun wrote:  
> > > > On Sun, 7 Mar 2021 05:46:24 +0100
> > > > Marek Vasut <marex at denx.de> wrote:
> > > >     
> > > >> On 3/7/21 5:25 AM, Marek Behún wrote:    
> > > >>> When compiling with LTO, the compiler fails with an error saying that
> > > >>> `crc_table` causes a section type conflict with `efi_var_buf`.
> > > >>>
> > > >>> This is because both are declared to be in the same section (via macro
> > > >>> `__efi_runtime_data`), but one is const while the other is not.
> > > >>>
> > > >>> Make this variable non-const in order to fix this.    
> > > >>
> > > >> This does not look right, the crc32 array is constant.
> > > >> Maybe what you want to do instead if create some __efi_constant_data
> > > >> section ?    
> > > > 
> > > > Yes, this was the easier solution, and maybe is not ideal.
> > > > 
> > > > I thought it would not be much of a problem since this array can be
> > > > nonconstant (generated after boot) if CONFIG_DYNAMIC_CRC_TABLE is
> > > > enabled.
> > > > 
> > > > Anyway I don't much understand the EFI code so I wanted to poke into it
> > > > as little as possible.    
> > > 
> > > Isn't the compiler capable of better optimization on constant stuff ?
> > > That's pretty much what prompted my suggestion to add separate section.  
> > 
> > Yes, but
> > - for this case I don't think the compiler actually can do any
> >   significat optimizations when declaring the table const, since it has
> >   to access
> >     tab[(crc ^ (x)) & 255]
> >   I tried with arm compiler just now, with -O2 and -Os, and it
> >   generated the same code either way
> > - when using LTO, the compiler theoretically should be able to deduce
> >   that the table is never written to
> > 
> > But if people insist on declaring the table const, I will look into
> > this...  
> 
> If you try to overwrite a const variable from the same code unit then
> compiler throw an error. So declaring read-only variable as a const
> could prevent people to unintentionally overwrite read-only variable.
> And detect possible bad code.

OK it seems all linker scripts also mention .rodata.efi_runtime*, not
just .data.efi_runtime*. I shall put it into the .rodata.efi_runtime
section.

Marek