From d242706f3b739bcb00de1d970a01a319db6ad77d Mon Sep 17 00:00:00 2001 From: grischka Date: Tue, 9 May 2017 18:10:02 +0200 Subject: [PATCH] bitfields: one more hack and a "-Wgcc-compat" switch bit_pos + bit_size > type_size * 8 must NEVER happen because the code generator can read/write only the basic integral types. Warn if tcc has to break GCC compatibility for that reason and if -Wgcc-compat is given. Example: struct __attribute__((packed)) _s { unsigned int x : 12; unsigned char y : 7; unsigned int z : 28; }; Expected (GCC) layout (sizeof struct = 6) .xxxxxxxx.xxxxyyyy.yyyzzzzz.zzzzzzzz.zzzzzzzz.zzzzzzz0. But we cannot read/write 'char y'from 2 bytes in memory. So we have to adjust: .xxxxxxxx.xxxx0000.yyyyyyyz.zzzzzzzz.zzzzzzzz.zzzzzzzz.zzz00000 Now 'int z' cannot be accessed from 5 bytes. So we arrive at this (sizeof struct = 7): .xxxxxxxx.xxxx0000.yyyyyyy0.zzzzzzzz.zzzzzzzz.zzzzzzzz.zzzz0000 Otherwise the bitfield load/store generator needs to be changed to allow byte-wise accesses. Also we may touch memory past the struct in some cases currently. The patch adds a warning for that too. 0: 55 push %ebp 1: 89 e5 mov %esp,%ebp 3: 81 ec 04 00 00 00 sub $0x4,%esp 9: 90 nop struct __attribute__((packed)) { unsigned x : 5; } b = {0} ; a: 8b 45 ff mov -0x1(%ebp),%eax d: 83 e0 e0 and $0xffffffe0,%eax 10: 89 45 ff mov %eax,-0x1(%ebp) This touches -0x1 ... +0x3(%ebp), hence 3 bytes beyond stack space. Since the data is not changed, nothing else happens here. --- libtcc.c | 1 + tcc.h | 1 + tccgen.c | 25 +++++++++++++++++++------ 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/libtcc.c b/libtcc.c index 331baf3..89e75fa 100644 --- a/libtcc.c +++ b/libtcc.c @@ -1578,6 +1578,7 @@ static const FlagDef options_W[] = { { offsetof(TCCState, warn_unsupported), 0, "unsupported" }, { offsetof(TCCState, warn_write_strings), 0, "write-strings" }, { offsetof(TCCState, warn_error), 0, "error" }, + { offsetof(TCCState, warn_gcc_compat), 0, "gcc-compat" }, { offsetof(TCCState, warn_implicit_function_declaration), WD_ALL, "implicit-function-declaration" }, { 0, 0, NULL } diff --git a/tcc.h b/tcc.h index 6627618..5f7bd10 100644 --- a/tcc.h +++ b/tcc.h @@ -660,6 +660,7 @@ struct TCCState { int warn_error; int warn_none; int warn_implicit_function_declaration; + int warn_gcc_compat; /* compile with debug symbol (and use them if error during execution) */ int do_debug; diff --git a/tccgen.c b/tccgen.c index 36c172a..e1017f5 100644 --- a/tccgen.c +++ b/tccgen.c @@ -3285,6 +3285,9 @@ static void struct_layout(CType *type, AttributeDef *ad) { int align, maxalign, offset, c, bit_pos, bt, prevbt, prev_bit_size; int pcc = !tcc_state->ms_bitfields; + int packwarn = tcc_state->warn_gcc_compat; + int typealign, bit_size, size; + Sym *f; if (ad->a.aligned) maxalign = 1 << (ad->a.aligned - 1); @@ -3295,9 +3298,10 @@ static void struct_layout(CType *type, AttributeDef *ad) bit_pos = 0; prevbt = VT_STRUCT; /* make it never match */ prev_bit_size = 0; + size = 0; + for (f = type->ref->next; f; f = f->next) { - int typealign, bit_size; - int size = type_size(&f->type, &typealign); + size = type_size(&f->type, &typealign); if (f->type.t & VT_BITFIELD) bit_size = (f->type.t >> (VT_STRUCT_SHIFT + 6)) & 0x3f; else @@ -3358,13 +3362,20 @@ static void struct_layout(CType *type, AttributeDef *ad) int ofs = (c * 8 + bit_pos) % (typealign * 8); int ofs2 = ofs + bit_size + (typealign * 8) - 1; if (bit_size == 0 || - ((typealign != 1 || size == 1) && + (typealign != 1 && (ofs2 / (typealign * 8)) > (size/typealign))) { c = (c + ((bit_pos + 7) >> 3) + typealign - 1) & -typealign; bit_pos = 0; } else if (bit_pos + bit_size > size * 8) { c += bit_pos >> 3; bit_pos &= 7; + if (bit_pos + bit_size > size * 8) { + c += 1, bit_pos = 0; + if ((ad->a.packed || f->r) && packwarn) { + tcc_warning("struct layout not compatible with GCC (internal limitation)"); + packwarn = 0; + } + } } offset = c; /* In PCC layout named bit-fields influence the alignment @@ -3407,8 +3418,8 @@ static void struct_layout(CType *type, AttributeDef *ad) if (align > maxalign) maxalign = align; #if 0 - printf("set field %s offset=%d c=%d", - get_tok_str(f->v & ~SYM_FIELD, NULL), offset, c); + printf("set field %s offset=%d", + get_tok_str(f->v & ~SYM_FIELD, NULL), offset); if (f->type.t & VT_BITFIELD) { printf(" pos=%d size=%d", (f->type.t >> VT_STRUCT_SHIFT) & 0x3f, @@ -3458,6 +3469,8 @@ static void struct_layout(CType *type, AttributeDef *ad) type->ref->c = (c + (pcc ? (bit_pos + 7) >> 3 : 0) + maxalign - 1) & -maxalign; type->ref->r = maxalign; + if (offset + size > type->ref->c) + tcc_warning("will touch memory past end of the struct (internal limitation)"); } /* enum/struct/union declaration. u is either VT_ENUM or VT_STRUCT */ @@ -3615,7 +3628,7 @@ static void struct_decl(CType *type, AttributeDef *ad, int u) } else if (ad1.a.packed || ad->a.packed) { alignoverride = 1; } else if (*tcc_state->pack_stack_ptr) { - if (align > *tcc_state->pack_stack_ptr) + if (align >= *tcc_state->pack_stack_ptr) alignoverride = *tcc_state->pack_stack_ptr; } if (bit_size >= 0) {