Re: [PATCH v2 04/93] tcg: Manage splitwx in tc_ptr_to_region_tree by hand

[Richard Henderson]

On 2/4/21 8:45 AM, Alex Bennée wrote:
> 
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> On 2/4/21 5:01 AM, Alex Bennée wrote:
>>>
>>> Richard Henderson <richard.henderson@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~