Re: [Qemu-devel] [PATCH v2 6/7] scripts/dump-guest-memory.py: add vmcoreinfo

[Laszlo Ersek]

On 07/11/17 15:35, Marc-André Lureau wrote:
> Hi
> 
> ----- Original Message -----
>> On 07/11/17 12:04, Marc-André Lureau wrote:
>>> Hi
>>>
>>> ----- Original Message -----
>>>> On 07/06/17 12:16, Marc-André Lureau wrote:
>>>>> Add vmcoreinfo ELF note if vmcoreinfo device is ready.
>>>>>
>>>>> To help the python script, add a little global vmcoreinfo_gdb
>>>>> structure, that is populated with vmcoreinfo_gdb_update().
>>>>>
>>>>> Signed-off-by: Marc-André Lureau <address@hidden>
>>
>>>>> @@ -181,6 +183,7 @@ static void vmcoreinfo_realize(DeviceState *dev,
>>>>> Error
>>>>> **errp)
>>>>>          return;
>>>>>      }
>>>>>
>>>>> +    vmcoreinfo_gdb_helper = VMCOREINFO(dev);
>>>>>      qemu_register_reset(vmcoreinfo_handle_reset, dev);
>>>>>  }
>>>>>
>>>>>
>>>>
>>>> I guess we don't build QEMU with link-time optimization at the
>>>> moment.
>>>>
>>>> With link-time optimization, I think gcc might reasonably optimize
>>>> away the assignment to "vmcoreinfo_gdb_helper", and
>>>> "vmcoreinfo_gdb_helper" itself. This is why I suggested "volatile":
>>>>
>>>> static VMCoreInfoState * volatile vmcoreinfo_gdb_helper;
>>>>
>>>> Do you think volatile is only superfluous, or do you actively dislike
>>>> it for some reason?
>>>
>>> Yeah, I am not convinced volatile is the best way, but nor is static.
>>>
>>> Let's export it?
>>
>> Volatile guarantees that the assignment will take place according to the
>> behavior of the "abstract C machine" described by ISO C. From ISO C99,
>>
>>   6.7.3 Type qualifiers
>>
>>   6 An object that has volatile-qualified type may be modified in ways
>>     unknown to the implementation or have other unknown side effects.
>>     Therefore any expression referring to such an object shall be
>>     evaluated strictly according to the rules of the abstract machine,
>>     as described in 5.1.2.3. Furthermore, at every sequence point the
>>     value last stored in the object shall agree with that prescribed by
>>     the abstract machine, except as modified by the unknown factors
>>     mentioned previously. [116] What constitutes an access to an object
>>     that has volatile-qualified type is implementation-defined.
>>
>>   Footnote 116: A volatile declaration may be used to describe an object
>>                 corresponding to a memory-mapped input/output port or an
>>                 object accessed by an asynchronously interrupting
>>                 function. Actions on objects so declared shall not be
>>                 "optimized out" by an implementation or reordered except
>>                 as permitted by the rules for evaluating expressions.
>>
>> So basically if you can find the symbol somehow, it will always have the
>> right value, due to the volatile qualifier, regardless of the
>> optimizations the compiler did.
>>
>> Internal vs. external linkage ("static" vs. "extern") is a different
>> question; it might affect whether you find the symbol at all. IME,
>> symbols for objects with internal linkage are preserved, if: (a) you
>> don't strip the final binary, and (b) gcc doesn't optimize the variable
>> away.
>>
>> With link-time / full program optimization, gcc is entitled to optimize
>> away variables with external linkage too, so "extern" in itself is not
>> bulletproof. Volatile is more important.
> 
> Ok, you seem confident about this sort of things, I am not, I'll follow you :)
>>
> 
>> Given volatile, I'm sort of neutral on extern vs. static. However, if
>> you make the variable extern, that makes abuse from other source files
>> easier. (The variable should only be looked at with gdb.) This is also
>> why I originally suggested to limit the scope of even the static
>> variable to function scope -- this way abuse would completely be
>> prevented (nothing else could access the variable even from the same
>> translation unit), and gdb would still find the variable (IME).
> 
> How do you access a static inside a function? Gdb can look it up but 
> complains that it's not in current context.
> 

According to <https://sourceware.org/gdb/onlinedocs/gdb/Variables.html>,
the magic incantation is

  function::variable

If this doesn't work (in case the realize function isn't on the stack at
all), then I agree the variable should be made file scope.

Thanks
Laszlo