Hi Richard,
On 2/8/19 4:55 AM, Richard Henderson wrote:
> Currently, a jump to a label that is not defined anywhere will
> be emitted not be relocated. This results in a jump to a random
> jump target. With tcg debugging, print a diagnostic to the -d op
> file and abort.
>
> This could help debug or detect errors like
> c2d9644e6d ("target/arm: Fix crash on conditional instruction in an IT block")
>
> Reported-by: Roman Kapl <c...@rkapl.cz>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> ---
> tcg/tcg-op.h | 1 +
> tcg/tcg.h | 12 +++++++++---
> tcg/tcg.c | 23 +++++++++++++++++++++++
> 3 files changed, 33 insertions(+), 3 deletions(-)
> ---
> This detects errors earlier than the patch that Roman posted.
>
>
> r~
>
>
> diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
> index 2d98868d8f..d3e51b15af 100644
> --- a/tcg/tcg-op.h
> +++ b/tcg/tcg-op.h
> @@ -255,6 +255,7 @@ static inline void tcg_gen_op6ii_i64(TCGOpcode opc,
> TCGv_i64 a1, TCGv_i64 a2,
>
> static inline void gen_set_label(TCGLabel *l)
> {
> + l->present = 1;
> tcg_gen_op1(INDEX_op_set_label, label_arg(l));
> }
>
> diff --git a/tcg/tcg.h b/tcg/tcg.h
> index 045c24a357..32b7cf3489 100644
> --- a/tcg/tcg.h
> +++ b/tcg/tcg.h
> @@ -244,16 +244,21 @@ typedef struct TCGRelocation {
> intptr_t addend;
> } TCGRelocation;
>
> -typedef struct TCGLabel {
> +typedef struct TCGLabel TCGLabel;
> +struct TCGLabel {
> + unsigned present : 1;
> unsigned has_value : 1;
> - unsigned id : 15;
> + unsigned id : 14;
> unsigned refs : 16;
> union {
> uintptr_t value;
> tcg_insn_unit *value_ptr;
> TCGRelocation *first_reloc;
> } u;
> -} TCGLabel;
> +#ifdef CONFIG_DEBUG_TCG
> + QSIMPLEQ_ENTRY(TCGLabel) next;
> +#endif
> +};
>
> typedef struct TCGPool {
> struct TCGPool *next;
> @@ -685,6 +690,7 @@ struct TCGContext {
> #endif
>
> #ifdef CONFIG_DEBUG_TCG
> + QSIMPLEQ_HEAD(, TCGLabel) labels;
> int temps_in_use;
> int goto_tb_issue_mask;
> #endif
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 20a5d8f315..9b2bf7f439 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -305,6 +305,9 @@ TCGLabel *gen_new_label(void)
Not related to this patch content, but I'm wondering why, TCGLabels
never get freed?
> *l = (TCGLabel){
> .id = s->nb_labels++
> };
> +#ifdef CONFIG_DEBUG_TCG
> + QSIMPLEQ_INSERT_TAIL(&s->labels, l, next);
> +#endif
>
> return l;
> }
> @@ -1092,6 +1095,9 @@ void tcg_func_start(TCGContext *s)
>
> QTAILQ_INIT(&s->ops);
> QTAILQ_INIT(&s->free_ops);
> +#ifdef CONFIG_DEBUG_TCG
> + QSIMPLEQ_INIT(&s->labels);
> +#endif
> }
>
> static inline TCGTemp *tcg_temp_alloc(TCGContext *s)
> @@ -3841,6 +3847,23 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
> }
> #endif
>
> +#ifdef CONFIG_DEBUG_TCG
> + /* Ensure all labels referenced have been emitted. */
> + {
> + TCGLabel *l;
> + bool error = false;
> +
> + QSIMPLEQ_FOREACH(l, &s->labels, next) {
> + if (unlikely(!l->present) && l->refs) {
> + qemu_log_mask(CPU_LOG_TB_OP,
> + "$L%d referenced but not present.\n", l->id);
> + error = true;
> + }
> + }
> + assert(!error);
> + }
> +#endif
> +
> #ifdef CONFIG_PROFILER
> atomic_set(&prof->opt_time, prof->opt_time - profile_getclock());
> #endif
>
Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
