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

size_t vs usize #1671

Closed
elichai opened this issue Nov 8, 2019 · 15 comments
Closed

size_t vs usize #1671

elichai opened this issue Nov 8, 2019 · 15 comments

Comments

@elichai
Copy link
Contributor

elichai commented Nov 8, 2019

Input C/C++ Header

#include <stddef.h>
static size_t A = 5;

bindgen converts size_t to `usize. but I'm not sure that's correct. the unsafe guidelines(https://rust-lang.github.io/unsafe-code-guidelines/layout/scalars.html) say that usize==uintptr_t. but the C standard doesn't define that uintptr_t==size_t therefore usize != size_t.

Am I missing something?

@highfive
Copy link

highfive commented Nov 9, 2019

Hi! If you have any questions regarding this issue, feel free to make a comment here, or ask it in the #servo channel in IRC.

If you intend to work on this issue, then add @highfive: assign me to your comment, and I'll assign this to you. 😄

@emilio
Copy link
Contributor

emilio commented Nov 9, 2019

You're technically correct, I think (which is the best kind of correct ;)).

In practice, in all platforms that I know of sizeof(size_t) == sizeof(uintptr_t). See https://stackoverflow.com/questions/1464174/size-t-vs-uintptr-t.

But we should probably avoid doing this transformation, should be straight-forward.

@cmcavity
Copy link
Contributor

Hi I'd like to work on this. Should the ssize_t to isize transformation be removed as well?
@highfive: assign me

@highfive
Copy link

Hey @cmcavity! Thanks for your interest in working on this issue. It's now assigned to you!

@cmcavity
Copy link
Contributor

@emilio This issue should be closed after pull request #1688.

@emilio
Copy link
Contributor

emilio commented Feb 2, 2020

Fixed by the above PR.

@emilio emilio closed this as completed Feb 2, 2020
@quidity
Copy link

quidity commented Feb 5, 2020

For posterity, a flag was added to revert to previous behavior: --size_t-is-usize

f96dcf9

@vext01
Copy link

vext01 commented Apr 3, 2020

Hi folks,

I have a scenario where bindgen has generated a u64 for a size_t. I think that's wrong. E.g. on i386 a size_t will be 4-bytes, but a u64 is always 8.

Here @emilio suggested I should try --size_t-is-usize, but I'm confused.

It's not clear to me if this is the same issue, as here you are talking about size_t and usize (not size_t and u64)...

The issue starts with version 0.53 of bindgen.

Can anyone shed some light?

@emilio
Copy link
Contributor

emilio commented Apr 3, 2020

Bindgen by default will peek the right size depending on the target architecture. So if you run bindgen with a 64-bit target it'll use u64, with a 32-bit target it'll use u32. If you enable size_t-is-usize then it'll assume that size_t is always usize, and generate usize instead of u32 / u64.

@vext01
Copy link

vext01 commented Apr 3, 2020

Ah, I see.

I wonder why size_t-is-usize isn't default. It seems to me that this is what most people would want/expect.

(Sorry for all the questions)

@geofft
Copy link
Contributor

geofft commented Aug 9, 2020

Is bindgen able to assert that size_t and uintptr_t are the same on my platform, such that it can tell me that --size_t-is-usize is safe?

I was thinking of writing something to test this, but it seems like bindgen probably has enough access to the C compiler to make that check doable?

geofft added a commit to fishinabarrel/linux-kernel-module-rust that referenced this issue Aug 9, 2020
Add an assertion that doing so is safe.
See rust-lang/rust-bindgen#1671

Can't use static_assert from <linux/build_bug.h> because that's
5.1+-only, but the underlying _Static_assert compiler builtin is
available widely.
See torvalds/linux@6bab69c
geofft added a commit to fishinabarrel/linux-kernel-module-rust that referenced this issue Aug 9, 2020
Add an assertion that doing so is safe.
See rust-lang/rust-bindgen#1671

Can't use static_assert from <linux/build_bug.h> because that's
5.1+-only, but the underlying _Static_assert compiler builtin is
available widely.
See torvalds/linux@6bab69c
geofft added a commit to fishinabarrel/linux-kernel-module-rust that referenced this issue Aug 9, 2020
Add an assertion that doing so is safe.
See rust-lang/rust-bindgen#1671

Can't use static_assert from <linux/build_bug.h> because that's
5.1+-only, but the underlying _Static_assert compiler builtin is
available widely.
See torvalds/linux@6bab69c
@emilio
Copy link
Contributor

emilio commented Aug 10, 2020

@geofft potentially, but other than asserting and aborting if we detect it's not safe we couldn't do much else. We have target_pointer_size. When generating size_t we also have the size of that type, so we could assert there and abort, or add a #[test] fn or something.

@geofft
Copy link
Contributor

geofft commented Aug 27, 2020

Yeah, asserting and aborting is the behavior I want - the platforms where this is not true are niche and I'd have to change many other parts of my bindings to safely handle them, I think.

This might be asking too much, but, one approach might be to revert the default back to treating size_t as usize, but with such an assertion added (i.e., it fails to compile on platforms where they're incompatible), and if you specifically want to support these other platforms, allow passing an option to say that size_t isn't usize.

@dkg
Copy link

dkg commented Oct 16, 2020

fwiw, i agree with @geofft that bindgen's defaults for size_t_is_usize should be true, and a warning should be issued on any platform where size_t isn't usize.

@dkg
Copy link

dkg commented Oct 16, 2020

I've opened #1901 to encourage bindgen to change the default back to true, and to abort when building on a platform where usize and size_t are not congruent unless it is deliberately reset to false.

TheSven73 pushed a commit to TheSven73/linux that referenced this issue Apr 13, 2021
The bug which this guard protects against
(rust-lang/rust-bindgen#1671)
was fixed upstream as of rust-bindgen v0.53:
rust-lang/rust-bindgen#1688
d650823839f7 ("Remove size_t to usize conversion")

The current recommended rust-bindgen version for building
the Linux kernel is v0.56, so the guard can be safely
dropped.

Signed-off-by: Sven Van Asbroeck <[email protected]>
TheSven73 pushed a commit to TheSven73/linux that referenced this issue Apr 14, 2021
The bug which this guard protects against
(rust-lang/rust-bindgen#1671)
was fixed upstream as of rust-bindgen v0.53:
rust-lang/rust-bindgen#1688
d650823839f7 ("Remove size_t to usize conversion")

The current recommended rust-bindgen version for building
the Linux kernel is v0.56, so the guard can be safely
dropped.

Signed-off-by: Sven Van Asbroeck <[email protected]>
TheSven73 pushed a commit to TheSven73/linux that referenced this issue Apr 14, 2021
The bug which this guard protects against
(rust-lang/rust-bindgen#1671)
was fixed upstream as of rust-bindgen v0.53:
rust-lang/rust-bindgen#1688
d650823839f7 ("Remove size_t to usize conversion")

The current recommended rust-bindgen version for building
the Linux kernel is v0.56, so the guard can be safely
dropped.

Signed-off-by: Sven Van Asbroeck <[email protected]>
TheSven73 pushed a commit to TheSven73/linux that referenced this issue Apr 15, 2021
On 32-bit arm, `size_t` and 'uintptr_t` are incompatible,
which will trigger the static assertion.

The bug which this guard protects against
(rust-lang/rust-bindgen#1671)
was fixed upstream as of rust-bindgen v0.53:
rust-lang/rust-bindgen#1688
d650823839f7 ("Remove size_t to usize conversion")

The current recommended rust-bindgen version for building
the Linux kernel is v0.56, so the guard can be safely
dropped.

Out of an abundance of caution, remove the guard only
if building for 32-bit arm.

Signed-off-by: Sven Van Asbroeck <[email protected]>
TheSven73 pushed a commit to TheSven73/linux that referenced this issue Apr 15, 2021
On 32-bit arm, `size_t` and 'uintptr_t` are incompatible,
which will trigger the static assertion.

The bug which this guard protects against
(rust-lang/rust-bindgen#1671)
was fixed upstream as of rust-bindgen v0.53:
rust-lang/rust-bindgen#1688
d650823839f7 ("Remove size_t to usize conversion")

The current recommended rust-bindgen version for building
the Linux kernel is v0.56, so the guard can be safely
dropped.

Out of an abundance of caution, remove the guard only
if building for 32-bit arm.

Signed-off-by: Sven Van Asbroeck <[email protected]>
TheSven73 pushed a commit to TheSven73/linux that referenced this issue Apr 15, 2021
On 32-bit arm, `size_t` and 'uintptr_t` are incompatible,
which will trigger the static assertion.

The bug which this guard protects against
(rust-lang/rust-bindgen#1671)
was fixed upstream as of rust-bindgen v0.53:
rust-lang/rust-bindgen#1688
d650823839f7 ("Remove size_t to usize conversion")

The current recommended rust-bindgen version for building
the Linux kernel is v0.56, so the guard can be safely
dropped.

Out of an abundance of caution, remove the guard only
if building for 32-bit arm.

Signed-off-by: Sven Van Asbroeck <[email protected]>
TheSven73 pushed a commit to TheSven73/linux that referenced this issue Apr 15, 2021
On 32-bit arm, `size_t` and 'uintptr_t` are incompatible,
which will trigger the static assertion.

The bug which this guard protects against
(rust-lang/rust-bindgen#1671)
was fixed upstream as of rust-bindgen v0.53:
rust-lang/rust-bindgen#1688
d650823839f7 ("Remove size_t to usize conversion")

The current recommended rust-bindgen version for building
the Linux kernel is v0.56, so the guard can be safely
dropped.

Out of an abundance of caution, remove the guard only
if building for 32-bit arm.

Signed-off-by: Sven Van Asbroeck <[email protected]>
TheSven73 pushed a commit to TheSven73/linux that referenced this issue Apr 16, 2021
On 32-bit arm, `size_t` and 'uintptr_t` are incompatible,
which will trigger the static assertion.

The bug which this guard protects against
(rust-lang/rust-bindgen#1671)
was fixed upstream as of rust-bindgen v0.53:
rust-lang/rust-bindgen#1688
d650823839f7 ("Remove size_t to usize conversion")

The current recommended rust-bindgen version for building
the Linux kernel is v0.56, so the guard can be safely
dropped.

Out of an abundance of caution, remove the guard only
if building for 32-bit arm.

Signed-off-by: Sven Van Asbroeck <[email protected]>
TheSven73 pushed a commit to TheSven73/linux that referenced this issue Apr 16, 2021
On 32-bit arm, `size_t` and 'uintptr_t` are incompatible,
which will trigger the static assertion.

The bug which this guard protects against
(rust-lang/rust-bindgen#1671)
was fixed upstream as of rust-bindgen v0.53:
rust-lang/rust-bindgen#1688
d650823839f7 ("Remove size_t to usize conversion")

The current recommended rust-bindgen version for building
the Linux kernel is v0.56, so the guard can be safely
dropped.

Out of an abundance of caution, remove the guard only
if building for 32-bit arm.

Signed-off-by: Sven Van Asbroeck <[email protected]>
ojeda pushed a commit to Rust-for-Linux/linux that referenced this issue Apr 27, 2021
On 32-bit arm, `size_t` and 'uintptr_t` are incompatible,
which will trigger the static assertion.

The bug which this guard protects against
(rust-lang/rust-bindgen#1671)
was fixed upstream as of rust-bindgen v0.53:
rust-lang/rust-bindgen#1688
d650823839f7 ("Remove size_t to usize conversion")

The current recommended rust-bindgen version for building
the Linux kernel is v0.56, so the guard can be safely
dropped.

Out of an abundance of caution, remove the guard only
if building for 32-bit arm.

Signed-off-by: Sven Van Asbroeck <[email protected]>
[normalized title]
Signed-off-by: Miguel Ojeda <[email protected]>
geofft added a commit to geofft/rust-bindgen that referenced this issue Jun 3, 2021
…ng#1901, rust-lang#1903)

This addresses the underlying issue identified in rust-lang#1671, that size_t
(integer that can hold any object size) isn't guaranteed to match usize,
which is defined more like uintptr_t (integer that can hold any
pointer). However, on almost all platforms, this is true, and in fact
Rust already uses usize extensively in contexts where size_t would be
more appropriate, such as slice indexing. So, it's better for ergonomics
when interfacing with C code to map the C size_t type to usize. (See
also discussion in rust-lang/rust#65473 about how usize really should be
defined as size_t, not uintptr_t.)

The previous fix for rust-lang#1671 removed the special case for size_t and
defaulted to binding it as a normal typedef.  This change effectively
reverts that and goes back to mapping size_t to usize (and ssize_t to
isize), but also ensures that if size_t is emitted, the typedef'd type
of size_t in fact is compatible with usize (defined by checking that the
size and alignment match the target pointer width). For (hypothetical)
platforms where this is not true, or for compatibility with the default
behavior of bindgen between 0.53 and this commit, onwards, you can
disable this mapping with --no-size_t-is-usize.
pvdrz pushed a commit to ferrous-systems/rust-bindgen that referenced this issue Sep 22, 2022
…ng#1901, rust-lang#1903)

This addresses the underlying issue identified in rust-lang#1671, that size_t
(integer that can hold any object size) isn't guaranteed to match usize,
which is defined more like uintptr_t (integer that can hold any
pointer). However, on almost all platforms, this is true, and in fact
Rust already uses usize extensively in contexts where size_t would be
more appropriate, such as slice indexing. So, it's better for ergonomics
when interfacing with C code to map the C size_t type to usize. (See
also discussion in rust-lang/rust#65473 about how usize really should be
defined as size_t, not uintptr_t.)

The previous fix for rust-lang#1671 removed the special case for size_t and
defaulted to binding it as a normal typedef.  This change effectively
reverts that and goes back to mapping size_t to usize (and ssize_t to
isize), but also ensures that if size_t is emitted, the typedef'd type
of size_t in fact is compatible with usize (defined by checking that the
size and alignment match the target pointer width). For (hypothetical)
platforms where this is not true, or for compatibility with the default
behavior of bindgen between 0.53 and this commit, onwards, you can
disable this mapping with --no-size_t-is-usize.
pvdrz pushed a commit to ferrous-systems/rust-bindgen that referenced this issue Sep 24, 2022
…ng#1901, rust-lang#1903)

This addresses the underlying issue identified in rust-lang#1671, that size_t
(integer that can hold any object size) isn't guaranteed to match usize,
which is defined more like uintptr_t (integer that can hold any
pointer). However, on almost all platforms, this is true, and in fact
Rust already uses usize extensively in contexts where size_t would be
more appropriate, such as slice indexing. So, it's better for ergonomics
when interfacing with C code to map the C size_t type to usize. (See
also discussion in rust-lang/rust#65473 about how usize really should be
defined as size_t, not uintptr_t.)

The previous fix for rust-lang#1671 removed the special case for size_t and
defaulted to binding it as a normal typedef.  This change effectively
reverts that and goes back to mapping size_t to usize (and ssize_t to
isize), but also ensures that if size_t is emitted, the typedef'd type
of size_t in fact is compatible with usize (defined by checking that the
size and alignment match the target pointer width). For (hypothetical)
platforms where this is not true, or for compatibility with the default
behavior of bindgen between 0.53 and this commit, onwards, you can
disable this mapping with --no-size_t-is-usize.
qsdrqs pushed a commit to qsdrqs/rust-bindgen that referenced this issue Oct 26, 2022
…ng#1901, rust-lang#1903)

This addresses the underlying issue identified in rust-lang#1671, that size_t
(integer that can hold any object size) isn't guaranteed to match usize,
which is defined more like uintptr_t (integer that can hold any
pointer). However, on almost all platforms, this is true, and in fact
Rust already uses usize extensively in contexts where size_t would be
more appropriate, such as slice indexing. So, it's better for ergonomics
when interfacing with C code to map the C size_t type to usize. (See
also discussion in rust-lang/rust#65473 about how usize really should be
defined as size_t, not uintptr_t.)

The previous fix for rust-lang#1671 removed the special case for size_t and
defaulted to binding it as a normal typedef.  This change effectively
reverts that and goes back to mapping size_t to usize (and ssize_t to
isize), but also ensures that if size_t is emitted, the typedef'd type
of size_t in fact is compatible with usize (defined by checking that the
size and alignment match the target pointer width). For (hypothetical)
platforms where this is not true, or for compatibility with the default
behavior of bindgen between 0.53 and this commit, onwards, you can
disable this mapping with --no-size_t-is-usize.
piscesyuma pushed a commit to piscesyuma/linux-kernel-rust that referenced this issue Mar 8, 2023
Add an assertion that doing so is safe.
See rust-lang/rust-bindgen#1671

Can't use static_assert from <linux/build_bug.h> because that's
5.1+-only, but the underlying _Static_assert compiler builtin is
available widely.
See torvalds/linux@6bab69c
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

8 participants