Annoying warnings on Windows build (engine/Port.hpp)

Development
1

Hi,

I’m building modules with MinGW suite on Windows, following the guideline on official site, but I got those annoying warning on engine/Port.hpp:

C:\repos\Rack/include/engine/Port.hpp: In member function 'virtual void Slug::Plugins::Tester::process(const rack::engine::Module::ProcessArgs&)':
C:\repos\Rack/include/engine/Port.hpp:163:14: warning: array subscript 16 is above array bounds of 'float [16]' [-Warray-bounds]  
  163 |    voltages[c] = 0.f;
      |    ~~~~~~~~~~^
C:\repos\Rack/include/engine/Port.hpp:18:9: note: while referencing 'rack::engine::Port::<unnamed union>::voltages'
   18 |   float voltages[PORT_MAX_CHANNELS] = {};
      |         ^~~~~~~~
C:\repos\Rack/include/engine/Port.hpp:163:14: warning: array subscript 16 is above array bounds of 'float [16]' [-Warray-bounds]  
  163 |    voltages[c] = 0.f;
      |    ~~~~~~~~~~^
C:\repos\Rack/include/engine/Port.hpp:18:9: note: while referencing 'rack::engine::Port::<unnamed union>::voltages'
   18 |   float voltages[PORT_MAX_CHANNELS] = {};
      |         ^~~~~~~~
C:\repos\Rack/include/engine/Port.hpp:163:14: warning: array subscript 16 is above array bounds of 'float [16]' [-Warray-bounds]  
  163 |    voltages[c] = 0.f;
      |    ~~~~~~~~~~^
C:\repos\Rack/include/engine/Port.hpp:18:9: note: while referencing 'rack::engine::Port::<unnamed union>::voltages'
   18 |   float voltages[PORT_MAX_CHANNELS] = {};

Are there any workaround to avoid these? I wouldn’t suppress -W settings, but I don’t think this happens on Linux? Any tips?

2
1 Like
3

Thanks :slight_smile:

4

If you look in Port.hpp, voltages is declared as

static const int PORT_MAX_CHANNELS = 16; float voltages[PORT_MAX_CHANNELS] = {};

So clearly voltages[16] is illegal. But the comments in setChannels imply that 16 is a legal value there (calling setChannels(16)). Not sure what’s going on.

1 Like
5

Funny things?

A clamp between 0 and PORT_MAX_CHANNELS before send the value to setChannels() suppress the thing.

But if you write setChannels(yourVariable) (having yourVariable const equal to PORT_MAX_CHANNELS) show the warning.

Very very weird…

1 Like
6

In a way, the compiler warning is correct. The method Port::setChannels() contains

		for (int c = channels; c < this->channels; c++) {
			voltages[c] = 0.f;
		}

which buffer-overruns if this->channels > 16. However, this should only happen if a plugin developer sets >16 channels which would be incorrect. Port methods purposely have no input sanitation because it prioritizes speed. Any extra instructions would make a measurable difference in performance.

I’ll look into a way to suppress this error, but in the meantime, just ignore it.

4 Likes
7

have you considered a channels = std::clamp(channels, 0, WHATEVER_THE_MAX_POLY_DEF_IS) at the top of the method body? Or alternately c < this->channels && c < MAX_POLY_DEF in the loop should do the same.

8

ahh i guess those both fail your extra instruction test though. Is setChannels called frequently in peoples code? if so, wouldn’t a top level bailout like if (channels == this->channels) return also help? Anyway peanut gallery, free comment, etc…

9

It also happens calling setChannels(16) (where 16 is not > 16).

this is what I’ve wrote above, and what I actually do to suppress the warnings:

void MyModule::setPortChannels(int outputId, int numChannels) {
	numChannels = clamp(numChannels, 0, PORT_MAX_CHANNELS);
	outputs[outputId].setChannels(numChannels);
}

which, generally, is not tha bad as “sanification” :slight_smile:

10

Does it? I don’t think voltages[16] is set if this->channels = 16 and channels = 16.

11

Yes, I’ve tried passing a constexpr int kMaxNumVoices = 16

12

You’re saying that my code snippet buffer-overruns if this->channels = 16 and channels = 16? Sorry, but this objectively isn’t true.

13

No, I meant the warning appairs passing directly “16”.

1 Like
14

GCC reports an error because the function parameter isn’t a data type constrained to never exceed the maximum. The compiler can’t know at compile time whether it’s out of bounds.

There may be a way to use C++ to constrain a parameter’s integral type to a range, but I can’t think of it. C++ is oceanic in its Syntax & semantics. Even though it was my primary language for over 30 years, there were still things I never learned.

15

in Rack/include/engine/Port.hpp I inserted the following pragma directives to get rid of the warnings

	void setChannels(int channels) {
		// If disconnected, keep the number of channels at 0.
		if (this->channels == 0) {
			return;
		}
		// Set higher channel voltages to 0
		for (int c = channels; c < this->channels; c++) {
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Warray-bounds"
			voltages[c] = 0.f;
#pragma GCC diagnostic pop
		}
		// Don't allow caller to set port as disconnected
		if (channels == 0) {
			channels = 1;
		}
		this->channels = channels;
	}
3 Likes
16

Did someone try using gcc built-ins for this? Like so:

	void setChannels(int channels) {
		// If disconnected, keep the number of channels at 0.
		if (this->channels == 0) {
			return;
		}
		// Set higher channel voltages to 0
		for (int c = channels; c < this->channels; c++) {
			if (c >= PORT_MAX_CHANNELS)
				__builtin_unreachable();
			voltages[c] = 0.f;
		}
		// Don't allow caller to set port as disconnected
		if (channels == 0) {
			channels = 1;
		}
		this->channels = channels;
	}

the __builtin_unreachable is a compile-time hint, it generates no extra code.

EDIT: just tried on Linux with GCC 11.3 and it works here, no more annoying warnings