Project

General

Profile

Bug #8465

aggressive-loop-optimizations error in rt2860.c

Added by Gary Mills about 3 years ago. Updated about 3 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
-
Start date:
2017-07-05
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage
Gerrit CR:

Description

In building illumos-gate with gcc-4.9.4 under Hipster, the following error appeared:

../../common/io/rwn/rt2860.c: In function 'rt2860_read_eeprom':
../../common/io/rwn/rt2860.c:528:28: error: iteration 35u invokes undefined behavior [-Werror=aggressive-loop-optimizations]
       rt2860_rf2850[14 + i].chan, sc->txpow1[14 + i],
                            ^
../../common/io/rwn/rt2860.c:520:2: note: containing loop
  for (i = 0; i < 36; i++) {
  ^
cc1: all warnings being treated as errors

The definition of the struct is in the source file, but the initializer for it is in uts/common/io/rwn/rt2860_reg.h . aggressive-loop-optimizations is a new type of optimization that's used by gcc when the limits are constant, as in this case. Undefined behavior is usually either variable overflow or array index overrun.

However, the code seems to be correct. The warning may be spurious. Redesigning the code may not help.

One solution seems to be to suppress the error message with CERRWARN += -_gcc=-Wno-aggressive-loop-optimizations . This may work, but may cause the compiler to generate bad machine code.

A better solution would be to prevent this type of optimization with CFLAGS += -_gcc=-fno-aggressive-loop-optimizations . This does work, but only if the compiler recognizes this option. It doesn't work with gcc-4.4.4, for example. The compiler also has no way to skip unrecognized -f options. We really need a way to make the Makefile assignment conditional on the compiler version. I don't know of any easy or official way to do that. illumos-gate currently treats all gcc versions the same. That, I suppose, is the bug.

History

#1

Updated by Andrew Stormont about 3 years ago

I agree with your analysis; the code looks fine. I am really not keen on putting fancy logic like this into the Makefiles though. Do you know if GCC 4.4.4 accepts the -Wno-aggressive-loop-optimizations flag?

#2

Updated by Gary Mills about 3 years ago

Gcc-4.4.4 will accept the -Wno-aggressive-loop-optimizations flag, but doesn't do anything with it. That's because gcc-4.4.4 doesn't do aggressive loop optimization. It's something that was introduced in later versions. I also prefer to modify the source, but if that's not possible, the Makefile is the only other choice.

#3

Updated by Andrew Stormont about 3 years ago

That's sensible. My only concern is including the version specific stuff in the Makefiles, which I'd rather not do because I think it sets a bad precedent. I understand your concerns about the -Wno-aggressive-loop-optimizations option; however, I think it's probably the best option here.

#4

Updated by Marcel Telka about 3 years ago

Please fix the real bug in the code instead of hacking the Makefile.

(The real bug is: When i = 35 you are accessing rt2860_rf2850[49], but the rt2860_rf2850 array have 49 entries only, not 50).

#5

Updated by Gary Mills about 3 years ago

I was hoping somebody would say that! I do want to fix the real bug, the one in the code. Last time I looked at the code, it seemed correct. I'll take another look. The compiler was right after all.

#6

Updated by Electric Monk about 3 years ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit e792dca6958d39dce4b70fc78a9c70e55fe4bbf2

commit  e792dca6958d39dce4b70fc78a9c70e55fe4bbf2
Author: Gary Mills <gary_mills@fastmail.fm>
Date:   2017-08-18T14:33:50.000Z

    8465 aggressive-loop-optimizations error in rt2860.c
    Reviewed by: Toomas Soome <tsoome@me.com>
    Reviewed by: Andy Stormont <astormont@racktopsystems.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Also available in: Atom PDF