Re: Please fix regressions from your sim changes

Mike Frysinger
2012-03-20T16:16:24Z

[-- Attachment #1: Type: Text/Plain, Size: 2743 bytes --]

On Tuesday 20 March 2012 01:54:27 Hans-Peter Nilsson wrote:
> From: Mike Frysinger <vapier@gentoo.org>
> > maybe strsignal() is crashing ?
>
> Yes:
> chimera-7:hp:/tmp/hpautotest-sim/cris-elf/sim/testsuite:
> /tmp/hpautotest-sim/cris-elf/sim/cris/run addqpc.ms.x General register
> read of PC is not implemented.
> zsh: segmentation fault /tmp/hpautotest-sim/cris-elf/sim/cris/run
> addqpc.ms.x
>
> And in the build-log I see:
> gcc -DHAVE_CONFIG_H -DWITH_DEFAULT_MODEL='"crisv32"' -DPROFILE=1
> -DWITH_PROFILE=-1 -DWITH_ALIGNMENT=NONSTRICT_ALIGNMENT
> -DWITH_ENVIRONMENT=ALL_ENVIRONMENT -DWITH_HW=0
> -DWITH_HOST_BYTE_ORDER=LITTLE_ENDIAN -DWITH_SCACHE=16384 -Wimplicit
> -Wreturn-type -Wcomment -Wtrigraphs -Wformat -Wparentheses -Wpointer-arith
> -Wuninitialized -I. -I/tmp/r/sim/cris -I../common
> -I/tmp/r/sim/cris/../common -I../../include
> -I/tmp/r/sim/cris/../../include -I../../bfd -I/tmp/r/sim/cris/../../bfd
> -I../../opcodes -I/tmp/r/sim/cris/../../opcodes -g -O2 -c -o nrun.o -MT
> nrun.o -MMD -MP -MF .deps/nrun.Tpo /tmp/r/sim/cris/../common/nrun.c
> /tmp/r/sim/cris/../common/nrun.c: In function 'main':
> /tmp/r/sim/cris/../common/nrun.c:206: warning: implicit declaration of
> function 'strsignal' /tmp/r/sim/cris/../common/nrun.c:206: warning: format
> '%s' expects type 'char *', but argument 4 has type 'int'
>
> Don't you see that warning?

the code compiles warning free for me

> Are you on a ILP32 host such as i686-linux-gnu, i.e.
> sizeof (char *) == sizeof(int)?

no, x86_64-pc-linux-gnu

> It seems nowhere is there an include of string.h (just
> pre-existing signal.h), see strerror(3). (TFM, not SIGQUIT :)
> What was missing from your patch introducing the strsignal call,
> is *not* the single obvious #define and #include.

the mips' sim-main.h code eventually includes string.h for me. the file should
have a string.h include for the strsignal prototype, so i'll update that.

> Instead, to #define _GNU_SOURCE in the Right Way AFAIK, there
> should be an AC_GNU_SOURCE (missing) in a configure.* used by
> all src/sim/* (common/configure.ac AFAICT), and an #include
> "cconfig.h" in nrun.c followed later by the mantra:

not entirely the right way. my guess is that your older glibc does not
conform to the newer POSIX spec. strsignal() was added in the 2008 spec, but
old glibc's provided it only when _GNU_SOURCE was defined.

> And right, it's time to add -Wall -Werror to the sim compilation
> options...

i posted a patch to improve that plumbing already. however, -Werror won't be
enabled by default because all the sim code and ports have problems, and even
if it was, it'd still pass on my side.