[PATCH] gcc: Fix tm.h plugin header

[PATCH] gcc: Fix tm.h plugin header

[Andrei Gherzan]

From: Andrei Gherzan <andrei.gherzan@huawei.com>

On x86-64, tm.h (needed to build gcc plugins) tries to include
config/i386/linux64.h, which isn't installed. Fortunately it also isn't
used, so simply removing the include statement is an ok fix.

Signed-off-by: Bernhard Rosenkränzer <bernhard.rosenkraenzer.ext@huawei.com>
Signed-off-by: Andrei Gherzan <andrei.gherzan@huawei.com>
---
 meta/recipes-devtools/gcc/gcc-cross-canadian.inc | 3 +++
 meta/recipes-devtools/gcc/gcc-cross.inc          | 3 +++
 meta/recipes-devtools/gcc/gcc-target.inc         | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/meta/recipes-devtools/gcc/gcc-cross-canadian.inc b/meta/recipes-devtools/gcc/gcc-cross-canadian.inc
index 495dd9070b..a64fcc50ce 100644
--- a/meta/recipes-devtools/gcc/gcc-cross-canadian.inc
+++ b/meta/recipes-devtools/gcc/gcc-cross-canadian.inc
@@ -139,6 +139,9 @@ do_install () {
 	cp ${S}/libquadmath/quadmath.h ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/include/
 	cp ${S}/libquadmath/quadmath_weak.h ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/include/
 
+	# Fix a plugin header including a nonexistant (and unnecessary) header
+	sed -i -e '/config\/i386\/linux64\.h/d' ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/plugin/include/tm.h
+
 	# install LTO linker plugins where binutils tools can find it
 	install -d ${D}${libdir}/bfd-plugins
 	ln -sf ${LIBRELPATH}/liblto_plugin.so ${D}${libdir}/bfd-plugins/liblto_plugin.so
diff --git a/meta/recipes-devtools/gcc/gcc-cross.inc b/meta/recipes-devtools/gcc/gcc-cross.inc
index 3ffa1f0c46..ac34d5116a 100644
--- a/meta/recipes-devtools/gcc/gcc-cross.inc
+++ b/meta/recipes-devtools/gcc/gcc-cross.inc
@@ -119,6 +119,9 @@ do_install () {
 	cp ${S}/libquadmath/quadmath.h ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/include/
 	cp ${S}/libquadmath/quadmath_weak.h ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/include/
 
+	# Fix a plugin header including a nonexistant (and unnecessary) header
+	sed -i -e '/config\/i386\/linux64\.h/d' ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/plugin/include/tm.h
+
 	find ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/include-fixed -type f -not -name "README" -not -name limits.h -not -name syslimits.h | xargs rm -f
 
 	# install LTO linker plugins where binutils tools can find it
diff --git a/meta/recipes-devtools/gcc/gcc-target.inc b/meta/recipes-devtools/gcc/gcc-target.inc
index bf55e692e6..4d087c5183 100644
--- a/meta/recipes-devtools/gcc/gcc-target.inc
+++ b/meta/recipes-devtools/gcc/gcc-target.inc
@@ -187,6 +187,9 @@ do_install () {
 	rm -f ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/plugin/include/auto-build.h
 	rm -f ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/plugin/include/bconfig.h
 
+	# Fix a plugin header including a nonexistant (and unnecessary) header
+	sed -i -e '/config\/i386\/linux64\.h/d' ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/plugin/include/tm.h
+
 	cd ${D}${bindir}
 
 	# We care about g++ not c++
-- 
2.25.1

Re: [OE-core] [PATCH] gcc: Fix tm.h plugin header

[Khem Raj]

On Thu, Dec 9, 2021 at 7:40 AM Andrei Gherzan <andrei@gherzan.com> wrote:
>
> From: Andrei Gherzan <andrei.gherzan@huawei.com>
>
> On x86-64, tm.h (needed to build gcc plugins) tries to include
> config/i386/linux64.h, which isn't installed. Fortunately it also isn't
> used, so simply removing the include statement is an ok fix.
>

is it due to multilib support ? if so we might be ok since we may not
be using it
but really it will be good to check it with multilib builds of x86_64/x86

> Signed-off-by: Bernhard Rosenkränzer <bernhard.rosenkraenzer.ext@huawei.com>
> Signed-off-by: Andrei Gherzan <andrei.gherzan@huawei.com>
> ---
>  meta/recipes-devtools/gcc/gcc-cross-canadian.inc | 3 +++
>  meta/recipes-devtools/gcc/gcc-cross.inc          | 3 +++
>  meta/recipes-devtools/gcc/gcc-target.inc         | 3 +++
>  3 files changed, 9 insertions(+)
>
> diff --git a/meta/recipes-devtools/gcc/gcc-cross-canadian.inc b/meta/recipes-devtools/gcc/gcc-cross-canadian.inc
> index 495dd9070b..a64fcc50ce 100644
> --- a/meta/recipes-devtools/gcc/gcc-cross-canadian.inc
> +++ b/meta/recipes-devtools/gcc/gcc-cross-canadian.inc
> @@ -139,6 +139,9 @@ do_install () {
>         cp ${S}/libquadmath/quadmath.h ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/include/
>         cp ${S}/libquadmath/quadmath_weak.h ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/include/
>
> +       # Fix a plugin header including a nonexistant (and unnecessary) header
> +       sed -i -e '/config\/i386\/linux64\.h/d' ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/plugin/include/tm.h
> +
>         # install LTO linker plugins where binutils tools can find it
>         install -d ${D}${libdir}/bfd-plugins
>         ln -sf ${LIBRELPATH}/liblto_plugin.so ${D}${libdir}/bfd-plugins/liblto_plugin.so
> diff --git a/meta/recipes-devtools/gcc/gcc-cross.inc b/meta/recipes-devtools/gcc/gcc-cross.inc
> index 3ffa1f0c46..ac34d5116a 100644
> --- a/meta/recipes-devtools/gcc/gcc-cross.inc
> +++ b/meta/recipes-devtools/gcc/gcc-cross.inc
> @@ -119,6 +119,9 @@ do_install () {
>         cp ${S}/libquadmath/quadmath.h ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/include/
>         cp ${S}/libquadmath/quadmath_weak.h ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/include/
>
> +       # Fix a plugin header including a nonexistant (and unnecessary) header
> +       sed -i -e '/config\/i386\/linux64\.h/d' ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/plugin/include/tm.h
> +
>         find ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/include-fixed -type f -not -name "README" -not -name limits.h -not -name syslimits.h | xargs rm -f
>
>         # install LTO linker plugins where binutils tools can find it
> diff --git a/meta/recipes-devtools/gcc/gcc-target.inc b/meta/recipes-devtools/gcc/gcc-target.inc
> index bf55e692e6..4d087c5183 100644
> --- a/meta/recipes-devtools/gcc/gcc-target.inc
> +++ b/meta/recipes-devtools/gcc/gcc-target.inc
> @@ -187,6 +187,9 @@ do_install () {
>         rm -f ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/plugin/include/auto-build.h
>         rm -f ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/plugin/include/bconfig.h
>
> +       # Fix a plugin header including a nonexistant (and unnecessary) header
> +       sed -i -e '/config\/i386\/linux64\.h/d' ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/plugin/include/tm.h
> +
>         cd ${D}${bindir}
>
>         # We care about g++ not c++
> --
> 2.25.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#159457): https://lists.openembedded.org/g/openembedded-core/message/159457
> Mute This Topic: https://lists.openembedded.org/mt/87614108/1997914
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [raj.khem@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

Re: [OE-core] [PATCH] gcc: Fix tm.h plugin header

[Andrei Gherzan]

On Thu, 9 Dec 2021, at 16:40, Khem Raj wrote:
> On Thu, Dec 9, 2021 at 7:40 AM Andrei Gherzan <andrei@gherzan.com> wrote:
>>
>> From: Andrei Gherzan <andrei.gherzan@huawei.com>
>>
>> On x86-64, tm.h (needed to build gcc plugins) tries to include
>> config/i386/linux64.h, which isn't installed. Fortunately it also isn't
>> used, so simply removing the include statement is an ok fix.
>>
>
> is it due to multilib support ? if so we might be ok since we may not
> be using it
> but really it will be good to check it with multilib builds of x86_64/x86

I don't think it is related to multilib but I'll give multilib a try. In my case, the issue was revealed when compiling a kernel plugin config. I can't remember right now which one but I can replicate it for logs.

--
Andrei

Re: [PATCH] gcc: Fix tm.h plugin header

[Bernhard Rosenkränzer]

[-- Attachment #1: Type: text/plain, Size: 2064 bytes --]

On Thu, Dec 9, 2021 at 07:40 PM, Andrei Gherzan wrote:

> 
> On Thu, 9 Dec 2021, at 16:40, Khem Raj wrote:
> 
>> On Thu, Dec 9, 2021 at 7:40 AM Andrei Gherzan <andrei@gherzan.com> wrote:
>> 
>>> From: Andrei Gherzan <andrei.gherzan@huawei.com>
>>> 
>>> On x86-64, tm.h (needed to build gcc plugins) tries to include
>>> config/i386/linux64.h, which isn't installed. Fortunately it also isn't
>>> used, so simply removing the include statement is an ok fix.
>> 
>> is it due to multilib support ? if so we might be ok since we may not
>> be using it
>> but really it will be good to check it with multilib builds of x86_64/x86
> 
> I don't think it is related to multilib but I'll give multilib a try. In
> my case, the issue was revealed when compiling a kernel plugin config. I
> can't remember right now which one but I can replicate it for logs.

Hi,
I've had a closer look at the problem, and can confirm the patch doesn't break even in multilib setups.

The problem is that in a non-multilib setup, the config/i386/linux64.h header is not generated, but tm.h tries to include it nevertheless, causing any gcc plugin that #includes tm.h (such as all the gcc plugins in the kernel -- tm.h is included by scripts/gcc-plugins/gcc-common.h) to fail to build.
An alternative fix would be adding
mkdir -p ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/plugin/include/config/i386
touch ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/plugin/include/config/i386/linux64.h
(creating the file if it isn't there) instead. This would make sure that in cases where config/i386/linux64.h does exist, it doesn't get removed from tm.h (potentially fixing plugins that reference any of the #defines in i386/linux64.h without including the file manually -- but I have yet to see any such plugin).

I will try to get a better fix (not adding the include statement to tm.h in the first place rather than having to remove it later in install scripts) upstream, but in the mean time, the patch fixes our builds before the better fix is ready/applied.

ttyl
bero

[-- Attachment #2: Type: text/html, Size: 2177 bytes --]

Re: [OE-core] [PATCH] gcc: Fix tm.h plugin header

[Andrei Gherzan]

[-- Attachment #1: Type: text/plain, Size: 2259 bytes --]

On Mon, 20 Dec 2021, at 01:39, Bernhard Rosenkränzer via lists.openembedded.org wrote:
> On Thu, Dec 9, 2021 at 07:40 PM, Andrei Gherzan wrote:
> 
>> On Thu, 9 Dec 2021, at 16:40, Khem Raj wrote:
>> 
>>> On Thu, Dec 9, 2021 at 7:40 AM Andrei Gherzan <andrei@gherzan.com> wrote:
>>> 
>>>> From: Andrei Gherzan <andrei.gherzan@huawei.com>
>>>> 
>>>> On x86-64, tm.h (needed to build gcc plugins) tries to include
>>>> config/i386/linux64.h, which isn't installed. Fortunately it also isn't
>>>> used, so simply removing the include statement is an ok fix.
>>> is it due to multilib support ? if so we might be ok since we may not
>>> be using it
>>> but really it will be good to check it with multilib builds of x86_64/x86
>> I don't think it is related to multilib but I'll give multilib a try. In my case, the issue was revealed when compiling a kernel plugin config. I can't remember right now which one but I can replicate it for logs.
> Hi,
> I've had a closer look at the problem, and can confirm the patch doesn't break even in multilib setups.
> 
> The problem is that in a non-multilib setup, the config/i386/linux64.h header is not generated, but tm.h tries to include it nevertheless, causing any gcc plugin that #includes tm.h (such as all the gcc plugins in the kernel -- tm.h is included by scripts/gcc-plugins/gcc-common.h) to fail to build.
> An alternative fix would be adding
> mkdir -p ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/plugin/include/config/i386
> touch ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/plugin/include/config/i386/linux64.h
> (creating the file if it isn't there) instead. This would make sure that in cases where config/i386/linux64.h does exist, it doesn't get removed from tm.h (potentially fixing plugins that reference any of the #defines in i386/linux64.h without including the file manually -- but I have yet to see any such plugin).
> 
> I will try to get a better fix (not adding the include statement to tm.h in the first place rather than having to remove it later in install scripts) upstream, but in the mean time, the patch fixes our builds before the better fix is ready/applied.

Ping on this patch. Any chance we can pull it in as a first iteration of the solution?

---
Andrei

[-- Attachment #2: Type: text/html, Size: 2884 bytes --]

Re: [OE-core] [PATCH] gcc: Fix tm.h plugin header

[Richard Purdie]

On Tue, 2021-12-21 at 11:56 +0000, Andrei Gherzan wrote:
> On Mon, 20 Dec 2021, at 01:39, Bernhard Rosenkränzer via
> lists.openembedded.org wrote:
> > On Thu, Dec 9, 2021 at 07:40 PM, Andrei Gherzan wrote:
> >  
> > > On Thu, 9 Dec 2021, at 16:40, Khem Raj wrote:
> > >  
> > > > On Thu, Dec 9, 2021 at 7:40 AM Andrei Gherzan <andrei@gherzan.com>
> > > > wrote:
> > > >  
> > > > > From: Andrei Gherzan <andrei.gherzan@huawei.com>
> > > > > 
> > > > > On x86-64, tm.h (needed to build gcc plugins) tries to include
> > > > > config/i386/linux64.h, which isn't installed. Fortunately it also
> > > > > isn't
> > > > > used, so simply removing the include statement is an ok fix.
> > > > is it due to multilib support ? if so we might be ok since we may not
> > > > be using it
> > > > but really it will be good to check it with multilib builds of
> > > > x86_64/x86
> > > I don't think it is related to multilib but I'll give multilib a try. In
> > > my case, the issue was revealed when compiling a kernel plugin config. I
> > > can't remember right now which one but I can replicate it for logs.
> > Hi,
> > I've had a closer look at the problem, and can confirm the patch doesn't
> > break even in multilib setups.
> > 
> > The problem is that in a non-multilib setup, the config/i386/linux64.h
> > header is not generated, but tm.h tries to include it nevertheless, causing
> > any gcc plugin that #includes tm.h (such as all the gcc plugins in the
> > kernel -- tm.h is included by scripts/gcc-plugins/gcc-common.h) to fail to
> > build.
> > An alternative fix would be adding
> > mkdir -p ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/plugin/include/config/i386
> > touch ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/plugin/include/config/i386/lin
> > ux64.h
> > (creating the file if it isn't there) instead. This would make sure that in
> > cases where config/i386/linux64.h does exist, it doesn't get removed from
> > tm.h (potentially fixing plugins that reference any of the #defines in
> > i386/linux64.h without including the file manually -- but I have yet to see
> > any such plugin).
> > 
> > I will try to get a better fix (not adding the include statement to tm.h in
> > the first place rather than having to remove it later in install scripts)
> > upstream, but in the mean time, the patch fixes our builds before the better
> > fix is ready/applied.
> 
> Ping on this patch. Any chance we can pull it in as a first iteration of the
> solution?

I don't like sed in this context as if the code changes, it silently stops
working and we struggle to know what broke or whether we need to fix it. We
don't have any test case.

Given all that, the touch of the file is a neater solution as it is more obvious
what it is doing. I'd also want to see some more detailed comments in the code
about what it is doing and why.

Maintainability of these kinds of fixes is key.

Cheers,

Richard

Re: [OE-core] [PATCH] gcc: Fix tm.h plugin header

[Andrei Gherzan]

Hi Richard,

On Tue, 21 Dec 2021, at 12:04, Richard Purdie wrote:
> On Tue, 2021-12-21 at 11:56 +0000, Andrei Gherzan wrote:
>> On Mon, 20 Dec 2021, at 01:39, Bernhard Rosenkränzer via
>> lists.openembedded.org wrote:
>> > On Thu, Dec 9, 2021 at 07:40 PM, Andrei Gherzan wrote:
>> >  
>> > > On Thu, 9 Dec 2021, at 16:40, Khem Raj wrote:
>> > >  
>> > > > On Thu, Dec 9, 2021 at 7:40 AM Andrei Gherzan <andrei@gherzan.com>
>> > > > wrote:
>> > > >  
>> > > > > From: Andrei Gherzan <andrei.gherzan@huawei.com>
>> > > > > 
>> > > > > On x86-64, tm.h (needed to build gcc plugins) tries to include
>> > > > > config/i386/linux64.h, which isn't installed. Fortunately it also
>> > > > > isn't
>> > > > > used, so simply removing the include statement is an ok fix.
>> > > > is it due to multilib support ? if so we might be ok since we may not
>> > > > be using it
>> > > > but really it will be good to check it with multilib builds of
>> > > > x86_64/x86
>> > > I don't think it is related to multilib but I'll give multilib a try. In
>> > > my case, the issue was revealed when compiling a kernel plugin config. I
>> > > can't remember right now which one but I can replicate it for logs.
>> > Hi,
>> > I've had a closer look at the problem, and can confirm the patch doesn't
>> > break even in multilib setups.
>> > 
>> > The problem is that in a non-multilib setup, the config/i386/linux64.h
>> > header is not generated, but tm.h tries to include it nevertheless, causing
>> > any gcc plugin that #includes tm.h (such as all the gcc plugins in the
>> > kernel -- tm.h is included by scripts/gcc-plugins/gcc-common.h) to fail to
>> > build.
>> > An alternative fix would be adding
>> > mkdir -p ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/plugin/include/config/i386
>> > touch ${D}${libdir}/gcc/${TARGET_SYS}/${BINV}/plugin/include/config/i386/lin
>> > ux64.h
>> > (creating the file if it isn't there) instead. This would make sure that in
>> > cases where config/i386/linux64.h does exist, it doesn't get removed from
>> > tm.h (potentially fixing plugins that reference any of the #defines in
>> > i386/linux64.h without including the file manually -- but I have yet to see
>> > any such plugin).
>> > 
>> > I will try to get a better fix (not adding the include statement to tm.h in
>> > the first place rather than having to remove it later in install scripts)
>> > upstream, but in the mean time, the patch fixes our builds before the better
>> > fix is ready/applied.
>> 
>> Ping on this patch. Any chance we can pull it in as a first iteration of the
>> solution?
>
> I don't like sed in this context as if the code changes, it silently stops
> working and we struggle to know what broke or whether we need to fix it. We
> don't have any test case.
>
> Given all that, the touch of the file is a neater solution as it is more obvious
> what it is doing. I'd also want to see some more detailed comments in the code
> about what it is doing and why.
>
> Maintainability of these kinds of fixes is key.

I totally agree so I dug a bit more into this and arrived at a different fix. Pushed it to the ml - https://lists.openembedded.org/g/openembedded-core/message/159978

---
Andrei