[Bug tree-optimization/97424] New: Warn on invalid shift amount after inlining

[Bug tree-optimization/97424] New: Warn on invalid shift amount after inlining

[fw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97424

            Bug ID: 97424
           Summary: Warn on invalid shift amount after inlining
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Keywords: diagnostic
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: fw at gcc dot gnu.org
  Target Milestone: ---

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.

[Bug tree-optimization/97424] Warn on invalid shift amount after inlining

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97424

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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.

[Bug tree-optimization/97424] Warn on invalid shift amount after inlining

[fw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97424

--- Comment #2 from Florian Weimer <fw at gcc dot gnu.org> ---
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

[Bug tree-optimization/97424] Warn on invalid shift amount after inlining

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97424

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dmalcolm at gcc dot gnu.org

--- Comment #3 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
This is probably implementable as a -fanalyzer warning.

[Bug tree-optimization/97424] Warn on invalid shift amount after inlining

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97424

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement
            Version|unknown                     |11.0

[Bug tree-optimization/97424] Warn on invalid shift amount after inlining

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97424

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
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.

[Bug tree-optimization/97424] Warn on invalid shift amount after inlining

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97424

--- Comment #5 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
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?

[Bug tree-optimization/97424] Warn on invalid shift amount after inlining

[fw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97424

--- Comment #6 from Florian Weimer <fw at gcc dot gnu.org> ---
(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.

[Bug tree-optimization/97424] Warn on invalid shift amount after inlining

[vincent-gcc at vinc17 dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97424

Vincent Lefèvre <vincent-gcc at vinc17 dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |vincent-gcc at vinc17 dot net

--- Comment #7 from Vincent Lefèvre <vincent-gcc at vinc17 dot net> ---
I get a false positive on "b + 1 >= 64 ? 0UL : 1UL << (b + 1)" with a 64-bit
unsigned long. See PR98447.