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

['a'].pack('P') => warning: unknown pack directive 'P' in 'P' #8051

Open
AndyObtiva opened this issue Dec 22, 2023 · 4 comments
Open

['a'].pack('P') => warning: unknown pack directive 'P' in 'P' #8051

AndyObtiva opened this issue Dec 22, 2023 · 4 comments

Comments

@AndyObtiva
Copy link

AndyObtiva commented Dec 22, 2023

Environment Information

Provide at least:

  • JRuby version (jruby -v) and command line (flags, JRUBY_OPTS, etc)
jruby 9.4.5.0 (3.1.4) 2023-11-02 1abae2700f Java HotSpot(TM) 64-Bit Server VM 21.0.1+12-LTS-29 on 21.0.1+12-LTS-29 +jit [arm64-darwin]
  • Operating system and platform (e.g. uname -a)
Darwin Andys-MacBook-Pro.local 23.2.0 Darwin Kernel Version 23.2.0: Wed Nov 15 21:55:06 PST 2023; root:xnu-10002.61.3~2/RELEASE_ARM64_T6020 arm64

Other relevant info you may wish to add:

  • Installed or activated gems
  • Application/framework version (e.g. Rails, Sinatra)
  • Environment variables

N/A

Expected Behavior

  • Describe your expectation of how JRuby should behave, perhaps by showing how CRuby/MRI behaves.

This code works in CRuby 3.2.1:

puts ['a'].pack('P').inspect
"\b\xE3\xCE\x02\x01\x00\x00\x00"
  • Provide an executable Ruby script or a link to an example repository.

You can reproduce in IRB or by putting the Ruby code above in a ruby script and executing it normally.

Actual Behavior

  • Describe or show the actual behavior.

In JRuby 9.4.5.0, the same code does not work:

puts ['a'].pack('P').inspect
  • Provide text or screen capture showing the behavior.
(irb):1: warning: unknown pack directive 'P' in 'P'
""

I discovered this because one of my Ruby gem dependencies in Glimmer executes that code (facets gem). I thought I had to update it at first, but then I realized the same code runs in CRuby. I was able to come up with a workaround by not loading all of facets yet only what I need, which is better anyways. That circumvented the code causing this issue, so there is no pressing emergency to fix this quickly.

BTW, the CRuby documentation does list 'P' as a supported option for Array#pack:

https://docs.ruby-lang.org/en/master/packed_data_rdoc.html#:~:text=yjit_hacking-,Packed%20Data,new%20array%3Breturns%20that%20array.

Help is appreciated.

@enebo
Copy link
Member

enebo commented Dec 22, 2023

@AndyObtiva We have considered adding 'P' but it is a weird thing to support in Java since Java doesn't have pointers. The implementation would be saving a reference into some data structure (probably weakly referenced) then writing that key. So we can mostly make it appear to work but it won't really be a pointer to a struct.

If we added it as a strong reference it would have a big memory leak potential because we could never know when it is no longer used. If we have it weak memory pressure could force it to disappear before the same process eventually unpacks it again.

Do you know what facets use this for? As I said we can do something for this but it would be useful to know what wants this.

@AndyObtiva
Copy link
Author

Facets is only using it to detect the operating system architecture as 64bit or 32bit:

https://github.com/rubyworks/facets/blob/main/lib/core/facets/kernel/object_hexid.rb

ARCH_SIZE = (['a'].pack('P').length > 4 ? 64 : 32)

@enebo
Copy link
Member

enebo commented Dec 23, 2023

@AndyObtiva It seems facets is not committing a lot these days - 1 commit in 4 years. I was going to suggest a PR for adding rescue 64 but that seems unlikely to get into an actual release (and to be fair it is hacky). Would it be possible to see whether a fix like that will eliminate any issues you see with facets? If so then perhaps this adds a little more fire into supporting 'P' since it will make a gem start to work.

If there are multiple problems I guess that would also be good to know. I was fairly sure people used facets with JRuby in the past (likely also including you) so I am hoping this is the only problem. I also noticed this check has existed for 12 years so possibly something changed in requiring this file?

Side-note: I was wondering what other ways this check could be done and the best I came up with was RbConfig::CONFIG["target_cpu"]. This is somewhat more involved because it will return the arch of the CPU not the bits so you would need more code to be able to use this value. I can see why they did this.

@headius
Copy link
Member

headius commented Jan 24, 2024

We probably would only ever return 64-bit values for P anyway since for our platform (the JVM) everything is 64-bit. I'd be reluctant to try to match the host platform for this but not for other things like the width of a Fixnum.

It sounds like all we need to do is make this not error, so perhaps we should just bite the bullet and implement it as 64-bit always.

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

No branches or pull requests

3 participants