Jaderné noviny – 20. 12. 2012: Umlčování GCC ne vždy přínosné (diskuse)

[Sten]

Za zkušenosti má kompilátor vždy pravdu, když varuje, i když tvrdí trochu něco jiného, než co je skutečný problém. Použití return ret ? ret : len; ukazuje na nepříliš promyšlenou strukturu kódu, protože zbytečně porovnávám ret na konci funkce, když už někde dříve muselo být jasné a musel jsem otestovat, že není nulové (a tedy jsem měl funkci ukončit už tam), protože jinak by len bylo inicializováno.

[kralyk z abclinuxu]

To není tak úplně jisté...

Jednak mám dojem, že dle C89 (s -pedantic) musíš mít deklarace proměnných na začátku scope. Ale nevim, podle jakýho standardu se kompiluje jádro.

No a jednak si umím představit i takovej kód, kde by to opravdu muselo být rozlišeno až na konci. Asi by to chtělo vidět ten konkrétní kód...

[Sten]

Jednak mám dojem, že dle C89 (s -pedantic) musíš mít deklarace proměnných na začátku scope. Ale nevim, podle jakýho standardu se kompiluje jádro.

To by nevadilo, to varování vyvolá až použití.

No a jednak si umím představit i takovej kód, kde by to opravdu muselo být rozlišeno až na konci. Asi by to chtělo vidět ten konkrétní kód...

Já si ho samozřejmě taky umí představit, to ale neznamená, že je strukturálně správně.

[Goheeca]

static int uhid_hid_get_raw(struct hid_device *hid, unsigned char rnum,
			    __u8 *buf, size_t count, unsigned char rtype)
{
	struct uhid_device *uhid = hid->driver_data;
	__u8 report_type;
	struct uhid_event *ev;
	unsigned long flags;
	int ret;
	size_t uninitialized_var(len);
	struct uhid_feature_answer_req *req;

	if (!uhid->running)
		return -EIO;

	switch (rtype) {
	case HID_FEATURE_REPORT:
		report_type = UHID_FEATURE_REPORT;
		break;
	case HID_OUTPUT_REPORT:
		report_type = UHID_OUTPUT_REPORT;
		break;
	case HID_INPUT_REPORT:
		report_type = UHID_INPUT_REPORT;
		break;
	default:
		return -EINVAL;
	}

	ret = mutex_lock_interruptible(&uhid->report_lock);
	if (ret)
		return ret;

	ev = kzalloc(sizeof(*ev), GFP_KERNEL);
	if (!ev) {
		ret = -ENOMEM;
		goto unlock;
	}

	spin_lock_irqsave(&uhid->qlock, flags);
	ev->type = UHID_FEATURE;
	ev->u.feature.id = atomic_inc_return(&uhid->report_id);
	ev->u.feature.rnum = rnum;
	ev->u.feature.rtype = report_type;

	atomic_set(&uhid->report_done, 0);
	uhid_queue(uhid, ev);
	spin_unlock_irqrestore(&uhid->qlock, flags);

	ret = wait_event_interruptible_timeout(uhid->report_wait,
				atomic_read(&uhid->report_done), 5 * HZ);

	/*
	 * Make sure "uhid->running" is cleared on shutdown before
	 * "uhid->report_done" is set.
	 */
	smp_rmb();
	if (!ret || !uhid->running) {
		ret = -EIO;
	} else if (ret < 0) {
		ret = -ERESTARTSYS;
	} else {
		spin_lock_irqsave(&uhid->qlock, flags);
		req = &uhid->report_buf.u.feature_answer;

		if (req->err) {
			ret = -EIO;
		} else {
			ret = 0;
			len = min(count,
				min_t(size_t, req->size, UHID_DATA_MAX));
			memcpy(buf, req->data, len);
		}

		spin_unlock_irqrestore(&uhid->qlock, flags);
	}

	atomic_set(&uhid->report_done, 1);

unlock:
	mutex_unlock(&uhid->report_lock);
	return ret ? ret : len;
}

Stačilo by přiřadit len do ret v té else větvi.

[kralyk z abclinuxu]

No to je teda prasečina... Nelíbí se mi na tom, že ret i celá ta funkce je int, kdežto len je size_t.

[Goheeca]

Jo to implicitní přetypování se mi taky nelíbí.

[j]

A nebylo by lepsi opravit ten kompilator?

[Ondrej 'SanTiago' Zajicek]

Jestli pod 'opravit' myslis upravit GCC tak, aby nehlasil to varovani tam, kde nemuze k problemu dojit, a hlasil tam, kde muze, tak to principielne neni mozne. At pouzijes sebelepsi algoritmus, tak vzdy jsou pripady, kdy nelze spolehlive ani potvrdit ani vyvratit chybu, a maximalne v takovych pripadech lze zvolit zda hlasit varovani (a riskovat false positive) nebo nehlasit (a riskovat false negative). Pritom ty priklady mohou byt treba prokazatelne korektni. Abych se nepoustel do konstrukci spojenych s vycislitelnosti, tak staci vzit v uvahu pripad, kdy chovani zavisi na nejakem vnejsim faktoru (napr. externi funkci), o niz kompilator nema zadne informace, ale programator ma.

[teoretik]

Opravit? U te analyzy inicializaci se v extremnich pripadech muze jednat o ekvivalent problemu zastaveni, tedy o neco algoritmicky neresitelneho. I kdyz mohl by to Honza Hubicka zadat jako semestralku studentum, treba mu to nekdo omylem vyresi... :)

[:::]

poslyste vy dva teoretici nademnou. Principielne resitelny to samozrejme je a s problemem zastaveni to nema nic spolecnyho.

Ale je pravda ze kvuli volani dalsich funkci nemusi mit kompilator vsechny potrebny informace a shanet je by zbytecne zpomalovalo preklad.

[petr_p]

Principielne resitelny to samozrejme je a s problemem zastaveni to nema nic spolecnyho.

Důkaz máte? :)

[teoretik]

Nejsem sam kdo si to mysli:

http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings <- "Unfortunately, detecting when the use of an uninitialized variable is equivalent, in the general case, to solving the halting problem."

[Grunt]

Co třeba inicializace na nějakou předefinovanou hodnotu nečíselného typu (podobně jako je NaN) kdy při libovolné operaci s touto hodnotou vyhodí procesor debugovací přerušení nebo si udělá zářez na křemík nebo tak něco? Bylo by to o moc blbější?

[Bluebear]

Ale co dělat u integerů?

Respektive, dalo by se přidat zvláštní flag "je-není inicializován". Bohužel myslím, že tohle tam dát, tak Linus vezme nunčaky a půjde po krvi toho, kdo ten patch do GCC poslal.

Ukazatele by samozřejmě šlo inicializovat na NULL, a osobně bych byl pro, ale spousta lidí by (právem) začala nadávat, že to je v drtivé většině případů zcela zbytečná instrukce a přístup do paměti.

[Grunt]

Ale co dělat u integerů?

To by se asi muselo implementovat na procesoru, kdy libovolná aritmetická operace s nějakým specifickým číslem (nebo číslem s flagem) by způsobila vyhození z běžného běhu.

Ukazatele by samozřejmě šlo inicializovat na NULL, a osobně bych byl pro, ale spousta lidí by (právem) začala nadávat, že to je v drtivé většině případů zcela zbytečná instrukce a přístup do paměti.

Tak třeba #ifdef DEBUG. Furt lepší kfree(NULL) než kfree(rand()).

[petr_p]

Pokud někdo potřebuje něco takového, tak nepoužije C, ale Adu. C má mnohem více záludnějších problémů ,jako ke třeba přetečení znaménkového integeru. Ostatně těšte se na GCC 4.8, které jde s optimalizacemi ještě více na dřeň specifikace, kdy například takovéto přiřazení do iterátoru povede k zacyklení programu.

[Grunt]

Pokud někdo potřebuje něco takového, tak nepoužije C, ale Adu.

Takže už jen stačí přepsat jádro z Céčka do Ady…

jako ke třeba přetečení znaménkového integeru

V tom problém nevidím. Když přeteče, tak se začne počítat znova od spodní hranice rozsahu, ne?

[Voty]

jako ke třeba přetečení znaménkového integeru

V tom problém nevidím. Když přeteče, tak se začne počítat znova od spodní hranice rozsahu, ne?

Dle normy je to nedefinovaná operace a tudíž se může stát cokoliv. Například v kódu:

int a, b;

...

if(a > 0 && b > 0)
{
    int c = a + b;
    
    if(c < 0)
    {
        printf("Overflow");
    }
}

může překladač usoudit, že při sčítání dvou kladných čísel nemůže nastat "legální" situace, kdy výsledek součtu bude záporný a Overflow se nikdy nevypíše.

[Grunt]

1. Furt nevidím problém
2. Tohle

   if(c < 0)
       {
           printf("Overflow");
       }

   mi moc validní nepřijde. Není k detekci přetečení určen overflow flag v SR?

[Voty]

Byla to jen ukázka, co se může stát (přišlo mi to lepší než příklad s auty :)

Každopádně jde o to, že u znaménkových integerů v jazyce C obecně neplatí (nelze se spolehnout) na to, že se začne počítat od spodní hranice rozsahu. A vyrobit podobnou chybu může být poměrně snadné, v příkladu je naznačen jeden možný (a zcela zřejmý) postup.

Norma C poměrně jasně říká, že pokud dojde k přetečení integeru, může se dále dít prakticky cokoliv a další chování programu je _zcela_ nedefinované. Mimojiné je tak překladači dovoleno jakékoliv testy na zjištění, že došlo k přetečení ignorovat a v rámci optimalizace je úplně vyhodit. Nehledě na to, že svět není jen x86 a některé arch overflow flag ani nemají.

[petr_p]

Doporučuji nečíst vlákno, proč bash vrací na $((2**63)) divný výstup. Mohl byste mít špatné spaní.