Consider this program: #include <stdint.h> static inline uint32_t _dl_hwcaps_subdirs_build_bitmask (int subdirs, int active) { /* Leading subdirectories that are not active. */ int inactive = subdirs - active; if (inactive == 32) return 0; uint32_t mask; if (subdirs != 32) mask = (1 << subdirs) - 1; else mask = -1; return mask ^ ((1U << inactive) - 1); } void f1 (int); void f2 (void) { f1 (_dl_hwcaps_subdirs_build_bitmask (1, 2)); f1 (_dl_hwcaps_subdirs_build_bitmask (33, 31)); } This has invalid shifts involving a negative shift amount and larger-than-width shift amount. This does not result in a warning because the current shift warnings are implemented in the front end. But the computed values as the argument to f1 are garbage, so it would make sense to warn.
Such a warning would suffer from the usual pain of late warnings, warning even about cases of this in unreachable code that the compiler can't prove is unreachable. An alternative to this is -fsanitize=undefined which detects only the reachable cases at runtime.
Indeed, Martin Sebor has suggested that it would have to be coupled with __builtin_warning: https://gcc.gnu.org/legacy-ml/gcc-patches/2019-10/msg01015.html
This is probably implementable as a -fanalyzer warning.
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>: https://gcc.gnu.org/g:5e00ad3ffbfb4df7242c313a0d836f5b538eb2fb commit r11-4930-g5e00ad3ffbfb4df7242c313a0d836f5b538eb2fb Author: David Malcolm <dmalcolm@redhat.com> Date: Wed Nov 11 21:16:45 2020 -0500 analyzer: warn on invalid shift counts [PR97424] This patch implements -Wanalyzer-shift-count-negative and -Wanalyzer-shift-count-overflow, analogous to the C/C++ warnings -Wshift-count-negative and -Wshift-count-overflow, but implemented via interprocedural path analysis rather than via parsing in a front end, and thus capable of detecting interprocedural cases that the warnings implemented in the front ends can miss. gcc/analyzer/ChangeLog: PR tree-optimization/97424 * analyzer.opt (Wanalyzer-shift-count-negative): New. (Wanalyzer-shift-count-overflow): New. * region-model.cc (class shift_count_negative_diagnostic): New. (class shift_count_overflow_diagnostic): New. (region_model::get_gassign_result): Complain about shift counts that are negative or are >= the operand's type's width. gcc/ChangeLog: PR tree-optimization/97424 * doc/invoke.texi (Static Analyzer Options): Add -Wno-analyzer-shift-count-negative and -Wno-analyzer-shift-count-overflow. (-Wno-analyzer-shift-count-negative): New. (-Wno-analyzer-shift-count-overflow): New. gcc/testsuite/ChangeLog: PR tree-optimization/97424 * gcc.dg/analyzer/invalid-shift-1.c: New test.
The above commit implements it as an analyzer warning. Should I close this out, or should we keep it open for the __builtin_warning approach?
(In reply to David Malcolm from comment #5) > The above commit implements it as an analyzer warning. Should I close this > out, or should we keep it open for the __builtin_warning approach? Thanks for the analyzer warning. I think the __builtin_warning approach is very desirable here. To me, it looks like GCC already did all the work to figure out this undefined.
I get a false positive on "b + 1 >= 64 ? 0UL : 1UL << (b + 1)" with a 64-bit unsigned long. See PR98447.