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

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
                 CC|                            |msebor at gcc dot gnu.org
         Resolution|---                         |INVALID

--- Comment #2 from Martin Sebor <msebor at gcc dot gnu.org> ---
The warning triggers for the IL below as designed.  Your analysis is also on
the mark: GCC cannot determine that the index is in the bounds of the vec
array, and so it emits code for the cases when it isn't.

  <bb 17> [local count: 883471484]:
  i.42_76 = (unsigned int) i_74;
  if (i.42_76 > 199)
    goto <bb 20>; [33.00%]   >>> i is in [200, UINT_MAX]
  else
    goto <bb 18>; [67.00%]

  ...

  <bb 20> [local count: 291545588]:
  nologmsg.43_77 = nologmsg;
  fc_assert_fail ("../../../src/server/techtools.c", &__FUNCTION__, 674, "(i)
>= 0 && (i) < (signed int) sizeof((eligible_techs).vec) * 8", nologmsg.43_77,
nologmsg.43_77);
  _78 = i_74 >> 3;
  _79 = eligible_techs.vec[_78];   <<< -Warray-bounds: _78 is in [25, UINT_MAX
>> 3]
  _80 = (unsigned int) _79;
  _81 = i_74 & 7;
  _82 = _80 >> _81;
  _83 = (_Bool) _82;
  if (_83 != 0)
    goto <bb 21>; [50.00%]
  else
    goto <bb 49>; [50.00%]


This kind of a transformation isn't uncommon and can be the result of optimizer
improvements or other changes.  Having said that, I see the function asserts
that the index is in bounds just before using it:

  { Tech_type_id i = (1); for (; i < advance_count(); i++) { {
    if (((((i) >= 0 && (i) < (signed int) sizeof((eligible_techs).vec) * 8) ?
(void) 0 : fc_assert_fail("../../../src/server/techtools.c", __FUNCTION__, 713,
"(i) >= 0 && (i) < (signed int) sizeof((eligible_techs).vec) * 8", nologmsg,
nologmsg)), ((eligible_techs).vec[((i) / 8)] & (1u << ((i) & 0x7))) != 0)) {
      chosen--;
      if (chosen == 0) {
        return i;
      }
    }
  } } };


Assuming fc_assert_fail() behaves similar to aborts() and doesn't return to the
caller, annotating it as such is one of the recommended ways to communicate pre
and postconditions to GCC, improve the emitted code, and avoid warnings for
that GCC cannot prove is unreachable, like so:

void fc_assert_fail(const char *file, const char *function, int line,
                    const char *assertion, const char *message, ...)
  __attribute__((__format__ (__printf__, 5, 6), noreturn));

With that, I'm going to resolve the problem as invalid.  (If I misunderstood
your point please clarify.)

Reply via email to