Re: [XEN PATCH v4 14/18] xen,symbols: rework file symbols selection

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.04.2020 15:19, Anthony PERARD wrote:
> On Fri, Apr 17, 2020 at 09:12:11AM +0200, Jan Beulich wrote:
>> On 16.04.2020 17:09, Anthony PERARD wrote:
>>> On Thu, Apr 16, 2020 at 04:22:05PM +0200, Jan Beulich wrote:
>>>> On 16.04.2020 14:44, Anthony PERARD wrote:
>>>>> On Wed, Apr 08, 2020 at 02:54:35PM +0200, Jan Beulich wrote:
>>>>>> On 31.03.2020 12:30, Anthony PERARD wrote:
>>>>>>> We want to use the same rune to build mm/*/guest_*.o as the one use to
>>>>>>> build every other *.o object. The consequence it that file symbols that
>>>>>>> the program ./symbols prefer changes with 
>>>>>>> CONFIG_ENFORCE_UNIQUE_SYMBOLS=y.
>>>>>>>
>>>>>>> (1) Currently we have those two file symbols:
>>>>>>>     guest_walk.c
>>>>>>>     guest_walk_2.o
>>>>>>> (2) with CONFIG_ENFORCE_UNIQUE_SYMBOLS used on guest_walk.c, we will 
>>>>>>> have:
>>>>>>>     arch/x86/mm/guest_walk.c
>>>>>>>     guest_walk_2.o
>>>>>>>
>>>>>>> The order in which those symbols are present may be different.
>>>>>>>
>>>>>>> Currently, in case (1) ./symbols chooses the *.o symbol (object file
>>>>>>> name). But in case (2), may choose the *.c symbol (source file name with
>>>>>>> path component) if it is first
>>>>>>>
>>>>>>> We want to have ./symbols choose the object file name symbol in both
>>>>>>> cases.
>>>>>>
>>>>>> I guess the reason for wanting this is somehow connected to the
>>>>>> statement at the beginning of the description, but I can't seem
>>>>>> to be able to make the connection.
>>>>>
>>>>> I'm not sure I can explain it better.
>>>>>
>>>>> The "object file name" file symbol is used to distinguish between symbols
>>>>> from all mm/*/guest_* objects. The other file symbol present in those
>>>>> object is a "source file name without any path component symbol".
>>>>>
>>>>> But building those objects with the same rune as any other objects, and
>>>>> having CONFIG_ENFORCE_UNIQUE_SYMBOLS=y, changes the file symbols present
>>>>> in the resulting object. We still have the "object file name" symbol,
>>>>> but now we also have "source file name with path components" symbol.
>>>>> Unfortunately, all mm/*/guest_*.o in one directory are built from the
>>>>> same source file, and thus have the same "source file name" symbol, but
>>>>> have different "object file name" symbol. We still want to be able to
>>>>> distinguish between guest_*.o in one dir, and the only way for that is
>>>>> to use the "object file name" symbol.
>>>>
>>>> So where's the difference from how things work right now? The "same rune"
>>>> aspect doesn't really change - right now we also build with effectively
>>>> the same logic, just that -DGUEST_PAGING_LEVELS=... gets added. I guess
>>>> it might help if you showed (for one particular example) how the set of
>>>> file symbols changes from what we have now (with and without
>>>> CONFIG_ENFORCE_UNIQUE_SYMBOLS=y) to what there would be with your changes
>>>> to the symbols utility to what there will be with those changes.
>>>
>>> The logic to build objects from C files changed in 81ecb38b83b0 ("build:
>>> provide option to disambiguate symbol names"), with objects build with
>>> __OBJECT_FILE__ explicitly left alone. So the logic is different now (at
>>> least when CONFIG_ENFORCE_UNIQUE_SYMBOLS=y).
>>>
>>> I did add the example of building arch/x86/mm/guest_walk_2.o to the
>>> commit message, reworded below:
>>>
>>> For example, when building arch/x86/mm/guest_walk_2.o from guest_walk.c,
>>> this would be the difference of file symbol present in the object when
>>> building with CONFIG_ENFORCE_UNIQUE_SYMBOLS=y:
>>>
>>> (1) Currently we have those two file symbols:
>>>     guest_walk.c
>>>     guest_walk_2.o
>>> (2) When building with the same rune, we will have:
>>>     arch/x86/mm/guest_walk.c
>>>     guest_walk_2.o
>>
>> Ah, yes, the changed introductory paragraph makes clear (to me)
>> what presence and what future (1) and (2) are talking about. Yet
>> what I then still don't understand - what is it that makes the
>> path appear when switching to the common rune? Oh - I finally
>> figured it: It's the objcopy step that will apply to all targets
>> uniformly then. Perhaps it's indeed obvious, but it clearly
>> wasn't to me when merely looking at the patch.
>>
>> With this I'd then wonder whether it wouldn't be a far smaller
>> adjustment to simply skip that --redefine-sym step in case the
>> object file already has a file symbol naming the object file,
>> thus simply retaining the status quo.
> 
> So, we should call `nm' thousands of time, to find out if calling
> `objcopy' is needed, for the 9 objects that doesn't need the extra step?

Well that (or rather objdump) was what I was thinking while writing
the earlier reply, but you have a point - I can see how treating
the bigger change for less build time might be worth it. Yet ...

> Or do you mean keeping exception to the rule? And hope that when someone
> changes the rule, it doesn't forget to check if the exception needs
> changing as well?

... "exception" like you put it (requiring special care to keep
multiple instances in sync) is not the only way this can be done
(and indeed I'd not want something like this). Since you have
(in patch 15) e.g.

guest_walk_%.o: guest_walk.c FORCE
        $(call if_changed_rule,cc_o_c)

anyway, the desire to skip the objcopy step could be communicated
to the command from here, without needing to clone the command.
One way might be a special (phony) dependency, another might be to
set some variable along the lines of

guest_walk_%.o: SPECIAL := y

> Also, I'm going to have to use this patch later anyway as sometime CC
> use a full path to the source as file symbol. So this is going to be
> important when we will run for example
> `clang -o arch/x86/mm/guest_walk_2.o arch/x86/mm/guest_walk.c`.
> (There isn't a patch for that yet.)

That's interesting - what will be the goal of that future adjustment?

Jan