Re: [U-Boot] [PATCH 1/1] net: remove superfluous __packed attribute
On Mon, Nov 04, 2019 at 07:07:46PM +0100, Heinrich Schuchardt wrote:
> On 10/12/19 3:29 PM, Heinrich Schuchardt wrote:
> > On 10/3/19 8:01 PM, Heinrich Schuchardt wrote:
> > > On 10/3/19 7:09 PM, Tom Rini wrote:
> > > > On Thu, Oct 03, 2019 at 07:01:22PM +0200, Heinrich Schuchardt wrote:
> > > >
> > > > > struct ip_udp_hdr is naturally packed. There is no point in adding a
> > > > > __packed attribute. With the attribute the network stack does not
> > > > > compile
> > > > > using GCC 9.2.1:
> > > > >
> > > > > net/net.c: In function ‘net_process_received_packet’:
> > > > > net/net.c:1288:23: error: taking address of packed member of ‘struct
> > > > > ip_udp_hdr’ may result in an unaligned pointer value
> > > > > [-Werror=address-of-packed-member]
> > > > > 1288 | sumptr = (ushort *)&(ip->udp_src);
> > > > > | ^~
> > > > >
> > > > > So let's remove the attribute.
> > > > >
> > > > > Signed-off-by: Heinrich Schuchardt
> > > > > ---
> > > > > include/net.h | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/include/net.h b/include/net.h
> > > > > index 75a16e4c8f..bd875b56f5 100644
> > > > > --- a/include/net.h
> > > > > +++ b/include/net.h
> > > > > @@ -390,7 +390,7 @@ struct ip_udp_hdr {
> > > > > u16 udp_dst; /* UDP destination port */
> > > > > u16 udp_len; /* Length of UDP packet */
> > > > > u16 udp_xsum; /* Checksum */
> > > > > -} __attribute__((packed));
> > > > > +};
> > > > >
> > > > > #define IP_UDP_HDR_SIZE (sizeof(struct ip_udp_hdr))
> > > > > #define UDP_HDR_SIZE (IP_UDP_HDR_SIZE - IP_HDR_SIZE)
> > > >
> > > > This came from:
> > > > commit 704f3acfcf55343043bbed01c5fb0a0094a68e8a
> > > > Author: Denis Pynkin
> > > > Date: Fri Jul 21 19:28:42 2017 +0300
> > > >
> > > > net: Use packed structures for networking
> > > >
> > > > PXE boot is broken with GCC 7.1 due option '-fstore-merging'
> > > > enabled
> > > > by default for '-O2':
> > > >
> > > > BOOTP broadcast 1
> > > > data abort
> > > > pc : [<8ff8bb30>] lr : [<4f1f>]
> > > > reloc pc : [<17832b30>] lr : [<878abf1f>]
> > > > sp : 8f558bc0 ip : fp : 8ffef5a4
> > > > r10: 8ffed248 r9 : 8f558ee0 r8 : 8ffef594
> > > > r7 : 000e r6 : 8ffed700 r5 : r4 : 8ffed74e
> > > > r3 : 00060101 r2 : 8ffed230 r1 : 8ffed706 r0 : 0ddd
> > > > Flags: nzcv IRQs off FIQs off Mode SVC_32
> > > > Resetting CPU ...
> > > >
> > > > Core reason is usage of structures for network headers without
> > > > packed
> > > > attribute.
> > > >
> > > > Reviewed-by: Yauheni Kaliuta
> > > > Signed-off-by: Denis Pynkin
> > > > Acked-by: Joe Hershberger
> > > >
> > > > ... so adding a few folks to the list and lets see if patchwork picks up
> > > > this tag:
> > > > Fixes: 704f3acfcf55 ("net: Use packed structures for networking")
> > > >
> > >
> > > Thanks for providing this background.
> > >
> > > -fstore-merging is enabled for O2 and Os. But it should not change the
> > > program behavior.
> > >
> > > The relevant code is in gcc/gimple-ssa-store-merging.c. It has been
> > > completely overhauled since GCC 7.1.
> > >
> > > I have seen no problems with DHCP, ping and tFTP on arm and arm64 with
> > > GCC 9.2.1.
> > >
> > > Best regards
> > >
> > > Heinrich
> > >
> > > Since GCC 7.1 some bugs have been fixed, e.g.
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86492
> > >
> >
> > Hello Joe,
> >
> > how will we carry on, to get the issue resolved?
> >
> > Best regards
> >
> > Heinrich
> > ___
>
> Hello Tom, hello Joe,
>
> I still cannot build sandbox_defconfig. How will we resolve this issue?
We can't drop gcc-7.1 support at this point. Perhaps use an accessor
function of some sort?
--
Tom
signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] net: remove superfluous __packed attribute
On 10/12/19 3:29 PM, Heinrich Schuchardt wrote:
On 10/3/19 8:01 PM, Heinrich Schuchardt wrote:
On 10/3/19 7:09 PM, Tom Rini wrote:
On Thu, Oct 03, 2019 at 07:01:22PM +0200, Heinrich Schuchardt wrote:
struct ip_udp_hdr is naturally packed. There is no point in adding a
__packed attribute. With the attribute the network stack does not
compile
using GCC 9.2.1:
net/net.c: In function ‘net_process_received_packet’:
net/net.c:1288:23: error: taking address of packed member of ‘struct
ip_udp_hdr’ may result in an unaligned pointer value
[-Werror=address-of-packed-member]
1288 | sumptr = (ushort *)&(ip->udp_src);
| ^~
So let's remove the attribute.
Signed-off-by: Heinrich Schuchardt
---
include/net.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net.h b/include/net.h
index 75a16e4c8f..bd875b56f5 100644
--- a/include/net.h
+++ b/include/net.h
@@ -390,7 +390,7 @@ struct ip_udp_hdr {
u16 udp_dst; /* UDP destination port */
u16 udp_len; /* Length of UDP packet */
u16 udp_xsum; /* Checksum */
-} __attribute__((packed));
+};
#define IP_UDP_HDR_SIZE (sizeof(struct ip_udp_hdr))
#define UDP_HDR_SIZE (IP_UDP_HDR_SIZE - IP_HDR_SIZE)
This came from:
commit 704f3acfcf55343043bbed01c5fb0a0094a68e8a
Author: Denis Pynkin
Date: Fri Jul 21 19:28:42 2017 +0300
net: Use packed structures for networking
PXE boot is broken with GCC 7.1 due option '-fstore-merging'
enabled
by default for '-O2':
BOOTP broadcast 1
data abort
pc : [<8ff8bb30>] lr : [<4f1f>]
reloc pc : [<17832b30>] lr : [<878abf1f>]
sp : 8f558bc0 ip : fp : 8ffef5a4
r10: 8ffed248 r9 : 8f558ee0 r8 : 8ffef594
r7 : 000e r6 : 8ffed700 r5 : r4 : 8ffed74e
r3 : 00060101 r2 : 8ffed230 r1 : 8ffed706 r0 : 0ddd
Flags: nzcv IRQs off FIQs off Mode SVC_32
Resetting CPU ...
Core reason is usage of structures for network headers without
packed
attribute.
Reviewed-by: Yauheni Kaliuta
Signed-off-by: Denis Pynkin
Acked-by: Joe Hershberger
... so adding a few folks to the list and lets see if patchwork picks up
this tag:
Fixes: 704f3acfcf55 ("net: Use packed structures for networking")
Thanks for providing this background.
-fstore-merging is enabled for O2 and Os. But it should not change the
program behavior.
The relevant code is in gcc/gimple-ssa-store-merging.c. It has been
completely overhauled since GCC 7.1.
I have seen no problems with DHCP, ping and tFTP on arm and arm64 with
GCC 9.2.1.
Best regards
Heinrich
Since GCC 7.1 some bugs have been fixed, e.g.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86492
Hello Joe,
how will we carry on, to get the issue resolved?
Best regards
Heinrich
___
Hello Tom, hello Joe,
I still cannot build sandbox_defconfig. How will we resolve this issue?
Best regards
Heinrich
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] net: remove superfluous __packed attribute
On 10/3/19 8:01 PM, Heinrich Schuchardt wrote:
On 10/3/19 7:09 PM, Tom Rini wrote:
On Thu, Oct 03, 2019 at 07:01:22PM +0200, Heinrich Schuchardt wrote:
struct ip_udp_hdr is naturally packed. There is no point in adding a
__packed attribute. With the attribute the network stack does not
compile
using GCC 9.2.1:
net/net.c: In function ‘net_process_received_packet’:
net/net.c:1288:23: error: taking address of packed member of ‘struct
ip_udp_hdr’ may result in an unaligned pointer value
[-Werror=address-of-packed-member]
1288 | sumptr = (ushort *)&(ip->udp_src);
| ^~
So let's remove the attribute.
Signed-off-by: Heinrich Schuchardt
---
include/net.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net.h b/include/net.h
index 75a16e4c8f..bd875b56f5 100644
--- a/include/net.h
+++ b/include/net.h
@@ -390,7 +390,7 @@ struct ip_udp_hdr {
u16 udp_dst; /* UDP destination port */
u16 udp_len; /* Length of UDP packet */
u16 udp_xsum; /* Checksum */
-} __attribute__((packed));
+};
#define IP_UDP_HDR_SIZE (sizeof(struct ip_udp_hdr))
#define UDP_HDR_SIZE (IP_UDP_HDR_SIZE - IP_HDR_SIZE)
This came from:
commit 704f3acfcf55343043bbed01c5fb0a0094a68e8a
Author: Denis Pynkin
Date: Fri Jul 21 19:28:42 2017 +0300
net: Use packed structures for networking
PXE boot is broken with GCC 7.1 due option '-fstore-merging' enabled
by default for '-O2':
BOOTP broadcast 1
data abort
pc : [<8ff8bb30>] lr : [<4f1f>]
reloc pc : [<17832b30>] lr : [<878abf1f>]
sp : 8f558bc0 ip : fp : 8ffef5a4
r10: 8ffed248 r9 : 8f558ee0 r8 : 8ffef594
r7 : 000e r6 : 8ffed700 r5 : r4 : 8ffed74e
r3 : 00060101 r2 : 8ffed230 r1 : 8ffed706 r0 : 0ddd
Flags: nzcv IRQs off FIQs off Mode SVC_32
Resetting CPU ...
Core reason is usage of structures for network headers without
packed
attribute.
Reviewed-by: Yauheni Kaliuta
Signed-off-by: Denis Pynkin
Acked-by: Joe Hershberger
... so adding a few folks to the list and lets see if patchwork picks up
this tag:
Fixes: 704f3acfcf55 ("net: Use packed structures for networking")
Thanks for providing this background.
-fstore-merging is enabled for O2 and Os. But it should not change the
program behavior.
The relevant code is in gcc/gimple-ssa-store-merging.c. It has been
completely overhauled since GCC 7.1.
I have seen no problems with DHCP, ping and tFTP on arm and arm64 with
GCC 9.2.1.
Best regards
Heinrich
Since GCC 7.1 some bugs have been fixed, e.g.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86492
Hello Joe,
how will we carry on, to get the issue resolved?
Best regards
Heinrich
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] net: remove superfluous __packed attribute
On 10/3/19 7:09 PM, Tom Rini wrote:
On Thu, Oct 03, 2019 at 07:01:22PM +0200, Heinrich Schuchardt wrote:
struct ip_udp_hdr is naturally packed. There is no point in adding a
__packed attribute. With the attribute the network stack does not compile
using GCC 9.2.1:
net/net.c: In function ‘net_process_received_packet’:
net/net.c:1288:23: error: taking address of packed member of ‘struct
ip_udp_hdr’ may result in an unaligned pointer value
[-Werror=address-of-packed-member]
1288 |sumptr = (ushort *)&(ip->udp_src);
| ^~
So let's remove the attribute.
Signed-off-by: Heinrich Schuchardt
---
include/net.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net.h b/include/net.h
index 75a16e4c8f..bd875b56f5 100644
--- a/include/net.h
+++ b/include/net.h
@@ -390,7 +390,7 @@ struct ip_udp_hdr {
u16 udp_dst;/* UDP destination port */
u16 udp_len;/* Length of UDP packet */
u16 udp_xsum; /* Checksum */
-} __attribute__((packed));
+};
#define IP_UDP_HDR_SIZE (sizeof(struct ip_udp_hdr))
#define UDP_HDR_SIZE (IP_UDP_HDR_SIZE - IP_HDR_SIZE)
This came from:
commit 704f3acfcf55343043bbed01c5fb0a0094a68e8a
Author: Denis Pynkin
Date: Fri Jul 21 19:28:42 2017 +0300
net: Use packed structures for networking
PXE boot is broken with GCC 7.1 due option '-fstore-merging' enabled
by default for '-O2':
BOOTP broadcast 1
data abort
pc : [<8ff8bb30>] lr : [<4f1f>]
reloc pc : [<17832b30>]lr : [<878abf1f>]
sp : 8f558bc0 ip : fp : 8ffef5a4
r10: 8ffed248 r9 : 8f558ee0 r8 : 8ffef594
r7 : 000e r6 : 8ffed700 r5 : r4 : 8ffed74e
r3 : 00060101 r2 : 8ffed230 r1 : 8ffed706 r0 : 0ddd
Flags: nzcv IRQs off FIQs off Mode SVC_32
Resetting CPU ...
Core reason is usage of structures for network headers without packed
attribute.
Reviewed-by: Yauheni Kaliuta
Signed-off-by: Denis Pynkin
Acked-by: Joe Hershberger
... so adding a few folks to the list and lets see if patchwork picks up
this tag:
Fixes: 704f3acfcf55 ("net: Use packed structures for networking")
Thanks for providing this background.
-fstore-merging is enabled for O2 and Os. But it should not change the
program behavior.
The relevant code is in gcc/gimple-ssa-store-merging.c. It has been
completely overhauled since GCC 7.1.
I have seen no problems with DHCP, ping and tFTP on arm and arm64 with
GCC 9.2.1.
Best regards
Heinrich
Since GCC 7.1 some bugs have been fixed, e.g.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86492
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] net: remove superfluous __packed attribute
On Thu, Oct 03, 2019 at 07:01:22PM +0200, Heinrich Schuchardt wrote:
> struct ip_udp_hdr is naturally packed. There is no point in adding a
> __packed attribute. With the attribute the network stack does not compile
> using GCC 9.2.1:
>
> net/net.c: In function ‘net_process_received_packet’:
> net/net.c:1288:23: error: taking address of packed member of ‘struct
> ip_udp_hdr’ may result in an unaligned pointer value
> [-Werror=address-of-packed-member]
> 1288 |sumptr = (ushort *)&(ip->udp_src);
> | ^~
>
> So let's remove the attribute.
>
> Signed-off-by: Heinrich Schuchardt
> ---
> include/net.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/net.h b/include/net.h
> index 75a16e4c8f..bd875b56f5 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -390,7 +390,7 @@ struct ip_udp_hdr {
> u16 udp_dst;/* UDP destination port */
> u16 udp_len;/* Length of UDP packet */
> u16 udp_xsum; /* Checksum */
> -} __attribute__((packed));
> +};
>
> #define IP_UDP_HDR_SIZE (sizeof(struct ip_udp_hdr))
> #define UDP_HDR_SIZE (IP_UDP_HDR_SIZE - IP_HDR_SIZE)
This came from:
commit 704f3acfcf55343043bbed01c5fb0a0094a68e8a
Author: Denis Pynkin
Date: Fri Jul 21 19:28:42 2017 +0300
net: Use packed structures for networking
PXE boot is broken with GCC 7.1 due option '-fstore-merging' enabled
by default for '-O2':
BOOTP broadcast 1
data abort
pc : [<8ff8bb30>] lr : [<4f1f>]
reloc pc : [<17832b30>]lr : [<878abf1f>]
sp : 8f558bc0 ip : fp : 8ffef5a4
r10: 8ffed248 r9 : 8f558ee0 r8 : 8ffef594
r7 : 000e r6 : 8ffed700 r5 : r4 : 8ffed74e
r3 : 00060101 r2 : 8ffed230 r1 : 8ffed706 r0 : 0ddd
Flags: nzcv IRQs off FIQs off Mode SVC_32
Resetting CPU ...
Core reason is usage of structures for network headers without packed
attribute.
Reviewed-by: Yauheni Kaliuta
Signed-off-by: Denis Pynkin
Acked-by: Joe Hershberger
... so adding a few folks to the list and lets see if patchwork picks up
this tag:
Fixes: 704f3acfcf55 ("net: Use packed structures for networking")
--
Tom
signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
