Re: [PATCH] [sim] Fix build failure in d10v sim

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

On 10 May 2021 23:01, Luis Machado via Gdb-patches wrote:
> On 5/10/21 7:30 PM, Mike Frysinger wrote:
> > On 10 May 2021 16:14, Luis Machado via Gdb-patches wrote:
> >> While building all targets on Ubuntu 20.04/aarch64, I ran into the following
> >> build error:
> >>
> >> In file included from /usr/include/string.h:495,
> >>                   from ../../bfd/bfd.h:48,
> >>                   from ../../../../repos/binutils-gdb/sim/d10v/interp.c:4:
> >> In function 'memset',
> >>      inlined from 'sim_create_inferior' at ../../../../repos/binutils-gdb/sim/d10v/interp.c:1146:3:
> >> /usr/include/aarch64-linux-gnu/bits/string_fortified.h:71:10: error: ‘__builtin_memset’ offset [33, 616] from the object at ‘State’ is out of the bounds of referenced subobject ‘regs’ with type ‘reg_t[16]’ {aka ‘short unsigned int[16]’} at offset 0 [-Werror=array-bounds]
> >>     71 |   return __builtin___memset_chk (__dest, __ch, __len, __bos0 (__dest));
> >>        |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> cc1: all warnings being treated as errors
> >> make[3]: *** [Makefile:558: interp.o] Error 1
> >>
> >> I looked at a different sim (cr16), and it zeroes out the entire State, not
> >> just the registers.
> > 
> > it's clearing more than just the regs.  it's clearing all the members in the
> > state struct from regs up to mem.  so regs, cregs, sp, a, slot, etc...  if
> > you want to change it, the comment in the header probably needs adjusting.
> 
> Heh... the comment in the header says something and the one before 
> memset says another that is completely different.
> 
> >> It is unclear why we have the casts to uintptr.
> > 
> > because pointer math on diff types isn't allowed, and the 3rd arg to memset
> > needs to be a scalar integer.  so casting to uintptr_t is the correct way to
> > calculate this value.
> 
> Right. What I mean is I don't understand why the complexity to clear 
> some data with memset while the comment is stating we should clear 
> everything. Then again, it looks like it was copy/pasted from somewhere 
> else and not documented properly in the .c file.
> 
> The same seems to be happening with cr16. The header says we reset 
> things until a certain member. But the .c code says we reset everything.

i don't think the comment/code is at odds.  it's just not as precise as it
should be.  it's clearing all the cpu state, but leaving the system (memory)
state alone.

if you look through the git history on these files, it was a bit more explicit:
it would save the fields it cared about, memset the whole thing, then restore
the fields.  at some point that was all deleted and replaced with this (clever?)
memset call.

copy pasta between some of these older sim ports is quite bad/heavy.  it's slow
going converting it over, especially for ports that don't seem to have anyone
who cares about them anymore.  some of these arches i know exist only because
there's a sim port for them ... never heard of them otherwise.
-mike