Mój artykuł zaczynał się od następujących słów: "Programując w językach C lub C++, bardzo łatwo jest popełnić błąd." - jak można się domyślić po tytule postu, słowa okazały się prorocze i faktycznie kod jednego z listingów w artykule jest błędny. Kudos za wypatrzenie błędu dla KrzaQ!

Błędy listing:
Listing 1c. Przykładowa funkcja inkrementująca zmienną typu unsigned char

bool uc_inc(unsigned char &a) {
 /* czy nastąpi przepełnienie */
 if(a > a + 1) {
   return false;
 }
 a++;
 return true;
}

Powinno być:
Listing 1c. Przykładowa funkcja inkrementująca zmienną typu unsigned char

bool uc_inc(unsigned char &a) {
 /* czy nastąpi przepełnienie */
 if(a > (unsigned char)(a + 1)) {
   return false;
 }
 a++;
 return true;
}

Błąd polega oczywiście na zignorowaniu integer promotion, czyli promocji typu unsigned char na int przed wykonaniem dodawania i potem porównania - a więc overflow nigdy nie następuje. Konkretniej, int(255)+int(1) to int(256), które jest oczywiście większe niż int(255), a więc overflow nie zostaje wykryty (zachęcam do rzucenia okiem na dyskusję w komentarzach do tego postu).

KrzaQ jako dowód na powyższe niechciane zachowanie wrzucił m.in. postać wynikową w assembly powyższej (nieprawidłowej) funkcji. Fragment w składni intela:

mov rax, QWORD PTR [rbp-8]
movzx eax, BYTE PTR [rax]
movzx edx, al
mov rax, QWORD PTR [rbp-8]
movzx eax, BYTE PTR [rax]
movzx eax, al
add eax, 1
cmp edx, eax

Jak widać powyżej, porównanie faktycznie dokonuje się na rejestrach 32-bitowych (edx, eax), a więc cast do int'a lub unsigned int'a musiał nastąpić.

Od siebie dorzucę jeszcze fragment zrzutu generowanego przez gcc -fdump-tree-original:

if ((int) *a > (int) *a + 1)
{
 return = 0;
}

Jeśli chodzi o odpowiedni cytat ze standardu (C++, jeden z nowszych draftów; sekcja Integer Promotion), to:

A prvalue of an integer type other than bool, char16_t, char32_t, or wchar_t whose integer conversion rank (4.13) is less than the rank of int can be converted to a prvalue of type int if int can represent all the values of the source type; otherwise, the source prvalue *can* be converted to a prvalue of type unsignedint.

W ramach post mortem sprawdziłem jak doszło do popełniania przeze mnie takiego błędu (revision history w Google Docs to świetny ficzer), i wygląda na to, że wcześniej listing 1c miał następującą formę:

bool uc_inc(unsigned char *a) {
 if(!a) return false;

 unsigned char tmp = *a + 1;

 if(*a > tmp) {
   /* nastąpi przepełnienie */
   return false;
 }

 (*a)++;
 return true;  
}

Jak widać w powyższym listingu mam dodatkową zmienną tmp, której celem było blokowanie promocji (w standardzie jest "can", nie "will" czy "must") lub implicit cast wyniku na unsigned char. I ten listing jest poprawny.
Niestety, wygląda na to, że później (o 3:15 AM wg revision history ;p) chciałem go uprościć, więc zamieniłem pointery na reference i pozbyłem się zmiennej tmp bez przeniesienia implicit castu na explicit castu. My bad.

Przepraszam za zamieszanie i wprowadzenie w błąd, i raz jeszcze podziękowania dla KrzaQ za wypatrzenie błędu.

Z drobniejszych rzeczy: na stronie 4 (24 w magazynie) listing na dole po prawej jest oznaczony jako "Listing 3a". Oczywiście poprawnym oznaczeniem jest "Listing 2a". Podziękowania dla rafalona.

I tyle ;)

Comments:

2013-03-01 01:58:44 = XANi
{
eee a nie lepiej po prostu sprawdzać czy a = UCHAR_MAX ?
}
2013-03-01 09:03:20 = KrzaQ
{
@ XANi: pewnie, można nawet pójść krok dalej i napisać generalne rozwiązanie szablonowe, np. http://ideone.com/dsEl0P ;)
}
2013-03-01 09:08:32 = Gynvael Coldwind
{
@XANi
Forma funkcji wynika ze "spisu treści" artykułu - dla typów unsigned gdzie overflow jest w pełni zdefiniowany chciałem pokazać ten sposób, natomiast dla typów signed gdzie overflow jest niezdefiniowany pokazałem sposób o którym piszesz.

@KrzaQ
Yep.

Chociaż osobiście najchętniej bym to napisał w assembly :)
}

Add a comment:

Nick:
URL (optional):
Math captcha: 7 ∗ 7 + 8 =