Build issues in PlatformIO when using -O3

[eosti]

Using the latest PlatformIO (6.1.5a1) and the arduino-pico core pulled today using platform-raspberrypi in PlatformIO.

Compiling with the following settings will throw a warning, which is reproducible using the most basic "hello world" script. It also at one point caused the USB port to become unresponsive (serial and flashing broke when not holding BOOTSEL), but I haven't been able to reproduce that.

; platformio.ini
build_unflags = -Os
build_flags = -O3 -Wall

Build warning:

Compiling .pio/build/pico/FrameworkArduino/main.cpp.o
In file included from /Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/api/ArduinoAPI.h:36,
                 from /Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/Arduino.h:28,
                 from /Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/FS.h:24,
                 from /Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/FS.cpp:21:
/Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/api/Stream.h: In member function 'fs::File fs::Dir::openFile(const char*)':
/Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/api/Stream.h:50:7: warning: 'f.arduino::Stream::_startMillis' may be used uninitialized in this function [-Wmaybe-uninitialized]
   50 | class Stream : public Print
      |       ^~~~~~
/Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/FS.cpp:241:10: note: 'f.arduino::Stream::_startMillis' was declared here
  241 |     File f(_impl->openFile(om, am), _baseFS);
      |          ^
Compiling .pio/build/pico/FrameworkArduino/posix.cpp.o
Compiling .pio/build/pico/FrameworkArduino/sdkoverride/cyw43_arch_threadsafe_background.c.o
In file included from /Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/api/ArduinoAPI.h:36,
                 from /Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/Arduino.h:28,
                 from /Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/FS.h:24,
                 from /Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/FS.cpp:21:
/Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/api/Stream.h: In member function 'fs::File fs::FS::open(const arduino::String&, const char*)':
/Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/api/Stream.h:50:7: warning: 'f.arduino::Stream::_startMillis' may be used uninitialized in this function [-Wmaybe-uninitialized]
   50 | class Stream : public Print
      |       ^~~~~~
/Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/FS.cpp:392:10: note: 'f.arduino::Stream::_startMillis' was declared here
  392 |     File f(_impl->open(path, om, am), this);
      |          ^
Compiling .pio/build/pico/FrameworkArduino/stdlib_noniso.cpp.o
In file included from /Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/api/ArduinoAPI.h:36,
                 from /Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/Arduino.h:28,
                 from /Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/FS.h:24,
                 from /Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/FS.cpp:21:
/Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/api/Stream.h: In member function 'fs::File fs::FS::open(const char*, const char*)':
/Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/api/Stream.h:50:7: warning: 'f.arduino::Stream::_startMillis' may be used uninitialized in this function [-Wmaybe-uninitialized]
   50 | class Stream : public Print
      |       ^~~~~~
/Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/FS.cpp:392:10: note: 'f.arduino::Stream::_startMillis' was declared here
  392 |     File f(_impl->open(path, om, am), this);
      |          ^
Compiling .pio/build/pico/FrameworkArduino/wiring_analog.cpp.o
In file included from /Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/api/ArduinoAPI.h:36,
                 from /Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/Arduino.h:28,
                 from /Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/FS.h:24,
                 from /Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/FS.cpp:21:
/Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/api/Stream.h: In member function 'fs::File fs::File::openNextFile()':
/Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/api/Stream.h:50:7: warning: 'f.arduino::Stream::_startMillis' may be used uninitialized in this function [-Wmaybe-uninitialized]
   50 | class Stream : public Print
      |       ^~~~~~
/Users/reid/.platformio/packages/framework-arduinopico/cores/rp2040/FS.cpp:241:10: note: 'f.arduino::Stream::_startMillis' was declared here
  241 |     File f(_impl->openFile(om, am), _baseFS);
      |   

Removing the unflags will let it compile correctly, as well as changing the optimization level to -O2 instead of -O3. Is this just a weird interaction between PlatformIO and the compiler, or is there some issue in the core?

[earlephilhower]

Wow, that's a confusing warning message. I suppose it could be worse if we threw in a template or two. 😱

That said, it's a warning and not an error. I can repro it in the IDE only under -O3, probably because it inlines something or other. (FWIW, -O3 can blow up the code making it slower, since you'll have more XIP cache misses which read from very slow SPI flash).

I'll look at it, but OTOH there is no place in the File constructor I actually use _startMillis so I think it's a false alarm related to the multiple inheritance of the File class...

[earlephilhower]

With a blank sketch at -O3 I get the same warning. I've done a basic grep to see where _startMillis is uses, and it is only in the ArduinoAPI, not in any core code:

earle@amd:~/Arduino/hardware/pico/rp2040$ grep -r _startMillis cores/
cores/rp2040/api/Stream.h:    unsigned long _startMillis;  // used for timeout measurement
cores/rp2040/api/Stream.cpp:  _startMillis = millis();
cores/rp2040/api/Stream.cpp:  } while(millis() - _startMillis < _timeout);
cores/rp2040/api/Stream.cpp:  _startMillis = millis();
cores/rp2040/api/Stream.cpp:  } while(millis() - _startMillis < _timeout);

If I look at where it's used in Stream.cpp I see:

// private method to read stream with timeout
int Stream::timedRead()
{
  int c;
  _startMillis = millis();
  do {
    c = read();
    if (c >= 0) return c;
  } while(millis() - _startMillis < _timeout);
  return -1;     // -1 indicates timeout
}

// private method to peek stream with timeout
int Stream::timedPeek()
{
  int c;
  _startMillis = millis();
  do {
    c = peek();
    if (c >= 0) return c;
  } while(millis() - _startMillis < _timeout);
  return -1;     // -1 indicates timeout
}

which is definitely set before use. So, at this point I have to say it is a weird optimizer issue and not a real coding one.

I've done a silly initialization in #770 which removes the warning, just for sanity's sake.