[BUG] clang.cmd doesn't work well with -march=armv7-a

[Berrysoft]

Description

I've installed NDK 25.2.9519653 and want to work with rust. However, the content of *-*-*-clang.cmd causes issues. The content of such cmd script is:

@echo off
setlocal
call :find_bin
if "%1" == "-cc1" goto :L

set "_BIN_DIR=" && "%_BIN_DIR%clang.exe" --target=armv7a-linux-androideabi24 %*
if ERRORLEVEL 1 exit /b 1
goto :done

:L
rem Target is already an argument.
set "_BIN_DIR=" && "%_BIN_DIR%clang.exe" %*
if ERRORLEVEL 1 exit /b 1
goto :done

:find_bin
rem Accommodate a quoted arg0, e.g.: "clang"
rem https://github.com/android-ndk/ndk/issues/616
set _BIN_DIR=%~dp0
exit /b

:done

The fourth line, if "%1" == "-cc1" goto :L, causes issues. Rust put a param "-march=armv7-a" with armv7a target, and cmd will throw error

=armv7-a"" was unexpected at this time.

That's because the line is interpreted as if ""-march=armv7-a"" == "-cc1" goto :L. When I comment out this line, it works well.

Affected versions

r25

Canary version

No response

Host OS

Windows

Host OS version

Windows 11

Affected ABIs

armeabi-v7a

Build system

ndk-build

Other build system

No response

minSdkVersion

24

Device API level

No response

[DanAlbert]

@rprichard could you look at this for r26? I think you understand the cmd stuff better than the rest of us.

[rprichard]

Maybe EnableDelayedExpansion would help?

https://stackoverflow.com/a/26079381

@echo off
SetLocal EnableDelayedExpansion

:: Test empty string
set Lhs=
set Rhs=
echo Lhs: !Lhs! & echo Rhs: !Rhs!
if !Lhs! == !Rhs! (echo Equal) else (echo Not Equal)
echo.

:: Test symbols
set Lhs= \ / : * ? " ' < > | %% ^^ ` ~ @ # $ [ ] & ( ) + - _ =
set Rhs= \ / : * ? " ' < > | %% ^^ ` ~ @ # $ [ ] & ( ) + - _ =
echo Lhs: !Lhs! & echo Rhs: !Rhs!
if !Lhs! == !Rhs! (echo Equal) else (echo Not Equal)
echo.

I wonder if our wrappers can forward arguments containing special characters, like %, !, ^, "...

[vbieleny]

I also encountered this problem. I've found that if %1 contains multiple " it can cause problems on this line if "%1" == "-cc1" goto :L.

What helped me was to replace this line

if "%1" == "-cc1" goto :L

with

if "%~1" == "-cc1" goto :L

The %~1 will remove the surrounding " from the argument and then it can be safely parsed. Although I am not sure about other special characters.

[rib]

I also just arrived at the same workaround as @vbieleny

[MarijnS95]

Isn't the idea to ditch the .cmd scripts and invoke clang directly, as per https://android-review.googlesource.com/c/platform/ndk/+/2134712?

[rib]

Isn't the idea to ditch the .cmd scripts and invoke clang directly, as per https://android-review.googlesource.com/c/platform/ndk/+/2134712?

At least for my current use case i'm using the toolchain via cargo/rustc and I'm not sure if there's any practical way yet to specify the target api level via cargo afaik? (adding the api level suffix wouldn't be a valid target name for rust) so at least for now it's still useful to have these wrappers.

[MarijnS95]

In ndk-build we provide that via C(XX)FLAGS and RUSTFLAGS. cargo-apk selects the API level via the manifest (and some other sources such as latest installed SDK?), cargo-ndk seems to pull it from cmdline.

[DanAlbert]

Isn't the idea to ditch the .cmd scripts and invoke clang directly, as per https://android-review.googlesource.com/c/platform/ndk/+/2134712?

At least for my current use case i'm using the toolchain via cargo/rustc and I'm not sure if there's any practical way yet to specify the target api level via cargo afaik? (adding the api level suffix wouldn't be a valid target name for rust) so at least for now it's still useful to have these wrappers.

CC="clang -target aarch64-linux-android24". You only need the wrappers in the rare cases where you're using a system that's going to fumble when escaping that. They're rare but I've seen it happen in not-actually-autoconf-but-almost projects.

[MarijnS95]

Rust unfortunately seems to be rare in how we can provide the linker... by needing a different nontrivial environment var (RUSTFLAGS with all kinds of mutual exclusivity with user-settable config files) to add this extra arg (https://github.com/rust-mobile/ndk/pull/306/files#diff-33520aaf2e3ca2264c50b4159237131b9804b93d42e50b2276a281da69567b8bR41-R42).

I wonder if we could have added it to the LINKER env var directly and do away with all this hassle, like you suggest for CC.

[rib]

I wonder if we could have added it to the LINKER env var directly and do away with all this hassle, like you suggest for CC.

Just for reference, I gave this a try and cargo expects to be able to resolve the _LINKER to an absolute path and so it didn't work to try and add the target argument to the _LINKER environment variable. Also just conceptually; without something similar to CARGO_ENCODED_RUSTFLAGS it would be awkward to deal with paths that have spaces, in conjunction with arguments.

Unfortunately right now it looks like it's very difficult to pass --target to Clang when cross compiling Rust crates with Cargo without risking trampling the user's own rustflags.

As discussed here bbqsrc/cargo-ndk#104 (comment); tools such as cargo ndk, cargo apk and xbuild have no real hope of being able to determine the rustflags that have been configured by the user and so they can't add the required -Clink-arg= to add the --target to the rustflags without potentially trampling the user's own configuration of rustflags.

Unfortunately that means that the solution mentioned above for ndk-build isn't compatible with Rust projects that configure rustflags via Cargo. The current project I'm working with, for example, configures rustflags via .cargo/config.toml

The best solution for Rust projects would probably be to look at adding cargo ndk functionality to cargo itself, considering that cargo is the only thing that can really extend rustflags reliably.

Alternatively I think Cargo and the cc crate would need to offer some alternative ways of specifying a linker, with flags that can be used by tools like cargo ndk / ndk-build that won't trample the user's configured rustflags. E.g. Cargo could potentially support a CARGO_ENCODED_<target>_LINKER that could accept arguments for the linker too.

Without something like that then I think we need to continue using these .cmd wrappers with Rust and we also need to patch them to fix this bug because rustc seems to be passing arguments to the linker with quotes (E.g. we see this with -Wl,--version-scripts created by rustc)

One other approach I may investigate is to treat cargo-ndk as a RUSTC_WRAPPER on Windows so we can handle adding the -Clink-arg= at a point where all the rustflags should have been resolved by Cargo. This will likely make builds noticeably slower on Windows if it works since it will add a redundant CreateProcess for every rustc invocation.

[leleliu008]

to package manager developers and build system developers, maybe write your own wrapper in C API is a better choice.

here is a example I'm using https://github.com/leleliu008/ndk-pkg/blob/master/ndk-pkg-wrapper-cc.c

write your own wrapper, you can do many things. let's show you some examples:

1. fix bugs. for exmaple, checking if --Wl,--soname,libxx.so is passed, if not, adding it. as discussed in [BUG]My compiled .so file depends on other shared object files, but has path prefixes included. #1865
2. implement verbose mode, likes make V=1
3. generate compile_commands.json
4. do whatever you want

[DanAlbert]

https://android-review.googlesource.com/c/platform/ndk/+/2925163 incorporates @vbieleny's suggested fix. Thanks! I need to wait for presubmit to finish with it before I can verify it, but I should be able to do that later today.

[DanAlbert]

Confirmed, fix merged.