Re: [hash2][review] Early review (due to holidays)

From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2024-12-06 17:42:44

On 12/6/24 20:30, Peter Dimov via Boost wrote:
> You can see practical manifestations of these examples in e.g
> 
> https://github.com/pdimov/hash2/blob/develop/include/boost/hash2/md5.hpp
> 
> where at
> 
> https://github.com/pdimov/hash2/blob/26b16023dc639ae354e246458719e4b569bc1d97/include/boost/hash2/md5.hpp#L110
> 
> you have
> 
>     BOOST_CXX14_CONSTEXPR void transform( unsigned char const block[ 64 ] )
> 
> with the implicit precondition that `block` points at at least 64 valid bytes,
> and then at
> 
> https://github.com/pdimov/hash2/blob/26b16023dc639ae354e246458719e4b569bc1d97/include/boost/hash2/md5.hpp#L119-L122
> 
> you have
> 
>         for( int i = 0; i < 16; ++i )
>         {
>             x[ i ] = detail::read32le( block + i * 4 );
>         }
> 
> which relies on this implicit precondition to read 16 subspans of size 4.

Assuming you had code like this:

  void transform( span<unsigned char const, 64> block )
  {
    for( int i = 0; i < 16; ++i )
    {
      x[ i ] = detail::read32le( block.subspan(i * 4, 4) );
    }
  }

I would presume, the subspan call would have a runtime check for the
offset and size. Then the converting constructor from span<const
unsigned char> returned from subspan to span<const unsigned char, 4>
accepted by read32le would have to check the size again. Yes, the
compiler could optimize those checks away, if it's smart enough. Or it
could not, e.g. if optimizations are disabled or it fails to propagate
constants for some reason. But none of these checks are actually
necessary, except maybe one at the entry, to enforce the size of the
entire block. Maybe not even that, if the callers of transform are
guaranteed to always pass the whole blocks.