Jeg har hen over de sidste par måneder været med i et projekt, hvor vi
skulle rydde op i noget C/C++ kode på Linux, hvor der var en del
problemer. I dette blog-indlæg sætter jeg fokus på hvor "hysterisk"
man kan sætte sin GCC/GLANG C eller C++ compiler op, og der er ikke
fokus på dynamisk kode-check. Det regner jeg med at vende tilbage til
i et andet blogindlæg.
Jeg startede projektet med at tilføje compile flag til GCC så vi fik
fockus på de værste problemer i koden. Jeg starter altid med "-Wall",
som giver en hel del advarsler af forskellig art, men f.eks. får man
ikke advarsler om variable som skygger for hinanden eller manglende
prototyper i headerfiler. Derfor starter jeg også med "-Wshadow
-Wimplicit-function-declaration"
CFLAGS := -Wall -Wshadow -Wimplicit-function-declaration
Efter at der kom styr på at alle basis-fejl blev rettet op, gik jeg
mere hårdt til værks, med at lede efter ubrugte variable, eller steder
hvor resultater ikke blev brugt, type-konverteringsfejl og endelig tre
hystade-flag "-ansi -pedantic -Wextra".
-Wunused-variable -Wunused -Wunused-result -Wconversion -ansi -pedantic -Wextra
Det skal lige nævnes at "-pedantic" er koblet til "-ansi", og måske er det mange sammenhænge
smartere at se på f.eks. "-std=c99" eller en tilsvarende C++ standard, da "-ansi" bl.a. blokerer
for v
For at få fokus på at selv advarsler blev undersøgt, konverterer jeg
advarsler til at blive flaget som fejl.
-fmax-errors=1 -Werror
Det har været hårdt at rette koden op, så der ikke er advarsler
tilbage, men det var også en rigtig god oplevelse at rette koden op.
Langt det meste, der kom op i søgelyset var fejl ala.
int c = 1; uint d = c;
Det er kode som næsten altid er ok, men var "c" initialiseret med et
negativt tal, så kan det være mere "underholdende".
Der var også få tilfælde, hvor mere subtile men rigtige fejl pludselig kom
til overfladen.
Efter at være kommet i bund med fejl og advarsler har det været skønt
at arbejde med koden fordi nye advarsler drukner i listen af gamle
advarsler. Nu ved at jeg en advarsel er noget nyt og noget at jeg bør
kigge på. Overraskende har der ikke været surhed fra de andre
udviklere over at køre så hysterisk. Alle har taget godt i mod det,
men det var en hård fase at luge alle advarsler ud.
Et godt trick til at skrive makefiler er også at lave understøttelse
for forskellige niveauer af advarsler, så man i en stor restruktureringsøvelse kan køre med lavt
niveau af advarsler og senere i et udviklingsforløb få de mere detaljerede advarsler frem.
Der er tonsvis af flag man kan give GCC og har du haft god anvendelse
af andre compilerflag, så skriv gerne om dem nedenfor.
En anden sjov erfaring er at selvom jeg f.eks. har slagtet alle
advarsler ned for en compiler, så er min erfaring at forskellige
compilere giver forskellige advarsler. De nyeste udgaver af Visual
Studio giver nogle valide advarsler, som GCC ikke fanger. At hoppe
mellem forskellige compilere er guld.
/pto
Jeg oversætter også gerne med både clang og gcc. Jenkins er din ven - oversæt altid med begge compilers hver gang og opfat warnings som fejl.
Men som en sjov øvelse, er det også værd at oversætte på forskellige platforme, f.eks. Intel og ARM (en billig Raspberry Pi 2) og køre dine tests. Du kan fange fejl grundet i subtile antagelser og typer og værdier af variable.
De eneste warnings der tillades i den C-kodebase jeg udvikler professionelt, er dem der indsættes med #warning-direktivet. FWIW: vi skriver embedded functional safety critical software.
Udviklerne overser ofte warnings om reelle fejl, som f.eks. 'a=b' i stedet for 'a==b' hvis der er nok warnings til at man skal scrolle for at se dem allesammen... Derfor har vi nul warnings som mål. Ellers er erfaringen, at det stikker af når projektet begynder at nærme sig en vis størrelse, fx. 100.000 liniers kode.
Og hvad angår dynamisk checks, så strør jeg om mig med assertions! De kan i mange tilfælde verificeres statisk i embedded kode, f.eks. med CBMC.
Jeg har haft ganske gode erfaringer med at tilføje ca. ditto linting til en håndfuld projekter efterhånden.
Siden risikoen ved et mega-commit med alle rettelser ofte er for stor, kan man sætte linter/compiler/hvadnu til at ignorere alle advarsler/fejl, men derved få CI/precommit/... infrastrukturen på plads. Husk endelig at have den tilhørende konfiguration i versionskontrol - så er der bred enighed, kan tweakes per-projekt og nemt opdateres over tid.
Derefter kan man slå et flag til ad gangen og i samme commit rette alle opståede fejl. Så får man ét commit med det ændrede flag og alle deraf følgende rettelser. Og der en god anti-overspringshandling at tilføje endnu et flag en gang imellem ☺.
Har du prøvet med Intel's propriotære compiler? Ud over optimering performance er de også kendte for at have gode warnings. Kræver selvf. at du er på x86/amd64 hvilket historien ikke melder noget om.
HP har et værktøj "cadvise" (code advisor). På pa-risc var det et standalone værktøj, men på itanium er det bygget ind i compileren. Det interessante er at compileren på pa-risc var deres eget produkt, men på itanium var den baseret på Intel-teknologi og havde ret meget til fælles med Intels compiler. Så mit spørgsmål er om Intel i deres toolset har "cadvise" eller om det var en ren HP-tilpasning? Det interessante ved cadvise, som jeg aldrig nåede at prøve da jeg havde adgang til det, er at man angiveligt kan skrive plugins til det som har adgang til hele parse-træet, inkl på link-tidspunkt, så man kunne lave nogle ret avancerede globale checks.
Sidst jeg prøvede (et par år siden), da havde Intels compilere bedre warnings end gcc. Så vidt jeg ved er den oprindeligt baseret på Kai C/C++ compiler, som de købte tilbage i 2000. Studerende, undervisere og opensource-udviklere kan få en gratis version. Andre må betale $699, hvilket stadig er billigt hvis den finder blot en fejl i ens program inden en kunde gør det.
På mit tidligere arbejde fik mine C++-programmer en tur med GCC 2.95.3 og 4.x, HP aCC 02.xx og 06.xx, clang, Flexelint, samt Insure++ (når jeg havde tålmodighed til det).
Men hvis man er seriøs med at finde fejl i kode vha. statisk analyse, så bør man investere i f.eks. Flexelint, som er beregnet til at gøre programmører kede af det :-) Se evt. min præsentation fra debugging-konferenced i 2013: http://i1.dk/misc/flexelint/
Nu har jeg ikke så meget erfaring med C udvikling, men jeg har brugt Coverity Scan til mit open source Java project og der har den fundet et 2-3 rigtig fejl og 1-2 conercases, plus lidt import sjusk. Så de par timer det to at få sat op med integration til build pipeline var godt givet ud. Og da Coverity er kendt for statisk analyse af C kode så burde der jo også være noget at hente der?
Eksemple med Travis-CI og Coverity scan kan ses her: https://github.com/tlbdk/copybook4java
Peter, du er ikke spor hysterisk. Her er vores C++ compiler flag:
Linux gcc: -std=c++11 -march=core2 -fno-common -pipe -Werror -Wall -Wextra -Wreorder -Wnon-virtual-dtor -Wpacked -Wcast-align -Wenum-compare -Wpointer-arith -Wunused -Wuninitialized -Winit-self -Winvalid-pch -Woverlength-strings -Woverloaded-virtual -Woverflow -Wno-unknown-pragmas -Wpacked-bitfield-compat -Wlogical-op -Wsync-nand -Wstrict-null-sentinel -Wnoexcept
Visual Studio 2013/2015: /Zc:inline /Zc:rvalueCast /volatile:iso /Zo /MP /WX /arch:SSE2 /nologo /Zm200 /W4
Derudover bygger vi også med clang og vi har cppcheck og clang-tidy på vores CI system til yderligere statiske checks. Vi har også længe snakket om at prøve Coverity og/eller PVS-studio.
Vi har brugt -Wall -Werror i mange år, mens yderligere statisk analyse værktøjer kom på bordet efter at have læst om Carmacks erfaringer i 2011:
http://www.gamasutra.com/view/news/128836/InDepth_Static_Code_Analysis.php
Så vidt jeg ved er den oprindeligt baseret på Kai C/C++ compiler, som de købte tilbage i 2000.
Intels C++ compilere bruger EDG (Edison Design Group - www.edg.com - https://en.wikipedia.org/wiki/Edison_Design_Group) som front-end. Den bliver brugt i en række sammenhænge - bl.a. til code completion i Microsoft Visual C++ IDE'et.
EDG havde i mange år den mest komplette implementering af C++. De var bl.a. de første (og eneste ?) som implementerede "export" - på trods af de havde været imod indførelsen af det i standarden. Det er god stil.
Det lyder ikke hysteriskt - det lyder mere som almindelig, håndværksmæssig forsvarlig opførsel. Det burde ikke give anledning til surhed fra kollegaer at have sådan en politik og hjælpe projektet til at komme i en tilstand, hvor det er praktisk muligt. Jeg mener det er et organisatorisk problem hvis det ikke var sådan.
Godt indlæg! Jeg har også presset "hysteriet" ned over vores software-afdeling. Her var der dog en stor gammel kodebase, som vi i første omgang lader køre videre og så fremadrettet kræver at alt compilerer rent. Hertil er "GCC Diagnostic pragma" (https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html) nyttigt så man selektivt kan enable/disable diagnostik på diverse filer.
Få at finde aliasing bugs er du nødt til at compilere med -O2.
Jeg er lidt overrasket over at der ikke er nogen af læserne som kryber til korset og indrømmer; jeg har koden, som slet ikke er clean og bliver det heller aldrig...
Jeg er skiftet til Rust...
Men ellers har vi vel alle været nødt til at arbejde på kodebaser som ikke er vores egne og hvor det vil være uoverkommeligt at rydde op i alt. Specielt hvis man bare skal lave en mindre rettelse.
I de flest større projekter jeg har arbejdet på har vi haft en politik om at oversætte med WallWerror, men det skal siges at det er en del arbejde. Hver gang der kommer en ny compiler version er der nye warnings der skal håndteres eller white-listes. Og der skal ofte white-listes en hel del warnings i moderne compilere. Den jeg rammer oftest nu om dage med GCC, er en rigtigt smart logik der regner ud om en 'int' på et givent tidspunkt kun kan være positiv og så giver en "hjælpsom" warning hvis man laver et mindre-end-nul check, desværre får man den også på mindre-end-nul checks i inline functions som kan kaldes fra flere steder og generelt giver mening, også selvom compileren nu er så klog at den kan regne ud når den ikke gør.
Nå ja, og standard flag: -Wall -Werror -Wextra
Wextra slår ekstra warnings til som Wall ikke inkluderer.
At man skal tilføje så meget ekstra til sin compiler, for at fange fejl og dårligdomme. Dvs. default er at man slipper afsted med dårligt håndværk/sjusk.
Nå et spørgsmål til Peter. I fik gjort rent, men fik I også rydtet op? Oftests synes jeg at når kode lider af enormt meget sjusk og dårligt håndværk, så lider det generelle design og kodens struktur i al almindelighed enormt, og der skal virkelig tages fat (eller måske nogen gange smides ud).
Hvad med automatiserede tests, var der nogen af dem, og kunne de bistå jer i arbejdet? Var i nødt til at skrive nogen, inden i gik i gang for at kunne verificere jeres rengøring/oprydning ikke havde ødelagt noget?
valgrind kan også finde mange fejl: valgrind ./din_binary
