[Bug c/57862] New: invalid read struct uint32_t member (ARMV5)

[Bug c/57862] New: invalid read struct uint32_t member (ARMV5)

[mendola at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57862

            Bug ID: 57862
           Summary: invalid read struct uint32_t member (ARMV5)
           Product: gcc
           Version: 4.7.2
            Status: UNCONFIRMED
          Severity: critical
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: mendola at gmail dot com

Created attachment 30481
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30481&action=edit
The preprocessed file

# uname -a
Linux tqma28 2.6.35.3 #1 PREEMPT Sun Sep 11 17:38:39 CEST 2011 armv5tejl
GNU/Linux

# gcc-4.7 -v -save-temps main.c -march=armv5t
Using built-in specs.
COLLECT_GCC=gcc-4.7
COLLECT_LTO_WRAPPER=/usr/lib/gcc/arm-linux-gnueabi/4.7/lto-wrapper
Target: arm-linux-gnueabi
Configured with: ../src/configure -v --with-pkgversion='Debian 4.7.2-5'
--with-bugurl=file:///usr/share/doc/gcc-4.7/README.Bugs
--enable-languages=c,c++,go,fortran,objc,obj-c++ --prefix=/usr
--program-suffix=-4.7 --enable-shared --enable-linker-build-id
--with-system-zlib --libexecdir=/usr/lib --without-included-gettext
--enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.7
--libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu
--enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-gnu-unique-object
--disable-libitm --enable-plugin --enable-objc-gc --disable-sjlj-exceptions
--with-arch=armv4t --with-float=soft --enable-checking=release
--build=arm-linux-gnueabi --host=arm-linux-gnueabi --target=arm-linux-gnueabi
Thread model: posix
gcc version 4.7.2 (Debian 4.7.2-5) 
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-march=armv5t' '-mfloat-abi=soft'
'-mtls-dialect=gnu'
 /usr/lib/gcc/arm-linux-gnueabi/4.7/cc1 -E -quiet -v -imultilib . -imultiarch
arm-linux-gnueabi main.c -march=armv5t -mfloat-abi=soft -mtls-dialect=gnu
-fpch-preprocess -o main.i
ignoring nonexistent directory "/usr/local/include/arm-linux-gnueabi"
ignoring nonexistent directory
"/usr/lib/gcc/arm-linux-gnueabi/4.7/../../../../arm-linux-gnueabi/include"
#include "..." search starts here:
#include <...> search starts here:
 /usr/lib/gcc/arm-linux-gnueabi/4.7/include
 /usr/local/include
 /usr/lib/gcc/arm-linux-gnueabi/4.7/include-fixed
 /usr/include/arm-linux-gnueabi
 /usr/include
End of search list.
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-march=armv5t' '-mfloat-abi=soft'
'-mtls-dialect=gnu'
 /usr/lib/gcc/arm-linux-gnueabi/4.7/cc1 -fpreprocessed main.i -quiet -dumpbase
main.c -march=armv5t -mfloat-abi=soft -mtls-dialect=gnu -auxbase main -version
-o main.s
GNU C (Debian 4.7.2-5) version 4.7.2 (arm-linux-gnueabi)
        compiled by GNU C version 4.7.2, GMP version 5.0.5, MPFR version
3.1.0-p10, MPC version 0.9
GGC heuristics: --param ggc-min-expand=38 --param ggc-min-heapsize=15631
GNU C (Debian 4.7.2-5) version 4.7.2 (arm-linux-gnueabi)
        compiled by GNU C version 4.7.2, GMP version 5.0.5, MPFR version
3.1.0-p10, MPC version 0.9
GGC heuristics: --param ggc-min-expand=38 --param ggc-min-heapsize=15631
Compiler executable checksum: da7c3c2f0b4be4af23076cf3c1dfbf58
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-march=armv5t' '-mfloat-abi=soft'
'-mtls-dialect=gnu'
 as -v -march=armv5t -mfloat-abi=soft -meabi=5 -o main.o main.s
GNU assembler version 2.22 (arm-linux-gnueabi) using BFD version (GNU Binutils
for Debian) 2.22
COMPILER_PATH=/usr/lib/gcc/arm-linux-gnueabi/4.7/:/usr/lib/gcc/arm-linux-gnueabi/4.7/:/usr/lib/gcc/arm-linux-gnueabi/:/usr/lib/gcc/arm-linux-gnueabi/4.7/:/usr/lib/gcc/arm-linux-gnueabi/
LIBRARY_PATH=/usr/lib/gcc/arm-linux-gnueabi/4.7/:/usr/lib/gcc/arm-linux-gnueabi/4.7/../../../arm-linux-gnueabi/:/usr/lib/gcc/arm-linux-gnueabi/4.7/../../../:/lib/arm-linux-gnueabi/:/lib/:/usr/lib/arm-linux-gnueabi/:/usr/lib/
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-march=armv5t' '-mfloat-abi=soft'
'-mtls-dialect=gnu'
 /usr/lib/gcc/arm-linux-gnueabi/4.7/collect2 --sysroot=/ --build-id
--no-add-needed --eh-frame-hdr -dynamic-linker /lib/ld-linux.so.3 -X
--hash-style=both -m armelf_linux_eabi
/usr/lib/gcc/arm-linux-gnueabi/4.7/../../../arm-linux-gnueabi/crt1.o
/usr/lib/gcc/arm-linux-gnueabi/4.7/../../../arm-linux-gnueabi/crti.o
/usr/lib/gcc/arm-linux-gnueabi/4.7/crtbegin.o
-L/usr/lib/gcc/arm-linux-gnueabi/4.7
-L/usr/lib/gcc/arm-linux-gnueabi/4.7/../../../arm-linux-gnueabi
-L/usr/lib/gcc/arm-linux-gnueabi/4.7/../../.. -L/lib/arm-linux-gnueabi
-L/usr/lib/arm-linux-gnueabi main.o -lgcc --as-needed -lgcc_s --no-as-needed
-lc -lgcc --as-needed -lgcc_s --no-as-needed
/usr/lib/gcc/arm-linux-gnueabi/4.7/crtend.o
/usr/lib/gcc/arm-linux-gnueabi/4.7/../../../arm-linux-gnueabi/crtn.o


This is the following output executing it:

# ./a.out 
myInAddr.s_addr: 513
theIpHeader->daddr: 513
1.2.3.4
1.2.0.0

and that 1.2.0.0 should be 1.2.3.4. The same code works fine on ARMV7
architectures or x86

I believe that the culprit is in the assignment:

myInAddr.s_addr = theIpHeader->daddr;

note that replacing that assignment with:

memcpy(&myInAddr.s_addr, &(theIpHeader->daddr), sizeof(theIpHeader->daddr));

nothing changes, while using gcc 4.4 I get:

# ./a.out 
myInAddr.s_addr: 67305985
theIpHeader->daddr: 513
1.2.3.4
1.2.3.4

note now the bytes behind the memory are corrected reported as 1.2.3.4 but
printing the fields ad integer they report a wrong value.

[Bug c/57862] invalid read struct uint32_t member (ARMV5)

[mendola at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57862

--- Comment #2 from Gaetano Mendola <mendola at gmail dot com> ---
I had 0. Putting 2 or 3 fixed the problem. Now my question is: who is faulty? 
Kernel configuration on this platform, the architecture, the compiler or even
me ?
BTW, compiling that code with clang even with 0 in /proc/cpu/alignment gives
the right "result".

[Bug c/57862] invalid read struct uint32_t member (ARMV5)

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57862

--- Comment #3 from Mikael Pettersson <mikpe at it dot uu.se> ---
(In reply to Gaetano Mendola from comment #2)
> who is faulty? 
> Kernel configuration on this platform, the architecture, the compiler or
> even me ?

All of the above.  The architecture for getting mis-alignment very very wrong,
the kernel for not enforcing correct handling of alignment faults, the compiler
for sometimes generating mis-aligned accesses where the original code had none
(there are PRs about that affecting at least ARM and I believe also SPARC), and
your code for (apparently) having a load from a mis-aligned address where
portable code should use memcpy() (the fact that memcpy() didn't help you is a
consequence of one of those PRs).

My ARMv5TE box' /proc/cpu/alignment currently reads

User:           100669

all of which come from gcc's own test suite.  That's just so wrong.

[Bug c/57862] invalid read struct uint32_t member (ARMV5)

[mendola at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57862

--- Comment #4 from Gaetano Mendola <mendola at gmail dot com> ---
At this point I'm very puzzled. The fact I have to use memcpy instead of an
assignment for sake of portability is plain wrong.

Consider that only looking at:

  struct in_addr myInAddr;
  myInAddr.s_addr = theIpHeader->daddr;

I have no idea if the theIpHeader pointer was the result of a malloc or the
result of an assignment with a not aligned offset. Unless of course I inspect
the pointer value. 

The entire application (my application) suppose, as I think it should, that an
assignment of a value retrieved from a structure member is correctly done, note
that I'm not relying if this is done with 1 bus access, 2 or whatever. 

I'm going to put 2 in /proc/cpu/alignment and considering it as a kernel
configuration problem. 

btw clang behaves correctly even with a "kernel not well configured".

[Bug c/57862] invalid read struct uint32_t member (ARMV5)

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57862

--- Comment #5 from Mikael Pettersson <mikpe at it dot uu.se> ---
Your test case contains this:

===snip===
struct iphdr
  {
...
  };
...
int main()
{
  char thePacket[1518];
  memset(thePacket, 0, 1518);

  thePacket[30] = 1;
  thePacket[31] = 2;
  thePacket[32] = 3;
  thePacket[33] = 4;

  struct ether_header* theEthHeader = (struct ether_header*)(thePacket);

  struct iphdr* theIpHeader = (struct iphdr*)((const unsigned
char*)(theEthHeader) + 14);

  struct in_addr myInAddr;
  myInAddr.s_addr = theIpHeader->daddr;
===snip===

The alignment of thePacket is arbitrary, so the alignment of theIpHeader is
unknown, and struct iphdr is not declared with attribute packed.  The final
load may therefore be misaligned.

[Bug c/57862] invalid read struct uint32_t member (ARMV5)

[mendola at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57862

--- Comment #6 from Gaetano Mendola <mendola at gmail dot com> ---
That's clear to me.

I'm writing in C not in assembler, what I'm trying to understand is if I have
to
threat the following code:

  struct in_addr myInAddr;
  myInAddr.s_addr = theIpHeader->daddr;

as not portable, where myInAddr.s_addr and theIpHeader->daddr are of the same
type. I'm not a c-standard lawyer but I'm hard believing that in the standard
that assignment is marked as: "undefined behaviour" unless you use memcpy.

And again printf-ing the two values, both are reported on screen as the same
number but the bytes behind that area of memory do not contain the same values.

[Bug c/57862] invalid read struct uint32_t member (ARMV5)

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57862

--- Comment #7 from Mikael Pettersson <mikpe at it dot uu.se> ---
(In reply to Gaetano Mendola from comment #6)
>   struct in_addr myInAddr;
>   myInAddr.s_addr = theIpHeader->daddr;
> 
> as not portable, where myInAddr.s_addr and theIpHeader->daddr are of the
> same type. I'm not a c-standard lawyer but I'm hard believing that in the
> standard
> that assignment is marked as: "undefined behaviour" unless you use memcpy.

The assignment is immaterial, it's the load (theIpHeader->daddr) that's
problematic because the base pointer (theIpHeader) is not correctly aligned for
its declared type (struct iphdr).

[Bug c/57862] invalid read struct uint32_t member (ARMV5)

[rearnsha at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57862

Richard Earnshaw <rearnsha at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #8 from Richard Earnshaw <rearnsha at gcc dot gnu.org> ---
As Mikael says, this is undefined behaviour.