From c1194ab7edbb17cb7371ac38e6eab5ae3ae72163 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Sun, 10 Apr 2022 19:31:50 +0200 Subject: [PATCH 1/2] Protocols use EA_LITERAL_* to set attributes --- lib/route.h | 11 ++++++++--- proto/babel/babel.c | 36 +++++++++++++----------------------- proto/bgp/attrs.c | 7 +++---- proto/ospf/rt.c | 40 +++++++++++++++++----------------------- proto/rip/rip.c | 24 ++++++------------------ 5 files changed, 47 insertions(+), 71 deletions(-) diff --git a/lib/route.h b/lib/route.h index 49810887..431da4d8 100644 --- a/lib/route.h +++ b/lib/route.h @@ -225,9 +225,14 @@ struct ea_one_attr_list { EA_LITERAL_GENERIC(_id, _type, _flags, .u.i = _val); \ }) -#define EA_LITERAL_ADATA(_id, _type, _flags, _buf, _len) ({ \ +#define EA_LITERAL_STORE_ADATA(_id, _type, _flags, _buf, _len) ({ \ ASSERT_DIE(!(_type & EAF_EMBEDDED)); \ - EA_LITERAL_GENERIC(_id, _type, _flags, .u.ad = tmp_store_adata(_buf, _len)); \ + EA_LITERAL_GENERIC(_id, _type, _flags, .u.ad = tmp_store_adata((_buf), (_len))); \ + }) + +#define EA_LITERAL_DIRECT_ADATA(_id, _type, _flags, _adata) ({ \ + ASSERT_DIE(!(_type & EAF_EMBEDDED)); \ + EA_LITERAL_GENERIC(_id, _type, _flags, .u.ad = _adata); \ }) #define EA_LITERAL_GENERIC(_id, _type, _flags, ...) \ @@ -261,7 +266,7 @@ ea_set_attr_u32(ea_list **to, uint id, uint flags, uint type, u32 data) static inline void ea_set_attr_data(ea_list **to, uint id, uint flags, uint type, void *data, uint len) -{ ea_set_attr(to, EA_LITERAL_ADATA(id, type, flags, data, len)); } +{ ea_set_attr(to, EA_LITERAL_STORE_ADATA(id, type, flags, data, len)); } #define NEXTHOP_MAX_SIZE (sizeof(struct nexthop) + sizeof(u32)*MPLS_MAX_LABEL_STACK) diff --git a/proto/babel/babel.c b/proto/babel/babel.c index a6cc7aa3..db710a0d 100644 --- a/proto/babel/babel.c +++ b/proto/babel/babel.c @@ -640,6 +640,18 @@ babel_announce_rte(struct babel_proto *p, struct babel_entry *e) if (r) { + struct { + ea_list l; + eattr a[3]; + } eattrs = { + .l.count = 3, + .a = { + EA_LITERAL_EMBEDDED(EA_BABEL_METRIC, T_INT, 0, r->metric), + EA_LITERAL_STORE_ADATA(EA_BABEL_ROUTER_ID, T_OPAQUE, 0, &r->router_id, sizeof(r->router_id)), + EA_LITERAL_EMBEDDED(EA_BABEL_SEQNO, T_INT, 0, r->seqno), + } + }; + rta a0 = { .source = RTS_BABEL, .scope = SCOPE_UNIVERSE, @@ -648,29 +660,7 @@ babel_announce_rte(struct babel_proto *p, struct babel_entry *e) .from = r->neigh->addr, .nh.gw = r->next_hop, .nh.iface = r->neigh->ifa->iface, - .eattrs = alloca(sizeof(ea_list) + 3*sizeof(eattr)), - }; - - *a0.eattrs = (ea_list) { .count = 3 }; - a0.eattrs->attrs[0] = (eattr) { - .id = EA_BABEL_METRIC, - .type = T_INT, - .u.data = r->metric, - }; - - struct adata *ad = alloca(sizeof(struct adata) + sizeof(u64)); - ad->length = sizeof(u64); - memcpy(ad->data, &(r->router_id), sizeof(u64)); - a0.eattrs->attrs[1] = (eattr) { - .id = EA_BABEL_ROUTER_ID, - .type = T_OPAQUE, - .u.ptr = ad, - }; - - a0.eattrs->attrs[2] = (eattr) { - .id = EA_BABEL_SEQNO, - .type = T_INT, - .u.data = r->seqno, + .eattrs = &eattrs.l, }; /* diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c index a7b6d0cd..4c67b161 100644 --- a/proto/bgp/attrs.c +++ b/proto/bgp/attrs.c @@ -94,13 +94,12 @@ void bgp_set_attr_u32(ea_list **to, uint code, uint flags, u32 val) void bgp_set_attr_ptr(ea_list **to, uint code, uint flags, const struct adata *ad) { ASSERT(bgp_attr_known(code)); - ASSERT_DIE(!(bgp_attr_table[code].type & EAF_EMBEDDED)); - ea_set_attr(to, EA_LITERAL_GENERIC( + ea_set_attr(to, EA_LITERAL_DIRECT_ADATA( EA_CODE(PROTOCOL_BGP, code), bgp_attr_table[code].type, flags & ~BAF_EXT_LEN, - .u.ad = ad + ad )); } @@ -109,7 +108,7 @@ bgp_set_attr_data(ea_list **to, uint code, uint flags, void *data, uint len) { ASSERT(bgp_attr_known(code)); - ea_set_attr(to, EA_LITERAL_ADATA( + ea_set_attr(to, EA_LITERAL_STORE_ADATA( EA_CODE(PROTOCOL_BGP, code), bgp_attr_table[code].type, flags & ~BAF_EXT_LEN, diff --git a/proto/ospf/rt.c b/proto/ospf/rt.c index 8643f456..5969b7c7 100644 --- a/proto/ospf/rt.c +++ b/proto/ospf/rt.c @@ -2062,39 +2062,33 @@ again1: if (reload || ort_changed(nf, &a0)) { - a0.eattrs = alloca(sizeof(ea_list) + 4 * sizeof(eattr)); - memset(a0.eattrs, 0, sizeof(ea_list)); - nf->old_metric1 = nf->n.metric1; nf->old_metric2 = nf->n.metric2; nf->old_tag = nf->n.tag; nf->old_rid = nf->n.rid; - a0.eattrs->attrs[a0.eattrs->count++] = (eattr) { - .id = EA_OSPF_METRIC1, - .type = T_INT, - .u.data = nf->n.metric1, - }; + struct { + ea_list l; + eattr a[4]; + } eattrs; + + eattrs.l = (ea_list) {}; + + eattrs.a[eattrs.l.count++] = + EA_LITERAL_EMBEDDED(EA_OSPF_METRIC1, T_INT, 0, nf->n.metric1); if (nf->n.type == RTS_OSPF_EXT2) - a0.eattrs->attrs[a0.eattrs->count++] = (eattr) { - .id = EA_OSPF_METRIC2, - .type = T_INT, - .u.data = nf->n.metric2, - }; + eattrs.a[eattrs.l.count++] = + EA_LITERAL_EMBEDDED(EA_OSPF_METRIC2, T_INT, 0, nf->n.metric2); if ((nf->n.type == RTS_OSPF_EXT1) || (nf->n.type == RTS_OSPF_EXT2)) - a0.eattrs->attrs[a0.eattrs->count++] = (eattr) { - .id = EA_OSPF_TAG, - .type = T_INT, - .u.data = nf->n.tag, - }; + eattrs.a[eattrs.l.count++] = + EA_LITERAL_EMBEDDED(EA_OSPF_TAG, T_INT, 0, nf->n.tag); - a0.eattrs->attrs[a0.eattrs->count++] = (eattr) { - .id = EA_OSPF_ROUTER_ID, - .type = T_QUAD, - .u.data = nf->n.rid, - }; + eattrs.a[eattrs.l.count++] = + EA_LITERAL_EMBEDDED(EA_OSPF_ROUTER_ID, T_QUAD, 0, nf->n.rid); + + a0.eattrs = &eattrs.l; rta *a = rta_lookup(&a0); rte *e = rte_get_temp(a, p->p.main_source); diff --git a/proto/rip/rip.c b/proto/rip/rip.c index fa5b1289..2b5babcb 100644 --- a/proto/rip/rip.c +++ b/proto/rip/rip.c @@ -195,26 +195,14 @@ rip_announce_rte(struct rip_proto *p, struct rip_entry *en) struct { ea_list l; - eattr e[3]; + eattr a[3]; struct rip_iface_adata riad; } ea_block = { - .l = { .count = 3, }, - .e = { - { - .id = EA_RIP_METRIC, - .type = T_INT, - .u.data = rt_metric, - }, - { - .id = EA_RIP_TAG, - .type = T_INT, - .u.data = rt_tag, - }, - { - .id = EA_RIP_FROM, - .type = T_IFACE, - .u.ptr = &ea_block.riad.ad, - } + .l.count = 3, + .a = { + EA_LITERAL_EMBEDDED(EA_RIP_METRIC, T_INT, 0, rt_metric), + EA_LITERAL_EMBEDDED(EA_RIP_TAG, T_INT, 0, rt_tag), + EA_LITERAL_DIRECT_ADATA(EA_RIP_FROM, T_IFACE, 0, &ea_block.riad.ad), }, .riad = { .ad = { .length = sizeof(struct rip_iface_adata) - sizeof(struct adata) }, From 1d309c4ce6e95b68c64a8f007f6dd2f1830a5707 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Thu, 14 Apr 2022 16:51:18 +0200 Subject: [PATCH 2/2] Enforcing certain data structure explicit paddings. Implicit paddings have undefined values in C. We want the eattr blocks to be comparable by memcmp and eattrs settable directly by structrure literals. This check ensures that all paddings in eattr and bval are explicit and therefore zeroed in all literals. --- aclocal.m4 | 26 +++++++++++++++++ configure.ac | 7 +++++ lib/Makefile | 2 +- lib/birdlib.h | 18 ++++++++++-- lib/route.h | 2 ++ lib/type.h | 12 ++++++-- lib/type_test.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++ nest/Makefile | 2 +- nest/bird.h | 1 - 9 files changed, 139 insertions(+), 9 deletions(-) create mode 100644 lib/type_test.c diff --git a/aclocal.m4 b/aclocal.m4 index 1613d680..58c48791 100644 --- a/aclocal.m4 +++ b/aclocal.m4 @@ -1,6 +1,32 @@ dnl ** Additional Autoconf tests for BIRD configure script dnl ** (c) 1999 Martin Mares +AC_DEFUN([BIRD_CHECK_POINTER_ALIGNMENT], +[ + AC_CACHE_CHECK( + [how pointers are aligned], + [bird_cv_pointer_alignment], + AC_COMPILE_IFELSE([ + AC_LANG_PROGRAM( + [ + _Static_assert(_Alignof(void *) == 8, "bad"); + ], [] + ) + ], + [bird_cv_pointer_alignment=8], + AC_COMPILE_IFELSE([ + AC_LANG_PROGRAM( + [ + _Static_assert(_Alignof(void *) == 4, "bad"); + ], [] + ) + ], + [bird_cv_pointer_alignment=4], + [bird_cv_pointer_alignment=unknown] + )) + ) +]) + AC_DEFUN([BIRD_CHECK_THREAD_LOCAL], [ AC_CACHE_CHECK( diff --git a/configure.ac b/configure.ac index 64181d29..321bed95 100644 --- a/configure.ac +++ b/configure.ac @@ -360,6 +360,13 @@ AC_C_BIGENDIAN( [AC_MSG_ERROR([Cannot determine CPU endianity.])] ) +BIRD_CHECK_POINTER_ALIGNMENT +if test "$bird_cv_pointer_alignment" = "unknown" ; then + AC_MSG_ERROR([Couldn't determine pointer alignment]) +else + AC_DEFINE_UNQUOTED([CPU_POINTER_ALIGNMENT], [$bird_cv_pointer_alignment], [Pointer alignment for macro usage]) +fi + BIRD_CHECK_ANDROID_GLOB if test "$bird_cv_lib_glob" = no ; then AC_MSG_ERROR([glob.h not found.]) diff --git a/lib/Makefile b/lib/Makefile index 853e0a22..15f757d9 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -2,6 +2,6 @@ src := a-path.c a-set.c bitmap.c bitops.c blake2s.c blake2b.c checksum.c event.c obj := $(src-o-files) $(all-daemon) -tests_src := a-set_test.c a-path_test.c bitmap_test.c heap_test.c buffer_test.c event_test.c flowspec_test.c bitops_test.c patmatch_test.c fletcher16_test.c slist_test.c checksum_test.c lists_test.c mac_test.c ip_test.c hash_test.c printf_test.c slab_test.c +tests_src := a-set_test.c a-path_test.c bitmap_test.c heap_test.c buffer_test.c event_test.c flowspec_test.c bitops_test.c patmatch_test.c fletcher16_test.c slist_test.c checksum_test.c lists_test.c mac_test.c ip_test.c hash_test.c printf_test.c slab_test.c type_test.c tests_targets := $(tests_targets) $(tests-target-files) tests_objs := $(tests_objs) $(src-o-files) diff --git a/lib/birdlib.h b/lib/birdlib.h index 6f0bab96..9b6e4a16 100644 --- a/lib/birdlib.h +++ b/lib/birdlib.h @@ -9,18 +9,30 @@ #ifndef _BIRD_BIRDLIB_H_ #define _BIRD_BIRDLIB_H_ +#include "sysdep/config.h" #include "lib/alloca.h" /* Ugly structure offset handling macros */ -struct align_probe { char x; long int y; }; - #define OFFSETOF(s, i) ((size_t) &((s *)0)->i) #define SKIP_BACK(s, i, p) ((s *)((char *)p - OFFSETOF(s, i))) #define BIRD_ALIGN(s, a) (((s)+a-1)&~(a-1)) -#define CPU_STRUCT_ALIGN (sizeof(struct align_probe)) +#define CPU_STRUCT_ALIGN (MAX_(_Alignof(void*), _Alignof(u64))) #define BIRD_CPU_ALIGN(s) BIRD_ALIGN((s), CPU_STRUCT_ALIGN) +/* Structure item alignment macros */ + +#define PADDING_NAME(id) _padding_##id +#define PADDING_(id, sz) u8 PADDING_NAME(id)[sz] + +#if CPU_POINTER_ALIGNMENT == 4 +#define PADDING(id, n32, n64) PADDING_(id, n32) +#elif CPU_POINTER_ALIGNMENT == 8 +#define PADDING(id, n32, n64) PADDING_(id, n64) +#else +#error "Strange CPU pointer alignment: " CPU_POINTER_ALIGNMENT +#endif + /* Utility macros */ #define MIN_(a,b) (((a)<(b))?(a):(b)) diff --git a/lib/route.h b/lib/route.h index 431da4d8..ba7bab61 100644 --- a/lib/route.h +++ b/lib/route.h @@ -144,6 +144,8 @@ typedef struct eattr { byte fresh:1; /* An uncached attribute (e.g. modified in export filter) */ byte undef:1; /* Explicitly undefined */ + PADDING(unused, 0, 4); + union bval u; } eattr; diff --git a/lib/type.h b/lib/type.h index 6d19a250..e43389f3 100644 --- a/lib/type.h +++ b/lib/type.h @@ -13,9 +13,15 @@ #include "lib/attrs.h" union bval { -#define BVAL_ITEMS \ - u32 data; /* Integer type inherited from eattrs */ \ - u32 i; /* Integer type inherited from filters */ \ +#define BVAL_ITEMS \ + struct { \ + u32 data; /* Integer type inherited from eattrs */ \ + PADDING(data, 0, 4); /* Must be padded on 64-bits */ \ + }; \ + struct { \ + u32 i; /* Integer type inherited from filters */ \ + PADDING(i, 0, 4); /* Must be padded on 64-bits */ \ + }; \ const struct adata *ptr; /* Generic attribute data inherited from eattrs */ \ const struct adata *ad; /* Generic attribute data inherited from filters */ \ diff --git a/lib/type_test.c b/lib/type_test.c new file mode 100644 index 00000000..20e7630b --- /dev/null +++ b/lib/type_test.c @@ -0,0 +1,78 @@ +/* + * BIRD Library -- Data Type Alignment Tests + * + * (c) 2022 Maria Matejka + * + * Can be freely distributed and used under the terms of the GNU GPL. + */ + +#include "test/birdtest.h" +#include "lib/type.h" +#include "lib/route.h" + +#define CHECK_ONE(val) \ + for (uint i=0; i $@ + $(Q)echo "#include \"lib/birdlib.h\"\n$(patsubst %,void %(void);\n,$(PROTO_BUILD)) void protos_build_gen(void) { $(patsubst %, %();\n,$(PROTO_BUILD))}" > $@ tests_src := tests_targets := $(tests_targets) $(tests-target-files) diff --git a/nest/bird.h b/nest/bird.h index 55712abe..931974a0 100644 --- a/nest/bird.h +++ b/nest/bird.h @@ -9,7 +9,6 @@ #ifndef _BIRD_BIRD_H_ #define _BIRD_BIRD_H_ -#include "sysdep/config.h" #include "lib/birdlib.h" #include "lib/ip.h" #include "lib/net.h"