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

Attempt to fix ARM32 Windows #685

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

bdbai
Copy link

@bdbai bdbai commented Dec 20, 2024

Currently thumbv7a-pc-windows-msvc and thumbv7a-pc-windows-msvc fails to build due to lack of a few symbols generated by windows-targets. As backtrace being a dependency of Rust std, build-std feature targeting these platforms also fails. The fix here is definitely not a good idea which is modifying a generated file, but it shows the possibility to have the issue solved at least to unblock build-std on these platforms.

This was how I verified it using 1.85.0-nightly (9e136a30a 2024-12-19) toolchain:

cargo +nightly build -Z build-std=core,alloc,panic_abort --target thumbv7a-uwp-windows-msvc --no-default-features
cargo +nightly build -Z build-std=core,alloc,panic_abort --target thumbv7a-pc-windows-msvc --no-default-features
which produced the following output:
cargo +nightly build -Z build-std=core,alloc,panic_abort --target thumbv7a-uwp-windows-msvc --no-default-features                                         ─╯
   Compiling backtrace v0.3.74 (<path-to>\backtrace-rs)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.21s

cargo +nightly build -Z build-std=core,alloc,panic_abort --target thumbv7a-pc-windows-msvc --no-default-features                                          ─╯
   Compiling backtrace v0.3.74 (<path-to>\backtrace-rs)
warning: unnecessary `unsafe` block
   --> src\backtrace\dbghelp32.rs:218:5
    |
218 |     unsafe {
    |     ^^^^^^ unnecessary `unsafe` block
    |
    = note: `#[warn(unused_unsafe)]` on by default

warning: creating a mutable reference to mutable static is discouraged
   --> src\dbghelp.rs:115:21
    |
115 |                       DBGHELP.$name().unwrap()
    |                       ^^^^^^^ mutable reference to mutable static
...
128 | / dbghelp! {
129 | |     extern "system" {
130 | |         fn SymGetOptions() -> u32;
131 | |         fn SymSetOptions(options: u32) -> u32;
...   |
223 | | }
    | |_- in this macro invocation
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html>
    = note: mutable references to mutable statics are dangerous; it's undefined behavior if any other pointer to the static is used or if any other reference is created for the static while the mutable reference lives
    = note: `#[warn(static_mut_refs)]` on by default
    = note: this warning originates in the macro `dbghelp` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: creating a mutable reference to mutable static is discouraged
   --> src\dbghelp.rs:115:21
    |
115 |                       DBGHELP.$name().unwrap()
    |                       ^^^^^^^ mutable reference to mutable static
...
128 | / dbghelp! {
129 | |     extern "system" {
130 | |         fn SymGetOptions() -> u32;
131 | |         fn SymSetOptions(options: u32) -> u32;
...   |
223 | | }
    | |_- in this macro invocation
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html>
    = note: mutable references to mutable statics are dangerous; it's undefined behavior if any other pointer to the static is used or if any other reference is created for the static while the mutable reference lives
    = note: this warning originates in the macro `dbghelp` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: creating a mutable reference to mutable static is discouraged
   --> src\dbghelp.rs:321:9
    |
321 |         DBGHELP.ensure_open()?;
    |         ^^^^^^^^^^^^^^^^^^^^^ mutable reference to mutable static
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html>
    = note: mutable references to mutable statics are dangerous; it's undefined behavior if any other pointer to the static is used or if any other reference is created for the static while the mutable reference lives

warning: creating a mutable reference to mutable static is discouraged
   --> src\dbghelp.rs:333:20
    |
333 |         let orig = DBGHELP.SymGetOptions()?();
    |                    ^^^^^^^^^^^^^^^^^^^^^^^ mutable reference to mutable static
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html>
    = note: mutable references to mutable statics are dangerous; it's undefined behavior if any other pointer to the static is used or if any other reference is created for the static while the mutable reference lives

warning: creating a mutable reference to mutable static is discouraged
   --> src\dbghelp.rs:338:9
    |
338 |         DBGHELP.SymSetOptions()?(orig | SYMOPT_DEFERRED_LOADS);
    |         ^^^^^^^^^^^^^^^^^^^^^^^ mutable reference to mutable static
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html>
    = note: mutable references to mutable statics are dangerous; it's undefined behavior if any other pointer to the static is used or if any other reference is created for the static while the mutable reference lives

warning: creating a mutable reference to mutable static is discouraged
   --> src\dbghelp.rs:352:9
    |
352 |         DBGHELP.SymInitializeW()?(GetCurrentProcess(), ptr::null_mut(), TRUE);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^ mutable reference to mutable static
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html>
    = note: mutable references to mutable statics are dangerous; it's undefined behavior if any other pointer to the static is used or if any other reference is created for the static while the mutable reference lives

warning: creating a mutable reference to mutable static is discouraged
   --> src\dbghelp.rs:366:12
    |
366 |         if DBGHELP.SymGetSearchPathW()?(
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^ mutable reference to mutable static
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html>
    = note: mutable references to mutable statics are dangerous; it's undefined behavior if any other pointer to the static is used or if any other reference is created for the static while the mutable reference lives

warning: creating a mutable reference to mutable static is discouraged
   --> src\dbghelp.rs:386:9
    |
386 |         DBGHELP.EnumerateLoadedModulesW64()?(
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ mutable reference to mutable static
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html>
    = note: mutable references to mutable statics are dangerous; it's undefined behavior if any other pointer to the static is used or if any other reference is created for the static while the mutable reference lives

warning: creating a mutable reference to mutable static is discouraged
   --> src\dbghelp.rs:395:9
    |
395 |         DBGHELP.SymSetSearchPathW()?(GetCurrentProcess(), new_search_path.as_ptr());
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^ mutable reference to mutable static
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html>
    = note: mutable references to mutable statics are dangerous; it's undefined behavior if any other pointer to the static is used or if any other reference is created for the static while the mutable reference lives

warning: `backtrace` (lib) generated 24 warnings (14 duplicates)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.23s

Any input on better ways to handle this will be appreciated! In Rust std I can see it's solved here https://github.com/rust-lang/rust/blob/8a1f8039a7ded79d3d4fe97b110016d89f2b11e2/src/tools/generate-windows-sys/src/main.rs#L8 . Not sure if we can apply something similar here.

Xref: #572 #634

@workingjubilee
Copy link
Member

@bdbai I am curious what use the 32-bit Windows Arm target is being put to? It is a target unmaintained by the OS vendor... as in, the entire OS is unmaintained for that target... and no one has signed up to maintain the target on the Rust end.

As for the CONTEXT structure, the std definition provided that you linked to is an uninhabited type (no variants), so it cannot actually be interacted with in the conventional sense... hmm...

@bdbai
Copy link
Author

bdbai commented Dec 25, 2024

@workingjubilee

I am curious what use the 32-bit Windows Arm target is being put to?

Some examples running Windows ARM32: Surface RT, Windows Phone and Windows 10 IoT Core

As for the CONTEXT structure, the std definition provided that you linked to is an uninhabited type (no variants), so it cannot actually be interacted with in the conventional sense... hmm...

In std the CONTEXT structure for ARM is only used in pointer form (like https://github.com/rust-lang/rust/blob/41f2f5c0b7d5fb68685030edc62e66a21cae38ef/library/std/src/sys/pal/windows/c/windows_sys.rs#L65) so we didn't bother to fill up all fields inside but instead treat it as an opaque FFI type. However in backtrace we are actually reading the values inside so the complete definition is required.

@workingjubilee
Copy link
Member

@bdbai Yes, we don't really have support for any of those targets except for their 64-bit versions. But I wasn't really asking you to quote OS names at me, I'm curious what you actually use it for.

@bdbai
Copy link
Author

bdbai commented Dec 25, 2024

@workingjubilee Personally I am writing UWP apps (https://github.com/YtFlow/YtFlowApp) specifically targeting both desktop and mobile as some users including me are still using Windows Phone. The core part is written in Rust hence needs to cross compile to thumbv7a-uwp-windows-msvc.

@workingjubilee
Copy link
Member

I see! @ChrisDenton You lost the bet, I suppose?

@bdbai Do you have a citation for the 32-bit Arm Windows target CONTEXT definition? It is, for perhaps obvious reasons, hard to find on Microsoft's documentation.

@bdbai
Copy link
Author

bdbai commented Dec 25, 2024

@bdbai
Copy link
Author

bdbai commented Dec 25, 2024

Copying these from Windows SDK (10.0.22621.0):

//
// Specify the number of breakpoints and watchpoints that the OS
// will track. Architecturally, ARM supports up to 16. In practice,
// however, almost no one implements more than 4 of each.
//

#define ARM_MAX_BREAKPOINTS     8
#define ARM_MAX_WATCHPOINTS     1

//
// Context Frame
//
//  This frame has a several purposes: 1) it is used as an argument to
//  NtContinue, 2) it is used to constuct a call frame for APC delivery,
//  and 3) it is used in the user level thread creation routines.
//
//
// The flags field within this record controls the contents of a CONTEXT
// record.
//
// If the context record is used as an input parameter, then for each
// portion of the context record controlled by a flag whose value is
// set, it is assumed that that portion of the context record contains
// valid context. If the context record is being used to modify a threads
// context, then only that portion of the threads context is modified.
//
// If the context record is used as an output parameter to capture the
// context of a thread, then only those portions of the thread's context
// corresponding to set flags will be returned.
//
// CONTEXT_CONTROL specifies Sp, Lr, Pc, and Cpsr
//
// CONTEXT_INTEGER specifies R0-R12
//
// CONTEXT_FLOATING_POINT specifies Q0-Q15 / D0-D31 / S0-S31
//
// CONTEXT_DEBUG_REGISTERS specifies up to 16 of DBGBVR, DBGBCR, DBGWVR,
//      DBGWCR.
//

typedef struct _NEON128 {
    ULONGLONG Low;
    LONGLONG High;
} NEON128, *PNEON128;

typedef struct DECLSPEC_ALIGN(8) DECLSPEC_NOINITALL _CONTEXT {

    //
    // Control flags.
    //

    DWORD ContextFlags;

    //
    // Integer registers
    //

    DWORD R0;
    DWORD R1;
    DWORD R2;
    DWORD R3;
    DWORD R4;
    DWORD R5;
    DWORD R6;
    DWORD R7;
    DWORD R8;
    DWORD R9;
    DWORD R10;
    DWORD R11;
    DWORD R12;

    //
    // Control Registers
    //

    DWORD Sp;
    DWORD Lr;
    DWORD Pc;
    DWORD Cpsr;

    //
    // Floating Point/NEON Registers
    //

    DWORD Fpscr;
    DWORD Padding;
    union {
        NEON128 Q[16];
        ULONGLONG D[32];
        DWORD S[32];
    } DUMMYUNIONNAME;

    //
    // Debug registers
    //

    DWORD Bvr[ARM_MAX_BREAKPOINTS];
    DWORD Bcr[ARM_MAX_BREAKPOINTS];
    DWORD Wvr[ARM_MAX_WATCHPOINTS];
    DWORD Wcr[ARM_MAX_WATCHPOINTS];

    DWORD Padding2[2];

} CONTEXT, *PCONTEXT;


#[repr(C)]
#[derive(Clone, Copy)]
pub struct CONTEXT_u([u64; 32]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be an actual union that matches the anon union declaration as exactly as possible. The name is unimportant, so let us call it CONTEXT_FloatRegs.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d468f71

pub R10: u32,
pub R11: u32,
pub R12: u32,
pub Sp: u32,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub Sp: u32,
// Control registers
pub Sp: u32,

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d468f71

pub Lr: u32,
pub Pc: u32,
pub Cpsr: u32,
pub Fpsrc: u32,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub Fpsrc: u32,
// Floating-point registers
pub Fpsrc: u32,

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d468f71

pub Fpsrc: u32,
pub Padding: u32,
pub u: CONTEXT_u,
pub Bvr: [u32; ARM_MAX_BREAKPOINTS],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub Bvr: [u32; ARM_MAX_BREAKPOINTS],
// Debug registers
pub Bvr: [u32; ARM_MAX_BREAKPOINTS],

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d468f71

@bdbai bdbai requested a review from workingjubilee December 25, 2024 08:26
@workingjubilee
Copy link
Member

Thank you. I suppose we will have to think a little on where exactly to put the shim filling, hm.

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

Successfully merging this pull request may close these issues.

2 participants