Change more x64 size checks to not apply to x32.

[hvdijk]

Commit 95e096d changed a bunch of size checks already, but more have
been added, so this fixes the new ones the same way: the various size
checks that are conditional on target_arch = "x86_64" were not intended
to apply to x86_64-unknown-linux-gnux32, so add
target_pointer_width = "64" to the conditions.

[rust-highfive]

r? @michaelwoerister

(rust-highfive has picked a reviewer for you, use r? to override)

[CryZe]

Do these even need the target_arch at all and not just the pointer width?

[hvdijk]

Do these even need the target_arch at all and not just the pointer width?

Good question. I think some of the checks could potentially fail if some target has 64-bit pointers that do not require 64-bit alignment, but I do not know whether Rust supports any such target. I wouldn't want to have that change as part of this one in case it needs to be reverted (the revert would break x32 again), but I can submit it as a followup PR if you like.

[michaelwoerister]

r? @joshtriplett who seems to know more about this

[jrtc27]

Does target_pointer_width mean sizeof(void *) or log2(__UINTPTR_MAX__ + 1) (i.e. is it defined to be the size of a pointer or the range you can address)? If the latter, using a bare target_pointer_width == 64 would make things worse for CHERI, where pointers carry additional metadata beyond the address by using "capabilities", so 64-bit architectures have 128-bit pointers but still only a 64-bit address space (even the current patch would cause issues for a CHERI x86_64, but that is so far just a sketch in our ISA spec). The former is fine for CHERI but is often not what people use these kinds of things to mean.

(See also #65473 that tracks usize as defined conflating the ideas of size_t and uintptr_t)

[hvdijk]

Does target_pointer_width mean sizeof(void *) or log2(__UINTPTR_MAX__ + 1) (i.e. is it defined to be the size of a pointer or the range you can address)?

That's not clear to me from the specification. https://doc.rust-lang.org/reference/conditional-compilation.html#target_pointer_width just says "Key-value option set once with the target's pointer width in bits."

If the latter, using a bare target_pointer_width == 64 would make things worse for CHERI, where pointers carry additional metadata beyond the address by using "capabilities",

Let's play it safe and forget about that for now, then.

(even the current patch would cause issues for a CHERI x86_64, but that is so far just a sketch in our ISA spec)

The current patch wouldn't cause issues for that, as it doesn't enable any static asserts that were previously disabled: any assert that would fail with this patch already fails the same way without this patch.

[jrtc27]

Does target_pointer_width mean sizeof(void *) or log2(__UINTPTR_MAX__ + 1) (i.e. is it defined to be the size of a pointer or the range you can address)?

That's not clear to me from the specification. https://doc.rust-lang.org/reference/conditional-compilation.html#target_pointer_width just says "Key-value option set once with the target's pointer width in bits."

If the latter, using a bare target_pointer_width == 64 would make things worse for CHERI, where pointers carry additional metadata beyond the address by using "capabilities",

Let's play it safe and forget about that for now, then.

(even the current patch would cause issues for a CHERI x86_64, but that is so far just a sketch in our ISA spec)

The current patch wouldn't cause issues for that, as it doesn't enable any static asserts that were previously disabled: any assert that would fail with this patch already fails the same way without this patch.

Agreed :) and there are already asserts of that form elsewhere

[joshtriplett]

This seems reasonable to me.

@bors r+

[bors]

📌 Commit cd75af2 has been approved by joshtriplett