Skip to contentSkip to conversation

bash: Add fix for cross compile issues

PATCH In review Richard Purdie 4 participants 12 messages Nov 13, 2012 – Nov 15, 2012

lore
Share & tools
lore

Richard Purdie

author
#0 lore raw

PATCH 1/1 bash: Add fix for cross compile issues

Signed-off-by Richard Purdie <[email protected]>

---

meta/recipes-extended/bash/bash-4.2/crossfix.patch renamed from dev/null +28 -0
new file mode 100644
index 0000000..f587c34
@@ -0,0 +1,28 @@
+Adding @CROSS_COMPILE@ to CFLAGS_FOR_BUILD causes errors like:
+
+mkbuiltins.o: In function `open':
+/usr/include/x86_64-linux-gnu/bits/fcntl2.h:54: undefined reference to `xopen'
+mkbuiltins.o: In function `read':
+/usr/include/x86_64-linux-gnu/bits/unistd.h:45: undefined reference to `xread'
+collect2: ld returned 1 exit status
+
+when compiling on a 64 bit x86 build system for a 32 bit x86 target since
+config.h confuses the compiler about settings. By removing the option, config.h 
+isn't used and the compiler stops getting confused.
+
+Upstream-Status: Pending
+RP 2012/11/13
+
+Index: bash-4.2/builtins/Makefile.in
+===================================================================
+--- bash-4.2.orig/builtins/Makefile.in	2010-12-21 13:37:18.000000000 +0000
++++ bash-4.2/builtins/Makefile.in	2012-11-13 11:36:47.761417446 +0000
+@@ -56,7 +56,7 @@
+ 
+ PROFILE_FLAGS = @PROFILE_FLAGS@
+ CFLAGS = @CFLAGS@
+-CFLAGS_FOR_BUILD = @CFLAGS_FOR_BUILD@ @CROSS_COMPILE@
++CFLAGS_FOR_BUILD = @CFLAGS_FOR_BUILD@
+ CPPFLAGS = @CPPFLAGS@
+ CPPFLAGS_FOR_BUILD = @CPPFLAGS_FOR_BUILD@
+ LOCAL_CFLAGS = @LOCAL_CFLAGS@ ${DEBUG}
meta/recipes-extended/bash/bash_4.2.bb +2 -1
index 5a0f015..18ddc7e 100644
@@ -4,7 +4,7 @@ require bash.inc
 LICENSE = "GPLv3+"
 LIC_FILES_CHKSUM = "file://COPYING;md5=d32239bcb673463ab874e80d47fae504"
 
-PR = "r5"
+PR = "r6"
 
 SRC_URI = "${GNU_MIRROR}/bash/${BPN}-${PV}.tar.gz;name=tarball \
            ${GNU_MIRROR}/bash/bash-4.2-patches/bash42-001;apply=yes;striplevel=0;name=patch001 \

@@ -18,6 +18,7 @@ SRC_URI = "${GNU_MIRROR}/bash/${BPN}-${PV}.tar.gz;name=tarball \
            ${GNU_MIRROR}/bash/bash-4.2-patches/bash42-009;apply=yes;striplevel=0;name=patch009 \
            ${GNU_MIRROR}/bash/bash-4.2-patches/bash42-010;apply=yes;striplevel=0;name=patch010 \
            file://execute_cmd.patch;striplevel=0 \
+           file://crossfix.patch \
            "
 
 SRC_URI[tarball.md5sum] = "3fb927c7c33022f1c327f14a81c0d4b0"
#1 lore raw
Quoted 23 lines · Richard Purdie
Signed-off-by Richard Purdie <[email protected]>

---

meta/recipes-extended/bash/bash-4.2/crossfix.patch renamed from dev/null +4 -0
new file mode 100644
index 0000000..f587c34
@@ -0,0 +1,28 @@
+Adding @CROSS_COMPILE@ to CFLAGS_FOR_BUILD causes errors like:
+
+mkbuiltins.o: In function `open':
+/usr/include/x86_64-linux-gnu/bits/fcntl2.h:54: undefined reference to

`xopen'

meta/recipes-extended/bash/bash-4.2/crossfix.patch renamed from dev/null +2 -0
+mkbuiltins.o: In function `read':
+/usr/include/x86_64-linux-gnu/bits/unistd.h:45: undefined reference to

`xread'

meta/recipes-extended/bash/bash-4.2/crossfix.patch renamed from dev/null +4 -0
+collect2: ld returned 1 exit status
+
+when compiling on a 64 bit x86 build system for a 32 bit x86 target since
+config.h confuses the compiler about settings. By removing the option,

config.h

meta/recipes-extended/bash/bash-4.2/crossfix.patch line 14

Quoted 4 lines
meta/recipes-extended/bash/bash-4.2/crossfix.patch renamed from dev/null +4 -0
+isn't used and the compiler stops getting confused.
+
+Upstream-Status: Pending
+RP 2012/11/13

Relying on the target config.h to build a host tool could fail if the build and target environments differ greatly, which is likely why mkbuiltins.c has hardcoded defines assuming less about the host (just POSIX) based on the CROSS_COMPILE define. I don't think removing that is the best fix, personally.

The reason for this failure has to do with a particular set of circumstances. A header in the bash source tree defines STRING() based on HAVE_STRINGIZE. This define overwrites the unistd.h define of the same macro. The unistd.h definitions of read() and open() wrap the call to the real functions to implement FORTIFY_SOURCES, and those wrappers use STRING() to do it. As a result, for any host that defaults to -DFORTIFY_SOURCES, STRING() returns 'x' resulting in a concatenation rather than an assembly level rename of the function being called.

If we add -DHAVE_STRINGIZE in the CROSS_COMPILE case, then STRING() will be defined to something useful, and therefore the FORTIFY_SOURCES wrappers don't get hosed.

See http://git.yoctoproject.org/cgit/cgit.cgi/meta-mentor/commit/?id=da0ff91for
an alternative fix which may be more likely to be accepted upstream.

Signature · 1 line
Christopher Larson
author
#2 lore raw
Quoted 33 lines · Chris Larson

On Tue, Nov 13, 2012 at 6:59 AM, Richard Purdie
<[email protected]> wrote:

        Signed-off-by: Richard Purdie
        <[email protected]>
        ---
        diff --git
        a/meta/recipes-extended/bash/bash-4.2/crossfix.patch
        b/meta/recipes-extended/bash/bash-4.2/crossfix.patch
        new file mode 100644
        index 0000000..f587c34
        --- a/dev/null
        +++ b/meta/recipes-extended/bash/bash-4.2/crossfix.patch
        @@ -0,0 +1,28 @@
        +Adding @CROSS_COMPILE@ to CFLAGS_FOR_BUILD causes errors
        like:
        +
        +mkbuiltins.o: In function `open':
        +/usr/include/x86_64-linux-gnu/bits/fcntl2.h:54: undefined
        reference to `xopen'
        +mkbuiltins.o: In function `read':
        +/usr/include/x86_64-linux-gnu/bits/unistd.h:45: undefined
        reference to `xread'
        +collect2: ld returned 1 exit status
        +
        +when compiling on a 64 bit x86 build system for a 32 bit x86
        target since
        +config.h confuses the compiler about settings. By removing
        the option, config.h
        +isn't used and the compiler stops getting confused.
        +
        +Upstream-Status: Pending
        +RP 2012/11/13
Quoted 5 lines

Relying on the target config.h to build a host tool could fail if the build and target environments differ greatly, which is likely why mkbuiltins.c has hardcoded defines assuming less about the host (just POSIX) based on the CROSS_COMPILE define. I don't think removing that is the best fix, personally.

Agreed. I'd actually misread that file and thought that it was using config.h in the CROSS_COMPILE case.

Quoted 10 lines

The reason for this failure has to do with a particular set of circumstances. A header in the bash source tree defines STRING() based on HAVE_STRINGIZE. This define overwrites the unistd.h define of the same macro. The unistd.h definitions of read() and open() wrap the call to the real functions to implement FORTIFY_SOURCES, and those wrappers use STRING() to do it. As a result, for any host that defaults to -DFORTIFY_SOURCES, STRING() returns 'x' resulting in a concatenation rather than an assembly level rename of the function being called.

Quoted 6 lines

If we add -DHAVE_STRINGIZE in the CROSS_COMPILE case, then STRING() will be defined to something useful, and therefore the FORTIFY_SOURCES wrappers don't get hosed.

See http://git.yoctoproject.org/cgit/cgit.cgi/meta-mentor/commit/?id=da0ff91 for an alternative fix which may be more likely to be accepted upstream.

This is better, yes. Is there a reason this is in meta-mentor and not OE-Core? I'd really like to pull it into OE-Core...

Cheers,

Richard

#3 lore raw
Quoted 14 lines · Richard Purdie
Quoted 14 lines

The reason for this failure has to do with a particular set of circumstances. A header in the bash source tree defines STRING() based on HAVE_STRINGIZE. This define overwrites the unistd.h define of the same macro. The unistd.h definitions of read() and open() wrap the call to the real functions to implement FORTIFY_SOURCES, and those wrappers use STRING() to do it. As a result, for any host that defaults to -DFORTIFY_SOURCES, STRING() returns 'x' resulting in a concatenation rather than an assembly level rename of the function being called.

If we add -DHAVE_STRINGIZE in the CROSS_COMPILE case, then STRING() will be defined to something useful, and therefore the FORTIFY_SOURCES wrappers don't get hosed.

Quoted 5 lines
Quoted 1 line

See

http://git.yoctoproject.org/cgit/cgit.cgi/meta-mentor/commit/?id=da0ff91for an alternative fix which may be more likely to be accepted upstream.

Quoted 1 line

This is better, yes. Is there a reason this is in meta-mentor and not OE-Core? I'd really like to pull it into OE-Core...

Nope, it's just in the pending upstream pile. I'll send a patch against oe-core to the list today if you'd like.

Signature · 1 line
Christopher Larson
author
#4 lore raw
Quoted 30 lines · Chris Larson

On Wed, Nov 14, 2012 at 6:01 AM, Richard Purdie
<[email protected]> wrote:
> The reason for this failure has to do with a particular set
of
> circumstances. A header in the bash source tree defines
STRING() based
> on HAVE_STRINGIZE. This define overwrites the unistd.h
define of the
> same macro. The unistd.h definitions of read() and open()
wrap the
> call to the real functions to implement FORTIFY_SOURCES, and
those
> wrappers use STRING() to do it. As a result, for any host
that
> defaults to -DFORTIFY_SOURCES, STRING() returns 'x'
resulting in a
> concatenation rather than an assembly level rename of the
function
> being called.
>
> If we add -DHAVE_STRINGIZE in the CROSS_COMPILE case, then
STRING()
> will be defined to something useful, and therefore the
FORTIFY_SOURCES
> wrappers don't get hosed.
>
> See
http://git.yoctoproject.org/cgit/cgit.cgi/meta-mentor/commit/?id=da0ff91 for an alternative fix which may be more likely to be accepted upstream. >

Quoted 6 lines

This is better, yes. Is there a reason this is in meta-mentor and not
OE-Core? I'd really like to pull it into OE-Core...

Nope, it's just in the pending upstream pile. I'll send a patch against oe-core to the list today if you'd like.

Please, I'd like to get that problem fixed.

Cheers,

Richard

#5 lore raw
In reply to Richard Purdie

meta/recipes-extended/bash/bash-4.2/crossfix.patch

Quoted 8 lines · Richard Purdie
Signed-off-by Richard Purdie <[email protected]>

---

meta/recipes-extended/bash/bash-4.2/crossfix.patch renamed from dev/null +0 -0
new file mode 100644
index 0000000..f587c34
@@ -0,0 +1,28 @@

Are you using some special tool to generate git patches or some weird git version?

Patches from you where you add some file usually does not apply here, It's caused by:
--- a/dev/null
+++ b/meta/recipes-extended/bash/bash-4.2/crossfix.patch which usually looks like
--- /dev/null
+++ b/meta/recipes-extended/bash/bash-4.2/crossfix.patch

2012-11-14 13:22:27 URL:http://patchwork.openembedded.org/patch/38919/mbox/ [2833] -> "pw-am-38919.patch" [1]

Applying: bash: Add fix for cross compile issues
fatal: git apply: bad git-diff - expected /dev/null on line 7 Patch failed at 0001 bash: Add fix for cross compile issues The copy of the patch that failed is found in:
/OE/openembedded-core/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort".

Cheers,

Signature · 1 line
Martin 'JaMa' Jansa     jabber: [email protected]
author
#6 lore raw
Quoted 12 lines · Martin Jansa

meta/recipes-extended/bash/bash-4.2/crossfix.patch

Quoted 8 lines · Richard Purdie
Signed-off-by Richard Purdie <[email protected]>

---

meta/recipes-extended/bash/bash-4.2/crossfix.patch renamed from dev/null +0 -0
new file mode 100644
index 0000000..f587c34
@@ -0,0 +1,28 @@

Are you using some special tool to generate git patches or some weird git version?

Basically, the data from cgit in the web interface is broken and that is causing this.

meta/recipes-extended/bash/bash-4.2/crossfix.patch

Quoted 7 lines

Patches from you where you add some file usually does not apply here, It's caused by:

meta/recipes-extended/bash/bash-4.2/crossfix.patch renamed from dev/null +0 -0

which usually looks like

meta/recipes-extended/bash/bash-4.2/crossfix.patch new file +0 -0

Right, there is a bug in cgit. I'm not sure if anyone fancies trying to find/fix it?

Cheers,

Richard

author
#7 lore raw
Quoted 17 lines · Richard Purdie
Quoted 12 lines · Martin Jansa

meta/recipes-extended/bash/bash-4.2/crossfix.patch

Quoted 8 lines · Richard Purdie
Signed-off-by Richard Purdie <[email protected]>

---

meta/recipes-extended/bash/bash-4.2/crossfix.patch renamed from dev/null +0 -0
new file mode 100644
index 0000000..f587c34
@@ -0,0 +1,28 @@

Are you using some special tool to generate git patches or some weird git version?

Basically, the data from cgit in the web interface is broken and that is causing this.

Quoted 10 lines

meta/recipes-extended/bash/bash-4.2/crossfix.patch

Quoted 7 lines

Patches from you where you add some file usually does not apply here, It's caused by:

meta/recipes-extended/bash/bash-4.2/crossfix.patch renamed from dev/null +0 -0

which usually looks like

meta/recipes-extended/bash/bash-4.2/crossfix.patch new file +0 -0

Right, there is a bug in cgit. I'm not sure if anyone fancies trying to find/fix it?

Actually, its not hard to find:

http://hjemli.net/git/cgit/tree/ui-patch.c

In header(), the:

		htmlf("\n--- a/%s\n", path1);
		htmlf("+++ b/%s\n", path2);
needs to be conditional on is_null_sha1(sha1)/is_null_sha1(sha2) with
the alternative of:
		htmlf("\n--- /%s\n", path1);
		htmlf("+++ /%s\n", path2);

which there are a variety of ways to achieve...

Michael: Fancy fixing that on the server and sending a patch upstream? :)

Cheers,

Richard

#8 lore raw
Quoted 37 lines · Richard Purdie
Quoted 15 lines · Richard Purdie
Quoted 11 lines · Martin Jansa

meta/recipes-extended/bash/bash-4.2/crossfix.patch

Quoted 8 lines · Richard Purdie
Signed-off-by Richard Purdie <[email protected]>

---

meta/recipes-extended/bash/bash-4.2/crossfix.patch renamed from dev/null +0 -0
new file mode 100644
index 0000000..f587c34
@@ -0,0 +1,28 @@

Are you using some special tool to generate git patches or some weird git version?

Basically, the data from cgit in the web interface is broken and that is causing this.

Quoted 9 lines

meta/recipes-extended/bash/bash-4.2/crossfix.patch

Quoted 7 lines

Patches from you where you add some file usually does not apply here, It's caused by:

meta/recipes-extended/bash/bash-4.2/crossfix.patch renamed from dev/null +0 -0

which usually looks like

meta/recipes-extended/bash/bash-4.2/crossfix.patch new file +0 -0

Right, there is a bug in cgit. I'm not sure if anyone fancies trying to find/fix it?

Actually, its not hard to find:

http://hjemli.net/git/cgit/tree/ui-patch.c

In header(), the:

		htmlf("\n--- a/%s\n", path1);
		htmlf("+++ b/%s\n", path2);
needs to be conditional on is_null_sha1(sha1)/is_null_sha1(sha2) with
the alternative of:
		htmlf("\n--- /%s\n", path1);
		htmlf("+++ /%s\n", path2);
Quoted 9 lines

which there are a variety of ways to achieve...

Michael: Fancy fixing that on the server and sending a patch upstream? :)

Cheers,

Richard

I'm happy to. I've fixed the code you've tracked down and recompiled for git.yoctoproject.org. You can see the result at
https://git.yoctoproject.org/cgit/cgit.cgi/poky/patch/?id=ed234aca98d0867c7b32801fc63820b19cf67df9
but I am still working on fixing the shared UI for this view
https://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=ed234aca98d0867c7b32801fc63820b19cf67df9.

Once I have both fixes I'll upgrade http://cgit.openembedded.org/ with the new version and submit my patch upstream.

Signature · 2 lines
Michael Halstead
Yocto Project / Sys Admin
#9 lore raw
Quoted 54 lines · Michael Halstead
Quoted 37 lines · Richard Purdie
Quoted 15 lines · Richard Purdie
Quoted 11 lines · Martin Jansa

meta/recipes-extended/bash/bash-4.2/crossfix.patch

Quoted 8 lines · Richard Purdie
Signed-off-by Richard Purdie <[email protected]>

---

meta/recipes-extended/bash/bash-4.2/crossfix.patch renamed from dev/null +0 -0
new file mode 100644
index 0000000..f587c34
@@ -0,0 +1,28 @@

Are you using some special tool to generate git patches or some weird git version?

Basically, the data from cgit in the web interface is broken and that is causing this.

Quoted 9 lines

meta/recipes-extended/bash/bash-4.2/crossfix.patch

Quoted 7 lines

Patches from you where you add some file usually does not apply here, It's caused by:

meta/recipes-extended/bash/bash-4.2/crossfix.patch renamed from dev/null +0 -0

which usually looks like

meta/recipes-extended/bash/bash-4.2/crossfix.patch new file +0 -0

Right, there is a bug in cgit. I'm not sure if anyone fancies trying to find/fix it?

Actually, its not hard to find:

http://hjemli.net/git/cgit/tree/ui-patch.c

In header(), the:

		htmlf("\n--- a/%s\n", path1);
		htmlf("+++ b/%s\n", path2);
needs to be conditional on is_null_sha1(sha1)/is_null_sha1(sha2) with
the alternative of:
		htmlf("\n--- /%s\n", path1);
		htmlf("+++ /%s\n", path2);
Quoted 9 lines

which there are a variety of ways to achieve...

Michael: Fancy fixing that on the server and sending a patch upstream? :)

Cheers,

Richard

I'm happy to. I've fixed the code you've tracked down and recompiled for git.yoctoproject.org. You can see the result at
https://git.yoctoproject.org/cgit/cgit.cgi/poky/patch/?id=ed234aca98d0867c7b32801fc63820b19cf67df9
but I am still working on fixing the shared UI for this view
https://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=ed234aca98d0867c7b32801fc63820b19cf67df9.

Quoted 3 lines

Once I have both fixes I'll upgrade http://cgit.openembedded.org/ with the new version and submit my patch upstream.

I've submitted the patch upstream and installed a fixed copy of cgit on cgit.openembedded.org. Please let me know if any errors in the formatting remain.

Michael Halstead
Yocto Project / Sys Admin

author
#10 lore raw
Quoted 55 lines · Michael Halstead
Quoted 54 lines · Michael Halstead
Quoted 37 lines · Richard Purdie
Quoted 15 lines · Richard Purdie
Quoted 11 lines · Martin Jansa

meta/recipes-extended/bash/bash-4.2/crossfix.patch

Quoted 8 lines · Richard Purdie
Signed-off-by Richard Purdie <[email protected]>

---

meta/recipes-extended/bash/bash-4.2/crossfix.patch renamed from dev/null +0 -0
new file mode 100644
index 0000000..f587c34
@@ -0,0 +1,28 @@

Are you using some special tool to generate git patches or some weird git version?

Basically, the data from cgit in the web interface is broken and that is causing this.

Quoted 9 lines

meta/recipes-extended/bash/bash-4.2/crossfix.patch

Quoted 7 lines

Patches from you where you add some file usually does not apply here, It's caused by:

meta/recipes-extended/bash/bash-4.2/crossfix.patch renamed from dev/null +0 -0

which usually looks like

meta/recipes-extended/bash/bash-4.2/crossfix.patch new file +0 -0

Right, there is a bug in cgit. I'm not sure if anyone fancies trying to find/fix it?

Actually, its not hard to find:

http://hjemli.net/git/cgit/tree/ui-patch.c

In header(), the:

		htmlf("\n--- a/%s\n", path1);
		htmlf("+++ b/%s\n", path2);
needs to be conditional on is_null_sha1(sha1)/is_null_sha1(sha2) with
the alternative of:
		htmlf("\n--- /%s\n", path1);
		htmlf("+++ /%s\n", path2);
Quoted 9 lines

which there are a variety of ways to achieve...

Michael: Fancy fixing that on the server and sending a patch upstream? :)

Cheers,

Richard

I'm happy to. I've fixed the code you've tracked down and recompiled for git.yoctoproject.org. You can see the result at
https://git.yoctoproject.org/cgit/cgit.cgi/poky/patch/?id=ed234aca98d0867c7b32801fc63820b19cf67df9
but I am still working on fixing the shared UI for this view
https://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=ed234aca98d0867c7b32801fc63820b19cf67df9.

Quoted 6 lines
Quoted 3 lines

Once I have both fixes I'll upgrade http://cgit.openembedded.org/ with the new version and submit my patch upstream.

I've submitted the patch upstream and installed a fixed copy of cgit on cgit.openembedded.org. Please let me know if any errors in the formatting remain.

Thanks for the fast turnaround, its much appreciated! :)

Cheers,

Richard

#11 lore raw
In reply to Richard Purdie
Quoted 14 lines · Richard Purdie
Quoted 12 lines · Martin Jansa

meta/recipes-extended/bash/bash-4.2/crossfix.patch

Quoted 8 lines · Richard Purdie
Signed-off-by Richard Purdie <[email protected]>

---

meta/recipes-extended/bash/bash-4.2/crossfix.patch renamed from dev/null +0 -0
new file mode 100644
index 0000000..f587c34
@@ -0,0 +1,28 @@

Are you using some special tool to generate git patches or some weird git version?

Quoted 2 lines

Basically, the data from cgit in the web interface is broken and that is causing this.

You're copy pasting cgit output to email when sending patches, or how is cgit involved in your patch flow?

I'm just curious, because I usually use only git send-email or create-pull-request script.

Cheers,

Quoted 15 lines

meta/recipes-extended/bash/bash-4.2/crossfix.patch

Quoted 7 lines

Patches from you where you add some file usually does not apply here, It's caused by:

meta/recipes-extended/bash/bash-4.2/crossfix.patch renamed from dev/null +0 -0

which usually looks like

meta/recipes-extended/bash/bash-4.2/crossfix.patch new file +0 -0

Right, there is a bug in cgit. I'm not sure if anyone fancies trying to find/fix it?

Cheers,

Richard

Signature · 1 line
Martin 'JaMa' Jansa     jabber: [email protected]