clang 6.0 and newer behave like gcc in regards for the FILE symbol, so
only the filename rather than the full path to the source file.

clang 3.8.1-24 (in our debian:stretch container) and 3.5.0-10
(in our debian:jessie container) do store the full path to the source
file in the FILE symbol.

Also, based on commit 81ecb38b83 ("build: provide option to
disambiguate symbol names"), which were using clang 5, the change of
behavior likely happened in clang 6.0.

This means that we also need to check clang version to figure out
which command we need to use to redefine symbol.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

"enforce unique symbols" works by chance with recent clang version.
The few object built from source in subdir don't pose an issue.
---

Notes:
    v9:
    - checking for clang 6 instead of clang 4, based on 81ecb38b83, and
      update commit message.
    
    v8:
    - new patch, extracted from "build: build everything from the root dir, use obj=$subdir"

 xen/Rules.mk               | 2 +-
 xen/scripts/Kbuild.include | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 60d1d6c4f583..1e7f47a3d8a8 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -166,7 +166,7 @@ SRCPATH := $(patsubst $(BASEDIR)/%,%,$(CURDIR))
 quiet_cmd_cc_o_c = CC      $@
 ifeq ($(CONFIG_ENFORCE_UNIQUE_SYMBOLS),y)
     cmd_cc_o_c = $(CC) $(c_flags) -c $< -o $(dot-target).tmp -MQ $@
-    ifeq ($(CONFIG_CC_IS_CLANG),y)
+    ifeq ($(CONFIG_CC_IS_CLANG)$(call clang-ifversion,-lt,600,y),yy)
         cmd_objcopy_fix_sym = $(OBJCOPY) --redefine-sym $<=$(SRCPATH)/$< $(dot-target).tmp $@
     else
         cmd_objcopy_fix_sym = $(OBJCOPY) --redefine-sym $(<F)=$(SRCPATH)/$< $(dot-target).tmp $@
diff --git a/xen/scripts/Kbuild.include b/xen/scripts/Kbuild.include
index 73caf238d42c..4875bb28c282 100644
--- a/xen/scripts/Kbuild.include
+++ b/xen/scripts/Kbuild.include
@@ -59,6 +59,8 @@ ld-option = $(call success,$(LD) -v $(1))
 # Usage:  EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
 cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4))
 
+clang-ifversion = $(shell [ $(CONFIG_CLANG_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4))
+
 # Shorthand for $(MAKE) clean
 # Usage:
 # $(MAKE) $(clean) dir
-- 
Anthony PERARD

On 25.01.2022 12:00, Anthony PERARD wrote:
> clang 6.0 and newer behave like gcc in regards for the FILE symbol, so
> only the filename rather than the full path to the source file.
> 
> clang 3.8.1-24 (in our debian:stretch container) and 3.5.0-10
> (in our debian:jessie container) do store the full path to the source
> file in the FILE symbol.
> 
> Also, based on commit 81ecb38b83 ("build: provide option to
> disambiguate symbol names"), which were using clang 5, the change of
> behavior likely happened in clang 6.0.
> 
> This means that we also need to check clang version to figure out
> which command we need to use to redefine symbol.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

The "likely" in the description still worries me some. Roger, would
you happen to know, or know of a way to find out for sure ("sure"
not meaning to exclude the usual risk associated with version
number checks)?

Jan

On Thu, Jan 27, 2022 at 04:57:20PM +0100, Jan Beulich wrote:
> On 25.01.2022 12:00, Anthony PERARD wrote:
> > clang 6.0 and newer behave like gcc in regards for the FILE symbol, so
> > only the filename rather than the full path to the source file.
> > 
> > clang 3.8.1-24 (in our debian:stretch container) and 3.5.0-10
> > (in our debian:jessie container) do store the full path to the source
> > file in the FILE symbol.
> > 
> > Also, based on commit 81ecb38b83 ("build: provide option to
> > disambiguate symbol names"), which were using clang 5, the change of
> > behavior likely happened in clang 6.0.
> > 
> > This means that we also need to check clang version to figure out
> > which command we need to use to redefine symbol.
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> The "likely" in the description still worries me some. Roger, would
> you happen to know, or know of a way to find out for sure ("sure"
> not meaning to exclude the usual risk associated with version
> number checks)?

I found f5040b9685a7 ("Make .file directive to have basename only") as
part of LLVM's "release/6.x" branch (and "llvmorg-6.0.0" tag), but not
in "release/5.x".

https://github.com/llvm/llvm-project/commit/f5040b9685a760e584c576e9185295e54635d51e

This patch would seems to be the one changing the behavior. This still
suggest clang 6.0.

Cheers,

-- 
Anthony PERARD

On 28.01.2022 13:03, Anthony PERARD wrote:
> On Thu, Jan 27, 2022 at 04:57:20PM +0100, Jan Beulich wrote:
>> On 25.01.2022 12:00, Anthony PERARD wrote:
>>> clang 6.0 and newer behave like gcc in regards for the FILE symbol, so
>>> only the filename rather than the full path to the source file.
>>>
>>> clang 3.8.1-24 (in our debian:stretch container) and 3.5.0-10
>>> (in our debian:jessie container) do store the full path to the source
>>> file in the FILE symbol.
>>>
>>> Also, based on commit 81ecb38b83 ("build: provide option to
>>> disambiguate symbol names"), which were using clang 5, the change of
>>> behavior likely happened in clang 6.0.
>>>
>>> This means that we also need to check clang version to figure out
>>> which command we need to use to redefine symbol.
>>>
>>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>>
>> The "likely" in the description still worries me some. Roger, would
>> you happen to know, or know of a way to find out for sure ("sure"
>> not meaning to exclude the usual risk associated with version
>> number checks)?
> 
> I found f5040b9685a7 ("Make .file directive to have basename only") as
> part of LLVM's "release/6.x" branch (and "llvmorg-6.0.0" tag), but not
> in "release/5.x".
> 
> https://github.com/llvm/llvm-project/commit/f5040b9685a760e584c576e9185295e54635d51e
> 
> This patch would seems to be the one changing the behavior. This still
> suggest clang 6.0.

Oh, thanks for digging this out. May I suggest to replace (or delete)
"likely" then in the description?

Acked-by: Jan Beulich <jbeulich@suse.com>

Jan

On Fri, Jan 28, 2022 at 01:43:38PM +0100, Jan Beulich wrote:
> On 28.01.2022 13:03, Anthony PERARD wrote:
> > On Thu, Jan 27, 2022 at 04:57:20PM +0100, Jan Beulich wrote:
> >> On 25.01.2022 12:00, Anthony PERARD wrote:
> >>> clang 6.0 and newer behave like gcc in regards for the FILE symbol, so
> >>> only the filename rather than the full path to the source file.
> >>>
> >>> clang 3.8.1-24 (in our debian:stretch container) and 3.5.0-10
> >>> (in our debian:jessie container) do store the full path to the source
> >>> file in the FILE symbol.
> >>>
> >>> Also, based on commit 81ecb38b83 ("build: provide option to
> >>> disambiguate symbol names"), which were using clang 5, the change of
> >>> behavior likely happened in clang 6.0.
> >>>
> >>> This means that we also need to check clang version to figure out
> >>> which command we need to use to redefine symbol.
> >>>
> >>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> >>
> >> The "likely" in the description still worries me some. Roger, would
> >> you happen to know, or know of a way to find out for sure ("sure"
> >> not meaning to exclude the usual risk associated with version
> >> number checks)?
> > 
> > I found f5040b9685a7 ("Make .file directive to have basename only") as
> > part of LLVM's "release/6.x" branch (and "llvmorg-6.0.0" tag), but not
> > in "release/5.x".
> > 
> > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project%2Fcommit%2Ff5040b9685a760e584c576e9185295e54635d51e&amp;data=04%7C01%7Canthony.perard%40citrix.com%7C1ce7898a15bb4024260008d9e25be6f9%7C335836de42ef43a2b145348c2ee9ca5b%7C0%7C0%7C637789706644173026%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=V73NmkJWAHpqlzY9sAysf6%2Fw7q8ik6twT6lMLgglR3s%3D&amp;reserved=0
> > 
> > This patch would seems to be the one changing the behavior. This still
> > suggest clang 6.0.
> 
> Oh, thanks for digging this out. May I suggest to replace (or delete)
> "likely" then in the description?

Maybe something like that? Or just delete the word might be enough.

    Also we have commit 81ecb38b83 ("build: provide option to
    disambiguate symbol names") which were using clang 5, and LLVM's
    commit f5040b9685a7 [1] ("Make .file directive to have basename
    only") which is part of "llvmorg-6.0.0" tag but not "release/5.x"
    branch. Both suggest that clang change of behavior happened with
    clang 6.0.

    [1] https://github.com/llvm/llvm-project/commit/f5040b9685a760e584c576e9185295e54635d51e

> Acked-by: Jan Beulich <jbeulich@suse.com>

Thanks,

-- 
Anthony PERARD

On 28.01.2022 16:52, Anthony PERARD wrote:
> On Fri, Jan 28, 2022 at 01:43:38PM +0100, Jan Beulich wrote:
>> On 28.01.2022 13:03, Anthony PERARD wrote:
>>> On Thu, Jan 27, 2022 at 04:57:20PM +0100, Jan Beulich wrote:
>>>> On 25.01.2022 12:00, Anthony PERARD wrote:
>>>>> clang 6.0 and newer behave like gcc in regards for the FILE symbol, so
>>>>> only the filename rather than the full path to the source file.
>>>>>
>>>>> clang 3.8.1-24 (in our debian:stretch container) and 3.5.0-10
>>>>> (in our debian:jessie container) do store the full path to the source
>>>>> file in the FILE symbol.
>>>>>
>>>>> Also, based on commit 81ecb38b83 ("build: provide option to
>>>>> disambiguate symbol names"), which were using clang 5, the change of
>>>>> behavior likely happened in clang 6.0.
>>>>>
>>>>> This means that we also need to check clang version to figure out
>>>>> which command we need to use to redefine symbol.
>>>>>
>>>>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>>>>
>>>> The "likely" in the description still worries me some. Roger, would
>>>> you happen to know, or know of a way to find out for sure ("sure"
>>>> not meaning to exclude the usual risk associated with version
>>>> number checks)?
>>>
>>> I found f5040b9685a7 ("Make .file directive to have basename only") as
>>> part of LLVM's "release/6.x" branch (and "llvmorg-6.0.0" tag), but not
>>> in "release/5.x".
>>>
>>> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project%2Fcommit%2Ff5040b9685a760e584c576e9185295e54635d51e&amp;data=04%7C01%7Canthony.perard%40citrix.com%7C1ce7898a15bb4024260008d9e25be6f9%7C335836de42ef43a2b145348c2ee9ca5b%7C0%7C0%7C637789706644173026%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=V73NmkJWAHpqlzY9sAysf6%2Fw7q8ik6twT6lMLgglR3s%3D&amp;reserved=0
>>>
>>> This patch would seems to be the one changing the behavior. This still
>>> suggest clang 6.0.
>>
>> Oh, thanks for digging this out. May I suggest to replace (or delete)
>> "likely" then in the description?
> 
> Maybe something like that? Or just delete the word might be enough.
> 
>     Also we have commit 81ecb38b83 ("build: provide option to
>     disambiguate symbol names") which were using clang 5, and LLVM's
>     commit f5040b9685a7 [1] ("Make .file directive to have basename
>     only") which is part of "llvmorg-6.0.0" tag but not "release/5.x"
>     branch. Both suggest that clang change of behavior happened with
>     clang 6.0.
> 
>     [1] https://github.com/llvm/llvm-project/commit/f5040b9685a760e584c576e9185295e54635d51e

This sounds better to me than just dropping the one word.

Jan