mirror of
http://git.haproxy.org/git/haproxy.git/
synced 2024-12-11 05:54:39 +00:00
855796bdc8
Ryan O'Hara reported that haproxy breaks on fedora-32 using gcc-10
(pre-release). It turns out that constructs such as:
while (item != head) {
item = LIST_ELEM(item.n);
}
loop forever, never matching <item> to <head> despite a printf there
showing them equal. In practice the problem is that the LIST_ELEM()
macro is wrong, it assigns the subtract of two pointers (an integer)
to another pointer through a cast to its pointer type. And GCC 10 now
considers that this cannot match a pointer and silently optimizes the
comparison away. A tested workaround for this is to build with
-fno-tree-pta. Note that older gcc versions even with -ftree-pta do
not exhibit this rather surprizing behavior.
This patch changes the test to instead cast the null-based address to
an int to get the offset and subtract it from the pointer, and this
time it works. There were just a few places to adjust. Ideally
offsetof() should be used but the LIST_ELEM() API doesn't make this
trivial as it's commonly called with a typeof(ptr) and not typeof(ptr*)
thus it would require to completely change the whole API, which is not
something workable in the short term, especially for a backport.
With this change, the emitted code is subtly different even on older
versions. A code size reduction of ~600 bytes and a total executable
size reduction of ~1kB are expected to be observed and should not be
taken as an anomaly. Typically this loop in dequeue_proxy_listeners() :
while ((listener = MT_LIST_POP(...)))
used to produce this code where the comparison is performed on RAX
while the new offset is assigned to RDI even though both are always
identical:
53ded8: 48 8d 78 c0 lea -0x40(%rax),%rdi
53dedc: 48 83 f8 40 cmp $0x40,%rax
53dee0: 74 39 je 53df1b <dequeue_proxy_listeners+0xab>
and now produces this one which is slightly more efficient as the
same register is used for both purposes:
53dd08: 48 83 ef 40 sub $0x40,%rdi
53dd0c: 74 2d je 53dd3b <dequeue_proxy_listeners+0x9b>
Similarly, retrieving the channel from a stream_interface using si_ic()
and si_oc() used to cause this (stream-int in rdi):
1cb7: c7 47 1c 00 02 00 00 movl $0x200,0x1c(%rdi)
1cbe: f6 47 04 10 testb $0x10,0x4(%rdi)
1cc2: 74 1c je 1ce0 <si_report_error+0x30>
1cc4: 48 81 ef 00 03 00 00 sub $0x300,%rdi
1ccb: 81 4f 10 00 08 00 00 orl $0x800,0x10(%rdi)
and now causes this:
1cb7: c7 47 1c 00 02 00 00 movl $0x200,0x1c(%rdi)
1cbe: f6 47 04 10 testb $0x10,0x4(%rdi)
1cc2: 74 1c je 1ce0 <si_report_error+0x30>
1cc4: 81 8f 10 fd ff ff 00 orl $0x800,-0x2f0(%rdi)
There is extremely little chance that this fix wakes up a dormant bug as
the emitted code effectively does what the source code intends.
This must be backported to all supported branches (dropping MT_LIST_ELEM
and the spoa_example parts as needed), since the bug is subtle and may
not always be visible even when compiling with gcc-10.
96 lines
3.8 KiB
C
96 lines
3.8 KiB
C
#ifndef _COMMON_MINI_CLIST_H
|
|
#define _COMMON_MINI_CLIST_H
|
|
|
|
/* these are circular or bidirectionnal lists only. Each list pointer points to
|
|
* another list pointer in a structure, and not the structure itself. The
|
|
* pointer to the next element MUST be the first one so that the list is easily
|
|
* cast as a single linked list or pointer.
|
|
*/
|
|
struct list {
|
|
struct list *n; /* next */
|
|
struct list *p; /* prev */
|
|
};
|
|
|
|
/* First undefine some macros which happen to also be defined on OpenBSD,
|
|
* in sys/queue.h, used by sys/event.h
|
|
*/
|
|
#undef LIST_HEAD
|
|
#undef LIST_INIT
|
|
#undef LIST_NEXT
|
|
|
|
/* ILH = Initialized List Head : used to prevent gcc from moving an empty
|
|
* list to BSS. Some older version tend to trim all the array and cause
|
|
* corruption.
|
|
*/
|
|
#define ILH { .n = (struct list *)1, .p = (struct list *)2 }
|
|
|
|
#define LIST_HEAD(a) ((void *)(&(a)))
|
|
|
|
#define LIST_INIT(l) ((l)->n = (l)->p = (l))
|
|
|
|
#define LIST_HEAD_INIT(l) { &l, &l }
|
|
|
|
/* adds an element at the beginning of a list ; returns the element */
|
|
#define LIST_ADD(lh, el) ({ (el)->n = (lh)->n; (el)->n->p = (lh)->n = (el); (el)->p = (lh); (el); })
|
|
|
|
/* adds an element at the end of a list ; returns the element */
|
|
#define LIST_ADDQ(lh, el) ({ (el)->p = (lh)->p; (el)->p->n = (lh)->p = (el); (el)->n = (lh); (el); })
|
|
|
|
/* removes an element from a list and returns it */
|
|
#define LIST_DEL(el) ({ typeof(el) __ret = (el); (el)->n->p = (el)->p; (el)->p->n = (el)->n; (__ret); })
|
|
|
|
/* returns a pointer of type <pt> to a structure containing a list head called
|
|
* <el> at address <lh>. Note that <lh> can be the result of a function or macro
|
|
* since it's used only once.
|
|
* Example: LIST_ELEM(cur_node->args.next, struct node *, args)
|
|
*/
|
|
#define LIST_ELEM(lh, pt, el) ((pt)(((const char *)(lh)) - ((size_t)&((pt)NULL)->el)))
|
|
|
|
/* checks if the list head <lh> is empty or not */
|
|
#define LIST_ISEMPTY(lh) ((lh)->n == (lh))
|
|
|
|
/* returns a pointer of type <pt> to a structure following the element
|
|
* which contains list head <lh>, which is known as element <el> in
|
|
* struct pt.
|
|
* Example: LIST_NEXT(args, struct node *, list)
|
|
*/
|
|
#define LIST_NEXT(lh, pt, el) (LIST_ELEM((lh)->n, pt, el))
|
|
|
|
|
|
/* returns a pointer of type <pt> to a structure preceding the element
|
|
* which contains list head <lh>, which is known as element <el> in
|
|
* struct pt.
|
|
*/
|
|
#undef LIST_PREV
|
|
#define LIST_PREV(lh, pt, el) (LIST_ELEM((lh)->p, pt, el))
|
|
|
|
/*
|
|
* Simpler FOREACH_ITEM macro inspired from Linux sources.
|
|
* Iterates <item> through a list of items of type "typeof(*item)" which are
|
|
* linked via a "struct list" member named <member>. A pointer to the head of
|
|
* the list is passed in <list_head>. No temporary variable is needed. Note
|
|
* that <item> must not be modified during the loop.
|
|
* Example: list_for_each_entry(cur_acl, known_acl, list) { ... };
|
|
*/
|
|
#define list_for_each_entry(item, list_head, member) \
|
|
for (item = LIST_ELEM((list_head)->n, typeof(item), member); \
|
|
&item->member != (list_head); \
|
|
item = LIST_ELEM(item->member.n, typeof(item), member))
|
|
|
|
/*
|
|
* Simpler FOREACH_ITEM_SAFE macro inspired from Linux sources.
|
|
* Iterates <item> through a list of items of type "typeof(*item)" which are
|
|
* linked via a "struct list" member named <member>. A pointer to the head of
|
|
* the list is passed in <list_head>. A temporary variable <back> of same type
|
|
* as <item> is needed so that <item> may safely be deleted if needed.
|
|
* Example: list_for_each_entry_safe(cur_acl, tmp, known_acl, list) { ... };
|
|
*/
|
|
#define list_for_each_entry_safe(item, back, list_head, member) \
|
|
for (item = LIST_ELEM((list_head)->n, typeof(item), member), \
|
|
back = LIST_ELEM(item->member.n, typeof(item), member); \
|
|
&item->member != (list_head); \
|
|
item = back, back = LIST_ELEM(back->member.n, typeof(back), member))
|
|
|
|
|
|
#endif /* _COMMON_MINI_CLIST_H */
|