Veselá úložka ze Dne firem na MFF

Dnes se na matfyzu konal Den firem. SUSE tam jako obvykle mělo stánek a mimo toho, že jsme nabízeli práci, dělali reklamu naší hackovací soutěži Po drátě a rozdávali korálky domorodcům, uspořádali jsme taky soutěž v řešení úložek o susí trička, plecháčky a jednoho obřího Tuxe. Měli jsme tam například roztomilý strace, jednu značně podezřelou transformaci, něco shellových podlostí... a zejména fůru rozbitého céčkového kódu.

Ten následující sem věším, protože jsem to slíbila kolegům vystavovatelům z Lundegaardu. Je v něm asi sedm chyb, o kterých vím (jedna "zajímavá," kvůli které úloha vznikla a spousta dalších maličkostí), minimálně jedna, o které jsem nevěděla... tak si to třeba můžete zkusit vyřešit a nalezené chyby nahlásit do komentářů. Příjemnou zábavu!

#include <stdio.h>
#include <stdlib.h>
#include <signal.h>
#include <sys/select.h>
#include <sys/socket.h>
#include <sys/wait.h>

char sigchld_arrived;

void
handler(int s)
{
	sigchld_arrived = 1;
}

struct sigaction act = { .sa_handler = handler };

int
main(void)
{
	int status, retval;
	int fds[2];
	fd_set rfds;
	
	if (socketpair(AF_UNIX, SOCK_STREAM, 0, fds) < 0)
		exit(1);
	FD_ZERO(&rfds);
	FD_SET(0, &rfds);
	FD_SET(fds[0], &rfds);
	sigaction(SIGCHLD, &act, NULL);
	for (;;) {
    		if (sigchld_arrived){
			sigchld_arrived = 0;
			pid_t pid;
			time_t t = time();
        		while ((pid = waitpid(-1, &status, WNOHANG)) >= 0)
				printf("%d: Child %d exited\n", t, pid);
        		continue;
      		}
		retval = select(2, &rfds, NULL, NULL);
		if (retval == -1)
			perror("select()");
		else if (retval){
			/* Zde je vynechan kus kodu, ktery pracuje s daty 
			a obcas zavola fork(). Zalozene procesy obcas 
			zapisi do fds[1] */
		}
	}
	return 0;
}
Re: Veselá úložka ze Dne firem na MFF Ondrej SanTiago Zajicek (21. 11. 2008 - 12:29) Sbalit(2)
Tak namatkou:

1) sigchld_arrived by mel byt volatile

2) je tam race condition mezi testem na signal a selectem.

3) select muze skoncit s chybou EINTR, coz by bylo dobre osetrit jinak nez ostatni chyby (i kdyz diky vnejsimu cyklu to je v podstate v poradku).

4) pred novym spustenim selectu by to chtelo znovu nastavit rfds.
Re: Veselá úložka ze Dne firem na MFF Michal Kára (21. 11. 2008 - 18:18) Sbalit(1)
5) Netestuje se vysledek sigaction()

6) Prvni parametr do select() (nfds) se nastavuje spatne (mel by byt fds[0]+1).

7) Volani select() nejak chybi parametr timeout, ale je mozne ze nekde je makro / verze kde se davat nemusi...
Re: Veselá úložka ze Dne firem na MFF Yenya (24. 11. 2008 - 10:22) Sbalit(5)
8) sigchld_arrived se nikde neinicializuje.
9) ma-li byt std. vystup korektni, asi by waitpid mel mit i WUNTRACED.
10) dalsi polozky act se nikde neinicializuji (zejmena sa_flags a pripadny SA_ONESHOT ktery tam byt nesmi a taky SA_NOCLDSTOP ktery by tam naopak byt mel, pokud neni WUNTRACED ve waitpid).

Tim 1) volatile si nejsem prilis jisty - po volani externi funkce (select()) musi stejne kompilator ten sigchld_arrived nechat nacist znovu (zvlast kdyz neni static).

Tak by me zajimalo, ktera ta chyba je "zajimava".

-Yenya
Re: Veselá úložka ze Dne firem na MFF mj (24. 11. 2008 - 10:33) Sbalit(4)
> 8) sigchld_arrived se nikde neinicializuje.

Je to staticka promenna, takze je zaruceno, ze je pri startu programu
nulova.

> 9) ma-li byt std. vystup korektni, asi by waitpid mel mit i WUNTRACED.

Procpak? Std. vystup oznamuje, ktere procesy skoncily, mezi coz jejich
pozastaveni logicky moc nepatri.

> 10) dalsi polozky act se nikde neinicializuji

Inicializatory v Cecku zarucuji, ze explicitne neuvedene polozky se
nuluji (aspon podle C99, co o tom rikaji starsi normy, zpameti nevim,
ale pochybuji, ze by se lisily a nebylo to v C99 explicitne zmineno).

> Tim 1) volatile si nejsem prilis jisty - po volani externi funkce
> (select()) musi stejne kompilator ten sigchld_arrived nechat nacist
> znovu (zvlast kdyz neni static).

Prekladac ale muze vedet, ze na sigchld_arrived neexistuje zadna
reference z ostatnich modulu, ani se na nej nikde nepredava pointer,
takze ho ani select() nemuze zmenit.

> Tak by me zajimalo, ktera ta chyba je "zajimava".

Puvodni chyba, kvuli ktere ulozka vznikla, je myslim (2). Ale kdyz ji
Anicka psala a ja ji koukal pres rameno, tu a tam jsme pridali nejakou
dalsi :-)

Jinak (1)-(7) jsou opravdove (byt (5) mi prijde dosti nepodstatna,
protoze snad ani neexistuje stav, kdy by system jeste byl rozumne
zivy a tento syscall selhal). Jeste vim o dvou :-)

Re: Veselá úložka ze Dne firem na MFF Yenya (24. 11. 2008 - 10:49) Sbalit(2)
Echh, mam zafixovany presne opacny vyznam WUNTRACED, to bych se zase nekdy divil...

ad 1) podle me to prekladac nemuze vedet - maximalne tak linker.
Re: Veselá úložka ze Dne firem na MFF mj (24. 11. 2008 - 10:50) Sbalit(1)
> ad 1) podle me to prekladac nemuze vedet - maximalne tak linker.

Ono je to u globalne optimalizujicich prekladacu casto tak, ze to, co se
vola jako linker, je ve skutecnosti dalsi cast prekladace -- treba SGI C,
pokud si dobre pamatuji, melo v .o mezikod a az linker to kompiloval
do strojaku :)

Re: Veselá úložka ze Dne firem na MFF anicka (24. 11. 2008 - 11:16) Sbalit(1)
> Jinak (1)-(7) jsou opravdove (byt (5) mi prijde dosti nepodstatna,
> protoze snad ani neexistuje stav, kdy by system jeste byl rozumne
> zivy a tento syscall selhal). Jeste vim o dvou :-)

Jojo, taky vím ještě o dvou. Z čehož u té jedné mě dost překvapuje, že
to ještě nikdo neřekl, když je tady taková diskuse kolem toho volatile
:-)

Re: Veselá úložka ze Dne firem na MFF BLEK. (28. 11. 2008 - 7:17) Sbalit(1)
Další bugy:
11) printf("%d: Child %d exited\n", t, pid);
nemusí být nutně typu int. t je typu time_t, pid je typu pid_t.

12) waitpid vrátí 0 když má děti, které ještě neskončily.

13) time() -> time(NULL)

14) pokud tomu signálu nedáš SA_RESTART, tak ti bude moct vracet chybu EINTR všechno možné --- např. i ten waitpid nebo printf.

Pokud signálu SA_RESTART dáš, tak je přerušení selectu signálem nekonzistentní (na Linuxu to přeruší, na Irixu ne). Já to vyřešil pomocí pipy, do které ten signál zapíše a tím select probudí. Další možné řešení je siglongjmp.

BTW. tu bugu - race condition se signálem a select se mi v první verzi Linksu podařilo vyrobit taky.