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

[BUG] r27c NDK: incorrect nullability on fields in group and passwd structs #2098

Open
hyp opened this issue Oct 29, 2024 · 3 comments
Open
Assignees
Labels

Comments

@hyp
Copy link

hyp commented Oct 29, 2024

Description

The Android NDK includes nullability annotations on some char * fields and parameters. Unfortunately it looks like some of the nullability annotations on fields of the group and passwd structs from grp.h and pwd.h headers are incorrect.
I looked over the sources for Android's libc and it looks like the situation is mixed:

  • pw_name doesn't seem to be nullable , so I think it should be _Nonnull instead of being _Nullable in the passwd struct.
  • pw_dir doesn't seem to be nullable, so I think it should be _Nonnul instead in the passwd struct.
  • gr_name doesn't seem to be nullable, so I think it should be _Nonnull instead in the group struct.

This is problematic as Clang depends on these nullability annotations when the NDK headers are being imported into Swift, and these values are marked as Optional in Swift, which is different to all other POSIX platforms. This causes an issue when building swift-foundation for Android:
swiftlang/swift-foundation#999

Affected versions

r27

Canary version

No response

Host OS

Windows

Host OS version

Windows 11

Affected ABIs

arm64-v8a

Build system

ndk-build

Other build system

Cmake

minSdkVersion

28

Device API level

No response

@hyp hyp added the bug label Oct 29, 2024
@github-project-automation github-project-automation bot moved this to Unconfirmed in NDK r28 Oct 29, 2024
@DanAlbert
Copy link
Member

@enh-google for the background. These were pretty closely reviewed when they were added and most of the cases like this were non-obviously correct, so it's possible that Android's right here but the other platforms aren't, or everyone's right and there's a reason for the discrepancy :)

These likely won't be fixed in r27 regardless, btw. It's unfortunately very difficult to update the r27 sysroot at this point due to how heavily the headers have diverged since then. It'd be quite a disruptive change to move it forward. If bionic's wrong about the annotation we'll get them fixed for r28 though. You could always use the bleeding edge artifacts for this rather than a published NDK. That's what I suggested to the Rust folks that are doing something similar. The ndk_platform.tar.bz2 artifact of the ndk target on ci.android.com (aosp-main branch, which should be the default) is where the NDK's sysroot comes from.

@enh-google
Copy link
Collaborator

yeah, this isn't great but i'm unconvinced it's wrong... i've replied in more detail at swiftlang/swift-foundation#999 (comment).

@enh-google
Copy link
Collaborator

(on the principle that me speaking up there might at least help you get your workaround in :-) that workaround seems like the least worst option to me atm, though i'm willing to be persuaded by more examples/other precedent.)

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

No branches or pull requests

3 participants