* [Qemu-devel] [PULL 1/4] tcg: Diagnose referenced labels that have not been emitted
2019-02-11 17:01 [Qemu-devel] [PULL 0/4] tcg queued patches Richard Henderson
@ 2019-02-11 17:01 ` Richard Henderson
2019-02-11 17:01 ` [Qemu-devel] [PULL 2/4] tcg/i386: fix unsigned vector saturating arithmetic Richard Henderson
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2019-02-11 17:01 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
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 <code@rkapl.cz>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
tcg/tcg-op.h | 1 +
tcg/tcg.h | 12 +++++++++---
tcg/tcg.c | 23 +++++++++++++++++++++++
3 files changed, 33 insertions(+), 3 deletions(-)
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)
*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
--
2.17.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PULL 2/4] tcg/i386: fix unsigned vector saturating arithmetic
2019-02-11 17:01 [Qemu-devel] [PULL 0/4] tcg queued patches Richard Henderson
2019-02-11 17:01 ` [Qemu-devel] [PULL 1/4] tcg: Diagnose referenced labels that have not been emitted Richard Henderson
@ 2019-02-11 17:01 ` Richard Henderson
2019-02-11 17:01 ` [Qemu-devel] [PULL 3/4] exec-all: document that tlb_fill can trigger a TLB resize Richard Henderson
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2019-02-11 17:01 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Mark Cave-Ayland
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Due to a cut/paste error in the original implementation, the unsigned
vector saturating arithmetic was erroneously being calculated as signed
vector saturating arithmetic.
Fixes: 8ffafbcec2 ("tcg/i386: Implement vector saturating arithmetic")
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-Id: <20190207224258.426-1-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
tcg/i386/tcg-target.inc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c
index 4d84aea3a9..e0670e5098 100644
--- a/tcg/i386/tcg-target.inc.c
+++ b/tcg/i386/tcg-target.inc.c
@@ -2615,7 +2615,7 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
OPC_PADDSB, OPC_PADDSW, OPC_UD2, OPC_UD2
};
static int const usadd_insn[4] = {
- OPC_PADDSB, OPC_PADDSW, OPC_UD2, OPC_UD2
+ OPC_PADDUB, OPC_PADDUW, OPC_UD2, OPC_UD2
};
static int const sub_insn[4] = {
OPC_PSUBB, OPC_PSUBW, OPC_PSUBD, OPC_PSUBQ
@@ -2624,7 +2624,7 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
OPC_PSUBSB, OPC_PSUBSW, OPC_UD2, OPC_UD2
};
static int const ussub_insn[4] = {
- OPC_PSUBSB, OPC_PSUBSW, OPC_UD2, OPC_UD2
+ OPC_PSUBUB, OPC_PSUBUW, OPC_UD2, OPC_UD2
};
static int const mul_insn[4] = {
OPC_UD2, OPC_PMULLW, OPC_PMULLD, OPC_UD2
--
2.17.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PULL 4/4] cputlb: update TLB entry/index after tlb_fill
2019-02-11 17:01 [Qemu-devel] [PULL 0/4] tcg queued patches Richard Henderson
` (2 preceding siblings ...)
2019-02-11 17:01 ` [Qemu-devel] [PULL 3/4] exec-all: document that tlb_fill can trigger a TLB resize Richard Henderson
@ 2019-02-11 17:01 ` Richard Henderson
2019-02-11 18:53 ` [Qemu-devel] [PULL 0/4] tcg queued patches Peter Maydell
4 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2019-02-11 17:01 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Emilio G. Cota
From: "Emilio G. Cota" <cota@braap.org>
We are failing to take into account that tlb_fill() can cause a
TLB resize, which renders prior TLB entry pointers/indices stale.
Fix it by re-doing the TLB entry lookups immediately after tlb_fill.
Fixes: 86e1eff8bc ("tcg: introduce dynamic TLB sizing", 2019-01-28)
Reported-by: Max Filippov <jcmvbkbc@gmail.com>
Tested-by: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Emilio G. Cota <cota@braap.org>
Message-Id: <20190209162745.12668-3-cota@braap.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
accel/tcg/softmmu_template.h | 8 ++++++++
accel/tcg/cputlb.c | 4 ++++
2 files changed, 12 insertions(+)
diff --git a/accel/tcg/softmmu_template.h b/accel/tcg/softmmu_template.h
index 1fdd262ea4..e970a8b378 100644
--- a/accel/tcg/softmmu_template.h
+++ b/accel/tcg/softmmu_template.h
@@ -129,6 +129,8 @@ WORD_TYPE helper_le_ld_name(CPUArchState *env, target_ulong addr,
if (!VICTIM_TLB_HIT(ADDR_READ, addr)) {
tlb_fill(ENV_GET_CPU(env), addr, DATA_SIZE, READ_ACCESS_TYPE,
mmu_idx, retaddr);
+ index = tlb_index(env, mmu_idx, addr);
+ entry = tlb_entry(env, mmu_idx, addr);
}
tlb_addr = entry->ADDR_READ;
}
@@ -198,6 +200,8 @@ WORD_TYPE helper_be_ld_name(CPUArchState *env, target_ulong addr,
if (!VICTIM_TLB_HIT(ADDR_READ, addr)) {
tlb_fill(ENV_GET_CPU(env), addr, DATA_SIZE, READ_ACCESS_TYPE,
mmu_idx, retaddr);
+ index = tlb_index(env, mmu_idx, addr);
+ entry = tlb_entry(env, mmu_idx, addr);
}
tlb_addr = entry->ADDR_READ;
}
@@ -294,6 +298,8 @@ void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
if (!VICTIM_TLB_HIT(addr_write, addr)) {
tlb_fill(ENV_GET_CPU(env), addr, DATA_SIZE, MMU_DATA_STORE,
mmu_idx, retaddr);
+ index = tlb_index(env, mmu_idx, addr);
+ entry = tlb_entry(env, mmu_idx, addr);
}
tlb_addr = tlb_addr_write(entry) & ~TLB_INVALID_MASK;
}
@@ -372,6 +378,8 @@ void helper_be_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
if (!VICTIM_TLB_HIT(addr_write, addr)) {
tlb_fill(ENV_GET_CPU(env), addr, DATA_SIZE, MMU_DATA_STORE,
mmu_idx, retaddr);
+ index = tlb_index(env, mmu_idx, addr);
+ entry = tlb_entry(env, mmu_idx, addr);
}
tlb_addr = tlb_addr_write(entry) & ~TLB_INVALID_MASK;
}
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index f580e4dd7e..88cc8389e9 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1045,6 +1045,8 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr)
if (unlikely(!tlb_hit(entry->addr_code, addr))) {
if (!VICTIM_TLB_HIT(addr_code, addr)) {
tlb_fill(ENV_GET_CPU(env), addr, 0, MMU_INST_FETCH, mmu_idx, 0);
+ index = tlb_index(env, mmu_idx, addr);
+ entry = tlb_entry(env, mmu_idx, addr);
}
assert(tlb_hit(entry->addr_code, addr));
}
@@ -1125,6 +1127,8 @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
if (!VICTIM_TLB_HIT(addr_write, addr)) {
tlb_fill(ENV_GET_CPU(env), addr, 1 << s_bits, MMU_DATA_STORE,
mmu_idx, retaddr);
+ index = tlb_index(env, mmu_idx, addr);
+ tlbe = tlb_entry(env, mmu_idx, addr);
}
tlb_addr = tlb_addr_write(tlbe) & ~TLB_INVALID_MASK;
}
--
2.17.2
^ permalink raw reply related [flat|nested] 6+ messages in thread