Möglicher GCC-Fehler beim Zurückgeben von struct von einer Funktion

133

Ich glaube, ich habe bei der Implementierung von O'Neills PCG PRNG einen Fehler in GCC gefunden. ( Anfangscode im Compiler Explorer von Godbolt )

Nach dem Multiplizieren oldstatemitMULTIPLIER , (Ergebnis in RDI gespeichert), GCC trägt nicht das Ergebnis an INCREMENT, movabs'ing INCREMENTstattdessen RDX, die dann als der Rückgabewert von rand32_ret.state verwendet wird

Ein minimal reproduzierbares Beispiel ( Compiler Explorer ):

#include <stdint.h>

struct retstruct {
    uint32_t a;
    uint64_t b;
};

struct retstruct fn(uint64_t input)
{
    struct retstruct ret;

    ret.a = 0;
    ret.b = input * 11111111111 + 111111111111;

    return ret;
}

Generierte Baugruppe (GCC 9.2, x86_64, -O3):

fn:
  movabs rdx, 11111111111     # multiplier constant (doesn't fit in imm32)
  xor eax, eax                # ret.a = 0
  imul rdi, rdx
  movabs rdx, 111111111111    # add constant; one more 1 than multiplier
     # missing   add rdx, rdi   # ret.b=... that we get with clang or older gcc
  ret
# returns RDX:RAX = constant 111111111111 : 0
# independent of input RDI, and not using the imul result it just computed

Interessanterweise führt das Ändern der Struktur so, dass uint64_t das erste Mitglied ist, zu korrektem Code , ebenso wie das Ändern beider Elemente in uint64_t

x86-64 System V gibt in RDX: RAX Strukturen zurück, die kleiner als 16 Byte sind, wenn sie trivial kopierbar sind. In diesem Fall befindet sich das 2. Element in RDX, da die hohe Hälfte von RAX die Polsterung für die Ausrichtung ist oder .bwenn .aes sich um einen schmaleren Typ handelt. ( sizeof(retstruct)ist so oder so 16; wir verwenden es nicht, __attribute__((packed))daher wird alignof (uint64_t) = 8 berücksichtigt.)

Enthält dieser Code ein undefiniertes Verhalten, das es GCC ermöglichen würde, die "falsche" Assembly auszugeben?

Wenn nicht, sollte dies unter https://gcc.gnu.org/bugzilla/ gemeldet werden.

vitorhnn
quelle
Kommentare sind nicht für eine ausführliche Diskussion gedacht. Dieses Gespräch wurde in den Chat verschoben .
Samuel Liew

Antworten:

102

Ich sehe hier keine UB; Ihre Typen sind nicht signiert, so dass ein UB mit signiertem Überlauf unmöglich ist und es nichts Seltsames gibt. (Und selbst wenn es signiert wäre, müsste es korrekte Ausgaben für Eingaben erzeugen, die dies nicht tun Überlauf UB verursachen, wie rdi=1). Es ist auch mit dem C ++ - Frontend von GCC kaputt.

Außerdem kompiliert GCC8.2 es korrekt für AArch64 und RISC-V ( maddnach Verwendung einer Anweisungmovk zum Konstanten oder RISC-V mul und Hinzufügen nach dem Laden der Konstanten). Wenn es UB war, das GCC fand, würden wir im Allgemeinen erwarten, dass es es findet und Ihren Code auch für andere ISAs bricht, zumindest für solche mit ähnlichen Typbreiten und Registerbreiten.

Clang kompiliert es auch korrekt.

Dies scheint eine Regression von GCC 5 auf 6 zu sein; GCC5.4-Kompilierungen sind korrekt, 6.1 und höher nicht. ( Godbolt ).

Sie können dies über GCCs Bugzilla mit dem MCVE aus Ihrer Frage melden.

Es sieht wirklich so aus, als wäre es ein Fehler in der Behandlung von x86-64 System V-Strukturrückgaben, möglicherweise von Strukturen, die Auffüllungen enthalten. Das würde erklären, warum es beim Inlining und beim Erweitern aauf uint64_t funktioniert (Auffüllen vermeiden).

Peter Cordes
quelle
34
Ich habe es gemeldet
vitorhnn
11
@vitorhnn Sieht aus wie es behoben wurde master.
SS Anne
19

Dies wurde am trunk/ behoben master.

Hier ist das relevante Commit .

Und dies ist ein Patch , um das Problem zu beheben.

Basierend auf einem Kommentar im Patch versuchte die reload_combine_recognize_patternFunktion, USE-Insns anzupassen .

SS Anne
quelle
14

Enthält dieser Code ein undefiniertes Verhalten, das es GCC ermöglichen würde, die "falsche" Assembly auszugeben?

Das Verhalten des in der Frage dargestellten Codes ist in Bezug auf die C99- und späteren C-Sprachstandards gut definiert. Insbesondere erlaubt C Funktionen, Strukturwerte ohne Einschränkung zurückzugeben.

John Bollinger
quelle
2
GCC erstellt eine eigenständige Definition der Funktion. Das ist es, worauf wir schauen, unabhängig davon, ob dies der Fall ist, wenn Sie es zusammen mit anderen Funktionen in einer Übersetzungseinheit kompilieren. Sie können es genauso einfach testen, ohne es tatsächlich zu verwenden, __attribute__((noinline))indem Sie es in einer Übersetzungseinheit selbst kompilieren und ohne LTO verknüpfen oder indem Sie kompilieren, -fPICwas impliziert, dass alle globalen Symbole (standardmäßig) interponierbar sind und daher nicht in Anrufer eingefügt werden können. Das Problem lässt sich jedoch nur anhand des generierten ASM erkennen, unabhängig von den Anrufern.
Peter Cordes
Fair genug, @PeterCordes, obwohl ich ziemlich sicher bin, dass dieses Detail in Godbolt unter mir geändert wurde.
John Bollinger
Version 1 der Frage, die mit Godbolt verknüpft ist und nur die Funktion für sich in einer Übersetzungseinheit hat, wie der Status der Frage selbst, als Sie sie beantwortet haben. Ich habe nicht alle Überarbeitungen oder Kommentare überprüft, die Sie sich hätten ansehen können. Wasser unter der Brücke, aber ich glaube nicht, dass jemals behauptet wurde, dass die eigenständige ASM-Definition nur gebrochen wurde, als die Quelle verwendet wurde __attribute__((noinline)). (Das wäre schockierend und nicht nur überraschend, wie ein GCC-Korrektheitsfehler ist). Wahrscheinlich wurde das nur erwähnt, um einen Testanrufer zu erstellen, der das Ergebnis druckt.
Peter Cordes