[ovs-dev] [PATCH] Fix compilation error on Fedora 34 with GCC 11
Numan Siddique
numans at ovn.org
Tue Aug 3 17:55:37 UTC 2021
On Thu, Jul 29, 2021 at 2:01 AM Guzowski Adrian via dev
<ovs-dev at openvswitch.org> wrote:
>
>
>
> W dniu śro, 28.07.2021 o godzinie 11∶52 -0400, użytkownik Numan Siddique napisał:
> >
> > On Wed, Jul 28, 2021 at 8:19 AM Guzowski Adrian via dev
> > <ovs-dev at openvswitch.org> wrote:
> > >
> > > With newest version of GCC, OVS fails to compile due to false-positive
> > > overread detection in hash-related functions. Those function declares
> > > "const uint32_t p[]" as argument and it throws off the compiler into
> > > thinking that it reads from memory region of size 0.
> > >
> > > To fix that behavior, a change in argument declaration needs to be made:
> > > instead of using "[]" notation for a pointer, simply use "*".
> > >
> > > The reported error in question:
> > >
> > > lib/conntrack.c: In function ‘conn_key_hash’:
> > > lib/conntrack.c:2154:12: error: ‘hash_words’ reading 4 bytes from a region of size 0 [-
> > > Werror=stringop-overread]
> > > 2154 | return hash_words((uint32_t *) (&key->dst + 1),
> > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > 2155 | (uint32_t *) (key + 1) - (uint32_t *) (&key->dst + 1),
> > > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > 2156 | hash);
> > > | ~~~~~
> > > lib/conntrack.c:2154:12: note: referencing argument 1 of type ‘const uint32_t *’ {aka ‘const
> > > unsigned int *’}
> > > In file included from lib/packets.h:31,
> > > from lib/ct-dpif.h:21,
> > > from lib/conntrack.h:23,
> > > from lib/conntrack.c:26:
> > > lib/hash.h:294:1: note: in a call to function ‘hash_words’
> > > 294 | hash_words(const uint32_t p[], size_t n_words, uint32_t basis)
> > > | ^~~~~~~~~~
> > >
> >
> > Can you please tell how you configured ovs ?
> >
> > On Fedora 34 with gcc version - gcc-11.1.1-3.fc34.x86_64 (gcc version
> > 11.1.1 20210531 (Red Hat 11.1.1-3) (GCC))
> > I don't see any compilation issues.
> >
> > I configured as - ./configure --enable-Werror --enable-sparse
> >
> > Thanks
> > Numan
>
> I disable all optimizations, enable debug and address sanitizer, my configure invocation looks like
> this:
>
> ./configure --enable-Werror CFLAGS='-O0 -g -fsanitize=address'
>
> GCC version: 11.1.1 20210531 (Red Hat 11.1.1-3)
Thanks. I'm able to reproduce the issue now.
And your patch fixes it.
Tested-by: Numan Siddique <numans at ovn.org>
Numan
>
> Regards
> Adrian
>
> >
> > > Signed-off-by: Guzowski Adrian <adrian.guzowski at exatel.pl>
> > > ---
> > > lib/hash.c | 4 ++--
> > > lib/hash.h | 18 +++++++++---------
> > > 2 files changed, 11 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/lib/hash.c b/lib/hash.c
> > > index 06f83395c..c722f3c3c 100644
> > > --- a/lib/hash.c
> > > +++ b/lib/hash.c
> > > @@ -61,13 +61,13 @@ hash_double(double x, uint32_t basis)
> > > }
> > >
> > > uint32_t
> > > -hash_words__(const uint32_t p[], size_t n_words, uint32_t basis)
> > > +hash_words__(const uint32_t *p, size_t n_words, uint32_t basis)
> > > {
> > > return hash_words_inline(p, n_words, basis);
> > > }
> > >
> > > uint32_t
> > > -hash_words64__(const uint64_t p[], size_t n_words, uint32_t basis)
> > > +hash_words64__(const uint64_t *p, size_t n_words, uint32_t basis)
> > > {
> > > return hash_words64_inline(p, n_words, basis);
> > > }
> > > diff --git a/lib/hash.h b/lib/hash.h
> > > index eb3776500..60a39a40b 100644
> > > --- a/lib/hash.h
> > > +++ b/lib/hash.h
> > > @@ -235,7 +235,7 @@ hash_words_inline(const uint32_t p_[], size_t n_words, uint32_t basis)
> > > /* A simpler version for 64-bit data.
> > > * 'n_words' is the count of 64-bit words, basis is 64 bits. */
> > > static inline uint32_t
> > > -hash_words64_inline(const uint64_t p[], size_t n_words, uint32_t basis)
> > > +hash_words64_inline(const uint64_t *p, size_t n_words, uint32_t basis)
> > > {
> > > uint64_t hash1 = basis;
> > > uint64_t hash2 = 0;
> > > @@ -284,14 +284,14 @@ static inline uint32_t hash_pointer(const void *p, uint32_t basis)
> > > }
> > > #endif
> > >
> > > -uint32_t hash_words__(const uint32_t p[], size_t n_words, uint32_t basis);
> > > -uint32_t hash_words64__(const uint64_t p[], size_t n_words, uint32_t basis);
> > > +uint32_t hash_words__(const uint32_t *p, size_t n_words, uint32_t basis);
> > > +uint32_t hash_words64__(const uint64_t *p, size_t n_words, uint32_t basis);
> > >
> > > /* Inline the larger hash functions only when 'n_words' is known to be
> > > * compile-time constant. */
> > > #if __GNUC__ >= 4
> > > static inline uint32_t
> > > -hash_words(const uint32_t p[], size_t n_words, uint32_t basis)
> > > +hash_words(const uint32_t *p, size_t n_words, uint32_t basis)
> > > {
> > > if (__builtin_constant_p(n_words)) {
> > > return hash_words_inline(p, n_words, basis);
> > > @@ -301,7 +301,7 @@ hash_words(const uint32_t p[], size_t n_words, uint32_t basis)
> > > }
> > >
> > > static inline uint32_t
> > > -hash_words64(const uint64_t p[], size_t n_words, uint32_t basis)
> > > +hash_words64(const uint64_t *p, size_t n_words, uint32_t basis)
> > > {
> > > if (__builtin_constant_p(n_words)) {
> > > return hash_words64_inline(p, n_words, basis);
> > > @@ -313,26 +313,26 @@ hash_words64(const uint64_t p[], size_t n_words, uint32_t basis)
> > > #else
> > >
> > > static inline uint32_t
> > > -hash_words(const uint32_t p[], size_t n_words, uint32_t basis)
> > > +hash_words(const uint32_t *p, size_t n_words, uint32_t basis)
> > > {
> > > return hash_words__(p, n_words, basis);
> > > }
> > >
> > > static inline uint32_t
> > > -hash_words64(const uint64_t p[], size_t n_words, uint32_t basis)
> > > +hash_words64(const uint64_t *p, size_t n_words, uint32_t basis)
> > > {
> > > return hash_words64__(p, n_words, basis);
> > > }
> > > #endif
> > >
> > > static inline uint32_t
> > > -hash_bytes32(const uint32_t p[], size_t n_bytes, uint32_t basis)
> > > +hash_bytes32(const uint32_t *p, size_t n_bytes, uint32_t basis)
> > > {
> > > return hash_words(p, n_bytes / 4, basis);
> > > }
> > >
> > > static inline uint32_t
> > > -hash_bytes64(const uint64_t p[], size_t n_bytes, uint32_t basis)
> > > +hash_bytes64(const uint64_t *p, size_t n_bytes, uint32_t basis)
> > > {
> > > return hash_words64(p, n_bytes / 8, basis);
> > > }
> > > --
> > > 2.31.1
> > >
> > > _______________________________________________
> > > dev mailing list
> > > dev at openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list