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

Code annotations require supporting code to use correctly #5

Open
zero318 opened this issue Nov 26, 2022 · 3 comments · May be fixed by #8
Open

Code annotations require supporting code to use correctly #5

zero318 opened this issue Nov 26, 2022 · 3 comments · May be fixed by #8

Comments

@zero318
Copy link

zero318 commented Nov 26, 2022

Currently @Code annotations do not implement any argument processing or any means of handling multiple architectures, unlike linked functions. This effectively forces all functions to go through a Java wrapper function at runtime to process arguments and select the appropriate implementation.

For example, even something as simple as querying CPUID output requires something like this:

@Code("4D01C8 4989D9 89D0 0FA2 418900 41895804 41894808 4189500C 4C89CB C3")
private static native void cpuid_amd64_win(int func, int[] out, long out_base_offset);
@Code("488D3C11 89F0 4889DE 0FA2 8907 895F04 894F08 89570C 4889F3 C3")
private static native void cpuid_amd64_linux(int func, int[] out, long out_base_offset);

private static final boolean is_windows = System.getProperty("os.name").toLowerCase().contains("windows");

public static void cpuid(int func, int[] out) {
  if (is_windows) {
    cpuid_amd64_win(func, out, UNSAFE.ARRAY_INT_BASE_OFFSET);
  } else {
    cpuid_amd64_linux(func, out, UNSAFE.ARRAY_INT_BASE_OFFSET);
  }
}

Instead, it seems reasonable to allow specifying multiple possible bytecode sequences that the linker can select at runtime when parsing a class. This would make the feature much simpler to use.

@Code(
  amd64_win = "4989D9 89D0 0FA2 418900 41895804 41894808 4189500C 4C89CB C3",
  amd64_linux = "4889D7 89F0 4889DE 0FA2 8907 895F04 894F08 89570C 4889F3 C3"
)
public static native void cpuid(int func, int[] out);
@apangin
Copy link
Owner

apangin commented Nov 26, 2022

The request definitely makes sense.
I was thinking of allowing multiple @Code annotations with different attributes, e.g.

@Code(os = OS.Windows, arch = Arch.AMD64, value = "12345678")
@Code(os = OS.Linux, arch = Arch.AMD64, value = "1234567890")
@Code(arch = Arch.AArch64, value = "aabbccdd")
public static native void func();

@zero318
Copy link
Author

zero318 commented Nov 26, 2022

That would definitely be nicer syntax than what I came up with, though it raises the question of how attributes would be parsed when there are multiple valid combinations present.

For example, it's unclear whether this first annotation would act as a fall through for any x64 that doesn't match the more specific annotation or if it would be an error since both annotations would be valid simultaneously.

@Code(arch = Arch.AMD64, value = "1234")
@Code(os = OS.Windows, arch = Arch.AMD64, value = "5678")
public static native void func();

trustin added a commit to trustin/nalim that referenced this issue Feb 13, 2023
Motivation:

A user might want to use a different dynamic library or machine code
depending on the current OS and architecture. Currently, such a user has
to introduce a runtime check in a wrapper method, e.g.

```
@code("4D01C8 4989D9 89D0 0FA2 418900 41895804 41894808 4189500C 4C89CB C3")
private static native void cpuid_amd64_win(int func, int[] out, long out_base_offset);
@code("488D3C11 89F0 4889DE 0FA2 8907 895F04 894F08 89570C 4889F3 C3")
private static native void cpuid_amd64_linux(int func, int[] out, long out_base_offset);

private static final boolean is_windows = System.getProperty("os.name").toLowerCase().contains("windows");

public static void cpuid(int func, int[] out) {
  if (is_windows) {
    cpuid_amd64_win(func, out, UNSAFE.ARRAY_INT_BASE_OFFSET);
  } else {
    cpuid_amd64_linux(func, out, UNSAFE.ARRAY_INT_BASE_OFFSET);
  }
}
```

We could improve the experience by allows a user specify multiple
annotations with the desired OS and architecture combination.

Modifications:

- Added two new enums, `Os` and `Arch`, to represent the current and
  desired OS and architecture.
- Added `AnnotationUtil.findLibrary()` and `findCode()` that looks for
  the annotation that best-matches the expected OS and architecture.
- Updated `Linker` to use `AnnotationUtil` to find the right annotation.
- Updated `CallingConvention` to use `Os.CURRENT` and `Arch.CURRENT` to
  detect the current OS and architecture.
- Added the `CpuId` example that demonstrates this feature.

Result:

A user can now greatly simplify the above example:

```
@code(
  os = Os.WINDOWS,
  arch = Arch.AMD64,
  value = "4989D9 89D0 0FA2 418900 41895804 41894808 4189500C 4C89CB C3"
)
@code(
  os = Os.LINUX,
  arch = Arch.AMD64,
  value = "4889D7 89F0 4889DE 0FA2 8907 895F04 894F08 89570C 4889F3 C3"
)
private static native void cpuid(int func, int[] out);
```

Closes apangin#5
@trustin
Copy link

trustin commented Feb 13, 2023

Here's the PR for this issue. Please take a look! 🙏 #8

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 a pull request may close this issue.

3 participants