[ptxdist] [PATCH] libmodbus3: add patch for fixing alignment problem on arm

[ptxdist] [PATCH] libmodbus3: add patch for fixing alignment problem on arm

[Alexander Dahl]

We use -Wcast-align on ARMv5 platform and libmodbus3 sets -Werror
which leads to a failing build in one case. This patch fixes the
alignment problem. Upstream pull requested.

Signed-off-by: Alexander Dahl <post@lespocky.de>
---
 ...nt-problem-build-fails-with-Werror-and-Wc.patch |   33 ++++++++++++++++++++
 patches/libmodbus-3.0.3/series                     |    4 ++
 2 files changed, 37 insertions(+), 0 deletions(-)
 create mode 100644 patches/libmodbus-3.0.3/0001-fix-alignment-problem-build-fails-with-Werror-and-Wc.patch
 create mode 100644 patches/libmodbus-3.0.3/series

diff --git a/patches/libmodbus-3.0.3/0001-fix-alignment-problem-build-fails-with-Werror-and-Wc.patch b/patches/libmodbus-3.0.3/0001-fix-alignment-problem-build-fails-with-Werror-and-Wc.patch
new file mode 100644
index 0000000..88db0df
--- /dev/null
+++ b/patches/libmodbus-3.0.3/0001-fix-alignment-problem-build-fails-with-Werror-and-Wc.patch
@@ -0,0 +1,33 @@
+From: Alexander Dahl <ada@ifak-system.com>
+Date: Wed, 13 Mar 2013 16:45:04 +0100
+Subject: [PATCH] fix alignment problem (build fails with -Werror and
+ -Wcast-align on ARMv5 platform)
+
+---
+ tests/unit-test-client.c |    6 ++++--
+ 1 file changed, 4 insertions(+), 2 deletions(-)
+
+diff --git a/tests/unit-test-client.c b/tests/unit-test-client.c
+index 533e6a7..7c3c64d 100644
+--- a/tests/unit-test-client.c
++++ b/tests/unit-test-client.c
+@@ -41,6 +41,7 @@ int main(int argc, char *argv[])
+     int nb_points;
+     int rc;
+     float real;
++    uint32_t ireal;
+     struct timeval old_response_timeout;
+     struct timeval response_timeout;
+     int use_backend;
+@@ -325,8 +326,9 @@ int main(int argc, char *argv[])
+         tab_rp_registers[0] == (UT_IREAL & 0xFFFF)) {
+         printf("OK\n");
+     } else {
+-        printf("FAILED (%x != %x)\n",
+-               *((uint32_t *)tab_rp_registers), UT_IREAL);
++        ireal = (uint32_t) tab_rp_registers[0] & 0xFFFF;
++        ireal |= (uint32_t) tab_rp_registers[1] << 16;
++        printf("FAILED (%x != %x)\n", ireal, UT_IREAL);
+         goto close;
+     }
+ 
diff --git a/patches/libmodbus-3.0.3/series b/patches/libmodbus-3.0.3/series
new file mode 100644
index 0000000..9d8674a
--- /dev/null
+++ b/patches/libmodbus-3.0.3/series
@@ -0,0 +1,4 @@
+# generated by git-ptx-patches
+#tag:base --start-number 1
+0001-fix-alignment-problem-build-fails-with-Werror-and-Wc.patch
+# d7fc60de785d0108680a1d6983f01a48  - git-ptx-patches magic
-- 
1.7.2.5


-- 
ptxdist mailing list
ptxdist@pengutronix.de

Re: [ptxdist] [PATCH] libmodbus3: add patch for fixing alignment problem on arm

[Michael Olbrich]

On Thu, Mar 14, 2013 at 01:16:36PM +0100, Alexander Dahl wrote:
> We use -Wcast-align on ARMv5 platform and libmodbus3 sets -Werror
> which leads to a failing build in one case. This patch fixes the
> alignment problem. Upstream pull requested.

Hmm, this is just unit test stuff, right? We don't actually need this.
Maybe we should do this instead:

LIBMODBUS3_MAKE_OPT	:= -C src
LIBMODBUS3_INSTALL_OPT	:= -C src install

Michael

> Signed-off-by: Alexander Dahl <post@lespocky.de>
> ---
>  ...nt-problem-build-fails-with-Werror-and-Wc.patch |   33 ++++++++++++++++++++
>  patches/libmodbus-3.0.3/series                     |    4 ++
>  2 files changed, 37 insertions(+), 0 deletions(-)
>  create mode 100644 patches/libmodbus-3.0.3/0001-fix-alignment-problem-build-fails-with-Werror-and-Wc.patch
>  create mode 100644 patches/libmodbus-3.0.3/series
> 
> diff --git a/patches/libmodbus-3.0.3/0001-fix-alignment-problem-build-fails-with-Werror-and-Wc.patch b/patches/libmodbus-3.0.3/0001-fix-alignment-problem-build-fails-with-Werror-and-Wc.patch
> new file mode 100644
> index 0000000..88db0df
> --- /dev/null
> +++ b/patches/libmodbus-3.0.3/0001-fix-alignment-problem-build-fails-with-Werror-and-Wc.patch
> @@ -0,0 +1,33 @@
> +From: Alexander Dahl <ada@ifak-system.com>
> +Date: Wed, 13 Mar 2013 16:45:04 +0100
> +Subject: [PATCH] fix alignment problem (build fails with -Werror and
> + -Wcast-align on ARMv5 platform)
> +
> +---
> + tests/unit-test-client.c |    6 ++++--
> + 1 file changed, 4 insertions(+), 2 deletions(-)
> +
> +diff --git a/tests/unit-test-client.c b/tests/unit-test-client.c
> +index 533e6a7..7c3c64d 100644
> +--- a/tests/unit-test-client.c
> ++++ b/tests/unit-test-client.c
> +@@ -41,6 +41,7 @@ int main(int argc, char *argv[])
> +     int nb_points;
> +     int rc;
> +     float real;
> ++    uint32_t ireal;
> +     struct timeval old_response_timeout;
> +     struct timeval response_timeout;
> +     int use_backend;
> +@@ -325,8 +326,9 @@ int main(int argc, char *argv[])
> +         tab_rp_registers[0] == (UT_IREAL & 0xFFFF)) {
> +         printf("OK\n");
> +     } else {
> +-        printf("FAILED (%x != %x)\n",
> +-               *((uint32_t *)tab_rp_registers), UT_IREAL);
> ++        ireal = (uint32_t) tab_rp_registers[0] & 0xFFFF;
> ++        ireal |= (uint32_t) tab_rp_registers[1] << 16;
> ++        printf("FAILED (%x != %x)\n", ireal, UT_IREAL);
> +         goto close;
> +     }
> + 
> diff --git a/patches/libmodbus-3.0.3/series b/patches/libmodbus-3.0.3/series
> new file mode 100644
> index 0000000..9d8674a
> --- /dev/null
> +++ b/patches/libmodbus-3.0.3/series
> @@ -0,0 +1,4 @@
> +# generated by git-ptx-patches
> +#tag:base --start-number 1
> +0001-fix-alignment-problem-build-fails-with-Werror-and-Wc.patch
> +# d7fc60de785d0108680a1d6983f01a48  - git-ptx-patches magic
> -- 
> 1.7.2.5
> 
> 
> -- 
> ptxdist mailing list
> ptxdist@pengutronix.de
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

-- 
ptxdist mailing list
ptxdist@pengutronix.de

Re: [ptxdist] [PATCH] libmodbus3: add patch for fixing alignment problem on arm

[Alexander Dahl]

Hei Michael, 

Am 2013-03-19 16:32, schrieb Michael Olbrich:
> On Thu, Mar 14, 2013 at 01:16:36PM +0100, Alexander Dahl wrote:
>> We use -Wcast-align on ARMv5 platform and libmodbus3 sets -Werror
>> which leads to a failing build in one case. This patch fixes the
>> alignment problem. Upstream pull requested.
> 
> Hmm, this is just unit test stuff, right? We don't actually need this.

Yes, I think so. The targetinstall rule does not install these binaries
anyway.

> Maybe we should do this instead:
> 
> LIBMODBUS3_MAKE_OPT	:= -C src
> LIBMODBUS3_INSTALL_OPT	:= -C src install

My colleague removed 'tests' from Makefile.am, I don't know which one
would be the best choice.

Greets
Alex

-- 
»With the first link, the chain is forged. The first speech censured,
the first thought forbidden, the first freedom denied, chains us all
irrevocably.« (Jean-Luc Picard, quoting Judge Aaron Satie)
*** GnuPG-FP: 02C8 A590 7FE5 CA5F 3601  D1D5 8FBA 7744 CC87 10D0 ***

-- 
ptxdist mailing list
ptxdist@pengutronix.de

Re: [ptxdist] [PATCH] libmodbus3: add patch for fixing alignment problem on arm

[Michael Olbrich]

On Tue, Mar 19, 2013 at 04:55:37PM +0100, Alexander Dahl wrote:
> Hei Michael, 
> 
> Am 2013-03-19 16:32, schrieb Michael Olbrich:
> > On Thu, Mar 14, 2013 at 01:16:36PM +0100, Alexander Dahl wrote:
> >> We use -Wcast-align on ARMv5 platform and libmodbus3 sets -Werror
> >> which leads to a failing build in one case. This patch fixes the
> >> alignment problem. Upstream pull requested.
> > 
> > Hmm, this is just unit test stuff, right? We don't actually need this.
> 
> Yes, I think so. The targetinstall rule does not install these binaries
> anyway.
> 
> > Maybe we should do this instead:
> > 
> > LIBMODBUS3_MAKE_OPT	:= -C src
> > LIBMODBUS3_INSTALL_OPT	:= -C src install
> 
> My colleague removed 'tests' from Makefile.am, I don't know which one
> would be the best choice.

I prefer to avoid patches when possible.

Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

-- 
ptxdist mailing list
ptxdist@pengutronix.de

Re: [ptxdist] [PATCH] libmodbus3: add patch for fixing alignment problem on arm

[Alexander Dahl]

Hei hei, 

Am 2013-03-19 17:27, schrieb Michael Olbrich:
>> > LIBMODBUS3_MAKE_OPT	:= -C src
>> > LIBMODBUS3_INSTALL_OPT	:= -C src install
>>
>> My colleague removed 'tests' from Makefile.am, I don't know which one
>> would be the best choice.
> 
> I prefer to avoid patches when possible.

Meanwhile the fix for this test tool was integrated upstream and
probably will be part of the next release. 

However I have the changes you suggested for the rule file ready and
tested. Do you still want it or keep it simple and just bump the version
when the next release comes out?

Greets
Alex

-- 
»With the first link, the chain is forged. The first speech censured,
the first thought forbidden, the first freedom denied, chains us all
irrevocably.« (Jean-Luc Picard, quoting Judge Aaron Satie)
*** GnuPG-FP: 02C8 A590 7FE5 CA5F 3601  D1D5 8FBA 7744 CC87 10D0 ***

-- 
ptxdist mailing list
ptxdist@pengutronix.de

Re: [ptxdist] [PATCH] libmodbus3: add patch for fixing alignment problem on arm

[Michael Olbrich]

On Tue, Mar 26, 2013 at 10:04:30AM +0100, Alexander Dahl wrote:
> Am 2013-03-19 17:27, schrieb Michael Olbrich:
> >> > LIBMODBUS3_MAKE_OPT	:= -C src
> >> > LIBMODBUS3_INSTALL_OPT	:= -C src install
> >>
> >> My colleague removed 'tests' from Makefile.am, I don't know which one
> >> would be the best choice.
> > 
> > I prefer to avoid patches when possible.
> 
> Meanwhile the fix for this test tool was integrated upstream and
> probably will be part of the next release. 

That's always good.

> However I have the changes you suggested for the rule file ready and
> tested. Do you still want it or keep it simple and just bump the version
> when the next release comes out?

When does this issue happen anyways? I've never seen it.
But I like the change, because we compile less that way.

Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

-- 
ptxdist mailing list
ptxdist@pengutronix.de

Re: [ptxdist] [PATCH] libmodbus3: add patch for fixing alignment problem on arm

[Alexander Dahl]

Hei hei, 

Am 2013-03-27 18:57, schrieb Michael Olbrich:
> When does this issue happen anyways? I've never seen it.

The problem is in one of the test programs, code was like this before:

  uint16_t *tab_rp_registers;
  /* load some data to this poiner */
  *((uint32_t *)tab_rp_registers) = something_else;

On ARMv5 (which we use here) and older the CPU can not access memory
bytewise, so fetching an 2 or 4 byte integer from an unaligned memory
address leads to wrong data. Get a detailed explanation here:
http://lecs.cs.ucla.edu/wiki/index.php/XScale_alignment

The cast from an *uint16_t to an *uint32_t could lead to such an
unaligned access. You can let the compiler spot such things and warn you
with -Wcast-align (which is btw not part of -Wall) and we set
-Wcast-align globally to spot those things in our own code for ARMv5
architecture.

The problem is with libmodbus3 setting -Werror which leads to all
warnings be handled as errors and thus the failing build.

In short: the combination of -Werror in libmodbus and -Wcast-align in
our PTXCONF_TARGET_EXTRA_CFLAGS triggered this.

> But I like the change, because we compile less that way.

Someone who wants the test would have to edit the rules then, but she
would need to edit the targetinstall rule then anyway.

Greets
Alex

-- 
»With the first link, the chain is forged. The first speech censured,
the first thought forbidden, the first freedom denied, chains us all
irrevocably.« (Jean-Luc Picard, quoting Judge Aaron Satie)
*** GnuPG-FP: 02C8 A590 7FE5 CA5F 3601  D1D5 8FBA 7744 CC87 10D0 ***

-- 
ptxdist mailing list
ptxdist@pengutronix.de

Re: [ptxdist] [PATCH] libmodbus3: add patch for fixing alignment problem on arm

[Michael Olbrich]

Hi,

On Thu, Mar 28, 2013 at 08:46:59AM +0100, Alexander Dahl wrote:
> Am 2013-03-27 18:57, schrieb Michael Olbrich:
> > When does this issue happen anyways? I've never seen it.
> 
> The problem is in one of the test programs, code was like this before:
> 
>   uint16_t *tab_rp_registers;
>   /* load some data to this poiner */
>   *((uint32_t *)tab_rp_registers) = something_else;
> 
> On ARMv5 (which we use here) and older the CPU can not access memory
> bytewise, so fetching an 2 or 4 byte integer from an unaligned memory
> address leads to wrong data. Get a detailed explanation here:
> http://lecs.cs.ucla.edu/wiki/index.php/XScale_alignment
> 
> The cast from an *uint16_t to an *uint32_t could lead to such an
> unaligned access. You can let the compiler spot such things and warn you
> with -Wcast-align (which is btw not part of -Wall) and we set
> -Wcast-align globally to spot those things in our own code for ARMv5
> architecture.
> 
> The problem is with libmodbus3 setting -Werror which leads to all
> warnings be handled as errors and thus the failing build.

Ah, here is the missing link. I know the alignment issue, but I also know
that this usually doesn't generate build errors.

> In short: the combination of -Werror in libmodbus and -Wcast-align in
> our PTXCONF_TARGET_EXTRA_CFLAGS triggered this.
> 
> > But I like the change, because we compile less that way.
> 
> Someone who wants the test would have to edit the rules then, but she
> would need to edit the targetinstall rule then anyway.

True.

Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

-- 
ptxdist mailing list
ptxdist@pengutronix.de