Re: Hotpatch construction and __LINE__

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 05/08/15 09:55, Martin Pohlack wrote:
> Hi,
>
> Another high-level point to think about is how we want to handle inlined
> __LINE__ references.  This problem is related to hotpatch construction
> and potentially has influence on the design of the hotpatching
> infrastructure in Xen.
>
> Let me try to explain the problem:
>
> We have file1.c with functions f1 and f2 (in that order).  f2 contains a
> BUG() (or WARN()) macro and at that point embeds the source line number
> into the generated code for f2.
>
> Now we want to hotpatch f1 and the hotpatch source-code patch adds 2
> lines to f1 and as a consequence shifts out f2 by two lines.  The newly
> constructed file1.o will now contain differences in both binary
> functions f1 (because we actually changed it with the applied patch) and
> f2 (because the contained BUG macro embeds the new line number).
>
> Without additional information, an algorithm comparing file1.o before
> and after hotpatch application will determine both functions to be
> changed and will have to include both into the binary hotpatch.
>
> Options:
>
> 1. Transform source code patches for hotpatches to be line-neutral for
>    each chunk.  This can be done in almost all cases with either
>    reformatting of the source code or by introducing artificial
>    preprocessor "#line n" directives to adjust for the introduced
>    differences.
>
>    This approach is low-tech and simple.  Potentially generated
>    backtraces and existing debug information refers to the original
>    build and does not reflect hotpatching state except for actually
>    hotpatched functions but should be mostly correct.
>
> 2. Ignoring the problem and living with artificially large hotpatches
>    that unnecessarily patch many functions.
>
>    This approach might lead to some very large hotpatches depending on
>    content of specific source file.  It may also trigger pulling in
>    functions into the hotpatch that cannot reasonable be hotpatched due
>    to limitations of a hotpatching framework (init-sections, parts of
>    the hotpatching framework itself, ...) and may thereby prevent us
>    from patching a specific problem.
>
>    The decision between 1. and 2. can be made on a patch--by-patch
>    basis.
>
> 3. Introducing an indirection table for storing line numbers and
>    treating that specially for binary diffing.  I believe Linux follows
>    this approach, but the details escape me ATM.
>
>    We might either use this indirection table for runtime use and patch
>    that with each hotpatch (similarly to exception tables) or we might
>    purely use it when building hotpatches to ignore functions that only
>    differ at exactly the location where a line-number is embedded.
>
> Similar considerations are true to a lesser extent for __FILE__, but I
> would argue that file renaming should be done outside of hotpatches.

Looking at `strings xen-syms`, we have very few embedded
__FILE__/__LINE__'s in other strings, meaning that most come from %s/%d
formatting.

bug/warn/assert frames have their references in the exception table, so
should be reasonably self contained to fix up.

Regular printk()s are definitely more problematic however.  Their
references will typically be in %rsi/%rdx and shifting any #line
reference across the 256 boundary will change the encoding size of
setting the print up.

Jan had a plan to make Xen read its own DWARF symbol table rather than
using the current cludge we have where we partially link Xen, extract
the public symbol table, rewrite symbol-offsets.c and relink it onto the
end.

Perhaps there is an opportunity to piggy-back onto that and have all
file/line references coming from the DWARF information.  I suspect this
will be similar to what Linux does, as it already has support for using
its own DWARF/STABS information.

Personally, I think #2 should be avoided wherever possible.  The bigger
the hotpatch, the greater the opportunity for something to go wrong.

Any panic/stack trace should be able to identify exactly which Xen is
running, including some hotpatch status.  Hopatches will be[1] small
targeted fixes, so I don't think it is unreasonable to expect someone
interpreting a stack trace to need to adjust file/line references by the
hotpatch delta.

~Andrew

[1] woe betide anyone attempting to use hotpatches for general software
updates.