Możliwy błąd GCC podczas zwracania struktury z funkcji

133

Wydaje mi się, że znalazłem błąd w GCC podczas wdrażania PCN PRNG O'Neilla. ( Kod początkowy w Godbolt's Compiler Explorer )

Po pomnożeniu oldstateprzez MULTIPLIER(wynik przechowywany w rdi), GCC nie dodaje tego wyniku do INCREMENTmovabs'ing INCREMENTzamiast do rdx, który następnie jest wykorzystywany jako wartość zwracana rand32_ret.state

Minimalny odtwarzalny przykład ( Eksplorator kompilatorów ):

#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;
}

Wygenerowany zespół (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

Co ciekawe, modyfikacja struktury tak, aby miała uint64_t jako pierwszy element, powoduje wygenerowanie poprawnego kodu , podobnie jak zmiana obu elementów na uint64_t

x86-64 System V zwraca struktury mniejsze niż 16 bajtów w RDX: RAX, gdy można je w prosty sposób skopiować. W tym przypadku drugi element jest w RDX, ponieważ górna połowa RAX jest wypełnieniem do wyrównania lub .bgdy .ajest węższy. ( sizeof(retstruct)ma 16 w obu kierunkach; nie korzystamy, __attribute__((packed))więc respektuje alignof (uint64_t) = 8.)

Czy ten kod zawiera jakieś niezdefiniowane zachowanie, które pozwoliłoby GCC na wysłanie „niepoprawnego” zestawu?

Jeśli nie, należy to zgłosić na https://gcc.gnu.org/bugzilla/

Vitorhnn
źródło
Komentarze nie są przeznaczone do rozszerzonej dyskusji; ta rozmowa została przeniesiona do czatu .
Samuel Liew

Odpowiedzi:

102

Nie widzę tu żadnego UB; twoje typy są niepodpisane, więc UB z podpisem jest niemożliwe i nie ma w tym nic dziwnego. (I nawet jeśli podpisany, musiałby generować prawidłowe dane wyjściowe dla danych wejściowych, które nie powodują przepełnienia UB, jak rdi=1). Jest również zepsuty w interfejsie GCC C ++.

Ponadto GCC8.2 kompiluje go poprawnie dla AArch64 i RISC-V (do maddinstrukcji po użyciu movkdo konstruowania stałych, lub mul RISC-V i dodawania po załadowaniu stałych). Jeśli GCC znajdowałby UB, zwykle spodziewalibyśmy się, że go znajdzie i złamie kod również dla innych ISA, przynajmniej tych, które mają podobne szerokości typów i szerokości rejestrów.

Clang również kompiluje go poprawnie.

Wydaje się, że jest to regresja z GCC 5 do 6; Kompilacje GCC5.4 są poprawne, 6.1 i nowsze nie. ( Godbolt ).

Możesz to zgłosić w Bugzilli GCC za pomocą MCVE z twojego pytania.

Wygląda na to, że jest to błąd w obsłudze struktur-return w systemie x86-64 System V, być może struktur zawierających padding. To by wyjaśniało, dlaczego działa podczas wstawiania i przy rozszerzaniu ado uint64_t (unikanie wypełniania).

Peter Cordes
źródło
34
Zgłosiłem to
vitorhnn
11
@vitorhnn Wygląda na to, że zostało to naprawione master.
SS Anne
19

Zostało to naprawione w trunk/ master.

Oto odpowiednie zatwierdzenie .

To jest łatka do rozwiązania problemu.

Na podstawie komentarza w łatcereload_combine_recognize_pattern funkcja próbowała dostosować ustawienia USE .

SS Anne
źródło
14

Czy ten kod zawiera jakieś niezdefiniowane zachowanie, które pozwoliłoby GCC na wysłanie „niepoprawnego” zestawu?

Zachowanie kodu przedstawionego w pytaniu jest dobrze określone w odniesieniu do standardów języka C99 i późniejszych C. W szczególności C umożliwia funkcjom zwracanie wartości struktury bez ograniczeń.

John Bollinger
źródło
2
GCC tworzy autonomiczną definicję funkcji; to jest to, na co patrzymy, niezależnie od tego, czy to działa, gdy kompilujesz go w jednostce tłumaczeniowej wraz z innymi funkcjami. Możesz równie łatwo przetestować go bez użycia __attribute__((noinline)), kompilując go sam w jednostce tłumaczeniowej i łącząc bez LTO, lub kompilując, z -fPICczego wynika, że ​​wszystkie symbole globalne są (domyślnie) interpolowalne, więc nie można ich wstawiać w wywołujące. Ale tak naprawdę problem można wykryć po spojrzeniu na wygenerowany asm, niezależnie od dzwoniących.
Peter Cordes
W porządku, @PeterCordes, chociaż jestem dość pewny, że ten szczegół został zmieniony spode mnie w Godbolt.
John Bollinger
Wersja 1 pytania powiązana z Godboltem z samą funkcją w jednostce tłumaczeniowej, podobnie jak stan samego pytania po udzieleniu odpowiedzi. Nie sprawdziłem wszystkich poprawek i komentarzy, na które mogłeś patrzeć. Woda pod mostem, ale nie sądzę, żeby kiedykolwiek istniało twierdzenie, że samodzielna definicja asm została zerwana tylko wtedy, gdy użyto źródła __attribute__((noinline)). (Byłoby to szokujące, a nie tylko zaskakujące jak błąd poprawności GCC). Prawdopodobnie wspomniano o tym tylko w przypadku wywołania testowego, które drukuje wynik.
Peter Cordes