Re: [PATCH 2/3] gdb/varobj: Fix use after free in varobj

[Lancelot SIX <Lancelot.Six@amd.com>]

Hi,

Thanks for the feedbacks and spot-on comments.  I'll integrate them in a 
V2 shortly.

>> +gdb_exit
>> +if { [mi_gdb_start] } {
>> +    return 0
>> +}
> 
> Could/should the gdb_exit and mi_gdb_start calls be replaced with:
> 
>    if { [mi_clean_restart] } {
>      # Should we have an unsupported call in here maybe?
>      return
>    }
> 

Yes, it should probably.  I'll change this in the V2.

This is what happens when we create a new test by copying an existing one…

>> +    # Floating varobj should still be valid, but out of scope at the moment.
>> +    mi_gdb_test "-var-update floating_local" \
>> +     "\\^done,changelist=\\\[{name=\"floating_local\",in_scope=\"false\",type_changed=\"false\",has_more=\"0\"}\\\]" \
>> +     "floating_local still valid but not in scope"
> 
> For me, this test is failing, the output looks like:
> 
>    -var-update floating_local
>    ^done,changelist=[{name="floating_local",in_scope="invalid",has_more="0"}]
>    (gdb)
>    FAIL: gdb.mi/mi-var-invalidate-shlib.exp: in new process: floating_local still valid but not in scope (unexpected output)
> 
> But, once the next patch is applied, the test starts to pass.  So maybe
> this test just needs moving into the next patch?

Indeed, this part of the test should really be in the patch #2.  Thanks 
for spotting this.


> 
> I notice that non of the tests in either this patch, or the next one,
> exercise this condition.
> 
> Is it possible to create a test for this case?

Will do.  Issuing a "-var-update" after the dlclose call in the test 
should exercise this.  I did that manually, but forgot to include it in 
the testcaes.

Best,
Lancelot.