Warum warnt der Kompilierer mich nicht? (eindeutige Speicherzugriffsverletzung)

Das sind die GCC-Kompilieroptionen, die ich verwende:

-std=c89 -g -D_FORTIFY_SOURCE=2 -Wc++-compat -Werror -Wall -Walloca -Warith-conversion -Warray-bounds=2 -Wcast-align=strict -Wcast-qual -Wconversion -Wduplicated-branches -Wduplicated-cond -Werror -Wextra -Wformat-overflow=2 -Wformat-security -Wformat-signedness -Wformat-truncation=2 -Wformat=2 -Wimplicit-fallthrough=3 -Wl,-z,noexecstack -Wl,-z,now -Wl,-z,relro -Wl,-z,separate-code -Wlogical-op -Wnull-dereference -Wpedantic -Wshadow -Wshift-overflow=2 -Wstack-protector -Wstack-usage=1000000 -Wstrict-overflow=4 -Wstrict-prototypes -Wstringop-overflow=4 -Wswitch-default -Wswitch-enum -Wtraditional-conversion -Wtrampolines -Wundef -Wvla -fPIE -fstack-clash-protection -fstack-protector-strong -fsanitize=address -fsanitize=pointer-compare -fsanitize=pointer-subtract -fsanitize=leak -fno-omit-frame-pointer -fsanitize=undefined -fsanitize=bounds-strict -fsanitize=float-divide-by-zero -fsanitize=float-cast-overflow -fanalyzer

Ich bekomme keine einzige Warnung, obschon ich absichtlich eine Speicherzugriffsverletzung begehe.

#include <stddef.h>
#include <stdio.h>

typedef struct CircularBuffer CircularBuffer;

enum { kRingBufferCapacity = sizeof(unsigned int) * 4 };

struct CircularBuffer {
    unsigned char data[kRingBufferCapacity];
    unsigned char* end;
    unsigned char* write_iterator;
    unsigned char* read_iterator;
    size_t value_size;
};

static void ClearBytes(void* first, void* last) {
    unsigned char* iterator = (unsigned char*)first;

    while (iterator != last) {
        *iterator = 0U;
        ++iterator;
    }
}

static void CopyBytes(void* first, void* last, void* d_first) {
    unsigned char* iterator = (unsigned char*)first;
    unsigned char* d_iterator = (unsigned char*)d_first;

    while (iterator != last) {
        *d_iterator = *iterator;
        ++iterator;
        ++d_iterator;
    }
}

void InitializeCircularBuffer(CircularBuffer* buffer, size_t value_size) {
    ClearBytes(buffer->data, (&buffer->data)[1]);
    buffer->end = (&buffer->data)[1];
    buffer->write_iterator = buffer->data;
    buffer->read_iterator = buffer->data;
    buffer->value_size = value_size;
}

void InsertValueToCircularBuffer(CircularBuffer* buffer, void* value) {
    if (buffer->write_iterator >= buffer->end) {
        buffer->write_iterator = buffer->data;
    }
    CopyBytes(value, (unsigned char*)value + buffer->value_size,
              buffer->write_iterator);
    buffer->write_iterator += buffer->value_size;
}

void* GetValueFromCircularBuffer(CircularBuffer* buffer) {
    unsigned char* old_iterator_value = NULL;
    if (buffer->read_iterator >= buffer->end) {
        buffer->read_iterator = buffer->data;
    }
    old_iterator_value = buffer->read_iterator;
    buffer->read_iterator += buffer->value_size;
    return old_iterator_value;
}

int main(void) {
    static CircularBuffer buffer;

    InitializeCircularBuffer(&buffer, sizeof(unsigned int));

    {
        unsigned int x = 42;
        InsertValueToCircularBuffer(&buffer, &x);
    }

    {
        unsigned int x = 1984;
        InsertValueToCircularBuffer(&buffer, &x);
    }

    {
        size_t i = 0U;

        for (i = 0U; i < kRingBufferCapacity / buffer.value_size; ++i) {
            printf("[%02lu] %02u\n", i,
                   *(unsigned int*)GetValueFromCircularBuffer(&buffer));
        }
    }

    return 0;
}

Statt enum { kRingBufferCapacity = sizeof(unsigned int) * 4 }; einfach enum { kRingBufferCapacity = 1}; machen.
Das data-Array kann hier nur 1 Byte haben, da (char meistens 1 Byte ist).

Das

    CopyBytes(value, (unsigned char*)value + buffer->value_size,
              buffer->write_iterator);
    buffer->write_iterator += buffer->value_size;

... sollte eigentlich nicht gehen.

Voll-Link: Compiler Explorer

Referenzen

https://airbus-seclab.github.io/c-compiler-security/

Th69

Die Funktion greift ja nicht direkt auf data zu, sondern über die Indirektion write_iterator (wo ja ein beliebiger Wert drin stehen kann) - daher kann hier keine Compilerwarnung (bzw. Fehler) generiert werden.

hustbaer

Selbst wenn man direkt

int* p = nullptr;
*p = 123;

schreibt bekommt man normalerweise keine Warnung.

Ich vermute der Grund ist, dass so eine Warnung eine extra Analyse erfordern würde, was zusätzliche Komplexität im Compiler bedeutet und langsamere Übersetzung.

Wenn du für solche Dinge Warnungen/Fehler bekommen willst, dann musst du dich bei sog. "Linter" bzw. Static-Code-Analysis Tools umsehen. Google mal nach CppCheck wenn dir danach ist.

Wobei zumindest MSVC auch ein Static-Code-Analysis Tool im Compiler eingebaut hat. Muss man aber extra anfordern, mit /analyze. Dann bekommt man zumindest in meinem trivialen Beispiel ne Warning: https://godbolt.org/z/ffMbzjEPj

hustbaer

ps: mit /O2 /analyze hat MSVC zumindest eine Chance auch für deinen originalen Code ne Warnung zu produzieren. Also je nachdem wie ich den abändere bekomm ich eine, oder auch nicht.

Und hier bekommst du immer ein Warnung:

    ClearBytes(buffer->data, (&buffer->data)[1]);
    buffer->end = (&buffer->data)[1];

Weil das mit "one past last element" Zeiger nur für Array-Elemente gilt. So wie du es schreibst, holst du die aber einen "one past last" Zeiger hinter das Array, nicht hinter das letzte Array Element.

Korrekt wäre:

    ClearBytes(buffer->data, &buffer->data[kRingBufferCapacity]);
    buffer->end = &buffer->data[kRingBufferCapacity];

Und %02lu passt natürlich nicht zu size_t. Für size_t bitte %02zu verwenden.

@hustbaer Nochmal danke für die Hinweise!

@hustbaer sagte in Warum warnt der Kompilierer mich nicht? (eindeutige Speicherzugriffsverletzung):

ps: mit /O2 /analyze hat MSVC zumindest eine Chance auch für deinen originalen Code ne Warnung zu produzieren. Also je nachdem wie ich den abändere bekomm ich eine, oder auch nicht.

Und hier bekommst du immer ein Warnung:
    ClearBytes(buffer->data, (&buffer->data)[1]);
    buffer->end = (&buffer->data)[1];
Weil das mit "one past last element" Zeiger nur für Array-Elemente gilt. So wie du es schreibst, holst du die aber einen "one past last" Zeiger hinter das Array, nicht hinter das letzte Array Element.

Korrekt wäre:
    ClearBytes(buffer->data, &buffer->data[kRingBufferCapacity]);
    buffer->end = &buffer->data[kRingBufferCapacity];

Das mit (&buffer_data)[1] und buffer->end = &buffer->data[kRingBufferCapacity]; war mir nicht ganz klar.

Aber Du hast recht, denn laut C89-Standard:

   An address constant is a pointer to an lvalue designating an object
of static storage duration, or to a function designator; it shall be
created explicitly, using the unary & operator, or implicitly, by the
use of an expression of array or function type.  The array-subscript
[] and member-access .  and -> operators, the address & and
indirection * unary operators, and pointer casts may be used in the
creation an address constant, but the value of an object shall not be
accessed by use of these operators.

Also:

Man kann auf MeinArray[N] zugreifen, solange man davor ein & hat.

    fprintf(stderr, "%p\n", (&buffer->data)[1]);
    fprintf(stderr, "%p\n", &buffer->data[kRingBufferCapacity]);

Bei Beiden bekomme ich die gleichen virtuellen Adressen raus ...

Das Problem ist, dass ich jetzt wohl all die Quellcodes anpassen muss, da ich (&MeinArray)[1] sehr oft verwendet habe.

Der Vorteil war einfach, dass ich hier keine Größen mitangeben muss.

Und %02lu passt natürlich nicht zu size_t. Für size_t bitte %02zu verwenden.

Bei C89 geht es offenbar nicht anders.
Danke aber dennoch für den Hinweis.

Randbemerkungen:

Niemand sollte wie ich hier C89 verwenden, weil er wie ich meint, dass es der meistunterstützte Sprach-Standard ist.
Und viele Kompilierer diesen Standard auch vollumfänglich implementieren.

Man kann statt stdint.h einfach (lt. Sprachstandard) annehmen, dass

char mindestens 1 Byte ist
short / int (mindestens 2 Byte)
long (mindestens 4 Byte ist)

Minimum von 8 Bytes sind bei keinem fundamentalen Typ festgelegt.

Siehe auch C89-Standard-Text (das Grundgesetz der C-Menschen).

hustbaer

@dozgon sagte in Warum warnt der Kompilierer mich nicht? (eindeutige Speicherzugriffsverletzung):

@hustbaer Nochmal danke für die Hinweise!

Bitte

@hustbaer sagte in Warum warnt der Kompilierer mich nicht? (eindeutige Speicherzugriffsverletzung):
ps: mit /O2 /analyze hat MSVC zumindest eine Chance auch für deinen originalen Code ne Warnung zu produzieren. Also je nachdem wie ich den abändere bekomm ich eine, oder auch nicht.

Und hier bekommst du immer ein Warnung:
    ClearBytes(buffer->data, (&buffer->data)[1]);
    buffer->end = (&buffer->data)[1];
Weil das mit "one past last element" Zeiger nur für Array-Elemente gilt. So wie du es schreibst, holst du die aber einen "one past last" Zeiger hinter das Array, nicht hinter das letzte Array Element.

Korrekt wäre:
    ClearBytes(buffer->data, &buffer->data[kRingBufferCapacity]);
    buffer->end = &buffer->data[kRingBufferCapacity];
Das mit (&buffer_data)[1] und buffer->end = &buffer->data[kRingBufferCapacity]; war mir nicht ganz klar.

Aber Du hast recht, denn laut C89-Standard:
   An address constant is a pointer to an lvalue designating an object
of static storage duration, or to a function designator; it shall be
created explicitly, using the unary & operator, or implicitly, by the
use of an expression of array or function type.  The array-subscript
[] and member-access .  and -> operators, the address & and
indirection * unary operators, and pointer casts may be used in the
creation an address constant, but the value of an object shall not be
accessed by use of these operators.
Also:

Man kann auf MeinArray[N] zugreifen, solange man davor ein & hat.
    fprintf(stderr, "%p\n", (&buffer->data)[1]);
    fprintf(stderr, "%p\n", &buffer->data[kRingBufferCapacity]);
Bei Beiden bekomme ich die gleichen virtuellen Adressen raus ...

Ja, klar. Es ist halt nur technisch gesehen UB, und deswegen macht der Static-Analyzer von MSVC mimimi.

Das Problem ist, dass ich jetzt wohl all die Quellcodes anpassen muss, da ich (&MeinArray)[1] sehr oft verwendet habe.

Der Vorteil war einfach, dass ich hier keine Größen mitangeben muss.

Ja. Du "musst" es auch nicht anpassen. Wird schon überall korrekt funktionieren denk ich mir.

Wenn du es wirklich anpassen willst, kannst du dir ein paar Hilfsmakros bauen. Mit sizeof(a)/sizeof(a[0]) bekommst du die Anzahl der Elemente raus. Und dann halt sowas in der Art

#define ARRAY_SIZE(a) (sizeof(a)/sizeof((a)[0]))
#define ARRAY_BEGIN(a) (&(a)[0])
#define ARRAY_END(a) (&(a)[ARRAY_SIZE(a)])

Und %02lu passt natürlich nicht zu size_t. Für size_t bitte %02zu verwenden.

Bei C89 geht es offenbar nicht anders.
Danke aber dennoch für den Hinweis.

Randbemerkungen:

Niemand sollte wie ich hier C89 verwenden, weil er wie ich meint, dass es der meistunterstützte Sprach-Standard ist.
Und viele Kompilierer diesen Standard auch vollumfänglich implementieren.

Machst du das nur weil du es dir halt so in den Kopf gesetzt hast, oder brauchst du wirklich C89?

@hustbaer Nee, also ich brauche C89 nicht wirklich.
Ich habe mir das tatsächlich so in den Kopf gesetzt.
Muss ich nicht so machen.

Danke nochmal für die Hinweise.

Undefiniertes Verhalten (UV) will ich nicht haben.
Der Grund, warum ich so viele Flaggen benutze, ist eben wegen Vermeidung von UV.

MSVC macht mimimimi, aber GCC und wohl Clang nicht.
Also ich hatte wie Du auch mal meintest cppcheck verwenden sollen.
Dachte die Flaggen reichen alle aus.
Aber offenbar nicht.
Enttäuschend.
Entweder ich bleibe bei Debian 11 oder ich installiere Windows 11.
Oder ich mach’ 'ne virtuelle Maschine mit Windows 11.
Letzteres wohl eher.
Ich bin mit Debian 11 sehr zufrieden.
Läuft stabil und schnell.

hustbaer

@dozgon sagte in Warum warnt der Kompilierer mich nicht? (eindeutige Speicherzugriffsverletzung):

@hustbaer Nee, also ich brauche C89 nicht wirklich.
Ich habe mir das tatsächlich so in den Kopf gesetzt.
Muss ich nicht so machen.

OK. Wenn du das so willst, dann willst du das halt so

Undefiniertes Verhalten (UV) will ich nicht haben.
Der Grund, warum ich so viele Flaggen benutze, ist eben wegen Vermeidung von UV.

Ultraviolette Fahnen, soso.
Also man kann's mit dem Eindeutschen von Fachbegriffen auch übertreiben

MSVC macht mimimimi, aber GCC und wohl Clang nicht.

MSVC auch nur wenn du mit /analyze baust. Sonst nicht.