ALT Linux Team development discussions
 help / color / mirror / Atom feed
From: "Kharitonov A. Dmitry" <kharpost@rambler.ru>
To: ALT Linux Team development discussions <devel@lists.altlinux.org>
Subject: Re: [devel] вопрос по языку Си - порядок вычисления операндов
Date: Thu, 01 Oct 2009 12:31:39 +0400
Message-ID: <4AC468EB.70603@rambler.ru> (raw)
In-Reply-To: <20091001012428.GL10769@altlinux.org>

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.



  parent reply	other threads:[~2009-10-01  8:31 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-01  1:24 Alexey Tourbin
2009-10-01  5:00 ` Damir Shayhutdinov
2009-10-01  5:02 ` Kirill A. Shutemov
2009-10-01  5:09   ` Kirill A. Shutemov
2009-10-01  8:31 ` Kharitonov A. Dmitry [this message]
2009-10-01  8:36   ` Kharitonov A. Dmitry
2009-10-01  9:16   ` Kirill A. Shutemov
2009-10-01  9:47     ` Kharitonov A. Dmitry
2009-10-01  9:51       ` Kirill A. Shutemov
2009-10-01 10:10         ` Kharitonov A. Dmitry
2009-10-01 10:14           ` Damir Shayhutdinov
2009-10-01 10:32             ` Kharitonov A. Dmitry
2009-10-01  9:30   ` Led

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4AC468EB.70603@rambler.ru \
    --to=kharpost@rambler.ru \
    --cc=devel@lists.altlinux.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

ALT Linux Team development discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://lore.altlinux.org/devel/0 devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 devel devel/ http://lore.altlinux.org/devel \
		devel@altlinux.org devel@altlinux.ru devel@lists.altlinux.org devel@lists.altlinux.ru devel@linux.iplabs.ru mandrake-russian@linuxteam.iplabs.ru sisyphus@linuxteam.iplabs.ru
	public-inbox-index devel

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://lore.altlinux.org/org.altlinux.lists.devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git