Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sizeof broken on armv7l / linux (rpi3 + raspbian) #9667

Closed
arnetheduck opened this issue Nov 9, 2018 · 8 comments
Closed

sizeof broken on armv7l / linux (rpi3 + raspbian) #9667

arnetheduck opened this issue Nov 9, 2018 · 8 comments

Comments

@arnetheduck
Copy link
Contributor

Nim 0.19

arnetheduck@raspberrypi:~/src/Nim $ tests/misc/tsizeof 
32 3 12 32
@krux02
Copy link
Contributor

krux02 commented Nov 9, 2018

Here is the source code that needs to be fixed

if conf.target.targetCPU == cpuI386 and size == 8:

Since you have the setup to run the test and it is you who is affected by this, do you work on it to fix it?

@arnetheduck
Copy link
Contributor Author

Actually, anybody can test this using docker (even on travis), no rpi needed (likewise ppc and anything else that qemu supports): https://ownyourbits.com/2018/06/27/running-and-building-arm-docker-containers-in-x86/

And right now, not fixing anything - just doing an inventory of things that will break on ARM for Nimbus/Status.. we'll likely be making some custom docker images with Nim on them as well, so as to ease testing

@krux02
Copy link
Contributor

krux02 commented Jan 10, 2019

I try to find a reliably source for correct sizes and alignments for different platforms. ARM is a very important platform and I would like to get the sizes correct. I went to https://godbolt.org/ and put this code in there:

#include <string.h>
#include <stdio.h>
#include <stdint.h>
#include <stddef.h>

typedef struct {
    uint8_t a; uint64_t b;
} MyStruct;

#define alignof(x) __alignof__(x)

int sizes[] = {
    sizeof(uint8_t),
    sizeof(uint16_t),
    sizeof(uint32_t),
    sizeof(uint64_t),
    sizeof(float),
    sizeof(double),
    sizeof(long),
};

int alignments[] = {
    alignof(uint8_t),
    alignof(uint16_t),
    alignof(uint32_t),
    alignof(uint64_t),
    alignof(float),
    alignof(double),
    alignof(long),
};

int other[] = {
    alignof(MyStruct),
    offsetof(MyStruct, b),
};

Then I selected the compiler to output ARM assembly, but I could not find any non self aligned values in the output. @arnetheduck can you explain this? Is raspberry pi a different ARM then what they are using on godbold.org?

@krux02
Copy link
Contributor

krux02 commented Jan 16, 2019

I tested this on an actual raspberry pi 3. There is no failing test.

@krux02 krux02 closed this as completed Jan 16, 2019
@arnetheduck
Copy link
Contributor Author

Perhaps there is no failing test because you were trying with a different version?

Since 0.19, the test has changed substantially, but the original test in 0.19 still fails, even when compiled with a recent devel like 28b3c8d - it used to look like this:

discard """
  file: "tsize.nim"
  output: "40 3 12 32"
"""
type
  TMyRecord {.final.} = object
    x, y: int
    b: bool
    r: float
    s: string

  TMyEnum = enum
    tmOne, tmTwo, tmThree, tmFour

  TMyArray1 = array[3, uint8]
  TMyArray2 = array[1..3, int32]
  TMyArray3 = array[TMyEnum, float64]

const 
  mysize1 = sizeof(TMyArray1)
  mysize2 = sizeof(TMyArray2)
  mysize3 = sizeof(TMyArray3)

write(stdout, sizeof(TMyRecord))
echo ' ', mysize1, ' ', mysize2, ' ',mysize3

and the output is what is in this bug report originally (wrong, compared to what the test says it should be). I'm not sure if the test is wrong or the code is wrong.

Is raspberry pi a different ARM then what they are using on godbold.org?

ARM is really a family of platforms, all with different features, sizes and ABI's. They're also called different things by different people. aarch64 is an arm for example. Notably, the rpi3 has a 64-bit CPU, so it could run an aarch64 ABI, but most people run a 32-bit OS and ABI on it (because it has so little memory) and call that armv7l or armhf or some other variation (https://superuser.com/questions/1009540/difference-between-arm64-armel-and-armhf), similar to the x32 ABI for x86_64.

reliably source for correct sizes and alignments

see #5664 (comment) - the code has since moved to https://github.com/rust-lang/rust/tree/master/src/librustc_target/spec - there are plenty of platforms there. clang will have a similar list, as will gcc, msvc etc.

In general, the difficulty lies in agreeing with the C compiler - either you have to duplicate the knowledge in nim by typing it out, or you have to fetch it from the c compiler. Like pointed out before, the key takeaway is that alignment is affected by compiler vendor (ms vs unix), compiler flags (lots), programming language (c / lowest common denominator / ffi vs optimal abi), arch/distro (android vs general linux for example), cpu (bits, mode etc), memory models and whole bunch of other things - the view that everything is aligned to its size is simplistic at best, but mostly just wrong.

As an example, sometimes, c compilers choose different alignments for efficiency reasons - code size vs speed, because a larger alignment will use more memory but be more efficient overall. Here's a very common case: https://stackoverflow.com/questions/11108328/double-alignment - what the right alignment is depends.

Adding alignof to the language without an accompanying plan / spec for ABI is in general not really that.. useful. In an ideal world, you want to use one ABI when interfacing with other languages (the c abi tends to be the lingua franca, and the packed ABI is common in network situations), and then play around with it more freely when you're in pure nim-land, but in order to do this, you need to isolate the types that will be used for FFI (by marking them somehow, with exportc for example) and then use an optimal ABI elsewhere..

@krux02
Copy link
Contributor

krux02 commented Jan 21, 2019

That test is wrong. It expects int to be of 64 bit (8 bytes). As you properly pointed out, raspbian is 32 bit and here int is 32 bit (4 bytes). The test has been improve a lot since then.

Adding alignof to the language without an accompanying plan / spec for ABI is in general not really that.. useful.

Nim support two ABI's, the C ABI and the packed ABI. Nim does not introduce a new ABI. There is no new nim specific ABI. The programmer has full control to use {.packed.} or to leave it out. Both are fully tested in the new sizeof test.

@arnetheduck
Copy link
Contributor Author

Nim support two ABI's, the C ABI and the packed ABI.

Is this documented somewhere? packed is in the manual, but other guarantees?

@krux02
Copy link
Contributor

krux02 commented Jan 21, 2019

Nim generates C code. So anything that is not {.packed.} is aligned like a C struct. No special rules added by Nim.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants