Re: [PATCH 08/65] xen: Annotate fnptr targets from acpi_table_parse()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.12.2021 15:44, Andrew Cooper wrote:
> On 06/12/2021 08:36, Jan Beulich wrote:
>> On 26.11.2021 13:33, Andrew Cooper wrote:
>>> --- a/xen/arch/x86/hvm/dom0_build.c
>>> +++ b/xen/arch/x86/hvm/dom0_build.c
>> Elsewhere in this file we have
>>
>>         rc = map ?   map_mmio_regions(d, _gfn(pfn), nr_pages, _mfn(pfn))
>>                  : unmap_mmio_regions(d, _gfn(pfn), nr_pages, _mfn(pfn));
>>
>> which has been in this shape only as of commit e3b418ac4919
>> ("x86/pvh-dom0: Remove unnecessary function pointer call from
>> modify_identity_mmio()"). Aren't we relying on the compiler not
>> transforming this back into the earlier
>>
>>         rc = (map ? map_mmio_regions : unmap_mmio_regions)
>>              (d, _gfn(pfn), nr_pages, _mfn(pfn));
>>
>> ?
> 
> That old code was especially dumb even before retpoline.  See also the
> damage caused by c/s 245a320ce2.

I must be lacking context here - what damage did that one cause again?
Or which subsequent fix of that damage am I overlooking when going
through the further commits on top of that one?

> Yes, we are relying on the compiler not to do transformations behind our
> backs, but it won't of its own accord.
> 
>>  And aren't we further relying on the compiler not transforming direct
>> calls into indirect ones for other reasons (I recall Microsoft's compiler
>> being pretty aggressive about this when the same function was called
>> more than once in close succession, it at least certain past versions)?
> 
> That sounds like a broken compiler.
> 
> There are legal cases where a direct call has to turn into an indirect
> one, and that's when we need to traverse more than disp32 distance.

Right, but that's certainly not happing anywhere in (relevant) practice
withing a single compiled binary.

> But without going to a larger mcmodel, we'd get linker errors before
> that becomes a problem, because R_X86_64_PLT32 relocations can't be
> retrofitted into an indirect call at link time.

I guess I don't see a connection to a PLT reloc: There wouldn't be any
if the compiler chose to make an indirect call out of a direct one. It
would be simple PC-relative relocations (generally coming from a RIP-
relative LEA) instead.

Jan

>> Is the widened effect of the annotation intended to also guarantee that
>> indirect calls will not be produced by the compiler for any reason when
>> the annotation is absent on a targeted function's declaration?
> 
> That would be one for the clang and gcc developers.
> 
> I don't see a plausible problem here.
> 
> ~Andrew
>