Re: Please fix regressions from your sim changes

Hans-Peter Nilsson
2012-03-20T05:55:00Z

> Date: Tue, 20 Mar 2012 03:30:20 +0100

> you can see the commit in question clearly doesn't remove the msg:
> --- a/sim/common/nrun.c
> +++ b/sim/common/nrun.c
> - fprintf (stderr, "program stopped with signal %d.\n", sigrc);
> + fprintf (stderr, "program stopped with signal %d (%s).\n", sigrc,
> + strsignal (sigrc));

When paying attention, I see nrun.c:1.16 is missing an #include
to declare it. See below.

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

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.

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:

#ifdef HAVE_STRING_H
#include <string.h>
#else
#ifdef HAVE_STRINGS_H
#include <strings.h>
#endif
#endif

Or shorter, see run.c for how nrun.c needs to be.

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

BTW, your testsuite fixes are both approved and obvious.

brgds, H-P