|
From: Mark W. <ma...@so...> - 2019-05-27 18:58:21
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=38cc5478bca0f858dbe69076ce033f8563b5d2e9 commit 38cc5478bca0f858dbe69076ce033f8563b5d2e9 Author: Mark Wielaard <ma...@kl...> Date: Mon May 27 20:50:02 2019 +0200 host_amd64_defs.c don't initialize opc and subopc_imm in emit_AMD64Instr. In the case of Ain_SseShiftN we first assign zero to opc and subopc_imm before handling the various subops. But since we will (and must) always assign a valid value to opc and subopc_imm we might get a compiler warning about the values never being read before storing a different value. So explicitly don't assign a value. Then the compiler will warn if we would ever forget to assign it a value value later on before using it. Diff: --- VEX/priv/host_amd64_defs.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/VEX/priv/host_amd64_defs.c b/VEX/priv/host_amd64_defs.c index f022b4f..29127c1 100644 --- a/VEX/priv/host_amd64_defs.c +++ b/VEX/priv/host_amd64_defs.c @@ -3952,8 +3952,6 @@ Int emit_AMD64Instr ( /*MB_MOD*/Bool* is_profInc, goto done; case Ain_SseShiftN: { - opc = 0; // invalid - subopc_imm = 0; // invalid UInt limit = 0; UInt shiftImm = i->Ain.SseShiftN.shiftBits; switch (i->Ain.SseShiftN.op) { |
|
From: John R. <jr...@bi...> - 2019-05-28 02:04:11
|
> So explicitly don't assign a value. Then the compiler will warn if we
> would ever forget to assign it a value value later on before using it.
That's too optimistic; the compiler is not that good.
Yes, the compiler usually catches such mistakes,
but sometimes it does not. For instance, the BEAM
static analysis tool ("Bugs, Errors, And Mistakes")
does report uses of uninitialized variables
that "ordinary" compilers do not notice.
For a hack: make it easy to switch between uninitialized an initialized.
Uninitialized probably is better for development, in order to take
advantage of "pretty good" compiler. But initialized is better
for a shipping product, because a 100% reproducible bug is MUCH better
(easier and faster to find and fix) than an intermittent bug.
|
|
From: Mark W. <ma...@kl...> - 2019-05-28 08:11:33
|
On Mon, 2019-05-27 at 19:03 -0700, John Reiser wrote:
> > So explicitly don't assign a value. Then the compiler will
> > warn if we
> > would ever forget to assign it a value value later on before
> > using it.
>
> That's too optimistic; the compiler is not that good.
> Yes, the compiler usually catches such mistakes,
> but sometimes it does not. For instance, the BEAM
> static analysis tool ("Bugs, Errors, And Mistakes")
> does report uses of uninitialized variables
> that "ordinary" compilers do not notice.
Yeah, Julian pointed out the same on irc. I am probably too optimistic
about the data flow analysis of compilers. In this particular case it
really is that simple for the compiler to notice though. Since the code
is a simple exhaustive switch case after which the variables are
immediately checked. I did test that part with various gcc versions
(forget to handle it in any switch case, and you do get a
warning/error). But I'll not change any other code.
Cheers,
Mark
|
|
From: Philippe W. <phi...@sk...> - 2019-05-30 13:58:21
|
On Mon, 2019-05-27 at 19:03 -0700, John Reiser wrote:
> > So explicitly don't assign a value. Then the compiler will warn if we
> > would ever forget to assign it a value value later on before using it.
>
> That's too optimistic; the compiler is not that good.
> Yes, the compiler usually catches such mistakes,
> but sometimes it does not. For instance, the BEAM
> static analysis tool ("Bugs, Errors, And Mistakes")
> does report uses of uninitialized variables
> that "ordinary" compilers do not notice.
>
> For a hack: make it easy to switch between uninitialized an initialized.
> Uninitialized probably is better for development, in order to take
> advantage of "pretty good" compiler. But initialized is better
> for a shipping product, because a 100% reproducible bug is MUCH better
> (easier and faster to find and fix) than an intermittent bug.
At my work, we have explicitly decided to *not* default initialise
in such cases, as default initialising variables if the code logic does
not require it means:
* some runtime cost (admittedly often small, but not always, e.g.
big struct or array).
* static code analysers, compilers and tools such as
valgrind/memcheck do not find/report 'uninit' bugs anymore.
* having many unneeded initialization means
the really needed initialisations are hidden in the forest
of useless initialisations.
Philippe
|