On 2/4/21 8:45 AM, Alex Bennée wrote: > > Richard Henderson <richard.hender...@linaro.org> writes: > >> On 2/4/21 5:01 AM, Alex Bennée wrote: >>> >>> Richard Henderson <richard.hender...@linaro.org> writes: >>> >>>> The use in tcg_tb_lookup is given a random pc that comes from the pc >>>> of a signal handler. Do not assert that the pointer is already within >>>> the code gen buffer at all, much less the writable mirror of it. >>> >>> Surely we are asserting that - or at least you can find a rt entry for >>> the pointer passed (which we always expect to work)? >> >> What? No. The pointer could be anything at all, depending on any other bug >> within qemu. > > But you do assert it: > > struct tcg_region_tree *rt = tc_ptr_to_region_tree(tb->tc.ptr); > > g_assert(rt != NULL); > > and rt is only NULL when it's !in_code_gen_buffer(p). > > In tcg_tb_lookup you haven't removed an assert - you just ensure you > don't fail if it's not.
I see your confusion. The arbitrary pointer is the one that is presented to tcg_tb_lookup, and passed on to tc_ptr_to_region_tree. The (dynamic) assert has been removed by not calling tcg_splitwx_to_rw in tc_ptr_to_region_tree, as called from tcg_tb_lookup. But tcg_tb_insert and tcg_tb_remove do not receive arbitrary pointers -- they always get a TranslationBlock. So I retain an assert along those paths. (As opposed to SEGV on the next lines, I suppose.) Suggestions on how to reword the commit? Just include most of the above? r~
