Re: [ptxdist] [PATCH] libmodbus3: add patch for fixing alignment problem on arm
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
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- | -- ptxdist mailing list ptxdist@pengutronix.de
Re: [ptxdist] [PATCH] libmodbus3: add patch for fixing alignment problem on arm
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- | -- ptxdist mailing list ptxdist@pengutronix.de
Re: [ptxdist] [PATCH] libmodbus3: add patch for fixing alignment problem on arm
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
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 p...@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 000..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 a...@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 0x)) {
+ printf(OK\n);
+ } else {
+-printf(FAILED (%x != %x)\n,
+- *((uint32_t *)tab_rp_registers), UT_IREAL);
++ireal = (uint32_t) tab_rp_registers[0] 0x;
++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 000..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- |
--
ptxdist mailing list
ptxdist@pengutronix.de
Re: [ptxdist] [PATCH] libmodbus3: add patch for fixing alignment problem on arm
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
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- | -- ptxdist mailing list ptxdist@pengutronix.de
