Re: [Xen-devel] Ping: [PATCH v2] build: provide option to disambiguate symbol names

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 20/11/2019 17:19, Jan Beulich wrote:
> On 20.11.2019 18:13, Andrew Cooper wrote:
>> On 20/11/2019 16:40, Jürgen Groß wrote:
>>> On 20.11.19 17:30, Jan Beulich wrote:
>>>> On 08.11.2019 12:18, Jan Beulich wrote:
>>>>> The .file assembler directives generated by the compiler do not include
>>>>> any path components (gcc) or just the ones specified on the command
>>>>> line
>>>>> (clang, at least version 5), and hence multiple identically named
>>>>> source
>>>>> files (in different directories) may produce identically named static
>>>>> symbols (in their kallsyms representation). The binary diffing
>>>>> algorithm
>>>>> used by xen-livepatch, however, depends on having unique symbols.
>>>>>
>>>>> Make the ENFORCE_UNIQUE_SYMBOLS Kconfig option control the (build)
>>>>> behavior, and if enabled use objcopy to prepend the (relative to the
>>>>> xen/ subdirectory) path to the compiler invoked STT_FILE symbols. Note
>>>>> that this build option is made no longer depend on LIVEPATCH, but
>>>>> merely
>>>>> defaults to its setting now.
>>>>>
>>>>> Conditionalize explicit .file directive insertion in C files where it
>>>>> exists just to disambiguate names in a less generic manner; note that
>>>>> at the same time the redundant emission of STT_FILE symbols gets
>>>>> suppressed for clang. Assembler files as well as multiply compiled C
>>>>> ones using __OBJECT_FILE__ are left alone for the time being.
>>>>>
>>>>> Since we now expect there not to be any duplicates anymore, also don't
>>>>> force the selection of the option to 'n' anymore in allrandom.config.
>>>>> Similarly COVERAGE no longer suppresses duplicate symbol warnings if
>>>>> enforcement is in effect, which in turn allows
>>>>> SUPPRESS_DUPLICATE_SYMBOL_WARNINGS to simply depend on
>>>>> !ENFORCE_UNIQUE_SYMBOLS.
>>>>>
>>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>> I've got acks from Konrad and Wei, but still need an x86 and a release
>>>> one here. Andrew? Or alternatively - Jürgen, would you rather not see
>>>> this go in anymore?
>>> In case the needed x86 Ack is coming in before RC3 I'm fine to give my
>>> Release-ack, but I'm hesitant to take it later.
>> Has anyone actually tried building a livepatch with this change in place?
> I've never tried building any live patch, so I also didn't test
> this angle of this change. But I did verify the results of what
> the change here does.
>
> I'm also a little puzzled about this response because I did the
> change upon your request.
>
>> I ask, because there is 0 testing of livepatches, and already one major
>> regression in 4.13 which forces XenServer to revert back to older build
>> tools.
> That's a build tools regression, isn't it? I.e. not really related
> to 4.13?

I believe it is a build tools regression, rather than a 4.13 regression.

However, we are in the position that there is a supported tool with no
adequate testing in place, with one rather terminal regression in the
4.13 timeframe.  All I'm doing is being cautious about making changes
which have a real likelihood of affecting the status quo.

~Andrew

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