fix: accessing inactive union members

[safocl]

Access to the inactive union members is an undefined behavior.
whole is active member here,

IronOS/source/Core/Drivers/OLED.cpp

Line 217 in ab1fa24

column.whole = (1 << height) - 1; // preload a set of set bits of height

but the strips (inactive) member accessed here.

IronOS/source/Core/Drivers/OLED.cpp

Line 222 in ab1fa24

fillArea(OLED_WIDTH - 1, 0, 1, 8, column.strips[0]);

Standard C++ (Working Draft):

[defns.undefined]

3.65[defns.undefined]undefined behavior
behavior for which this document imposes no requirements
[Note 1: Undefined behavior may be expected when this document omits any explicit definition of behavior or when a program uses an incorrect construct or invalid data. Permissible undefined behavior ranges from ignoring the situation completely with unpredictable results, to behaving during translation or program execution in a documented manner characteristic of the environment (with or without the issuance of a diagnostic message ([defns.diagnostic])), to terminating a translation or execution (with the issuance of a diagnostic message). Many incorrect program constructs do not engender undefined behavior; they are required to be diagnosed. Evaluation of a constant expression ([expr.const]) never exhibits behavior explicitly specified as undefined in [intro] through [cpp]. — end note]

[defns.undefined.runtime]

3.50[defns.undefined.runtime]runtime-undefined behavior
behavior that is undefined except when it occurs during constant evaluation
[Note 1: During constant evaluation,
it is implementation-defined whether runtime-undefined behavior results in the expression being deemed non-constant (as specified in [expr.const]) and
runtime-undefined behavior has no other effect.
— end note]

[basic.life]

except that if the object is a union member or subobject thereof, its lifetime only begins if that union member is the initialized member in the union ([dcl.init.aggr], [class.base.init]), or as described in [class.union], [class.copy.ctor], and [class.copy.assign], and except as described in [allocator.members]. The lifetime of an object o of type T ends when:
-- if T is a non-class type, the object is destroyed, or
-- if T is a class type, the destructor call starts, or
-- the storage which the object occupies is released, or is reused by an object that is not nested within o ([intro.object]).

[class.union#general-2]

In a union, a non-static data member is active if its name refers to an object whose lifetime has begun and has not ended ([basic.life]). At most one of the non-static data members of an object of union type can be active at any time, that is, the value of at most one of the non-static data members can be stored in a union at any time.

[basic.life#8]

Similarly, before the lifetime of an object has started but after the storage which the object will occupy has been allocated or, after the lifetime of an object has ended and before the storage which the object occupied is reused or released, any glvalue that refers to the original object may be used but only in limited ways. For an object under construction or destruction, see [class.cdtor]. Otherwise, such a glvalue refers to allocated storage ([basic.stc.dynamic.allocation]), and using the properties of the glvalue that do not depend on its value is well-defined. The program has undefined behavior if
-- the glvalue is used to access the object, or
-- the glvalue is used to call a non-static member function of the object, or
-- the glvalue is bound to a reference to a virtual base class ([dcl.init.ref]), or
-- the glvalue is used as the operand of a dynamic_cast ([expr.dynamic.cast]) or as the operand of typeid.

https://en.cppreference.com/w/cpp/language/union :

It is undefined behavior to read from the member of the union that wasn't most recently written.

#include <cstdint>
#include <iostream>
 
union S
{
    std::int32_t n;     // occupies 4 bytes
    std::uint16_t s[2]; // occupies 4 bytes
    std::uint8_t c;     // occupies 1 byte
};                      // the whole union occupies 4 bytes
 
int main()
{
    S s = {0x12345678}; // initializes the first member, s.n is now the active member
    // At this point, reading from s.s or s.c is undefined behavior,
    // but most compilers define it.
    std::cout << std::hex << "s.n = " << s.n << '\n';
 
    s.s[0] = 0x0011; // s.s is now the active member
    // At this point, reading from s.n or s.c is undefined behavior,
    // but most compilers define it.
    std::cout << "s.c is now " << +s.c << '\n' // 11 or 00, depending on platform
              << "s.n is now " << s.n << '\n'; // 12340011 or 00115678
}

Same issue: https://gitlab.com/libeigen/eigen/-/issues/2898

Additional Note: Accessing parts of a multi-byte integer via a byte array may result in different output data, since the byte order may differ across systems. However, without pointer casting, access via bit shifts is guaranteed to result in the same behavior across platforms.

• Please check if the PR fulfills these requirements

• [] The changes have been tested locally
• [] There are no breaking changes

• What kind of change does this PR introduce?

• What is the current behavior?

• What is the new behavior (if this is a feature change)?

• Other information:

[Ralim]

Eugh, I didn't even know this was undefined.

Thank you for pointing this out. Frustrating that it's my fault for not checking the article more carefully.

[safocl]

Eugh, I didn't even know this was undefined.

Thank you for pointing this out. Frustrating that it's my fault for not checking the article more carefully.

This is why opensource exists, so that the community can make changes (corrections) whenever possible.

[Ralim]

Thank you,
Do you know of a compiler warning or a linter to find this if I make this mistake again?

[safocl]

Do you know of a compiler warning or a linter to find this if I make this mistake again?

Unfortunately, no...