From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on sa.int.altlinux.org X-Spam-Level: X-Spam-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.2.5 Message-ID: <4AC468EB.70603@rambler.ru> Date: Thu, 01 Oct 2009 12:31:39 +0400 From: "Kharitonov A. Dmitry" User-Agent: Thunderbird 2.0.0.21 (X11/20090323) MIME-Version: 1.0 To: ALT Linux Team development discussions References: <20091001012428.GL10769@altlinux.org> In-Reply-To: <20091001012428.GL10769@altlinux.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [devel] =?utf-8?b?0LLQvtC/0YDQvtGBINC/0L4g0Y/Qt9GL0LrRgyDQodC4?= =?utf-8?b?IC0g0L/QvtGA0Y/QtNC+0Log0LLRi9GH0LjRgdC70LXQvdC40Y8g0L7Qv9C1?= =?utf-8?b?0YDQsNC90LTQvtCy?= X-BeenThere: devel@lists.altlinux.org X-Mailman-Version: 2.1.12 Precedence: list Reply-To: ALT Linux Team development discussions List-Id: ALT Linux Team development discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Oct 2009 08:28:33 -0000 Archived-At: List-Archive: List-Post: Alexey Tourbin wrote: > У нас в rpm есть такой код. > > lib/depends.h: > 108 typedef /*@abstract@*/ struct availableList_s { > 109 /*@owned@*/ /*@null@*/ struct availablePackage * list; /*!< Set of packages. */ > 110 struct availableIndex index; /*!< Set of available items. */ > 111 int delta; /*!< Delta for pkg list reallocation. */ > 112 int size; /*!< No. of pkgs in list. */ > 113 int alloced; /*!< No. of pkgs allocated for list. */ > 114 int numDirs; /*!< No. of directories. */ > 115 /*@owned@*/ /*@null@*/ dirInfo dirs; /*!< Set of directories. */ > 116 } * availableList; > ... > > lib/depends.c: > 213 static /*@exposed@*/ struct availablePackage * > 214 alAddPackage(availableList al, > 215 Header h, /*@null@*/ /*@dependent@*/ const void * key, > 216 /*@null@*/ FD_t fd, /*@null@*/ rpmRelocation * relocs) > 217 /*@modifies al, h @*/ > 218 { > 219 HGE_t hge = (HGE_t)headerGetEntryMinMemory; > 220 HFD_t hfd = headerFreeData; > 221 rpmTagType dnt, bnt; > 222 struct availablePackage * p; > 223 rpmRelocation * r; > 224 int i; > 225 int_32 * dirIndexes; > 226 const char ** dirNames; > 227 int numDirs, dirNum; > 228 int * dirMapping; > 229 struct dirInfo_s dirNeedle; > 230 dirInfo dirMatch; > 231 int first, last, fileNum; > 232 int origNumDirs; > 233 int pkgNum; > 234 > 235 if (al->size == al->alloced) { > 236 al->alloced += al->delta; > 237 al->list = xrealloc(al->list, sizeof(*al->list) * al->alloced); > 238 } > ... > 703 int rpmtransAddPackage(rpmTransactionSet ts, Header h, FD_t fd, > 704 const void * key, int upgrade, rpmRelocation * relocs) > 705 { > 706 HGE_t hge = (HGE_t)headerGetEntryMinMemory; > 707 HFD_t hfd = headerFreeData; > 708 rpmTagType ont, ovt; > 709 /* this is an install followed by uninstalls */ > 710 const char * name; > 711 int count; > 712 const char ** obsoletes; > 713 int alNum; > 714 > 715 /* > 716 * FIXME: handling upgrades like this is *almost* okay. It doesn't > 717 * check to make sure we're upgrading to a newer version, and it > 718 * makes it difficult to generate a return code based on the number of > 719 * packages which failed. > 720 */ > 721 if (ts->orderCount == ts->orderAlloced) { > 722 ts->orderAlloced += ts->delta; > 723 ts->order = xrealloc(ts->order, sizeof(*ts->order) * ts->orderAlloced); > 724 } > 725 ts->order[ts->orderCount].type = TR_ADDED; > 726 if (ts->addedPackages.list == NULL) > 727 return 0; > 728 > 729 alNum = alAddPackage(&ts->addedPackages, h, key, fd, relocs) - > 730 ts->addedPackages.list; > 731 ts->order[ts->orderCount++].u.addedIndex = alNum; > ... > > Вопрос касается строк 729-730. Следите за движением рук. > > ts->addedPackages -- это некоторая структура данных, которая содержит > ts->addedPackages.list -- указатель на массив, выделяемый в куче. > Функция alAddPackage() добавляет новый элемент в этот массив и возвращает > на него указатель. Соответственно (по правилам типизированной адресной > арифметики) переменная alNum -- это будет индекс последнего добавленного > элемента в массиве. > > Но! Когда массив становится слишком маленьким, функция alAddPackage() > делает realloc этого массива (строка 237). Соотвественно, адрес массива > ts->addedPackages.list может измениться. > > Теперь посмотрим на вычитание указателей в строках 729-730. Левый > операнд вычитания может изменить ts->addedPackages.list, а правым > операндом является сам ts->addedPackages.list. Получается, что этот > код зависит от порядка вычисления операндов -- а именно, может > использоваться либо старое значение ts->addedPackages.list, либо > уже новое значение. > > Воопрос соответствено насколько легален этот код, и вообще любая > нетривиальная информация на эту тему. > Неоднократно натыкался на такое: исходное выражение а+b+c менялся порядок вычисления случайным образом то так а+(b+c) то так (а+b)+c то так (с+b)+a естественно получалась случайно работающая программа. Багу ловил 3 месяца. Во всех руководствах написано, что расстановка скобок принудительно должна обеспечивать порядок вычислений, но я опять таки натыкался на отступления и от этого. Разбивание выражения с применением промежуточной переменной однозначно определяло порядок, именно тот, который мне нужен. Промежуточная переменная в конечном коде в результате оптимизации исчезала. Так же трудно уловимый баг связан с подстановкой в аргументы функции скрытых временных особенно, если эти аргументы помеченны модификатором const.