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

Replace Unsafe-based object scanning with a spec-compliat mechanism. #632

Closed
mukel opened this issue Mar 14, 2021 · 1 comment
Closed

Replace Unsafe-based object scanning with a spec-compliat mechanism. #632

mukel opened this issue Mar 14, 2021 · 1 comment

Comments

@mukel
Copy link

mukel commented Mar 14, 2021

Environment

  • LWJGL version: 3.3.0 SNAPSHOT
  • LWJGL build #: X
  • Java version: ALL
  • Platform: Linux|macOS|Windows
  • Module: core

Description

To find private field offsets of DirectByteBuffer, objects are scanned using Unsafe e.g. see MemoryUtil#getFieldOffset

Scanning objects like that is very fragile, field offsets are just a token/cookie, the physical location remains an implementation detail. The JVM may decide the shuffle fields (packing), or even remove fields that are always constant or unused and provide a "fake" offset instead.
The issue was originally reported/found here. Espresso is an alternative, spec-compliant JVM; it uses the OpenJDK class library but an different object model/layout that is not compatible with this pattern.

A spec-compliant improvement would be to scan all fields (instead of all offsets) while searching for a magic constant. This should be more resilient to object layout changes and alignment issues.

@Spasi
Copy link
Member

Spasi commented Mar 15, 2021

Thanks @mukel!

Sorenon added a commit to Sorenon/lwjgl3 that referenced this issue Jul 8, 2021
* fix(GL): gl(En/Dis)ableClientState function lookup. Close LWJGL#602

Fixes lookup of deprecated functions that have been undeprecated by an
OpenGL extension.

* feat(libffi): restore libffi bindings

* feat(build): minor improvements and fixes

* feat(core): replace dyncall with libffi. Close LWJGL#283

* feat(core): remove dyncall bindings

The only feature maintained is the dlGetLibraryPath function from
dynload.h (very useful when org.lwjgl.system.DebugLoader is enabled).

* build: bump version to 3.3.0

The replacement of dyncall with libffi is a major breaking change, so
3.2.4 is canceled and the next release will be 3.3.0.

* feat(assimp): update to 5.0.1

* feat(bgfx): update to API version 112

* build: update dependencies

* feat(generator): add link to invoke in callback class javadoc

This enables better javadoc navigation inside IDEs.

* feat(assimp): javadoc improvements

* feat(demo): use custom aiFileIO to import models

Creating a correct/complete custom aiFileIO solution is a bit tricky, so
this implementation may be a good reference for LWJGL users.

* feat(shaderc): update to 2020.5

* feat(spvc): update to 0.45.0

* fix(build): use https DTD URLs

* feat(rpmalloc): update to 1.4.2

This is a pre-release build, needed for macos/arm64 compatibility.

* build: add support for windows-arm64 & macos-arm64. Close LWJGL#601

* fix(rpmalloc): spin intrinsic on windows-arm64

* build: simplify cache-kotlinc

Remove the annoying AWS credential check and allow skipping Kotlin code
recompilation.

* build: move kotlinc cache to the ci folder

* build: javadoc fixes

* build: increase javadoc Xmx

* build: release macOS/Windows ARM64 natives

* build: skip bgfx natives for Windows ARM64

* build: fix build.gradle.kts error

* fix(core): bump version to 3.3.0

* fix(demo): font error check in nanovg demo

* chore: remove deprecated methods

* docs: new build status badge, add discord server

* build: add .gitattributes file

* feat(generator): automate virtual bitfield member generation

* feat(Vulkan): update to 1.2.172

* feat(Vulkan): update to 1.2.172 (generated)

* fix(generator): move library init before constants. Close LWJGL#630

A constant may be initialized with an expression that ends up calling a
native method. If the library has not been initialized first, it will
trigger an UnsatisfiedLinkError.

* fix(core): use Unsafe cookies for field reads. Close LWJGL#632

* fix(Vulkan): javadoc issues

* fix(Vulkan): make VK12 extend VK11

* fix(Vulkan): add 1.2 to the known versions array

* fix(Vulkan): values of aliased tokens

* build(vma): update Vulkan headers to 1.2.172

* feat(vma): update to 3.3.0-development

* build(vma): fix compilation on clang

* fix(vma): javadoc issues

* feat(lmdb): update to 0.9.28

* chore(Vulkan): remove redundant statements

* fix(Vulkan): default instance/device API versions

The default instance & device API versions, when not specified by the
user, now match what the Vulkan specification says they should be.

- The default instance API version, when VkApplicationInfo is not
specified or when its apiVersion value is 0, is now VK_API_VERSION_1_0
instead of the maximum version supported by the instance.
- The default device API version, when not specified in the VkDevice
constructor, is now the minimum of the instance API version and the
API version supported by the physical device.
- The API version stored in VKCapabilitiesDevice is now the version
specified/derived in the VkDevice constructor, instead of always equal
to the instance API version.

Note that the highest API version the application targets, set with
VkApplicationInfo and stored in VKCapabilitiesInstance, can be higher
than the actual maximum version supported by the instance, as described
in the Vulkan specification.

* chore(Vulkan): remove experimental extension

* docs: caps buffer factories must zero prefill

* fix(Vulkan): VkAccelerationStructureInstance bitfield order

* fix(stb): remove overloads with allocation context

stb is already configured to use the LWJGL allocator internally. Passing
a custom allocation context is unnecessary.

* chore(Vulkan): simplify bitwise operations

* feat(tinyfd): update to 3.8.7. Close LWJGL#623

* fix(tinyfd): javadoc issue

* fix(GL): ListDrawCommandsStatesClientNV parameter type

* feat(generator): move struct member javadoc to getters

* feat(generator): struct member javadoc (generated)

* feat(nanovg): update to latest version

* feat(tinyexr): update to 1.0.0

* perf(xxhash): enable XXH3 dispatching

* feat(zstd): update to 1.4.9

* build(xxhash): disabled AVX-512

Until LWJGL moves to a modern GCC.

* build(xxhash): use GCC 4.9+ for AVX-512

* build(demo): update google fonts branch

* fix(bullet): remove misplaced greek characters in JavaDoc

* feat(lz4): update to 1.9.3

* fix(core): closure allocation tracking

Also now logging which closure registry implementation is being used. On
Linux, libffi switched to static trampolines and the executable address
is now different from the closure address, so the ConcurrentHashMap
implementation is necessary.

* Start with the example for Vulkan with OpenXR

* Create the session loop for the OpenXR-Vulkan example

* My work-arounds to get my dev env for lwjgl working. DONT commit this in the actual lwjgl repository!

* Create the swapchains

* Prepare for rendering on the swapchains

* Use an empty command buffer to prepare the actual rendering. This gets rid of the warnings that were shown due to some monado command buffer being used simultaneously

* Remove some debug printlns

* Process the improvements done by Sorenon

* Start with the render pass and graphics pipeline

* Continue with the graphics pipeline

* Continue with the graphics pipeline

* Nearly finish the initial graphics pipeline(s)

* Fix the shaders and compile them

* Finish initial graphics pipeline creation

* Start with the render pass commands and some preparation I forgot

* Nearly finish framebuffers

* Clear color works now

* Start with the vertex buffer and index buffer

* DONT MAKE THIS STUPID MISTAKE EVER AGAIN

* Finish the initial vertex and index buffer

* Add a hardcoded camera matrix

* Make it possible to reuse some code in the OpenXR-Vulkan example

* Use the right camera matrix rather than the hardcoded one

* Start with the small cube

* Fix the small model

* Start with tracking the hand position

* Add hand pose tracking, but it is not really used yet.

* Continue with hands tracking

* Add push constant to invert colors (but still needs to be bound to inputs)

* Split some utilities into a separate XRHelper class

* Fix a stupid string terminator problem that messed things up on a Windows machine

* Fix the handpose tracking math

* Invert the Y-axis of the projection matrix (needed due to the Vulkan coordinate system)

* Use a better projection fix

* Improve the checking and handling of presence/absence of Vulkan extensions

* Improve checking of Vulkan and OpenXR results

* Improve selection of swapchain depth format

* Performance improvements (not needed for 5k cubes, but examples are supposed to show the right way of doing things)

* Invert the hand cube colors if the corresponding trigger is pressed. Unfortunately, I cant test this, so lets hope it works

* Work on XRHelper

* Use XRHelper also in the OpenXR with OpenGL example

* Rename HelloOpenXR to HelloOpenXRGL

* Rename Shaders to ShadersGL

* Restore ShadersGL

* Remove unneeded line

* Document XRHelper.createGraphicsBindingOpenGL

* Disable debugging safety check by default

* Update OpenXR generated bindings

Co-authored-by: Ioannis Tsakpinis <[email protected]>
Co-authored-by: adam-risberg <[email protected]>
Co-authored-by: Leon Linhart <[email protected]>
Co-authored-by: Sorenon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants