Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

A bug story: data alignment on x86

pzemtsov.github.io

Post reply on HN
Print view View original

61–70 of 111 posts

Re: A bug story: data alignment on x86

#61

Post by Thiez »

tropo wrote: post #40
qb45 wrote: post #11

The correct solution for GCC is specifying 1-byte alignment for this particular array: #include #include typedef uint32_t __attribute__((__aligned__(1))) uint32_t_unaligned; uint64_t sum (const uint32_t_unaligned * p, size_t nwords) { uint64_t res = 0; size_t i; for (i = 0; i Probably works on clang too and IIRC the MS compiler provides similar functionality with different syntax. AFAIK there is no portable solution.…

You also need the may_alias attribute to prevent other problems. Data written as int may be read as char, but going the other way is usually a standards violation. (an exception being if you had used char to implement a memcpy-like function, but in that case you should expect compiler bugs to bite you)

Implementing `memcpy` can be a challenge of its own. Gcc has a tendency to replace memcpy implementations with a call to... memcpy. Which is quite smart, just not in that particular context...

[1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56888

Re: A bug story: data alignment on x86

#62

Post by steveklabnik »

Thiez wrote: post #58
steveklabnik wrote: post #23

Earlier quoted context omitted.

Structs already insert padding to give them alignment: struct One { foo: u8, } struct Two { bar: u16, } struct Three { foo: u8, bar: u16, } struct Four { foo: u16, bar: u16, } fn main() { assert_eq!(1, std::mem::size_of:: ()); assert_eq!(2, std::mem::size_of:: ()); assert_eq!(4, std::mem::size_of:: ()); assert_eq!(4, std::mem::size_of:: ()); }

Well given that Rust leaves struct layout undefined unless #[repr(C)] is specified, std::mem::size_of:: is actually not guaranteed to be 4.

While this is true, it's a weird angle that's not itself super well defined. That is, the definition of repr(packed) implies that the alignment will always be there without it...

Re: A bug story: data alignment on x86

#63

Post by scott_s »

Sagiri wrote: post #38
ogoffart wrote: post #33

Again someone who relies on undefined behavior. Casting pointer of wrong alignement is not a platform specific behavior, it's an undefined behavior. Relying on it is an error. The author did not know "What Every C Programmer Should Know About Undefined Behavior": http://blog.llvm.org/2011/05/what-every-c-programmer-should-... Another good link about that: http://blog.regehr.org/archives/213

If a particular compiler specified that casting pointers of wrong alignments causes a segfault, it'd be perfectly acceptable to rely on that behavior. The standard would consider it UB, but that compiler has defined that behavior sufficiently. Note, though, that a compiler simply doing a particular thing now isn't good enough to specify it in the sense that I mean. The compiler writers would have to explain (in a blo…

I believe you are confusing undefined behavior and implementation defined behavior. Undefined behavior is illegal under all compilers, and all bets are off if you do it. Implementation defined behavior is always legal, but different compilers are allowed to do different things.

Re: A bug story: data alignment on x86

#64

Post by loup-vaillant »

ogoffart wrote: post #33

Again someone who relies on undefined behavior. Casting pointer of wrong alignement is not a platform specific behavior, it's an undefined behavior. Relying on it is an error. The author did not know "What Every C Programmer Should Know About Undefined Behavior": http://blog.llvm.org/2011/05/what-every-c-programmer-should-... Another good link about that: http://blog.regehr.org/archives/213

Again someone blaming the victim. I'm kind of sick of this.

While true, your comment doesn't get at the root of the problem. The obvious fact here is that there is a mismatch between the C standard and how the users really use it. The less obvious fact/opinion/fallacy, is that it is not automatically the user's fault.

The standard could be wrong.

Sure, there are reasons why such and such behaviour ended up undefined. Those reason are sometimes weak however: some behaviour ended up undefined on all platforms because some of them couldn't handle it reasonably. The alignment bug here is such an example.

To this day I don't understand why undefined behaviour wasn't specified on a platform-by-platform basis. We already have implementation defined behaviour, after all. I guess this is because it lets compiler writers unify their front-ends and optimizers, but as a result, we cannot use our platforms to their fullest potential.

Re: A bug story: data alignment on x86

#65

Post by pklausler »

So much HTML to complain about C working the way C is defined rather than the way the OP wants it to work! It's not that hard to write a fast ones'-complement checksum that's portable and compliant, but whining's always easier than coding.

Re: A bug story: data alignment on x86

#66

Post by qb45 »

MaulingMonkey wrote: post #47
qb45 wrote: post #11

The correct solution for GCC is specifying 1-byte alignment for this particular array: #include #include typedef uint32_t __attribute__((__aligned__(1))) uint32_t_unaligned; uint64_t sum (const uint32_t_unaligned * p, size_t nwords) { uint64_t res = 0; size_t i; for (i = 0; i Probably works on clang too and IIRC the MS compiler provides similar functionality with different syntax. AFAIK there is no portable solution.…

Specifying alignment might make the crash go away. Like the "disable SSE" solution of the post, it does not make the undefined behavior go away and is not actually a solution.

If certain compilers provide functionality like __aligned__ or __packed__ it isn't undefined anymore on these particular compilers. I was specific about this being a GCC/clang thing (GCC to be exact, but somebody else confirmed clang).

Re: A bug story: data alignment on x86

#67

Post by qb45 »

acqq wrote: post #32
qb45 wrote: post #11

The correct solution for GCC is specifying 1-byte alignment for this particular array: #include #include typedef uint32_t __attribute__((__aligned__(1))) uint32_t_unaligned; uint64_t sum (const uint32_t_unaligned * p, size_t nwords) { uint64_t res = 0; size_t i; for (i = 0; i Probably works on clang too and IIRC the MS compiler provides similar functionality with different syntax. AFAIK there is no portable solution.…

Have you actually tried and verified that? Reading the docs I've had an impression that it says that the "aligned" is for the alignment inside of the structures , and you here declare the simple type?

I wouldn't have posted this without checking. And as far as documentation goes, they say this attribute applies to "types" and the example they give is

            struct S { short f[3]; } __attribute__ ((aligned (8)));
where it actually is used outside of a struct so that this whole 6B thing is aligned to 8B instead of the default 2B.

Re: A bug story: data alignment on x86

#68

Post by alkonaut »

wyldfire wrote: post #21
alkonaut wrote: post #19

Earlier quoted context omitted.

Not sure if this is what you are asking: Last time I tried alignment in Rust I worked around the lack of explicit alignment support by adding a zero length array of the correct size to the end of the struct. Not sure if alignment support from proper attributes has landed yet. [repr(C)] struct Something { pub foo: f32, pub _alignment: [EightBytes, 0] } where "EightBytes" is a data type of size 8, to align the whole st…

I doubt you can count on all items being allocated at addresses that are multiples if their size. It's not optimal but you can always use libc::posix_memalign()

The final element of the struct is a zero length array of elements of size N bytes. So that element isn't padding, it has size 0! It's pure hinting. I'm not sure why or how this works under the hood I'm afraid. I used it successfully to call a library with pretty strict alignment requirements (Intel Embree).

Re: A bug story: data alignment on x86

#69

Post by ambrop7 »

Note that even if you try to manually correct the pointer to work on aligned data (read any initial bytes via char pointer and read the rest via uint32_t pointer), you still generally have undefined behavior: strict-aliasing violation. And the worst thing here is that whether you do have a violation depends on how other code accesses the same data / how the object is initially declared. E.g., you're fine if the original declaration is char[] or uint32_t[], but not if it's uint16_t[]. Because that would entail access to the same data via both uint16_t and uint32_t, a violation of strict-aliasing.

Actually two out of three inet checksum implementations in lwIP have this bug [1].

And like the problem discovered in the article, this is NOT theoretical. I have personally seen code "miscompiled" due to strict aliasing violations (in that case, packed structures were involved).

I think the only way to do this "manual alignment handling" is to use assembly, either by writing the entire thing in assembly, or using inline asm sections for doing the individual 32-bit memory reads/writes.

Funny story... When I was looking for a fast inet checksum implementation to use for an embedded ARM project, I took the one from RTEMS, which is written in C with much inline asm, and like the lwIP code, it has strict aliasing violations (and also problems compiling correctly with clang). What I did was, compiled it to assembly with gcc once, then included this compiled assembly in the source code. Assuming that this was compiled correctly, I don't need to be afraid of future compiler change breaking it.

[1] http://git.savannah.gnu.org/cgit/lwip.git/tree/src/core/inet...

Re: A bug story: data alignment on x86

#70

Post by qb45 »

stephencanon wrote: post #34
qb45 wrote: post #11

The correct solution for GCC is specifying 1-byte alignment for this particular array: #include #include typedef uint32_t __attribute__((__aligned__(1))) uint32_t_unaligned; uint64_t sum (const uint32_t_unaligned * p, size_t nwords) { uint64_t res = 0; size_t i; for (i = 0; i Probably works on clang too and IIRC the MS compiler provides similar functionality with different syntax. AFAIK there is no portable solution.…

This is documented as not working[1] in all but the most recent GCC versions. E.g. gcc-5.4 documents: > "The aligned attribute can only increase the alignment; but you can decrease it by specifying packed as well. See below." but gcc-6.2 documentation adds: > "When used as part of a typedef, the aligned attribute can both increase and decrease alignment, and specifying the packed attribute generates a warning." FWIW,…

Must have been supported for a while because this stuff is even used in Linux:

  #define __packed2__     __attribute__((packed, aligned(2)))
  
  /*
   * SystemV FS comes in two variants:
   * sysv2: System V Release 2 (e.g. Microport), structure elements aligned(2).
   * sysv4: System V Release 4 (e.g. Consensys), structure elements aligned(4).
   */
  
  struct sysv2_super_block {
          __fs16  s_isize;                /* index of first data zone */
          __fs32  s_fsize __packed2__;    /* total number of zones of this fs */
http://lxr.free-electrons.com/source/include/linux/sysv_fs.h...
Post reply on HN