(In reply to Martin Stránský [:stransky] (ni? me) from comment #0) > I tested latest trunk with gcc-13.2.1-4.fc38.x86_64 / Fedora 38 and the bug is here. Trunk built with clang doesn't show this bug. I looked at what the loop code looks like in Firefox 120 for Windows x64 (so before patch): ``` // edx is rnd3 cmp dl, 21 mov r11d, 20 cmovae r11d, edx // r11b is now std::clamp<uint8_t>(rnd3, 20, 255) inc r11b movzx edx, r11b cmp dl, 2 mov r11d, 1 cmovae r11d, edx // r11b is now 1 if std::clamp<uint8_t>(rnd3, 20, 255) was 0 or 255 // otherwise, r11b is now std::clamp<uint8_t>(rnd3, 20, 255) + 1 [...] loop: [...] dec r11b jne loop ``` When applied to a `rnd3` value in 0..254, this code yields a loop with `rnd3+1` turns, as expected from reading the C++ source code. When applied to a `rnd3` value of 255 however, it yields a loop with only 1 turn, and not an infinite loop like we may expect. I assume that clang didn't care about guaranteeing a particular behavior when `rnd3` is 255 because this case breaks [forward progress](https://en.cppreference.com/w/cpp/language/memory_model#Forward_progress) ("infinite loops without side effects are undefined behavior"), in other words the case where `rnd3` had a value of 255 was undefined behavior.
Bug 1866409 Comment 19 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
(In reply to Martin Stránský [:stransky] (ni? me) from comment #0) > I tested latest trunk with gcc-13.2.1-4.fc38.x86_64 / Fedora 38 and the bug is here. Trunk built with clang doesn't show this bug. I looked at what the loop code looks like in Firefox 120 for Windows x64 (so before patch, compiled with clang too): ``` // edx is rnd3 cmp dl, 21 mov r11d, 20 cmovae r11d, edx // r11b is now std::clamp<uint8_t>(rnd3, 20, 255) inc r11b movzx edx, r11b cmp dl, 2 mov r11d, 1 cmovae r11d, edx // r11b is now 1 if std::clamp<uint8_t>(rnd3, 20, 255) was 0 or 255 // otherwise, r11b is now std::clamp<uint8_t>(rnd3, 20, 255) + 1 [...] loop: [...] dec r11b jne loop ``` When applied to a `rnd3` value in 0..254, this code yields a loop with `rnd3+1` turns, as expected from reading the C++ source code. When applied to a `rnd3` value of 255 however, it yields a loop with only 1 turn, and not an infinite loop like we may expect. I assume that clang didn't care about guaranteeing a particular behavior when `rnd3` is 255 because this case breaks [forward progress](https://en.cppreference.com/w/cpp/language/memory_model#Forward_progress) ("infinite loops without side effects are undefined behavior"), in other words the case where `rnd3` had a value of 255 was undefined behavior.
(In reply to Martin Stránský [:stransky] (ni? me) from comment #0) > I tested latest trunk with gcc-13.2.1-4.fc38.x86_64 / Fedora 38 and the bug is here. Trunk built with clang doesn't show this bug. I looked at what the loop code looks like in Firefox 120 for Windows x64 (so before patch, compiled with clang too): ``` // edx is rnd3 cmp dl, 21 mov r11d, 20 cmovae r11d, edx // r11b is now std::clamp<uint8_t>(rnd3, 20, 255) inc r11b movzx edx, r11b cmp dl, 2 mov r11d, 1 cmovae r11d, edx // r11b is now 1 if std::clamp<uint8_t>(rnd3, 20, 255) was 0 or 255 // otherwise, r11b is now std::clamp<uint8_t>(rnd3, 20, 255) + 1 [...] loop: [...] dec r11b jne loop ``` When applied to a `rnd3` value in 0..254, this code yields a loop with `rnd3+1` turns, as expected from reading the C++ source code. When applied to a `rnd3` value of 255 however, it yields a loop with only 1 turn, and not an infinite loop like we may expect. I assume that clang didn't care about guaranteeing a particular behavior when `rnd3` is 255 because this case breaks [forward progress](https://en.cppreference.com/w/cpp/language/memory_model#Forward_progress) ("infinite loops without side effects are undefined behavior"), in other words I belive that the case where `rnd3` had a value of 255 was undefined behavior.
(In reply to Martin Stránský [:stransky] (ni? me) from comment #0) > I tested latest trunk with gcc-13.2.1-4.fc38.x86_64 / Fedora 38 and the bug is here. Trunk built with clang doesn't show this bug. I looked at what the loop code looks like in Firefox 120 for Windows x64 (so before patch, compiled with clang too): ``` // edx is rnd3 cmp dl, 21 mov r11d, 20 cmovae r11d, edx // r11b is now std::clamp<uint8_t>(rnd3, 20, 255) inc r11b movzx edx, r11b cmp dl, 2 mov r11d, 1 cmovae r11d, edx // r11b is now 1 if std::clamp<uint8_t>(rnd3, 20, 255) was 0 or 255 // otherwise, r11b is now std::clamp<uint8_t>(rnd3, 20, 255) + 1 [...] loop: [...] dec r11b jne loop ``` When applied to a `rnd3` value in 0..254, this code yields a loop with `rnd3+1` turns, as expected from reading the C++ source code. When applied to a `rnd3` value of 255 however, it yields a loop with only 1 turn, and not an infinite loop like we may expect. I assume that clang didn't care about guaranteeing a particular behavior when `rnd3` is 255 because this case breaks [forward progress](https://en.cppreference.com/w/cpp/language/memory_model#Forward_progress) ("infinite loops without side effects are undefined behavior"), in other words I believe that the case where `rnd3` had a value of 255 was undefined behavior.
(In reply to Martin Stránský [:stransky] (ni? me) from comment #0) > I tested latest trunk with gcc-13.2.1-4.fc38.x86_64 / Fedora 38 and the bug is here. Trunk built with clang doesn't show this bug. I looked at what the loop code looks like in Firefox 120 for Windows x64 (so before patch, compiled with clang too): ``` // edx is rnd3 cmp dl, 21 mov r11d, 20 cmovae r11d, edx // r11b is now std::clamp<uint8_t>(rnd3, 20, 255) inc r11b movzx edx, r11b cmp dl, 2 mov r11d, 1 cmovae r11d, edx // r11b is now 1 if std::clamp<uint8_t>(rnd3, 20, 255) was 255 // otherwise, r11b is now std::clamp<uint8_t>(rnd3, 20, 255) + 1 [...] loop: [...] dec r11b jne loop ``` When applied to a `rnd3` value in 0..254, this code yields a loop with `rnd3+1` turns, as expected from reading the C++ source code. When applied to a `rnd3` value of 255 however, it yields a loop with only 1 turn, and not an infinite loop like we may expect. I assume that clang didn't care about guaranteeing a particular behavior when `rnd3` is 255 because this case breaks [forward progress](https://en.cppreference.com/w/cpp/language/memory_model#Forward_progress) ("infinite loops without side effects are undefined behavior"), in other words I believe that the case where `rnd3` had a value of 255 was undefined behavior.
(In reply to Martin Stránský [:stransky] (ni? me) from comment #0) > I tested latest trunk with gcc-13.2.1-4.fc38.x86_64 / Fedora 38 and the bug is here. Trunk built with clang doesn't show this bug. I looked at what the loop code looks like in Firefox 120 for Windows x64 (so before patch, compiled with clang too): ``` // edx is rnd3 cmp dl, 21 mov r11d, 20 cmovae r11d, edx // r11b is now std::clamp<uint8_t>(rnd3, 20, 255) inc r11b movzx edx, r11b cmp dl, 2 mov r11d, 1 cmovae r11d, edx // r11b is now 1 if std::clamp<uint8_t>(rnd3, 20, 255) was 255 // otherwise, r11b is now std::clamp<uint8_t>(rnd3, 20, 255) + 1 [...] loop: [...] dec r11b jne loop ``` When applied to a `rnd3` value in 0..254, this code yields a loop with `rnd3+1` turns, as expected from reading the C++ source code. When applied to a `rnd3` value of 255 however, it yields a loop with only 1 turn, and not an infinite loop like we may expect. I assume that clang didn't care about guaranteeing a particular behavior when `rnd3` is 255 because this case breaks [forward progress](https://en.cppreference.com/w/cpp/language/memory_model#Forward_progress) ("infinite loops without side effects are undefined behavior"), in other words I believe that the case where `rnd3` had a value of 255 was undefined behavior, thus explaining the difference in behavior across different builds.
(In reply to Martin Stránský [:stransky] (ni? me) from comment #0) > I tested latest trunk with gcc-13.2.1-4.fc38.x86_64 / Fedora 38 and the bug is here. Trunk built with clang doesn't show this bug. I looked at what the loop code looks like in Firefox 120 for Windows x64 (so before patch, compiled with clang too): ``` // edx is rnd3 cmp dl, 21 mov r11d, 20 cmovae r11d, edx // r11b is now std::clamp<uint8_t>(rnd3, 20, 255) inc r11b movzx edx, r11b cmp dl, 2 mov r11d, 1 cmovae r11d, edx // r11b is now 1 if std::clamp<uint8_t>(rnd3, 20, 255) was 255 // otherwise, r11b is now std::clamp<uint8_t>(rnd3, 20, 255) + 1 [...] loop: [...] dec r11b jne loop ``` When applied to a `rnd3` value in 0..254, this code yields a loop with `rnd3+1` turns, as expected from reading the C++ source code. When applied to a `rnd3` value of 255 however, it yields a loop with only 1 turn, and not an infinite loop like we may expect. I assume that clang didn't care about guaranteeing a particular behavior when `rnd3` is 255 because this case breaks [forward progress](https://en.cppreference.com/w/cpp/language/memory_model#Forward_progress) (in simple terms, "infinite loops without side effects are undefined behavior"), in other words I believe that the case where `rnd3` had a value of 255 was undefined behavior, thus explaining the difference in behavior across different builds.
(In reply to Martin Stránský [:stransky] (ni? me) from comment #0) > I tested latest trunk with gcc-13.2.1-4.fc38.x86_64 / Fedora 38 and the bug is here. Trunk built with clang doesn't show this bug. I looked at what the loop code looks like in Firefox 120 for Windows x64 (so before patch, compiled with clang too): ``` // edx is rnd3 cmp dl, 21 mov r11d, 20 cmovae r11d, edx // r11b is now std::clamp<uint8_t>(rnd3, 20, 255) inc r11b movzx edx, r11b cmp dl, 2 mov r11d, 1 cmovae r11d, edx // r11b is now 1 if std::clamp<uint8_t>(rnd3, 20, 255) was 255 // otherwise, r11b is now std::clamp<uint8_t>(rnd3, 20, 255) + 1 [...] loop: [...] dec r11b jne loop ``` When applied to a `rnd3` value in 0..254, this code yields a loop with `rnd3+1` turns, as expected from reading the C++ source code. When applied to a `rnd3` value of 255 however, it yields a loop with only 1 turn, and not an infinite loop like we may expect. I assume that clang didn't care about guaranteeing a particular behavior when `rnd3` is 255 because this case breaks [forward progress](https://en.cppreference.com/w/cpp/language/memory_model#Forward_progress) (in simple terms, "infinite loops without side effects are undefined behavior"). In other words I believe that the case where `rnd3` had a value of 255 was undefined behavior, thus explaining the difference in behavior across different builds.
(In reply to Martin Stránský [:stransky] (ni? me) from comment #0) > I tested latest trunk with gcc-13.2.1-4.fc38.x86_64 / Fedora 38 and the bug is here. Trunk built with clang doesn't show this bug. I looked at what the loop code looks like in Firefox 120 for Windows x64 (so before patch, compiled with clang too): ``` // edx is rnd3 cmp dl, 21 mov r11d, 20 cmovae r11d, edx // r11b is now std::clamp<uint8_t>(rnd3, 20, 255) inc r11b movzx edx, r11b cmp dl, 2 mov r11d, 1 cmovae r11d, edx // r11b is now 1 if std::clamp<uint8_t>(rnd3, 20, 255) was 255 // otherwise, r11b is now std::clamp<uint8_t>(rnd3, 20, 255) + 1 [...] loop: [...] dec r11b jne loop ``` When applied to a `rnd3` value in 0..254, this code yields a loop with `rnd3+1` turns, as expected from reading the C++ source code. When applied to a `rnd3` value of 255 however, it yields a loop with only 1 turn, and not an infinite loop like we may expect. I assume that clang didn't care about guaranteeing a particular behavior when `rnd3` is 255 because this case breaks [forward progress](https://en.cppreference.com/w/cpp/language/memory_model#Forward_progress) (in simple terms, "infinite loops without side effects are undefined behavior"). I believe that the case where `rnd3` had a value of 255 was undefined behavior, thus explaining the difference in behavior across different builds.
(In reply to Martin Stránský [:stransky] (ni? me) from comment #0) > I tested latest trunk with gcc-13.2.1-4.fc38.x86_64 / Fedora 38 and the bug is here. Trunk built with clang doesn't show this bug. I looked at what the loop code looks like in Firefox 120 for Windows x64 (so before patch, compiled with clang too): ``` // edx is rnd3 cmp dl, 21 mov r11d, 20 cmovae r11d, edx // r11b is now std::clamp<uint8_t>(rnd3, 20, 255) inc r11b movzx edx, r11b cmp dl, 2 mov r11d, 1 cmovae r11d, edx // r11b is now 1 if std::clamp<uint8_t>(rnd3, 20, 255) was 255 // otherwise, r11b is now std::clamp<uint8_t>(rnd3, 20, 255) + 1 [...] loop: [...] dec r11b jne loop ``` When applied to a `rnd3` value in 0..254, this code yields a loop with `rnd3+1` turns, as expected from reading the C++ source code. When applied to a `rnd3` value of 255 however, it yields a loop with only 1 turn, and not an infinite loop like we may expect. I assume that clang didn't care about guaranteeing a particular behavior when `rnd3` is 255 because this case breaks [forward progress](https://en.cppreference.com/w/cpp/language/memory_model#Forward_progress) (in simple terms, "infinite loops without side effects are undefined behavior"). I believe that the case where `rnd3` had a value of 255 was therefore undefined behavior, thus explaining the difference in behavior across different builds.
(In reply to Martin Stránský [:stransky] (ni? me) from comment #0) > I tested latest trunk with gcc-13.2.1-4.fc38.x86_64 / Fedora 38 and the bug is here. Trunk built with clang doesn't show this bug. I looked at what the loop code looks like in Firefox 120 for Windows x64 (so before patch, compiled with clang too): ``` // edx is rnd3 cmp dl, 21 mov r11d, 20 cmovae r11d, edx // r11b is now std::clamp<uint8_t>(rnd3, 20, 255) inc r11b movzx edx, r11b cmp dl, 2 mov r11d, 1 cmovae r11d, edx // r11b is now 1 if std::clamp<uint8_t>(rnd3, 20, 255) was 255 // otherwise, r11b is now std::clamp<uint8_t>(rnd3, 20, 255) + 1 [...] loop: [...] dec r11b jne loop ``` When applied to a `rnd3` value in 0..254, this code yields a loop with `rnd3+1` turns, as expected from reading the C++ source code. When applied to a `rnd3` value of 255 however, it yields a loop with only 1 turn, and not an infinite loop like we may expect. I think that clang didn't care about guaranteeing a particular behavior when `rnd3` is 255 because this case breaks [forward progress](https://en.cppreference.com/w/cpp/language/memory_model#Forward_progress) (in simple terms, "infinite loops without side effects are undefined behavior"). I believe that the case where `rnd3` had a value of 255 was therefore undefined behavior, thus explaining the difference in behavior across different builds.