| Message ID | 20220125110103.3527686-9-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: Build system improvements, now with out-of-tree build! | expand |
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,
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&data=04%7C01%7Canthony.perard%40citrix.com%7C1ce7898a15bb4024260008d9e25be6f9%7C335836de42ef43a2b145348c2ee9ca5b%7C0%7C0%7C637789706644173026%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=V73NmkJWAHpqlzY9sAysf6%2Fw7q8ik6twT6lMLgglR3s%3D&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,
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&data=04%7C01%7Canthony.perard%40citrix.com%7C1ce7898a15bb4024260008d9e25be6f9%7C335836de42ef43a2b145348c2ee9ca5b%7C0%7C0%7C637789706644173026%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=V73NmkJWAHpqlzY9sAysf6%2Fw7q8ik6twT6lMLgglR3s%3D&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
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
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(-)