Enkla men fatala buggar

PIC, AVR, Arduino, Raspberry Pi, Basic Stamp, PLC mm.
Användarvisningsbild
TomasL
EF Sponsor
Inlägg: 45173
Blev medlem: 23 september 2006, 23:54:55
Ort: Borås
Kontakt:

Enkla men fatala buggar

Inlägg av TomasL »

Kan vi ha denna tråden typ klistrad eller nått, tanken är att lista fallgropar, vilka kan generera buggar, varav vissa kan vara svåra att hitta.

Nåväl, exempel 1:

Kod: Markera allt

if (i=1){......}
En lurig sak, dessutom extremt lätt att göra.
Man kan undvika den genom att slå på alla varningar, till exempel "wall" i gcc.
Dock, en mycket bättre variant är att vända på det.
Om man istället konsekvent sätter konstanten på vänster sida, dvs

Kod: Markera allt

if (1=i){....}
Då får man garanterat ett felmeddelande, så man kan fixa tioll raden till dess rätta innehåll, dvs

Kod: Markera allt

]if (1==i){....}
Exempel 2.

Kod: Markera allt

uint32 i;
sint16 arr[..]
i = arr[x++]
i |= arr[x++]<<16
Ovanstående kod funkar, kanske ibland, ibland alltid, ibland inte, ibland funkar den slumpmässigt.
Problemet uppstår när den högre byten i "arr" inte är 0, eftersom "i" i detta fallet är 32 bitar, och man läser in från en 16 bitars array, så kan vad som helst hamna i de övre 16 bitarna i variabeln "i",
Samma gäller ju även om "arr" är en vanlig variabel, då får de övre 16 bitarna "slumpmässiga" värden, dvs värden från dataomrrådet intill "arr".
för att slippa detta måste man naturligtvis casta 16bitars variabeln, så man enbart läser in 16 bitar, dvs

Kod: Markera allt

uint32 i;
sint16 arr[..]
i = (uint16)arr[x++]
i |= arr[x++]<<16
Det var två exempel på fallgropar, fyll gärna på med fler.
blueint
Inlägg: 23238
Blev medlem: 4 juli 2006, 19:26:11
Kontakt:

Re: Enkla men fatala buggar

Inlägg av blueint »

Kod: Markera allt

for(x=0; x<10; x++);
{ printf(..); outportb(..); }
Exekverar inte så mycket.. ;)
Användarvisningsbild
Icecap
Inlägg: 26106
Blev medlem: 10 januari 2005, 14:52:15
Ort: Aabenraa, Danmark

Re: Enkla men fatala buggar

Inlägg av Icecap »

Och förklaringen till blueint's exempel:
Efter for(x=0; x<10; x++) finns det en ';' vilket gör att den räknar upp x till 10 - och inget mer under den uppräkning.

Därefter utförs utskriften och portgrejen - men bara en enda gång.
Senast redigerad av Icecap 18 maj 2013, 08:24:45, redigerad totalt 1 gång.
idiotdea
Inlägg: 467
Blev medlem: 26 juli 2006, 16:11:34
Ort: Vasa, Finland
Kontakt:

Re: Enkla men fatala buggar

Inlägg av idiotdea »

TomasL: Är du säker på att du tänker rätt om exempel 2? För mig förefaller det mycket märkligt att kopiering av en 16 bitsvariabel till en 32 bitsvariabel inte skulle rensa de övre 16 bitarna i destinationen. Åtminstone har jag mig veterligen aldrig stött på något sådant problem.

Däremot skulle jag vara försiktig med bitshiften:

Kod: Markera allt

i |= arr[x++]<<16
Det förefaller som att man shiftar en 16 bitsvariabel med 16 bitar, vilket inte kan ha önskad effekt. Jag hade nog castat arr[x++] först:

Kod: Markera allt

i |= ((uint32)arr[x++])<<16
Där finns kanske en onödig parentes, jag kommer inte ihåg rangordningen för operatorerna. Men genom att casta till 32 bitsvariabel först så får 16 bitsshiften rum i resultatet. Det är väl också så att man skall vara lite försiktig med att shifta signed-variabler också, då "minustecknet" kan ställa till det om man inte vet vad man gör.

Jag är absolut ingen mästare på programmering, så jag kan gott ha fel.
Användarvisningsbild
bit96
Inlägg: 2492
Blev medlem: 3 september 2007, 10:04:29
Ort: Säffle

Re: Enkla men fatala buggar

Inlägg av bit96 »

Exempel 2 av TomasL belyser ett problem, men det är inte slumpmässigt och inte en bugg.

Exemplet innehåller två del-problem, dels en teckenomvandling och dels ett bitshift.

Om teckenomvandlingen:
Man måste vara på det klara med att omvandla från sint16 till uint32 inte alltid är möjligt.
Om sint16 är noll eller positivt funkar det, men naturligtvis inte om sint16 är negativt eftersom uint32 inte kan representera ett negativt tal.

Om sint16 är negativt sker omvandlingen i flera steg, enligt C-standard.
Först omvandlas det till sint32 så att antal bitar är korrekt.
Sedan adderas talet 2**32, vilket inte förändrar bitmönstret, men gör att tolkningen (flaggor m.m.) blir korrekt.

Kod: Markera allt

/* Exempel med omvandling från 8 bitars signed till 16 bitars unsigned */
sint8 i;
uint16 j;
i=-2;
j=i; /* j får värdet 65534 */
Om bitshiftet:
Som ideotdea skriver måste man se upp med bitshiften.
Shiftar man ut 16 bitar ur ett 16-bitarstal blir det bara 0 kvar. Att sen omvandla till 32-bitars tal ger fortfarande noll.
Alltså måste man först omvandla till 32-bitars och därefter har man plats att shifta 16 bitar och behålla dom.

Jag vill också återigen slå ett slag för boken "Vägen till C" av Ulf Bilting och Jan Skansholm.
Där finns allt detta förklarat redan på sidan 53. :) (Ja, jag slog upp det för att vara säker)
Det är alltså grundkunskap för en C-programmerare att känna till typ-omvandlingar.

Mitt råd är:
Se ALLTID upp vid typomvandlingar.
Även om det är definerat vad som händer så kanske det inte är vad DU vill ska hända.
Använd därför type-casting, och rikligt med paranteser, vid minsta tveksamhet.
Användarvisningsbild
arvidb
Inlägg: 4537
Blev medlem: 8 maj 2004, 12:56:24
Ort: Stockholm

Re: Enkla men fatala buggar

Inlägg av arvidb »

"Pointer alignment"/aliasing

Exempel:

Kod: Markera allt

int i;
char data[DATA_LEN];
int number;

for (i = 0; i < DATA_LEN; i++)
	data[i] = read_byte(); /* Läs bytes någonstans ifrån */

...

/* Hantera data[2]..data[5] som en 32-bit int */
number = *((int *) &data[2]); /* Odefinierat resultat! */
Ovanstående kan fungera på vissa arkitekturer (ibland med sämre prestanda), men fungerar inte på alla. Problemet är att många arkitekturer kräver att pekare till datatyper ska vara alignade på jämna multiplar av storleken på datatypen, d.v.s. pekare till int måste ha värden som är multiplar av 4 (eller vad nu sizeof(int) är på respektive arkitektur). Och det lömska är att detta är ett "tyst" fel; kompilatorn kan inte upptäcka det och man får ingen exception eller likande om det inte fungerar, bara väldigt konstiga värden...

Evalueringsordning inom uttryck med bieffekter

Exampel:

Kod: Markera allt

i = ++i + 1;
Om i == 3 innan yttrycket, så är i == 4 eller i == 5 efter uttrycket.

Kod: Markera allt

x[i] = i++;
x[2] == 2 eller x[2] == 3 efter uttrycket.

Kod: Markera allt

a[i] = b[i++];
a[n] == b[n] eller a[n] == b[n + 1] efter uttrycket

Evalueringsordningen inom ett uttryck (egentligen mellan "sequence points", t.ex. ';') är odefinierad i C. Detta gör pre- & postfix increment/decrement farliga! Använd i =+ 1; på en separat rad istället.
Användarvisningsbild
JimmyAndersson
Inlägg: 26308
Blev medlem: 6 augusti 2005, 21:23:33
Ort: Oskarshamn (En bit utanför)
Kontakt:

Re: Enkla men fatala buggar

Inlägg av JimmyAndersson »

bit96:
"Exempel 2 av TomasL belyser ett problem, men det är inte slumpmässigt och inte en bugg."

Med "bugg" så menar TomasL säkerligen att det kan leda till en bugg i ens egna kod/program.


TomasL:
Bra initiativ! :tumupp: Jag klistrade.
Användarvisningsbild
arvidb
Inlägg: 4537
Blev medlem: 8 maj 2004, 12:56:24
Ort: Stockholm

Re: Enkla men fatala buggar

Inlägg av arvidb »

TomasL skrev:

Kod: Markera allt

uint32 i;
sint16 arr[..]
i = arr[x++]
i |= arr[x++]<<16
Ovanstående kod funkar, kanske ibland, ibland alltid, ibland inte, ibland funkar den slumpmässigt.
Problemet uppstår när den högre byten i "arr" inte är 0, eftersom "i" i detta fallet är 32 bitar, och man läser in från en 16 bitars array, så kan vad som helst hamna i de övre 16 bitarna i variabeln "i",
Samma gäller ju även om "arr" är en vanlig variabel, då får de övre 16 bitarna "slumpmässiga" värden, dvs värden från dataomrrådet intill "arr".
för att slippa detta måste man naturligtvis casta 16bitars variabeln, så man enbart läser in 16 bitar, dvs

Kod: Markera allt

uint32 i;
sint16 arr[..]
i = (uint16)arr[x++]
i |= arr[x++]<<16
Det finns problem med exempelkoden, men jag tror inte heller att TomasLs förklaring (eller fix) är korrekt.

Innan operatorer appliceras på en heltalsoperand så uppgraderas typen ("type promotion") enligt följande:

signed char, short, signed bitfield --> int
char, unsigned char, unsigned short, unsigned bitfields --> int (om alla originaltypens möjliga värden får plats) eller unsigned int annars.

Alltså (givet sizeof(unsigned int) == 4 och sizeof(short) == 2):

Kod: Markera allt

unsigned int i;
short arr[..];
i = arr[x++]; /* typeof(arr[..]) --> int */ 
i |= arr[x++]<<16; /* Här shiftar vi en int (signed) */

Det finns flera problem här:
Rad 3: Type conversion int -> unsigned int. OK om originalvärdet är >= 0. Ingen varning från kompilatorn (definierat och korrekt enligt C-standarden).
Rad 4a: Bit-shift av signed är odefinierat om värdet antingen är negativt från början eller om det shiftas in en 1:a i sign bit. Här är det alltså viktigt att casta till unsigned int (ej unsigned short om jag förstår rätt).
Rad 4b: Type conversion int -> unsigned int. OK om originalvärdet är >= 0.

Originalkoden använder dessutom icke-standard-typer vilket gör det omöjligt att se hur koden kommer att fungera. Även standard (C99) fixed-width-typer (int16_t, uint32_t) är definierade i termer av bastyperna int etc, så även här kommer koden att bete sig olika på olika plattformar.


Slutsats 1: Använd alltid unsigned int för bit-operationer (lite yxigt men säkert!). Alternativt: skifta aldrig upp så att resultatet fyller alla bitar i den "uppgraderade" typen, och aldrig ett negativt värde.

Slutsats 2: Vill du ändra signedness, casta till int eller unsigned int, även om både originaltypen och resultattypen är kortare. Mer generellt: Använd om möjligt variabler av typen int/unsigned int, så slipper du otrevliga överraskningar.

Slutsats 3: Undvik typedefs eftersom de gör koden svårare att förstå (sällsynta undantag finns dock, t.ex. funktionspekare).

Edit: la till Slutsats 2 & 3.
Senast redigerad av arvidb 18 maj 2013, 16:34:50, redigerad totalt 1 gång.
blueint
Inlägg: 23238
Blev medlem: 4 juli 2006, 19:26:11
Kontakt:

Re: Enkla men fatala buggar

Inlägg av blueint »

Kod: Markera allt

i = ++i + 1;
Leder till i += 2;
Eftersom först inkrementeras i++ med ett och därefter läggs +1 till vilket resulterar 2 mer än man började med. ++ har högre precedens än + om jag minns rätt.

De övriga exemplen arvidb visar upp markerar betydelsen över att fundera på evalueringsordningen.
Användarvisningsbild
arvidb
Inlägg: 4537
Blev medlem: 8 maj 2004, 12:56:24
Ort: Stockholm

Re: Enkla men fatala buggar

Inlägg av arvidb »

Ok, efter lite mer googling:

Resultatet av uttrycket 'i = ++i + 1' (liksom av 'i = i++ + 1') är odefinierat i C-standarden, eftersom två tilldelningar till samma variabel sker utan sekvenspunkt emellan. Resultatet kan alltså vara att inget alls händer, att det motsvarar i += 2, att programmet kraschar eller något annat, beroende på kompilator och månens fas...
blueint
Inlägg: 23238
Blev medlem: 4 juli 2006, 19:26:11
Kontakt:

Re: Enkla men fatala buggar

Inlägg av blueint »

Gäller visst att se upp.. ;)
Användarvisningsbild
TomasL
EF Sponsor
Inlägg: 45173
Blev medlem: 23 september 2006, 23:54:55
Ort: Borås
Kontakt:

Re: Enkla men fatala buggar

Inlägg av TomasL »

TomasL: Är du säker på att du tänker rätt om exempel 2? För mig förefaller det mycket märkligt att kopiering av en 16 bitsvariabel till en 32 bitsvariabel inte skulle rensa de övre 16 bitarna i destinationen.
Det gör den inte (i alla fall inte gcc), det var därför jag hittade felet.

Beträffande mina typer uint32, sint16 osv, är det en mycket bra skola att INTE använda de inbyggda typerna, eftersom konstigheter kan uppkomma när man porterar kod.
I C är det väl i princip bara "char" som har samma längd oavsett system.

Därför skall man använda egna typer, där man vet längden, därav till exempel "uint32", som namnet framgår är det en unsigned int, vilken är 32 bitar lång, oavsett vilket system den körs på.
Användarvisningsbild
arvidb
Inlägg: 4537
Blev medlem: 8 maj 2004, 12:56:24
Ort: Stockholm

Re: Enkla men fatala buggar

Inlägg av arvidb »

blueint skrev:Gäller visst att se upp.. ;)
Jupp. :) Vill man skriva buggfri kod så är det faktiskt bra att låta bli "++" och "--".

En tänkbar mekanism bakom att man får resultatet i += 1:

Operationerna som krävs för '++':
A1: Hämta värdet av i
A2: Addera 1
A3: Spara nya värdet till minne

Operationerna som krävs för '+1':
B1: Hämta värdet av ++i
B2: Addera 1
B3: Spara nya värdet till minne

Dessa är inte sekvenserade (förutom att A2 antagligen måste göras innan B1). Om de görs i denna ordning blir det fel:

A1: Hämta värdet av i
A2: Addera 1
B1: Hämta värdet av ++i
B2: Addera 1
B3: Spara nya värdet till minne
A3: Spara nya värdet till minne

Det är antagligen mer komplicerat i verkligheten. :)

Edit: Här är en mycket bättre förklaring till vad som händer: http://c-faq.com/~scs/cgi-bin/faqcat.cgi?sec=expr
blueint
Inlägg: 23238
Blev medlem: 4 juli 2006, 19:26:11
Kontakt:

Re: Enkla men fatala buggar

Inlägg av blueint »

TomasL skrev:Beträffande mina typer uint32, sint16 osv, är det en mycket bra skola att INTE använda de inbyggda typerna, eftersom konstigheter kan uppkomma när man porterar kod.
I C är det väl i princip bara "char" som har samma längd oavsett system.
Är det "int" mm som du avser med inbyggda typer och "uint32" som egen typ?

Är ju vanligt att typer definieras som:

Kod: Markera allt

typedef unsigned char ui32[4];

int main(){
ui32  no;
 printf("len=%d\n", sizeof(no));
return 0;
}
Alternativt: typedef unsigned int ui32; /* Förutsatt att int är 32-bit vilket måste kontrolleras */

T.ex if( sizeof(int) != 4 ) { printf("FAIL!\n"); exit(1); }

Länkar:
http://publications.gbdirect.co.uk/c_bo ... pedef.html
http://www.tutorialspoint.com/cprogramm ... ypedef.htm
Användarvisningsbild
arvidb
Inlägg: 4537
Blev medlem: 8 maj 2004, 12:56:24
Ort: Stockholm

Re: Enkla men fatala buggar

Inlägg av arvidb »

TomasL skrev:
TomasL: Är du säker på att du tänker rätt om exempel 2? För mig förefaller det mycket märkligt att kopiering av en 16 bitsvariabel till en 32 bitsvariabel inte skulle rensa de övre 16 bitarna i destinationen.
Det gör den inte (i alla fall inte gcc), det var därför jag hittade felet.
Troligtvis är det något annat som var fel? Din beskrivning av att "värden från dataområdet intill "arr"" används stämmer i alla fall inte. Det låter snarare som att du beskriver en typecast från (sint16 *) till (uint32 *) - alltså mellan olika pekartyper? Derefererar du sådana så får du precis den effekt som du beskriver (givet att du inte får aliasingproblem... :roll:)
TomasL skrev:Beträffande mina typer uint32, sint16 osv, är det en mycket bra skola att INTE använda de inbyggda typerna, eftersom konstigheter kan uppkomma när man porterar kod.
I C är det väl i princip bara "char" som har samma längd oavsett system.

Därför skall man använda egna typer, där man vet längden, därav till exempel "uint32", som namnet framgår är det en unsigned int, vilken är 32 bitar lång, oavsett vilket system den körs på.
Om man är beroende av en viss bitlängd - som i ditt fall - så kan jag hålla med om att koden blir tydligare av att använda just fixed width-typer. Det jag är mest allergisk mot är den typiska typedef struct {} new_type; ...

Dock måste man komma ihåg att fixed width-typer inte är tillräckligt för att få samma beteende på en annan plattform. Du kan fortfarande få ändrat beteende p.g.a. annorlunda type promotion, som beskrivet ovan.
Skriv svar