Re: Sim bfin build failure with gcc 11

[Mike Frysinger via Gdb-patches <gdb-patches@sourceware.org>]

On 23 May 2021 21:41, Simon Marchi via Gdb-patches wrote:
> On 2021-05-23 9:37 p.m., Mike Frysinger wrote:
> > On 23 May 2021 20:51, Simon Marchi via Gdb-patches wrote:
> >> I see this with gcc 11:
> >>
> >> $ ccache gcc  -DHAVE_CONFIG_H -DWITH_DEFAULT_MODEL='"bf537"' -DWITH_DEFAULT_ALIGNMENT=STRICT_ALIGNMENT  -DWITH_TARGET_BYTE_ORDER=BFD_ENDIAN_LITTLE  -DWITH_HW=1 -DDEFAULT_INLINE=0   -Wall -Wdeclaration-after-statement -Wpointer-arith -Wpointer-sign -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wmissing-prototypes -Wdeclaration-after-statement -Wempty-body -Wmissing-parameter-type -Wold-style-declaration -Werror  -I. -I/home/simark/src/binutils-gdb/sim/bfin -I../common -I/home/simark/src/binutils-gdb/sim/bfin/../common -I../../include -I/home/simark/src/binutils-gdb/sim/bfin/../../include -I../../bfd -I/home/simark/src/binutils-gdb/sim/bfin/../../bfd -I../../opcodes -I/home/simark/src/binutils-gdb/sim/bfin/../../opcodes  -I/usr/include/SDL -D_GNU_SOURCE=1 -D_REENTRANT  -DHAVE_SDL -g3 -O0 -fsanitize=address -fmax-errors=1     -c -o dv-bfin_otp.o -MT dv-bfin_otp.o -MMD -MP -MF .deps/dv-bfin_otp.Tpo /home/simark/src/binutils-gdb/sim/bfin/dv-bfin_otp.c
> >> /home/simark/src/binutils-gdb/sim/bfin/dv-bfin_otp.c: In function ‘bfin_otp_write_page’:
> >> /home/simark/src/binutils-gdb/sim/bfin/dv-bfin_otp.c:94:3: error: ‘bfin_otp_write_page_val’ accessing 16 bytes in a region of size 4 [-Werror=stringop-overflow=]
> >>    94 |   bfin_otp_write_page_val (otp, page, (void *)&otp->data0);
> >>       |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> /home/simark/src/binutils-gdb/sim/bfin/dv-bfin_otp.c:94:3: note: referencing argument 3 of type ‘bu64 *’ {aka ‘long unsigned int *’}
> >> /home/simark/src/binutils-gdb/sim/bfin/dv-bfin_otp.c:81:1: note: in a call to function ‘bfin_otp_write_page_val’
> >>    81 | bfin_otp_write_page_val (struct bfin_otp *otp, bu16 page, bu64 val[2])
> >>       | ^~~~~~~~~~~~~~~~~~~~~~~
> >>
> >> It's not immediately obvious to me why it says that.
> > 
> > gcc really wants its pointer types to be in harmony.  i cheated here.
> > 
> > write_page_val wants a pointer to an array of 2 64-bit values.  i know the
> > otp struct has "bu64 data0, data1, data2, data3;", and taking the address
> > of data0 has the same memory layout as if it were "bu64 data[3];".  but with
> > the fortify work gcc has long been doing, they've stopped accepting these
> > kinds of hacks.
> > 
> > this code is not perf sensitive, so i can be less "clever" while keeping the
> > compiler happy.
> 
> Ah, I see.  Well if what you want is make sure the fields are
> contiguous, why not make that an array of four bu32 instead?

i'm not worried about the ordering ... it's a struct, so that's guaranteed.
the code uses a style to make it easy to compare against the datasheets and
to make it easy to access individual fields.  the datasheets have "DATA0"
an "DATA1" and such as explicit MMRs, not DATA[3].

but i misread the code and thought it said bu64 data0... when it's really
bu32 as you point out.  which is why i was being clever in the first place,
and the patch i just pushed is incorrect.

i guess the existing code was assuming the host cpu was little endian, so
doing a manual construction of the 64bit fields is unavoidable.  so let's
try this again.
-mike

commit d699be882b42f36677836c320edbf7db24021a30
Author: Mike Frysinger <vapier@gentoo.org>
Date:   Sun May 23 22:15:01 2021 -0400

    sim: bfin: fix the otp fix
    
    I misread the code and thought data0/... were bu64 when they were
    actually bu32.  Fix the call to assemble the 2 64-bit values instead
    of passing the 2 halves of the first 64-bit value.

diff --git a/sim/bfin/ChangeLog b/sim/bfin/ChangeLog
index 9c517d20bbeb..29dfde8fe6e3 100644
--- a/sim/bfin/ChangeLog
+++ b/sim/bfin/ChangeLog
@@ -1,3 +1,8 @@
+2021-05-23  Mike Frysinger  <vapier@gentoo.org>
+
+	* dv-bfin_otp.c (bfin_otp_write_page): Fix args to
+	bfin_otp_write_page_val2.
+
 2021-05-23  Mike Frysinger  <vapier@gentoo.org>
 
 	* dv-bfin_otp.c (bfin_otp_write_page): Call bfin_otp_write_page_val2.
diff --git a/sim/bfin/dv-bfin_otp.c b/sim/bfin/dv-bfin_otp.c
index 65afdf58b27f..cdc010ae551b 100644
--- a/sim/bfin/dv-bfin_otp.c
+++ b/sim/bfin/dv-bfin_otp.c
@@ -91,7 +91,8 @@ bfin_otp_write_page_val2 (struct bfin_otp *otp, bu16 page, bu64 lo, bu64 hi)
 static void
 bfin_otp_write_page (struct bfin_otp *otp, bu16 page)
 {
-  bfin_otp_write_page_val2 (otp, page, otp->data0, otp->data1);
+  bfin_otp_write_page_val2 (otp, page, (bu64)otp->data1 | otp->data0,
+			    (bu64)otp->data3 | otp->data2);
 }
 
 static unsigned