Re: [Xen-devel] [PATCH] x86/livepatch: Fail the build if duplicate symbols exist

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 08/04/2019 10:52, Jan Beulich wrote:
>>>> On 05.04.19 at 23:02, <konrad.wilk@xxxxxxxxxx> wrote:
>> On Fri, Apr 05, 2019 at 08:26:04PM +0100, Andrew Cooper wrote:
>>> From: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
>>>
>>> The binary diffing algorithm used by xen-livepatch depends on having unique
>>> symbols.
> Question: Is this an inherent requirement, or an effect of the
> current implementation? It would seem to me that at least in the
> common case (binary didn't changer overly heavily) it ought to
> be possible to re-associate the duplicates with their correct
> origins. And by making the build fail (including in cases like the
> one we have right now), we're painting ourselves into the
> corner of having to "fix" code which isn't actually buggy.

It is inherent.  The tool needs a unique way to identify each block of
code, so it can work out if the binary has changed by more than just
relocations.

How do you propose disambiguating ambiguous symbols after the fact?

(This is semi rhetorical, because even if there is a possible way, there
is no plausible way it is the sensible option to take.)

>
> Was the option of altering (amending) the symbol name instead
> of causing failure considered?

Amending how?  We've already got one bodge to prepend the filename to
static symbols.

The problem here is that we have two domctl.c's which both out-of-line
the functions, and the compiler-adjustment of .isra.0 is deterministic
based on the function parameters.

> Again, in the common case this
> shouldn't have overly bad implications: Typically the order of
> symbols wouldn't change between builds, so tagging the
> duplicates with a sequence number might be good enough.

This is painting ourselves into a corner with LTO.

Outside of LTO, the link order of files doesn't change, but the order of
symbols in a file changes a fair bit.

Also, this reasoning depends on people only making trivial changes via
livepatch, which, as it turns out, isn't the case in practice.

> And even if the order changed, as long as both instances in fact
> derive from the same inline function, all would still be fine afaict.

How do you propose that we determine this?

> Granted there's some heuristic underlying this, in that we then
> would hope for there not to be actually differing functions with
> the same source file and symbol names.

The more assumptions we start making, the more subtly this will go wrong
in the future.

>
>>> Signed-off-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> As for the time being this is hopefully going to do (once, as you
> say, the currently present issue has been addressed):
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks,

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel