Clang generates a warning if there is a function declaration/definition
with zero arguments. Such declarations/definitions without a prototype (an
argument list) are deprecated forms of indefinite arguments
("-Wdeprecated-non-prototype"). On the default configuration, it causes a
build failure (unless "--disable-werror" is specified).
But there is another issue. This function declaration in sim/sh/interp.c
is completely redundant. This commit just removes that declaration.
The variable "list" is only initialized when arg1 > 0 and when arg1 == 0,
an uninitialized value is passed to translate_endian_h2t function.
Although this behavior is harmless, this commit adds initialization to avoid
a GCC warning ("-Wmaybe-uninitialized").
Clang generates a warning if an argument is passed to a function without
prototype (zero arguments, even without (void)). Such calls are deprecated
forms of indefinite arguments passing ("-Wdeprecated-non-prototype").
On the default configuration, it (somehow) doesn't cause a build failure but
a warning is generated.
But because the cause is the same as the issue the author fixed in
"sim/erc32: Use int32_t as event callback argument", it would be better to
fix it now to prevent problems in the future.
To fix the issue, this commit makes struct irqcall to use int32_t as a
callback (callback) argument of an IRQ.
Clang generates a warning if an argument is passed to a function without
prototype (zero arguments, even without (void)). Such calls are deprecated
forms of indefinite arguments passing ("-Wdeprecated-non-prototype").
On the default configuration, it causes a build failure (unless
"--disable-werror" is specified).
To fix that, this commit makes struct evcell to use int32_t as a callback
(cfunc) argument of an event. int32_t is chosen because "event" function
accepts "int32_t arg".
Clang generates a warning if there is a function declaration/definition
with zero arguments. Such declarations/definitions without a prototype (an
argument list) are deprecated forms of indefinite arguments
("-Wdeprecated-non-prototype"). On the default configuration, it causes a
build failure (unless "--disable-werror" is specified).
This commit replaces () with (void) to avoid this warning.
Because of a Libiberty hack, getopt on GNU libc (2.25 or earlier) is
currently unusable on sim, causing a regression on CentOS 7.
This is caused as follows:
1. If HAVE_DECL_GETOPT is defined (getopt declaration with known prototype
is detected while configuration), a declaration of getopt in
"include/getopt.h" is suppressed.
The author started to define HAVE_DECL_GETOPT in sim with the commit
340aa4f687 ("sim: Check known getopt definition existence").
2. GNU libc (2.25 or earlier)'s <unistd.h> includes <getopt.h> with a
special purpose macro defined to declare only getopt function but due
to include path (not tested while configuration), it causes <unistd.h>
to include Libiberty's "include/getopt.h".
3. If both 1. and 2. are satisfied, despite that <unistd.h> tries to
declare getopt by including <getopt.h>, "include/getopt.h" does not do
so, causing getopt function undeclared.
Getting rid of "include/getopt.h" (e.g. renaming this header file) is the
best solution to avoid hacking but as a short-term solution, this commit
replaces getopt with getopt_long under sim/.
Zero initialize engine_fns entirely at creation, then override those
fields we intend to use, rather than zero just initializing the unused
fields later on.
There should be no user visible changes after this commit.
The current logic limits itself to a maxdepth of 4 when looking for
results. This wouldn't be a problem if cris didn't have a testsuite
at a depth of 5 which we end up ignoring when summarizing. Rather
than bump the number from 4 to 5, rework the code so that we gather
the exact set of tests that we tried to run.
In the lm32 simulator, I was seeing some warnings about missing
function declarations.
The lm32 simulator has a weird header structure, in order to pull in
the full cpu.h header we need to define WANT_CPU_LM32BF. This is done
in some files, but not in others. Critically, it's not done in some
files that then use functions declared in cpu.h
In this commit I added the missing #define so that the full cpu.h can
be included.
After doing this there are still a few functions that are used
undeclared, these functions appear to be missing any declarations at
all, so I've added some to cpu.h.
With this done all the warnings when compiling lm32 are resolved for
both gcc and clang, so I've removed the SIM_WERROR_CFLAGS line from
Makefile.in, this allows lm32 to build with -Werror.
There are two places in the h8300 simulator where we assign a variable
to itself. Clang gives a warning for this, which is converted into an
error by -Werror.
Silence the warning by removing the self assignments. As these
assignments were in a complex if/then/else tree, rather than try to
adjust all the conditions, I've just replaced the self assignments
with a comment and an empty statement.
In the ppc simulator, clang was warning about some code like this:
busy_ptr->nr_writebacks = 1 + (PPC_ONE_BIT_SET_P(out_vmask)) ? 1 : 2;
The warning was:
operator '?:' has lower precedence than '+'; '+' will be evaluated first
I suspect that this is not the original authors intention.
PPC_ONE_BIT_SET_P is going to be 0 or 1, so if we evaluate the '+'
first, the condition will always be non-zero, so true. The whole
expression could then be simplified to just '1', which doesn't make
much sense.
I suspect the answer the author was expecting was either 2 or 3. Why
they didn't just write:
busy_ptr->nr_writebacks = (PPC_ONE_BIT_SET_P(out_vmask)) ? 2 : 3;
I have no clue, however, to keep the structure of the code unchanged,
I've updated things to:
busy_ptr->nr_writebacks = 1 + (PPC_ONE_BIT_SET_P (out_vmask) ? 1 : 2);
which silences the warning from clang, and is, I am guessing, what the
original author intended.
In the ppc simulator's do_fstat function, which provides the fstat
call for the simulator, if the fstat is going to fail then we
currently write an uninitialized buffer into the simulated target.
In theory, I think this is fine, we also write the error status into
the simulated target, so, given that the fstat has failed, the target
shouldn't be relying on the buffer contents.
However, writing an uninitialized buffer means we might leak simulator
private data into the simulated target, which is probably a bad thing.
Plus it probably makes life easier if something consistent, like all
zeros, is written rather than random junk, which might look like a
successful call (except for the error code).
So, in this commit, I initialize the stat buffer to zero before
it is potentially used. If the stat call is not made then the buffer
will be left initialized as all zeros.
The ppc simulator, in sim_create_inferior, tries to print the function
local entry_point variable before the variable is initialized.
In this commit, I defer the debug print line until the variable has
been initialized.
Now that we run `check/foo.exp` instead of `check/./foo.exp`,
update the config/ & lib/ exceptions to cover both paths.
Bug: https://sourceware.org/PR29596
Make sure we invoke runtest with the same exp filenames when running in
parallel as it will find when run single threaded. When `runtest` finds
files itself, it will use paths like "aarch64/allinsn.exp". When we run
`find .` with the %p option, it produces "./aarch64/allinsn.exp". Switch
to %P to get "aarch64/allinsn.exp".
Bug: https://sourceware.org/PR29596
These configure scripts check $target and change behavior. They
shouldn't be doing that, but until we can rework the sim to change
behavior based on the input ELF, restore AC_CANONICAL_SYSTEM to
these so that $target is correctly populated.
This was lost in the d3562f83a7
("sim: unify toolchain probing logic") refactor as the logic was
hoisted up to the common code. But the fact the vars weren't
passed down to the sub-configure scripts was missed.
Bug: https://sourceware.org/PR29439
Not only that sim/configure.ac does not AC_SUBST CXXFLAGS,
unless we need C++ compiler like CXX, substitution @CXXFLAGS@ is useless.
Because of this, this commit removes this substitution.
When building the iq2000 simulator I see a few warnings like this:
/tmp/build/sim/../../src/sim/iq2000/iq2000.c: In function ‘fetch_str’:
/tmp/build/sim/../../src/sim/iq2000/iq2000.c:50:54: error: pointer targets in passing argument 3 of ‘sim_read’ differ in signedness [-Werror=pointer-sign]
50 | sim_read (CPU_STATE (current_cpu), CPU2DATA(addr), buf, nr);
| ^~~
| |
| char *
I've silenced these warnings by casting buf to 'unsigned char *'.
With this change I now see no warnings when compiling iq2000.c, so
I've removed the line from Makefile.in that disables -Werror.
Makefile.in was also disabling -Werror when compiling mloop.c,
however, I'm not seeing any warnings when compiling that file, so I've
removed the -Werror disable in that case too.
When building the erc32 simulator I get a few warnings like this:
/tmp/build/sim/../../src/sim/erc32/exec.c:1377:21: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
1377 | sregs->fs[rd] = *((float32 *) & ddata[0]);
| ~^~~~~~~~~~~~~~~~~~~~~~~
The type of '& ddata[0]' will be 'uint32_t *', which is what triggers
the warning.
This commit makes use of memcpy when performing the type-punning,
which resolves the above warnings.
With this change, I now see no warnings when compiling exec.c, which
means that the line in Makefile.in that disables -Werror can be
removed.
There should be no change in behaviour after this commit.
The device_error function always ends up calling the error function,
which is itself marked as ATTRIBUTE_NORETURN, so it makes sense that
device_error should also be marked ATTRIBUTE_NORETURN.
Doing this resolves a few warnings from hw_ide.c about possibly
uninitialized variables - the variables are only uninitialized after
passing through a call to device_error, which obviously means the
variables are never really used uninitialized, the simulation will
terminate with the device_error call.
This commit is a follow on to:
commit 182421c9d2
Date: Tue Oct 11 15:02:08 2022 +0100
sim/ppc: fixes for arguments to printf style functions
where commit 182421c9d2 addressed issues with printf format
arguments that were causing the compiler to give an error, this commit
addresses issues that caused the compiler to emit a warning.
This commit is mostly either changing the format string to match the
argument, or in some cases, excess, unused arguments are removed.
I see an uninitialized variable warning (with gcc 9.3.1) from
cgen-run.c, like this:
/tmp/build/sim/../../src/sim/cris/../common/cgen-run.c: In function ‘sim_resume’:
/tmp/build/sim/../../src/sim/cris/../common/cgen-run.c:259:5: warning: ‘engine_fns$’ may be used uninitialized in this function [-Wmaybe-uninitialized]
259 | (* engine_fns[next_cpu_nr]) (cpu);
| ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/build/sim/../../src/sim/cris/../common/cgen-run.c:232:14: note: ‘engine_fns$’ was declared here
232 | ENGINE_FN *engine_fns[MAX_NR_PROCESSORS];
| ^~~~~~~~~~
This is a false positive - we over allocate engine_fn, and then only
initialize the nr_cpus entries which we will later go on to use.
However, we can easily silence this warning by initializing the unused
entries in engine_fns to NULL, this might also help if anyone ever
looks at engine_fns in a debugger, it should now be obvious which
entries are in use, and which are not.
With this change the warning is gone.
There should be no change in behaviour with this commit.
Because "%p" is the pointer conversion specifier to print a pointer in an
implementation-defined manner, the result with format string containing
"0x%p" can be strange. For instance, core_map_find_mapping prints error
containing "0x0x...." (processor is not NULL) or "0x(null)" (processor is
NULL) on glibc.
This commit replaces "0x%p" with "%p" to prevent unpredictable behavior.
After the recent series of fixes to mark more functions in the
simulator with ATTRIBUTE_PRINTF, there were some build failures in the
ppc sim due, in some cases, to bugs with the arguments being passed,
and in other cases, the issues were (maybe) less serious, with
arguments being the wrong size, or type, for the printf format being
used.
This commit fixes all of the issues that I ran into.
In each case I selected the easiest solution to the problem, which is
usually just casting the argument to the correct type. If anyone
later on thinks the print format should change, please feel free to do
that. What we have here should keep the simulator basically working
as it does currently, which is my goal with this commit.
On the files generated by sim/common/genmloop.sh, variables pbb_br_type and
pbb_br_npc are declared uninitialized and passed to other functions in some
cases. Despite that those are harmless, they will generate GCC warnings
("-Wmaybe-uninitialized").
This commit ensures that pbb_br_type and pbb_br_npc variables are
initialized to a harmless value.
Clang generates a warning if there is a function declaration/definition
with zero arguments. Such declarations/definitions without a prototype (an
argument list) are deprecated forms of indefinite arguments
("-Wdeprecated-non-prototype"). On the default configuration, it causes a
build failure (unless "--disable-werror" is specified).
include/getopt.h defines some getopt function definitions but one of them
has a form "extern int getopt ();". If this form is selected in
include/getopt.h, Clang generates a warning and the build fails by default.
In really old environments, this getopt definition with no arguments is
necessary (because the definition may change between environments).
However, this definition is now a cause of problems on modern environments.
A good news is, this definition is not always selected (e.g. if used by
binutils/*.c). This is because configuration scripts of binutils, gas,
gprof and ld tries to find known definition of getopt function is used and
defines HAVE_DECL_GETOPT macro. If this macro is defined when getopt.h is
included, a good form of getopt is used and Clang won't generate warnings.
This commit adds a modified portion of ld/configure.ac to find the known
getopt definition. If we could find one (and we *will* in most modern
environments), we don't need to rely on the deprecated definition.
Clang generates a warning if the format string of a printf-like function is
not a literal ("-Wformat-nonliteral"). On the default configuration, it
causes a build failure (unless "--disable-werror" is specified).
To avoid this warning, this commit now uses vsnprintf to format error
message and pass the message to sim_engine_abort function with another
printf-style formatting.
This patch is mostly authored by Andrew Burgess and slightly modified by
Tsukasa OI.
Co-authored-by: Andrew Burgess <aburgess@redhat.com>
Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
Clang generates a warning if there is an ambiguous expression (possibly a
bitwise operation (& or |), but a logical operator (&& or ||) is used;
"-Wconstant-logical-operand"). On the default configuration, it causes a
build failure (unless "--disable-werror" is specified).
This is caused by predicate macros that use the form (base_variable & flag).
Clang considers them as regular integer values (not boolean) and
generates that warning.
This commit makes Clang think those predicate macros to be boolean.
Clang generates a warning if there is a redundant self-assignment
("-Wself-assign"). On the default configuration, it causes a build failure
(unless "--disable-werror" is specified).
This commit removes redundant self-assignments from two files.
Clang generates a warning if the format string of a printf-like function is
not a literal ("-Wformat-nonliteral"). On the default configuration, it
causes a build failure (unless "--disable-werror" is specified).
To avoid warnings on the printf-like wrapper, it requires proper
__attribute__((format)) and we have ATTRIBUTE_PRINTF macro for this reason.
This commit adds ATTRIBUTE_PRINTF to the printf-like functions.
Clang generates a warning if the format string of a printf-like function is
not a literal ("-Wformat-nonliteral"). On the default configuration, it
causes a build failure (unless "--disable-werror" is specified).
To avoid warnings on the printf-like wrapper, it requires proper
__attribute__((format)) and we have ATTRIBUTE_PRINTF macro for this reason.
This commit adds ATTRIBUTE_PRINTF to the printf-like functions.
For the error function defined in sim_calls.c, the ATTRIBUTE_NORETURN
has been moved to the function declaration.
Clang generates a warning if the format string of a printf-like function is
not a literal ("-Wformat-nonliteral"). On the default configuration, it
causes a build failure (unless "--disable-werror" is specified).
To avoid warnings on the printf-like wrapper, it requires proper
__attribute__((format)) and we have ATTRIBUTE_PRINTF macro for this reason.
This commit adds ATTRIBUTE_PRINTF to a printf-like function.
Clang generates a warning if the format string of a printf-like function is
not a literal ("-Wformat-nonliteral"). On the default configuration, it
causes a build failure (unless "--disable-werror" is specified).
To avoid warnings on the printf-like wrapper, it requires proper
__attribute__((format)) and we have ATTRIBUTE_PRINTF macro for this reason.
This commit adds ATTRIBUTE_PRINTF to the printf-like functions.
Clang generates a warning if the format string of a printf-like function is
not a literal ("-Wformat-nonliteral"). On the default configuration, it
causes a build failure (unless "--disable-werror" is specified).
To avoid warnings on the printf-like wrapper, it requires proper
__attribute__((format)) and we have ATTRIBUTE_PRINTF macro for this reason.
This commit adds ATTRIBUTE_PRINTF to the printf-like functions.
Clang generates a warning if the format string of a printf-like function is
not a literal ("-Wformat-nonliteral"). On the default configuration, it
causes a build failure (unless "--disable-werror" is specified).
To avoid warnings on the printf-like wrapper, it requires proper
__attribute__((format)) and we have ATTRIBUTE_PRINTF macro for this reason.
This commit adds ATTRIBUTE_PRINTF to a printf-like function.
Clang generates a warning if the format string of a printf-like function is
not a literal ("-Wformat-nonliteral"). On the default configuration, it
causes a build failure (unless "--disable-werror" is specified).
To avoid warnings on the printf-like wrapper, it requires proper
__attribute__((format)) and we have ATTRIBUTE_PRINTF macro for this reason.
This commit adds ATTRIBUTE_PRINTF to a printf-like function.
After this commit:
commit 0938b032da
Date: Wed Feb 2 10:06:15 2022 +0900
RISC-V: Add 'Zmmul' extension in assembler.
some instructions in the RISC-V simulator stopped working as a new
instruction class 'INSN_CLASS_ZMMUL' was added, and some existing
instructions were moved into this class.
The simulator doesn't currently handle this instruction class, and so
the instructions will now cause an illegal instruction trap.
This commit adds support for INSN_CLASS_ZMMUL, and adds a test that
ensures the affected instructions can be executed by the simulator.
Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
Reviewed-by: Andrew Burgess <aburgess@redhat.com>
Because sim/moxie/moxie-gdb.dtb is neither a program nor a library, automake
does not generate dirstamp file ($builddir/sim/moxie/.dirstamp) for it.
When maintainer mode is enabled, it tries to rebuild sim/moxie/moxie-gdb.dtb
but fails because there's no rules for automake-generated dirstamp file
which moxie-gdb.dtb depends.
This commit adds its own rule for the directory stamp (modified copy of the
automake output) and adds the directory stamp file to DISTCLEANFILES to
mimic automake-generated behavior (although "make distclean" does not work
when maintainer mode is enabled).
This commit removes SBREAK-related references on the simulator as it's
renamed to EBREAK in 2016 (the RISC-V ISA, version 2.1).
sim/ChangeLog:
* riscv/sim-main.c (execute_i): Use "ebreak" instead of "sbreak".
In commit:
commit 7b01c1cc1d
Date: Mon Apr 4 22:38:04 2022 +0100
sim: fixes for libopcodes styled disassembler
changes were made to the simulator source to handle the new libopcodes
disassembler styling API.
Unfortunately, these changes broke building GDB with the erc32 (sparc)
simulator, like this:
../src/configure --target=sparc-linux
make all-gdb
....
/usr/bin/ld: ../sim/erc32/libsim.a(interf.o): in function `sim_open':
/tmp/build/sim/../../src/sim/erc32/interf.c:247: undefined reference to `fprintf_styled'
collect2: error: ld returned 1 exit status
The problem is that in commit 7b01c1cc1d the fprintf_styled function
was added into sis.c. This file is only used when building the 'run'
binary, that is, the standalone simulator, and is not included in the
libsim.a library.
Now, the obvious fix would be to move fprintf_styled into libsim.a,
however, that turns out to be tricky.
The erc32 simulator currently has two copies of the function run_sim,
one in sis.c, and one in interf.c, both of these copies are global.
Currently, the 'run' binary links fine, though I suspect this might be
pure luck. When I tried moving fprintf_styled into interf.c, I ran
into multiple-definition (of run_sim) errors. I suspect that by
requiring the linker to pull in fprintf_styled from libsim.a I was
changing the order in which symbols were loaded, and the linker was
now seeing both copies of run_sim, while currently we only see one
copy.
The ideal solution of course, would be to merge the two similar, but
slightly different copies of run_sim, and just use the one copy. Then
we could safely move fprintf_styled into interf.c too, and all would
be good.
But I don't have time right now to start debugging the erc32
simulator, so I wanted a solution that fixes the build without
introducing multiple definition errors.
The easiest solution I think is to just have two copies of
fprintf_styled, one in sis.c, and one in interf.c. Unlike run_sim,
these two copies are both static, so we will not run into multiple
definition issues with this function. The functions themselves are
not very big, so it's not a huge amount of duplicate code.
I am very aware that this is not an ideal solution, and I would
welcome anyone who wants to take on fixing the run_sim problem
properly, and then cleanup the fprintf_styled duplication.
The commit bf11021653 "* MAINTAINERS: Perform some obvious fixups."
back in 2009 changed the mailing list address gdb-patches@sources.redhat.com
to gdb-patches@sourceware.org.
This commit does the same to sim/MAINTAINERS.
sim/ChangeLog:
* MAINTAINERS: Update mailing list address.
Change-Id: I56c6bf21a4bddfb35ffc3336ffcba7ff9b39926e
Like commit b82817674f, this replaces BFD_VMA_FMT "x" in sim/ with
PRIx64 and casts to promote bfd_vma to uint64_t. The one file using
BFD_VMA_FMT in gdb/ instead now uses hex_string, and a typo in the
warning message is fixed.
Noticed format mismatch when attempted to build gdb on i686-linux-gnu
in --enable-64-bit-bfd mode:
sim/../../sim/cris/sim-if.c:576:28:
error: format '%lx' expects argument of type 'long unsigned int',
but argument 4 has type 'bfd_size_type' {aka 'long long unsigned int'} [-Werror=format=]
576 | sim_do_commandf (sd, "memory region 0x%" BFD_VMA_FMT "x,0x%lx",
| ^~~~~~~~~~~~~~~~~~~
577 | interp_load_addr, interpsiz);
| ~~~~~~~~~
| |
| bfd_size_type {aka long long unsigned int}
While at it fixed format string for time-related types.
PTR will soon disappear from ansidecl.h. Remove uses in sim. Where
a PTR cast is used in assignment or function args to a void* I've
simply removed the unnecessary (in C) cast rather than replacing with
(void *).
This is the last of the correctness fixes I've been carrying around for the
v850.
Like the other recent fixes, this is another case where we haven't been as
careful as we should WRT host vs target types. For the divq instruction
both operands are 32 bit types. Yet in the simulator code we convert them
from unsigned int to signed long by assignment. So 0xfffffffb (aka -5)
turns into 4294967291 and naturally that changes the result of our division.
The fix is simple, insert a cast to int32_t to force interpretation as a
signed value.
Testcase for the simulator is included. It has a trivial dependency on the
bins patch.
I've been carrying this for a few years. One test in the GCC testsuite is
failing due to a bug in the handling of the v850e3v5 instruction "bins".
When the "bins" instruction specifies a 32bit bitfield size, the simulator
exhibits undefined behavior by trying to shift a 32 bit quantity by 32 bits.
In the case of a 32 bit shift, we know what the resultant mask should be. So
we can just set it.
That seemed better than using 1UL for the constant (on a 32bit host unsigned
long might still just be 32 bits) or needlessly forcing everything to
long long types.
Thankfully the case where this shows up is only bins <src>, 0, 32, <dest>
which would normally be encoded as a simple move.
* testsuite/v850/allinsns.exp: Add v850e3v5.
* testsuite/v850/bins.cgs: New test.
* v850/simops.c (v850_bins): Avoid undefined behavior on left shift.
