From 42e2a67f23448dc1ff6a23da650176ebad6e1681 Mon Sep 17 00:00:00 2001 From: Michael Matz Date: Tue, 20 Dec 2016 04:49:22 +0100 Subject: [PATCH] Fix some code suppression fallout Some more subtle issues with code suppression: - outputting asms but not their operand setup is broken - but global asms must always be output - statement expressions are transparent to code suppression - vtop can't be transformed from VT_CMP/VT_JMP when nocode_wanted Also remove .exe files from tests2 if they don't fail. --- tccasm.c | 32 +++++----- tccgen.c | 19 +++++- tests/tcctest.c | 32 ++++++++++ tests/tests2/85-asm-outside-function.c | 2 + tests/tests2/85-asm-outside-function.expect | 1 + tests/tests2/88_codeopt.c | 68 +++++++++++++++++++++ tests/tests2/88_codeopt.expect | 2 + tests/tests2/Makefile | 2 +- 8 files changed, 138 insertions(+), 20 deletions(-) create mode 100644 tests/tests2/88_codeopt.c create mode 100644 tests/tests2/88_codeopt.expect diff --git a/tccasm.c b/tccasm.c index ee368f4..67e9239 100644 --- a/tccasm.c +++ b/tccasm.c @@ -32,7 +32,7 @@ ST_FUNC int asm_get_local_label_name(TCCState *s1, unsigned int n) } ST_FUNC void asm_expr(TCCState *s1, ExprValue *pe); -static int tcc_assemble_internal(TCCState *s1, int do_preprocess); +static int tcc_assemble_internal(TCCState *s1, int do_preprocess, int global); static Sym sym_dot; /* Return a symbol we can use inside the assembler, having name NAME. @@ -462,7 +462,7 @@ static void pop_section(TCCState *s1) use_section1(s1, prev); } -static void asm_parse_directive(TCCState *s1) +static void asm_parse_directive(TCCState *s1, int global) { int n, offset, v, size, tok1; Section *sec; @@ -644,7 +644,8 @@ static void asm_parse_directive(TCCState *s1) save_parse_state(&saved_parse_state); begin_macro(init_str, 1); while (repeat-- > 0) { - tcc_assemble_internal(s1, (parse_flags & PARSE_FLAG_PREPROCESS)); + tcc_assemble_internal(s1, (parse_flags & PARSE_FLAG_PREPROCESS), + global); macro_ptr = init_str->str; } end_macro(); @@ -913,14 +914,10 @@ static void asm_parse_directive(TCCState *s1) /* assemble a file */ -static int tcc_assemble_internal(TCCState *s1, int do_preprocess) +static int tcc_assemble_internal(TCCState *s1, int do_preprocess, int global) { - int saved_nocode_wanted; int opcode; - saved_nocode_wanted = nocode_wanted; - nocode_wanted = 0; - /* XXX: undefine C labels */ ch = file->buf_ptr[0]; @@ -940,7 +937,7 @@ static int tcc_assemble_internal(TCCState *s1, int do_preprocess) while (tok != TOK_LINEFEED) next(); } else if (tok >= TOK_ASMDIR_FIRST && tok <= TOK_ASMDIR_LAST) { - asm_parse_directive(s1); + asm_parse_directive(s1, global); } else if (tok == TOK_PPNUM) { Sym *sym; const char *p; @@ -964,7 +961,7 @@ static int tcc_assemble_internal(TCCState *s1, int do_preprocess) /* handle "extern void vide(void); __asm__("vide: ret");" as "__asm__("globl vide\nvide: ret");" */ Sym *sym = sym_find(opcode); - if (sym && (sym->type.t & VT_EXTERN) && saved_nocode_wanted) { + if (sym && (sym->type.t & VT_EXTERN) && global) { sym = label_find(opcode); if (!sym) { sym = label_push(&s1->asm_labels, opcode, 0); @@ -993,7 +990,6 @@ static int tcc_assemble_internal(TCCState *s1, int do_preprocess) asm_free_labels(s1); - nocode_wanted = saved_nocode_wanted; return 0; } @@ -1017,7 +1013,7 @@ ST_FUNC int tcc_assemble(TCCState *s1, int do_preprocess) ELFW(ST_INFO)(STB_LOCAL, STT_FILE), 0, SHN_ABS, file->filename); - ret = tcc_assemble_internal(s1, do_preprocess); + ret = tcc_assemble_internal(s1, do_preprocess, 1); cur_text_section->data_offset = ind; @@ -1032,7 +1028,7 @@ ST_FUNC int tcc_assemble(TCCState *s1, int do_preprocess) /* assemble the string 'str' in the current C compilation unit without C preprocessing. NOTE: str is modified by modifying the '\0' at the end */ -static void tcc_assemble_inline(TCCState *s1, char *str, int len) +static void tcc_assemble_inline(TCCState *s1, char *str, int len, int global) { int saved_parse_flags; const int *saved_macro_ptr; @@ -1044,7 +1040,7 @@ static void tcc_assemble_inline(TCCState *s1, char *str, int len) memcpy(file->buffer, str, len); macro_ptr = NULL; - tcc_assemble_internal(s1, 0); + tcc_assemble_internal(s1, 0, global); tcc_close(); parse_flags = saved_parse_flags; @@ -1277,7 +1273,7 @@ ST_FUNC void asm_instr(void) clobber_regs, out_reg); /* assemble the string with tcc internal assembler */ - tcc_assemble_inline(tcc_state, astr1.data, astr1.size - 1); + tcc_assemble_inline(tcc_state, astr1.data, astr1.size - 1, 0); /* restore the current C token */ next(); @@ -1299,7 +1295,10 @@ ST_FUNC void asm_instr(void) ST_FUNC void asm_global_instr(void) { CString astr; + int saved_nocode_wanted = nocode_wanted; + /* Global asm blocks are always emitted. */ + nocode_wanted = 0; next(); parse_asm_str(&astr); skip(')'); @@ -1315,7 +1314,7 @@ ST_FUNC void asm_global_instr(void) ind = cur_text_section->data_offset; /* assemble the string with tcc internal assembler */ - tcc_assemble_inline(tcc_state, astr.data, astr.size - 1); + tcc_assemble_inline(tcc_state, astr.data, astr.size - 1, 1); cur_text_section->data_offset = ind; @@ -1323,5 +1322,6 @@ ST_FUNC void asm_global_instr(void) next(); cstr_free(&astr); + nocode_wanted = saved_nocode_wanted; } #endif /* CONFIG_TCC_ASM */ diff --git a/tccgen.c b/tccgen.c index 8281d1e..70e6fb6 100644 --- a/tccgen.c +++ b/tccgen.c @@ -575,8 +575,16 @@ static void vsetc(CType *type, int r, CValue *vc) tcc_error("memory full (vstack)"); /* cannot let cpu flags if other instruction are generated. Also avoid leaving VT_JMP anywhere except on the top of the stack - because it would complicate the code generator. */ - if (vtop >= vstack) { + because it would complicate the code generator. + + Don't do this when nocode_wanted. vtop might come from + !nocode_wanted regions (see 88_codeopt.c) and transforming + it to a register without actually generating code is wrong + as their value might still be used for real. All values + we push under nocode_wanted will eventually be popped + again, so that the VT_CMP/VT_JMP value will be in vtop + when code is unsuppressed again. */ + if (vtop >= vstack && !nocode_wanted) { v = vtop->r & VT_VALMASK; if (v == VT_CMP || (v & ~1) == VT_JMP) gv(RC_INT); @@ -4367,13 +4375,18 @@ ST_FUNC void unary(void) gen_cast(&type); } } else if (tok == '{') { + int saved_nocode_wanted = nocode_wanted; if (const_wanted) tcc_error("expected constant"); /* save all registers */ save_regs(0); /* statement expression : we do not accept break/continue - inside as GCC does */ + inside as GCC does. We do retain the nocode_wanted state, + as statement expressions can't ever be entered from the + outside, so any reactivation of code emission (from labels + or loop heads) can be disabled again after the end of it. */ block(NULL, NULL, 1); + nocode_wanted = saved_nocode_wanted; skip(')'); } else { gexpr(); diff --git a/tests/tcctest.c b/tests/tcctest.c index 2789255..6d5c4b4 100644 --- a/tests/tcctest.c +++ b/tests/tcctest.c @@ -1404,6 +1404,15 @@ void optimize_out(void) if (defined_function() && 0) refer_to_undefined(); + if (0) { + (void)sizeof( ({ + do { } while (0); + 0; + }) ); + undefined_function(); + } + + /* Leave the "if(1)return; printf()" in this order and last in the function */ if (1) return; printf ("oor:%d\n", undefined_function()); @@ -3251,6 +3260,28 @@ void trace_console(long len, long len2) } #endif } + +void test_asm_dead_code(void) +{ + long rdi; + /* Try to make sure that xdi contains a zero, and hence will + lead to a segfault if the next asm is evaluated without + arguments being set up. */ + asm volatile ("" : "=D" (rdi) : "0" (0)); + (void)sizeof (({ + int var; + /* This shouldn't trigger a segfault, either the argument + registers need to be set up and the asm emitted despite + this being in an unevaluated context, or both the argument + setup _and_ the asm emission need to be suppressed. The latter + is better. Disabling asm code gen when suppression is on + also fixes the above trace_console bug, but that came earlier + than asm suppression. */ + asm volatile ("movl $0,(%0)" : : "D" (&var) : "memory"); + var; + })); +} + void asm_test(void) { char buf[128]; @@ -3334,6 +3365,7 @@ void asm_test(void) printf ("regvar=%x\n", regvar); test_high_clobbers(); trace_console(8, 8); + test_asm_dead_code(); return; label1: goto label2; diff --git a/tests/tests2/85-asm-outside-function.c b/tests/tests2/85-asm-outside-function.c index 0aa7e33..dc5639a 100644 --- a/tests/tests2/85-asm-outside-function.c +++ b/tests/tests2/85-asm-outside-function.c @@ -1,7 +1,9 @@ +extern int printf (const char *, ...); extern void vide(void); __asm__("vide: ret"); int main() { vide(); + printf ("okay\n"); return 0; } diff --git a/tests/tests2/85-asm-outside-function.expect b/tests/tests2/85-asm-outside-function.expect index e69de29..dcf02b2 100644 --- a/tests/tests2/85-asm-outside-function.expect +++ b/tests/tests2/85-asm-outside-function.expect @@ -0,0 +1 @@ +okay diff --git a/tests/tests2/88_codeopt.c b/tests/tests2/88_codeopt.c new file mode 100644 index 0000000..647626f --- /dev/null +++ b/tests/tests2/88_codeopt.c @@ -0,0 +1,68 @@ +/* Check some way in where code suppression caused various + miscompilations. */ +extern int printf (const char *, ...); +typedef unsigned long size_t; + +size_t _brk_start, _brk_end; +void * extend_brk(size_t size, size_t align) +{ + size_t mask = align - 1; + void *ret = 0; + + do { + if (__builtin_expect(!!(_brk_start == 0), 0)) + do { + printf("wrong1\n"); + } while (0); + } while (0); + _brk_end = (_brk_end + mask) & ~mask; + ret = (void *)_brk_end; + _brk_end += size; + + return ret; +} + +static void get_args (int a, int b) +{ + if (a != 1) + printf("wrong2\n"); + else + printf("okay\n"); +} + +void bla(void) +{ + int __ret = 42; + ({ + if (__builtin_expect(!!(0), 0)) { + if (__builtin_expect(!!__ret, 0)) + printf("wrong3\n"); + int x = !!(__ret); + } + __ret; + }); + get_args(!!__ret, sizeof(__ret)); +} + +_Bool chk(unsigned long addr, unsigned long limit, unsigned long size) +{ + _Bool ret; + /* This just needs to compile, no runtime test. (And it doesn't compile + only with certain internal checking added that's not committed). */ + if (0) + ret = 0 != (!!(addr > limit - size)); +} + +int main() +{ + void *r; + _brk_start = 1024; + _brk_end = 1024; + r = extend_brk (4096, 16); + if (!r) + printf("wrong4\n"); + else + printf("okay\n"); + bla(); + return 0; +} diff --git a/tests/tests2/88_codeopt.expect b/tests/tests2/88_codeopt.expect new file mode 100644 index 0000000..439edfd --- /dev/null +++ b/tests/tests2/88_codeopt.expect @@ -0,0 +1,2 @@ +okay +okay diff --git a/tests/tests2/Makefile b/tests/tests2/Makefile index ababb43..b52b54a 100644 --- a/tests/tests2/Makefile +++ b/tests/tests2/Makefile @@ -59,7 +59,7 @@ all test: $(filter-out $(SKIP),$(TESTS)) $(TCC) $< -o ./$*.exe $(FILTER) 2>&1 && \ ./$*.exe $(ARGS) >$*.output 2>&1 || true; \ fi - @diff -Nbu $(SRC)/$*.expect $*.output && rm -f $*.output + @diff -Nbu $(SRC)/$*.expect $*.output && rm -f $*.output $*.exe # automatically generate .expect files with gcc: %.expect : %.c