gcc outputs a warning about shifts >= width of type inside dead code when that code is dead because of a const variable. This can easily happen in autogenerated code, or when you do operations on preprocessor defines. Release: GNU C version 3.0.1 20010626 (prerelease) (i386-pc-linux-gnu) How-To-Repeat: Compile the following with -Wall -O2: int rol0(int a) { const int b = 0; if (b) { a = (a << b) | (a >> (8 * sizeof a - b)); } return a; }
State-Changed-From-To: open->analyzed State-Changed-Why: Confirmed. I may be wrong, but I believe Toon Moene has recently filed a similar bug (recently=it's number should be in the high 8000s) where the dead code was actually created by a template parameter, instead of a plain constant. Whoever looks at this one might want to look for the other report as well.
From: Wolfgang Bangerth <bangerth@ticam.utexas.edu> To: gcc-gnats@gcc.gnu.org Cc: Subject: Re: c/4210: bad warning with dead code Date: Wed, 6 Nov 2002 16:12:43 -0600 (CST) I was wrong on the name of the originator, the range of numbers, and maybe more of the duplicate I suspected. It had templates in it, though :-) In any case, the duplicate was c++/8419, which I closed because it is almost an exact duplicate. W. ------------------------------------------------------------------------- Wolfgang Bangerth email: bangerth@ticam.utexas.edu www: http://www.ticam.utexas.edu/~bangerth
stills warns on mainline (20030524).
I think the problem is the warning happens before dead code elimation happens.
suspending as the problem is that the warning comes before striping of dead code and this will not fixed for a while.
Is there a technical reason why the bug is difficult to fix? Otherwise I might look at it myself.
As I said the point at which the shift error happens is before GCC knows that the code is dead code at all. The warning happens when generating the trees while dead code dection happens after that.
Fine. Since this is about the only warning that I get from dead code, I assumed that there was a mechanism already in place that silences other warnings from removed code, but perhaps this is exactly what is missing :) Or could the test be moved to a later phase (though it's obviously a frontend issue)? I'm just looking for an opportunity to do it myself if it's not too complicated.
In gcc 3.3, warning in dead code also occurs in another case. Testcase: unsigned f(unsigned x) { if (sizeof x > 4) return x >= 1ull << 32 ? 1 : 0; return 0; } yields: foo.c:4: warning: comparison is always false due to limited range of data type This is a regression from 3.2.2. [Should I open a new bug for it?]
Subject: Re: should not warning with dead code > This is a regression from 3.2.2. [Should I open a new bug for it?] Yes, always. W. ------------------------------------------------------------------------- Wolfgang Bangerth email: bangerth@ices.utexas.edu www: http://www.ices.utexas.edu/~bangerth/
*** Bug 12150 has been marked as a duplicate of this bug. ***
*** Bug 26516 has been marked as a duplicate of this bug. ***
Subject: Re: should not warning with dead code A workaround is to use ? : and statement expressions instead of "if". This way, the front-end setting of skip_evaluation disables these warnings. (skip_evaluation can't be set for if (0) because you can jump into if (0), whereas jumps into statement expressions are not permitted. Thus in general you need to parse the whole function body to tell if the if (0) is dead.)
But that's a) clearly a kludge, b) may not help in the future if our optimizers become more elaborate c) doesn't work with code where the code guarded by the if(0) is more than a single statement. It would definitely be better to have this fixed in the right place... W.
Subject: Re: should not warning with dead code On Wed, 1 Mar 2006, bangerth at dealii dot org wrote: > c) doesn't work with code where the code guarded by the if(0) is more > than a single statement. It does; I've used it to eliminate all these warnings in glibc's soft-fp code. Use statement expressions, i.e. surround the whole if body with ({ }).
> It does; I've used it to eliminate all these warnings in glibc's soft-fp > code. Use statement expressions, i.e. surround the whole if body with ({ > }). Ugh. Do we really want to advocate serious code obfuscation to avoid warnings? W.
We have resorted to case-by-case workarounds, usually a cast which would have been an identity operation had the condition been true. That is, if (sizeof x == 8) return x << 32 | x; would have its second line mutated to return (unsigned long long)x << 32 | x; The cast is always a no-op unless it occurs in dead code.
*** Bug 28106 has been marked as a duplicate of this bug. ***
*** Bug 30343 has been marked as a duplicate of this bug. ***
*** Bug 40114 has been marked as a duplicate of this bug. ***
*** Bug 44842 has been marked as a duplicate of this bug. ***
The way Clang gets this right is to perform some very-fast bitmap common constant propagation in the FE. I personally think this would be helpful if implemented correctly, even if it slows down the FE a little. But do not wait for a fix soon because this is not trivial to fix and no one has both the desire and the free time to work on fixing this.
*** Bug 60513 has been marked as a duplicate of this bug. ***
*** Bug 62074 has been marked as a duplicate of this bug. ***
I'd like this bug to be changed from SUSPENDED to CONFIRMED, given that it's continuing to be a problem (e.g., bug#79479). Also, I'd like to suggest what I hope is a simple fix. In 2006 Joseph wrote "skip_evaluation can't be set for if (0) because you can jump into if (0), whereas jumps into statement expressions are not permitted". So, how about if we merely set skip_evaluation for "if (0)" when the then-part lacks labels? This should be an easy test, as it shouldn't require parsing the whole function body. The test might still generate false alarms for code containing gotos, but in practice such gotos are rare, so the proposed change should be a significant improvement even if it's not perfect.
*** Bug 79479 has been marked as a duplicate of this bug. ***
(In reply to Paul Eggert from comment #25) > I'd like this bug to be changed from SUSPENDED to CONFIRMED, given that it's > continuing to be a problem (e.g., bug#79479). Well, NEW, but OK.
*** Bug 85962 has been marked as a duplicate of this bug. ***
(In reply to Paul Eggert from comment #25) > I'd like this bug to be changed from SUSPENDED to CONFIRMED, given that it's > continuing to be a problem (e.g., bug#79479). > > Also, I'd like to suggest what I hope is a simple fix. In 2006 Joseph wrote > "skip_evaluation can't be set for if (0) because you can jump into if (0), > whereas jumps into statement expressions are not permitted". So, how about > if we merely set skip_evaluation for "if (0)" when the then-part lacks > labels? This should be an easy test, as it shouldn't require parsing the > whole function body. The test might still generate false alarms for code > containing gotos, but in practice such gotos are rare, so the proposed > change should be a significant improvement even if it's not perfect. Joseph, what do you think about this solution?
At the point where the then block starts being processed (and, thus, warnings may be given) it wouldn't be known whether there are labels in there or not; cf. the discussion in bug 68193 regarding such warnings and _Generic. If you did things based on settings on entry to if (0), you'd need some way to end that setting (i.e. restore that from the containing block) early on encountering a label (allowing appropriately for nested if (0), of course).
*** Bug 94773 has been marked as a duplicate of this bug. ***
I've checked out the gcc sources, to see if I can understand how to move the warning around. The example input I'm looking at now is unsigned shift_dead (unsigned x) { if (0) return x >> 32; else return x >> 1; } unsigned shift_invalid (unsigned x) { return x >> 32; } where I'd like gcc -Wall to warn for the second function, but not from the first (currently, it warns for both). The warning is emitted from build_binary_op, in gcc/c/c-typeck.c, close to line 11880. Deleting it there silences the warning for *both* functions above. I have a few questions. Keep in mind that I only have a very vague understanding of the different phases in gcc, so any guidance is appreciated. 1. There's similar code in c_fully_fold_internal, in gcc/c/c-fold.c, close to line 400. But that code does *not* emit any warning for the example above, which surprised me a bit. Maybe that's only for the case that both operands to the shift operator are constants? 2. More importantly, if the warning is deleted from build_binary_op, we need to add a check for this case, and an appropriate warning, somewhere later in the compilation process. It has to be done after dead code is identified, i.e., in a phase processing only non-dead code. And preferably as early as possibly, when we're still working close to the parse-tree representation. Is there such a place? Some other functions traversing the tree are c_gimplify_expr (gcc/c-family/c-gimplify.c) and verify_tree (gcc/c-family/c-common.c), are any of those called after elimination of dead code? 3. Alternatively, if there's no place after dead code elimination where the parse tree is still easily available, a different alternative could be to leave the check for invalid shift counts in c-typeck.c, but instead of emitting a warning, construct a special tree object representing an expression with invalid/undefined behavior, and any meta data needed to emit a warning or error to describe it? Then emission of the warning could be postponed to later, say, close to the conversion to SSA form? 4. I also wonder what happens if, for some reason, a constant invalid shift count is passed through all the way to code generation? Most architectures would represent a constant shift count for a 32-bit value as 5-bit field in the opcode, and then the invalid shift counts aren't representable at all. Will gcc silently ignore higher bits, or is it an internal compiler error, or would it make sense to produce a friendly warning at this late stage of the compilation?
(In reply to Niels Möller from comment #32) > 4. I also wonder what happens if, for some reason, a constant invalid shift > count is passed through all the way to code generation? Most architectures > would represent a constant shift count for a 32-bit value as 5-bit field in > the opcode, and then the invalid shift counts aren't representable at all. > Will gcc silently ignore higher bits, That's undefined behavior, so that GCC can do whatever it wants for the generated code. > or is it an internal compiler error, This would not be acceptable.
(In reply to Niels Möller from comment #32) The front ends can eliminate simple subexpressions (as in '0 ? x >> 32 : x >> 1') but they don't do the same for statements. Moving the warning from the front end to some later pass would avoid diagnosing code in those cases (it would also avoid duplicating the same code between different front ends). The earliest is probably gimplify.c. That would avoid warning on statements rendered dead as a result of constant expressions (as defined by the language) but not those whose constant value GCC later propagates from prior assignments, such as in const int zero = 0; unsigned shift_dead (unsigned x) { if (zero) return x >> 32; else return x >> 1; } The later the warning is moved the more statements will be eliminated as dead, but the more other transformations will also be applied that might eliminate the warning where it might be desirable, or perhaps even introduce it where it wouldn't be issued otherwise.
(In reply to Niels Möller from comment #32) > 1. There's similar code in c_fully_fold_internal, in gcc/c/c-fold.c, close > to line 400. But that code does *not* emit any warning for the example > above, which surprised me a bit. Maybe that's only for the case that both > operands to the shift operator are constants? In general, front-ends should avoid emitting warnings while folding (simplifying) expressions. I think there is an open bug about this but I cannot find it now. Of course, if the code is doing the same, it should be factored out into a common function. A good first patch to submit: https://gcc.gnu.org/wiki/GettingStarted#Basics:_Contributing_to_GCC_in_10_easy_steps > 2. More importantly, if the warning is deleted from build_binary_op, we need > to add a check for this case, and an appropriate warning, somewhere later in > the compilation process. It has to be done after dead code is identified, > i.e., in a phase processing only non-dead code. And preferably as early as > possibly, when we're still working close to the parse-tree representation. > Is there such a place? Some other functions traversing the tree are > c_gimplify_expr (gcc/c-family/c-gimplify.c) and verify_tree > (gcc/c-family/c-common.c), are any of those called after elimination of dead > code? There is no such place. Dead code is identified in the middle-end and by then, there is no parse tree, only GIMPLE and SSA: https://gcc.gnu.org/onlinedocs/gccint/Passes.html#Passes Of course, you could add some kind of unreachable code pass to the FE, but that would require a lot of work from your side. Clang has something similar that you could use as an inspiration: https://github.com/llvm/llvm-project/blob/2946cd701067404b99c39fb29dc9c74bd7193eb3/clang/include/clang/Analysis/Analyses/ReachableCode.h > 3. Alternatively, if there's no place after dead code elimination where the > parse tree is still easily available, a different alternative could be to > leave the check for invalid shift counts in c-typeck.c, but instead of > emitting a warning, construct a special tree object representing an > expression with invalid/undefined behavior, and any meta data needed to emit > a warning or error to describe it? Then emission of the warning could be > postponed to later, say, close to the conversion to SSA form? That is still too early, since the dead code elimination happens after SSA.
(In reply to Martin Sebor from comment #34) > > The front ends can eliminate simple subexpressions (as in '0 ? x >> 32 : x > >> 1') but they don't do the same for statements. Moving the warning from > the front end to some later pass would avoid diagnosing code in those cases > (it would also avoid duplicating the same code between different front > ends). The earliest is probably gimplify.c. That would avoid warning on > statements rendered dead as a result of constant expressions (as defined by > the language) but not [...] Thanks. So moving the warning to the gimplify pass would be an improvement over the current situation. Maybe I can try it out, it sounds reasonably simple.
(In reply to Manuel López-Ibáñez from comment #35) > There is no such place. Dead code is identified in the middle-end and by > then, there is no parse tree, only GIMPLE and SSA: > https://gcc.gnu.org/onlinedocs/gccint/Passes.html#Passes So if emission of the warning is postponed to after the Tree SSA passes (https://gcc.gnu.org/onlinedocs/gccint/Tree-SSA-passes.html#Tree-SSA-passes). Could perhaps be inserted just after (or as part of) pass_remove_useless_stmts? > > 3. Alternatively, [...] construct a special tree object representing an > > expression with invalid/undefined behavior, and any meta data needed to emit > > a warning or error to describe it? Then emission of the warning could be > > postponed to later, say, close to the conversion to SSA form? > > That is still too early, since the dead code elimination happens after SSA. Then that special value needs to be passed through the conversions to gimple and SSA. I imagine it could be treated much like a compile time constant, but with an annotation saying it's invalid, and why, to be displayed in case the compiler thinks it needs to generate any code to evaluate it. (And if we go that route, we should probably do the same for most or all warnings on invalid operands detected by the frontend).
Just a brief update. 1. Tried adding fprintf warnings to c_gimplify_expr (btw, what's the right way to display a pretty warning with line numbers etc in later passes?). But it seems that's too early, I still get warnings for dead code. 2. The pass_remove_useless_stmts, mentioned in the docs, was deleted in 2009 (see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41573), and I take it it was obsoleted earlier, since there's no mention of a replacement. So what pass should I look at that is related to basic control flow analysis, and discarding unreachable statements?
I think these questions are more appropriate for the mailing list, since few people are subscribed to this bug. You can easily find which pass does something by dumping (-ftree-dump-*) all of them and comparing them. On Wed, 6 May 2020, 09:11 nisse at lysator dot liu.se, < gcc-bugzilla@gcc.gnu.org> wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=4210 > > --- Comment #38 from Niels Möller <nisse at lysator dot liu.se> --- > Just a brief update. > > 1. Tried adding fprintf warnings to c_gimplify_expr (btw, what's the right > way > to display a pretty warning with line numbers etc in later passes?). But it > seems that's too early, I still get warnings for dead code. > > 2. The pass_remove_useless_stmts, mentioned in the docs, was deleted in > 2009 > (see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41573), and I take it > it was > obsoleted earlier, since there's no mention of a replacement. So what pass > should I look at that is related to basic control flow analysis, and > discarding > unreachable statements? > > -- > You are receiving this mail because: > You are on the CC list for the bug.
(In reply to Manuel López-Ibáñez from comment #39) > I think these questions are more appropriate for the mailing list, since > few people are subscribed to this bug. There were more previously, but a lot of people got dropped from cc lists all throughout bugzilla in the process of transferring servers... I was on this one previously, for example, but now I'm having to re-subscribe... > > You can easily find which pass does something by dumping (-ftree-dump-*) > all of them and comparing them. > > On Wed, 6 May 2020, 09:11 nisse at lysator dot liu.se, < > gcc-bugzilla@gcc.gnu.org> wrote: > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=4210 > > > > --- Comment #38 from Niels Möller <nisse at lysator dot liu.se> --- > > Just a brief update. > > > > 1. Tried adding fprintf warnings to c_gimplify_expr (btw, what's the right > > way > > to display a pretty warning with line numbers etc in later passes?). But it > > seems that's too early, I still get warnings for dead code. > > > > 2. The pass_remove_useless_stmts, mentioned in the docs, was deleted in > > 2009 > > (see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41573), and I take it > > it was > > obsoleted earlier, since there's no mention of a replacement. So what pass > > should I look at that is related to basic control flow analysis, and > > discarding > > unreachable statements? > > > > -- > > You are receiving this mail because: > > You are on the CC list for the bug.
(In reply to Manuel López-Ibáñez from comment #39) > You can easily find which pass does something by dumping (-ftree-dump-*) > all of them and comparing them. It's -ftree-dump-all, and also -fdump-passes was useful. Thanks! I'm now compiling without optimization to (i) reduce number of passes, and (ii) because it would be nice to get it right also in absence of optimization options. It looks like the dead code is eliminated by the "cfg" (control flow graph) pass, in gcc/tree-cfg.c. In the .cfg dumpfile it says "Removing basic block 3", and the invalid shift disappears in that dump. That's nice. Immediately after this pass comes *warn_function_return, implemented in the same file. It would make sense to me to add a pass to warn about shift operations with invalid constant operands at about the same place. Is it easy to traverse a gimple function, and check all expressions? The "*warn_unused_result" pass seems to do a similar traversal (but examining statements rather than expressions, and done *before* the cfg pass).
Created attachment 48678 [details] Add a new pass for emitting the warning (not working) Since adding a new pass seems to be the right way, I've given that a try. See patch. By my printf debugging I can see that the pass is instantiated and the execute method is invoked twice (same example program): unsigned shift_dead (unsigned x) { if (0) return x >> 32; else return x >> 1; } unsigned shift_invalid (unsigned x) { return x >> 32; } But no further printouts, it seems my loop +void +do_warn_shift_count_range (gimple_seq seq) +{ + gimple_stmt_iterator i; + for (i = gsi_start (seq); !gsi_end_p (i); gsi_next (&i)) exits before a single iteration. My intention was that it should iterate over the body of the current function (i.e., fun->gimple_body), which I expect to be a single return statement after the cfg pass has removed dead code. I'm probably misunderstanding the data structures. And what's the easiest way to run the the right compiler process (I guess that's cc1) under gdb?
(In reply to Niels Möller from comment #42) > And what's the easiest way to run the the right compiler process (I guess > that's cc1) under gdb? gcc -c t.c -wrapper gdb,--args