From e2bd36388f713e515266381e99f9a54e03b2f32e Mon Sep 17 00:00:00 2001 From: Burdette Lamar Date: Thu, 31 Jul 2025 08:13:01 -0500 Subject: [PATCH 01/16] [DOC] Tweak for String#encode --- doc/string/encode.rdoc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/doc/string/encode.rdoc b/doc/string/encode.rdoc index 65872fdfd4658d..14b959ffffb276 100644 --- a/doc/string/encode.rdoc +++ b/doc/string/encode.rdoc @@ -1,4 +1,6 @@ -Returns a copy of +self+ transcoded as determined by +dst_encoding+. +Returns a copy of +self+ transcoded as determined by +dst_encoding+; +see {Encodings}[rdoc-ref:encodings.rdoc]. + By default, raises an exception if +self+ contains an invalid byte or a character not defined in +dst_encoding+; that behavior may be modified by encoding options; see below. @@ -45,3 +47,4 @@ given, conversion from an encoding +enc+ to the same encoding +enc+ no-op, i.e. the string is simply copied without any changes, and no exceptions are raised, even if there are invalid bytes. +Related: see {Converting to New String}[rdoc-ref:String@Converting+to+New+String]. From ff6a8e95b73f84fafbe4b6cfa4cd73607fb17362 Mon Sep 17 00:00:00 2001 From: BurdetteLamar Date: Wed, 30 Jul 2025 12:38:04 -0500 Subject: [PATCH 02/16] [DOc] Tweaks for String#end_with? --- doc/string/end_with_p.rdoc | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/doc/string/end_with_p.rdoc b/doc/string/end_with_p.rdoc index f959cf7aaab371..fcd92421225ca8 100644 --- a/doc/string/end_with_p.rdoc +++ b/doc/string/end_with_p.rdoc @@ -1,11 +1,10 @@ -Returns whether +self+ ends with any of the given +strings+. +Returns whether +self+ ends with any of the given +strings+: -Returns +true+ if any given string matches the end, +false+ otherwise: + 'foo'.end_with?('oo') # => true + 'foo'.end_with?('bar', 'oo') # => true + 'foo'.end_with?('bar', 'baz') # => false + 'foo'.end_with?('') # => true + 'тест'.end_with?('т') # => true + 'こんにちは'.end_with?('は') # => true - 'hello'.end_with?('ello') #=> true - 'hello'.end_with?('heaven', 'ello') #=> true - 'hello'.end_with?('heaven', 'paradise') #=> false - 'тест'.end_with?('т') # => true - 'こんにちは'.end_with?('は') # => true - -Related: String#start_with?. +Related: see {Querying}[rdoc-ref:String@Querying]. From 6d466a55bdee4b85e31d63bcd0113a0e645e5fad Mon Sep 17 00:00:00 2001 From: BurdetteLamar Date: Wed, 30 Jul 2025 13:00:33 -0500 Subject: [PATCH 03/16] [DOC] Tweaks for String#eql? --- doc/string/eql_p.rdoc | 18 ++++++++++++++++++ string.c | 12 +----------- 2 files changed, 19 insertions(+), 11 deletions(-) create mode 100644 doc/string/eql_p.rdoc diff --git a/doc/string/eql_p.rdoc b/doc/string/eql_p.rdoc new file mode 100644 index 00000000000000..85409c5ed687aa --- /dev/null +++ b/doc/string/eql_p.rdoc @@ -0,0 +1,18 @@ +Returns whether +self+ and +object+ have the same length and content: + + s = 'foo' + s.eql?('foo') # => true + s.eql?('food') # => false + s.eql?('FOO') # => false + +Returns +false+ if the two strings' encodings are not compatible: + + s0 = "äöü" # => "äöü" + s1 = s0.encode(Encoding::ISO_8859_1) # => "\xE4\xF6\xFC" + s0.encoding # => # + s1.encoding # => # + s0.eql?(s1) # => false + +See {Encodings}[rdoc-ref:encodings.rdoc]. + +Related: see {Querying}[rdoc-ref:String@Querying]. diff --git a/string.c b/string.c index 58fe632463c50f..235ead3fa085c6 100644 --- a/string.c +++ b/string.c @@ -4243,17 +4243,7 @@ rb_str_equal(VALUE str1, VALUE str2) * call-seq: * eql?(object) -> true or false * - * Returns +true+ if +object+ has the same length and content; - * as +self+; +false+ otherwise: - * - * s = 'foo' - * s.eql?('foo') # => true - * s.eql?('food') # => false - * s.eql?('FOO') # => false - * - * Returns +false+ if the two strings' encodings are not compatible: - * - * "\u{e4 f6 fc}".encode(Encoding::ISO_8859_1).eql?("\u{c4 d6 dc}") # => false + * :include: doc/string/eql_p.rdoc * */ From d7bc1378d25d2ec3fc940e977865c024350d278b Mon Sep 17 00:00:00 2001 From: BurdetteLamar Date: Wed, 30 Jul 2025 13:13:43 -0500 Subject: [PATCH 04/16] [DOC] Tweaks for String#force_encoding --- doc/string/force_encoding.rdoc | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/doc/string/force_encoding.rdoc b/doc/string/force_encoding.rdoc index fd9615caaa30f4..a509e67f80508e 100644 --- a/doc/string/force_encoding.rdoc +++ b/doc/string/force_encoding.rdoc @@ -1,5 +1,6 @@ -Changes the encoding of +self+ to +encoding+, +Changes the encoding of +self+ to the given +encoding+, which may be a string encoding name or an Encoding object; +does not change the underlying bytes; returns self: s = 'łał' @@ -7,14 +8,14 @@ returns self: s.encoding # => # s.force_encoding('ascii') # => "\xC5\x82a\xC5\x82" s.encoding # => # - -Does not change the underlying bytes: - + s.valid_encoding? # => true s.bytes # => [197, 130, 97, 197, 130] Makes the change even if the given +encoding+ is invalid for +self+ (as is the change above): - s.valid_encoding? # => false - s.force_encoding(Encoding::UTF_8) # => "łał" - s.valid_encoding? # => true + s.valid_encoding? # => false + +See {Encodings}[rdoc-ref:encodings.rdoc]. + +Related: see {Modifying}[rdoc-ref:String@Modifying]. From 6f7a4f9c9630c538f1921d59821265fe2564121b Mon Sep 17 00:00:00 2001 From: BurdetteLamar Date: Wed, 30 Jul 2025 13:33:51 -0500 Subject: [PATCH 05/16] [DOC] Tweaks for String#getbyte --- doc/string/getbyte.rdoc | 26 ++++++++++++++++++++++++++ string.c | 8 +------- 2 files changed, 27 insertions(+), 7 deletions(-) create mode 100644 doc/string/getbyte.rdoc diff --git a/doc/string/getbyte.rdoc b/doc/string/getbyte.rdoc new file mode 100644 index 00000000000000..ba1c06fd27cb67 --- /dev/null +++ b/doc/string/getbyte.rdoc @@ -0,0 +1,26 @@ +Returns the byte at zero-based +index+ as an integer: + + s = 'foo' + s.getbyte(0) # => 102 + s.getbyte(1) # => 111 + s.getbyte(2) # => 111 + +Counts backward from the end if +index+ is negative: + + s.getbyte(-3) # => 102 + +Returns +nil+ if +index+ is out of range: + + s.getbyte(3) # => nil + s.getbyte(-4) # => nil + +More examples: + + s = 'тест' + s.bytes # => [209, 130, 208, 181, 209, 129, 209, 130] + s.getbyte(2) # => 208 + s = 'こんにちは' + s.bytes # => [227, 129, 147, 227, 130, 147, 227, 129, 171, 227, 129, 161, 227, 129, 175] + s.getbyte(2) # => 147 + +Related: see {Converting to Non-String}[rdoc-ref:String@Converting+to+Non--5CString]. diff --git a/string.c b/string.c index 235ead3fa085c6..68c4f5f1d7948c 100644 --- a/string.c +++ b/string.c @@ -6685,14 +6685,8 @@ rb_str_chr(VALUE str) * call-seq: * getbyte(index) -> integer or nil * - * Returns the byte at zero-based +index+ as an integer, or +nil+ if +index+ is out of range: + * :include: doc/string/getbyte.rdoc * - * s = 'abcde' # => "abcde" - * s.getbyte(0) # => 97 - * s.getbyte(-1) # => 101 - * s.getbyte(5) # => nil - * - * Related: String#setbyte. */ VALUE rb_str_getbyte(VALUE str, VALUE index) From 05353ab4b7ac12eb1dc289cbd619b51e37ea402d Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Wed, 30 Jul 2025 14:31:20 -0400 Subject: [PATCH 06/16] Make cross_ractor_require write barrier protected --- ractor.c | 66 +++++++++++++++++++++++++++++++++----------------------- 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/ractor.c b/ractor.c index 0f4da1599a05c3..721234a98bde2f 100644 --- a/ractor.c +++ b/ractor.c @@ -2284,58 +2284,67 @@ static const rb_data_type_t cross_ractor_require_data_type = { NULL, // memsize NULL, // compact }, - 0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_DECL_MARKING + 0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED | RUBY_TYPED_DECL_MARKING }; static VALUE -require_body(VALUE data) +require_body(VALUE crr_obj) { - struct cross_ractor_require *crr = (struct cross_ractor_require *)data; + struct cross_ractor_require *crr; + TypedData_Get_Struct(crr_obj, struct cross_ractor_require, &cross_ractor_require_data_type, crr); ID require; CONST_ID(require, "require"); if (crr->silent) { int rb_require_internal_silent(VALUE fname); - crr->result = INT2NUM(rb_require_internal_silent(crr->feature)); + + RB_OBJ_WRITE(crr_obj, &crr->result, INT2NUM(rb_require_internal_silent(crr->feature))); } else { - crr->result = rb_funcallv(Qnil, require, 1, &crr->feature); + RB_OBJ_WRITE(crr_obj, &crr->result, rb_funcallv(Qnil, require, 1, &crr->feature)); } return Qnil; } static VALUE -require_rescue(VALUE data, VALUE errinfo) +require_rescue(VALUE crr_obj, VALUE errinfo) { - struct cross_ractor_require *crr = (struct cross_ractor_require *)data; - crr->exception = errinfo; + struct cross_ractor_require *crr; + TypedData_Get_Struct(crr_obj, struct cross_ractor_require, &cross_ractor_require_data_type, crr); + + RB_OBJ_WRITE(crr_obj, &crr->exception, errinfo); + return Qundef; } static VALUE -require_result_copy_body(VALUE data) +require_result_copy_body(VALUE crr_obj) { - struct cross_ractor_require *crr = (struct cross_ractor_require *)data; + struct cross_ractor_require *crr; + TypedData_Get_Struct(crr_obj, struct cross_ractor_require, &cross_ractor_require_data_type, crr); if (crr->exception != Qundef) { VM_ASSERT(crr->result == Qundef); - crr->exception = ractor_copy(crr->exception); + RB_OBJ_WRITE(crr_obj, &crr->exception, ractor_copy(crr->exception)); } else{ VM_ASSERT(crr->result != Qundef); - crr->result = ractor_copy(crr->result); + RB_OBJ_WRITE(crr_obj, &crr->result, ractor_copy(crr->result)); } return Qnil; } static VALUE -require_result_copy_resuce(VALUE data, VALUE errinfo) +require_result_copy_resuce(VALUE crr_obj, VALUE errinfo) { - struct cross_ractor_require *crr = (struct cross_ractor_require *)data; - crr->exception = errinfo; // ractor_move(crr->exception); + struct cross_ractor_require *crr; + TypedData_Get_Struct(crr_obj, struct cross_ractor_require, &cross_ractor_require_data_type, crr); + + RB_OBJ_WRITE(crr_obj, &crr->exception, errinfo); + return Qnil; } @@ -2353,16 +2362,16 @@ ractor_require_protect(VALUE crr_obj, VALUE (*func)(VALUE)) } // catch any error - rb_rescue2(func, (VALUE)crr, - require_rescue, (VALUE)crr, rb_eException, 0); + rb_rescue2(func, crr_obj, + require_rescue, crr_obj, rb_eException, 0); if (silent) { ruby_debug = debug; rb_set_errinfo(errinfo); } - rb_rescue2(require_result_copy_body, (VALUE)crr, - require_result_copy_resuce, (VALUE)crr, rb_eException, 0); + rb_rescue2(require_result_copy_body, crr_obj, + require_result_copy_resuce, crr_obj, rb_eException, 0); ractor_port_send(GET_EC(), crr->port, Qtrue, Qfalse); RB_GC_GUARD(crr_obj); @@ -2386,8 +2395,8 @@ rb_ractor_require(VALUE feature, bool silent) FL_SET_RAW(crr_obj, RUBY_FL_SHAREABLE); // Convert feature to proper file path and make it shareable as fstring - crr->feature = rb_fstring(FilePathValue(feature)); - crr->port = ractor_port_new(GET_RACTOR()); + RB_OBJ_WRITE(crr_obj, &crr->feature, rb_fstring(FilePathValue(feature))); + RB_OBJ_WRITE(crr_obj, &crr->port, ractor_port_new(GET_RACTOR())); crr->result = Qundef; crr->exception = Qundef; crr->silent = silent; @@ -2422,10 +2431,13 @@ ractor_require(rb_execution_context_t *ec, VALUE self, VALUE feature) } static VALUE -autoload_load_body(VALUE data) +autoload_load_body(VALUE crr_obj) { - struct cross_ractor_require *crr = (struct cross_ractor_require *)data; - crr->result = rb_autoload_load(crr->module, crr->name); + struct cross_ractor_require *crr; + TypedData_Get_Struct(crr_obj, struct cross_ractor_require, &cross_ractor_require_data_type, crr); + + RB_OBJ_WRITE(crr_obj, &crr->result, rb_autoload_load(crr->module, crr->name)); + return Qnil; } @@ -2441,9 +2453,9 @@ rb_ractor_autoload_load(VALUE module, ID name) struct cross_ractor_require *crr; VALUE crr_obj = TypedData_Make_Struct(0, struct cross_ractor_require, &cross_ractor_require_data_type, crr); FL_SET_RAW(crr_obj, RUBY_FL_SHAREABLE); - crr->module = module; - crr->name = name; - crr->port = ractor_port_new(GET_RACTOR()); + RB_OBJ_WRITE(crr_obj, &crr->module, module); + RB_OBJ_WRITE(crr_obj, &crr->name, name); + RB_OBJ_WRITE(crr_obj, &crr->port, ractor_port_new(GET_RACTOR())); crr->result = Qundef; crr->exception = Qundef; From 68a03167a5d2be2b73196d6437a01fe5f069dff1 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Thu, 31 Jul 2025 19:44:36 +0900 Subject: [PATCH 07/16] Suppress maybe-uninitialized warnings --- lib/mkmf.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mkmf.rb b/lib/mkmf.rb index 7ac4f916b8ae24..d0974b05436928 100644 --- a/lib/mkmf.rb +++ b/lib/mkmf.rb @@ -860,7 +860,7 @@ def try_func(func, libs, headers = nil, opt = "", &b) v } unless strvars.empty? - prepare << "char " << strvars.map {|v| "#{v}[1024]"}.join(", ") << "; " + prepare << "char " << strvars.map {|v| %[#{v}[1024] = ""]}.join(", ") << "; " end when nil call = "" From 6c24904a690eb7c4e20c3fa8c3751acc03454100 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Mon, 28 Jul 2025 09:36:19 -0400 Subject: [PATCH 08/16] Make static symbol ID atomic We don't need the VM lock if we make static symbol IDs atomic. --- symbol.c | 43 +++++++++---------------------------------- 1 file changed, 9 insertions(+), 34 deletions(-) diff --git a/symbol.c b/symbol.c index 840bb6752332b5..43ab0ffa3271c5 100644 --- a/symbol.c +++ b/symbol.c @@ -93,7 +93,7 @@ enum id_entry_type { }; typedef struct { - rb_id_serial_t last_id; + rb_atomic_t next_id; VALUE sym_set; VALUE ids; @@ -212,31 +212,12 @@ rb_str_symname_type(VALUE name, unsigned int allowed_attrset) return type; } -static ID -next_id_base_with_lock(rb_symbols_t *symbols) -{ - ID id; - rb_id_serial_t next_serial = symbols->last_id + 1; - - if (next_serial == 0) { - id = (ID)-1; - } - else { - const size_t num = ++symbols->last_id; - id = num << ID_SCOPE_SHIFT; - } - - return id; -} - static ID next_id_base(void) { - ID id; - GLOBAL_SYMBOLS_LOCKING(symbols) { - id = next_id_base_with_lock(symbols); - } - return id; + rb_atomic_t serial = RUBY_ATOMIC_FETCH_ADD(ruby_global_symbols.next_id, 1); + + return (ID)serial << ID_SCOPE_SHIFT; } static void @@ -293,13 +274,7 @@ sym_set_create(VALUE sym, void *data) ID id = rb_str_symname_type(str, IDSET_ATTRSET_FOR_INTERN); if (id == (ID)-1) id = ID_INTERNAL; - ID nid = next_id_base(); - if (nid == (ID)-1) { - str = rb_str_ellipsize(str, 20); - rb_raise(rb_eRuntimeError, "symbol table overflow (symbol %"PRIsVALUE")", str); - } - - id |= nid; + id |= next_id_base(); id |= ID_STATIC_SYM; static_sym = STATIC_ID2SYM(id); @@ -729,7 +704,7 @@ get_id_serial_entry(rb_id_serial_t num, ID id, const enum id_entry_type t) VALUE result = 0; GLOBAL_SYMBOLS_LOCKING(symbols) { - if (num && num <= symbols->last_id) { + if (num && num < RUBY_ATOMIC_LOAD(symbols->next_id)) { size_t idx = num / ID_ENTRY_UNIT; VALUE ids = symbols->ids; VALUE ary; @@ -983,7 +958,7 @@ rb_sym2id(VALUE sym) if (UNLIKELY(!(id & ~ID_SCOPE_MASK))) { VALUE fstr = RSYMBOL(sym)->fstr; - ID num = next_id_base_with_lock(symbols); + ID num = next_id_base(); RSYMBOL(sym)->id = id |= num; /* make it permanent object */ @@ -1061,7 +1036,7 @@ rb_make_temporary_id(size_t n) { const ID max_id = RB_ID_SERIAL_MAX & ~0xffff; const ID id = max_id - (ID)n; - if (id <= ruby_global_symbols.last_id) { + if (id < RUBY_ATOMIC_LOAD(ruby_global_symbols.next_id)) { rb_raise(rb_eRuntimeError, "too big to make temporary ID: %" PRIdSIZE, n); } return (id << ID_SCOPE_SHIFT) | ID_STATIC_SYM | ID_INTERNAL; @@ -1102,7 +1077,7 @@ rb_sym_all_symbols(void) size_t rb_sym_immortal_count(void) { - return (size_t)ruby_global_symbols.last_id; + return (size_t)(RUBY_ATOMIC_LOAD(ruby_global_symbols.next_id) - 1); } int From b3598cf2a355497693bb66097edc156af3152e9b Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Thu, 31 Jul 2025 16:45:09 +0900 Subject: [PATCH 09/16] Win: Strip CRs from `cpp` and `nm` output The combination of mingw tools and cygin/msys2 ruby leaves CRs. --- template/fake.rb.in | 1 + win32/mkexports.rb | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/template/fake.rb.in b/template/fake.rb.in index b592fbd253a0e7..a02582a9dc012f 100644 --- a/template/fake.rb.in +++ b/template/fake.rb.in @@ -9,6 +9,7 @@ while /\A(\w+)=(.*)/ =~ ARGV[0] end if inc = arg['i'] src = inc == '-' ? STDIN.read : File.read(inc) + src.tr!("\r", " ") src.gsub!(/^#.*\n/, '') else src = "" diff --git a/win32/mkexports.rb b/win32/mkexports.rb index 389b49def83544..97939cdd093436 100755 --- a/win32/mkexports.rb +++ b/win32/mkexports.rb @@ -146,7 +146,9 @@ def exports(*) end def each_line(objs, &block) - IO.foreach("|#{self.class.nm} --extern-only --defined-only #{objs.join(' ')}", &block) + IO.popen(%W[#{self.class.nm} --extern-only --defined-only] + objs) do |f| + f.each(&block) + end end def each_export(objs) @@ -155,7 +157,7 @@ def each_export(objs) re = /\s(?:(T)|[[:upper:]])\s#{symprefix}((?!#{PrivateNames}).*)$/ objdump(objs) do |l| next if /@.*@/ =~ l - yield $2, !$1 if re =~ l + yield $2.strip, !$1 if re =~ l end end end From 771804248e8a7b0e65bbc5b85fb604622a285174 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Thu, 31 Jul 2025 09:16:47 -0400 Subject: [PATCH 10/16] Make ARGF not pin references during marking ARGF supports compaction, but it pins all of its references, which means that none of it can move. This commit changes it to actually support compaction. --- io.c | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/io.c b/io.c index 9dcfff76a33522..4ee45c13442f7c 100644 --- a/io.c +++ b/io.c @@ -9999,14 +9999,14 @@ io_wait(int argc, VALUE *argv, VALUE io) } static void -argf_mark(void *ptr) +argf_mark_and_move(void *ptr) { struct argf *p = ptr; - rb_gc_mark(p->filename); - rb_gc_mark(p->current_file); - rb_gc_mark(p->argv); - rb_gc_mark(p->inplace); - rb_gc_mark(p->encs.ecopts); + rb_gc_mark_and_move(&p->filename); + rb_gc_mark_and_move(&p->current_file); + rb_gc_mark_and_move(&p->argv); + rb_gc_mark_and_move(&p->inplace); + rb_gc_mark_and_move(&p->encs.ecopts); } static size_t @@ -10017,20 +10017,9 @@ argf_memsize(const void *ptr) return size; } -static void -argf_compact(void *ptr) -{ - struct argf *p = ptr; - p->filename = rb_gc_location(p->filename); - p->current_file = rb_gc_location(p->current_file); - p->argv = rb_gc_location(p->argv); - p->inplace = rb_gc_location(p->inplace); - p->encs.ecopts = rb_gc_location(p->encs.ecopts); -} - static const rb_data_type_t argf_type = { "ARGF", - {argf_mark, RUBY_TYPED_DEFAULT_FREE, argf_memsize, argf_compact}, + {argf_mark_and_move, RUBY_TYPED_DEFAULT_FREE, argf_memsize, argf_mark_and_move}, 0, 0, RUBY_TYPED_FREE_IMMEDIATELY }; From 69ff8f736b0ad1f6ad70fa3ce288bafb364b021c Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Fri, 1 Aug 2025 01:57:31 +0900 Subject: [PATCH 11/16] [ruby/openssl] pkcs7: only set error_string in the error path Set the error_string attribute to nil if PKCS7_verify() succeeds, since the error queue should be empty in that case. With AWS-LC, OpenSSL::PKCS#verify currently sets error_string to "invalid library (0)" when the verification succeeds, whereas with OpenSSL and LibreSSL, it becomes nil. ERR_reason_error_string() appears to behave differently when an invalid error code is passed. The branch to raise OpenSSL::PKCS7::PKCS7Error is removed because it does not appear to be reachable. https://github.com/ruby/openssl/commit/c11c6631fa --- ext/openssl/ossl_pkcs7.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/ext/openssl/ossl_pkcs7.c b/ext/openssl/ossl_pkcs7.c index 944cbb5e97f5d8..c53e512e884e1c 100644 --- a/ext/openssl/ossl_pkcs7.c +++ b/ext/openssl/ossl_pkcs7.c @@ -770,7 +770,6 @@ ossl_pkcs7_verify(int argc, VALUE *argv, VALUE self) BIO *in, *out; PKCS7 *p7; VALUE data; - const char *msg; GetPKCS7(self, p7); rb_scan_args(argc, argv, "22", &certs, &store, &indata, &flags); @@ -794,14 +793,16 @@ ossl_pkcs7_verify(int argc, VALUE *argv, VALUE self) ok = PKCS7_verify(p7, x509s, x509st, in, out, flg); BIO_free(in); sk_X509_pop_free(x509s, X509_free); - if (ok < 0) ossl_raise(ePKCS7Error, "PKCS7_verify"); - msg = ERR_reason_error_string(ERR_peek_error()); - ossl_pkcs7_set_err_string(self, msg ? rb_str_new2(msg) : Qnil); - ossl_clear_error(); data = ossl_membio2str(out); ossl_pkcs7_set_data(self, data); - - return (ok == 1) ? Qtrue : Qfalse; + if (ok != 1) { + const char *msg = ERR_reason_error_string(ERR_peek_error()); + ossl_pkcs7_set_err_string(self, msg ? rb_str_new_cstr(msg) : Qnil); + ossl_clear_error(); + return Qfalse; + } + ossl_pkcs7_set_err_string(self, Qnil); + return Qtrue; } static VALUE From 865a6191d06902cebbebc41774faa947aeaea06f Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Fri, 25 Jul 2025 03:14:04 +0900 Subject: [PATCH 12/16] [ruby/openssl] pkcs7: clean up tests This includes: - Update test keys to the generic rsa-{1,2,3}.pem. - Add omissions for enveloped-data tests so that the rest can be tested in the FIPS mode. - Add tests for PKCS7#error_string and #data. - Check more error paths. - Various style fixes. https://github.com/ruby/openssl/commit/58f0022de3 --- test/openssl/test_pkcs7.rb | 172 +++++++++++++++++++++++-------------- 1 file changed, 109 insertions(+), 63 deletions(-) diff --git a/test/openssl/test_pkcs7.rb b/test/openssl/test_pkcs7.rb index 5a52f4ce5f346b..5245ec1a6c7657 100644 --- a/test/openssl/test_pkcs7.rb +++ b/test/openssl/test_pkcs7.rb @@ -6,95 +6,125 @@ class OpenSSL::TestPKCS7 < OpenSSL::TestCase def setup super - @rsa1024 = Fixtures.pkey("rsa1024") - @rsa2048 = Fixtures.pkey("rsa2048") - ca = OpenSSL::X509::Name.parse("/DC=org/DC=ruby-lang/CN=CA") - ee1 = OpenSSL::X509::Name.parse("/DC=org/DC=ruby-lang/CN=EE1") - ee2 = OpenSSL::X509::Name.parse("/DC=org/DC=ruby-lang/CN=EE2") + @ca_key = Fixtures.pkey("rsa-1") + @ee1_key = Fixtures.pkey("rsa-2") + @ee2_key = Fixtures.pkey("rsa-3") + ca = OpenSSL::X509::Name.new([["CN", "CA"]]) + ee1 = OpenSSL::X509::Name.new([["CN", "EE1"]]) + ee2 = OpenSSL::X509::Name.new([["CN", "EE2"]]) ca_exts = [ - ["basicConstraints","CA:TRUE",true], - ["keyUsage","keyCertSign, cRLSign",true], - ["subjectKeyIdentifier","hash",false], - ["authorityKeyIdentifier","keyid:always",false], + ["basicConstraints", "CA:TRUE", true], + ["keyUsage", "keyCertSign, cRLSign", true], + ["subjectKeyIdentifier", "hash", false], + ["authorityKeyIdentifier", "keyid:always", false], ] - @ca_cert = issue_cert(ca, @rsa2048, 1, ca_exts, nil, nil) + @ca_cert = issue_cert(ca, @ca_key, 1, ca_exts, nil, nil) ee_exts = [ - ["keyUsage","Non Repudiation, Digital Signature, Key Encipherment",true], - ["authorityKeyIdentifier","keyid:always",false], - ["extendedKeyUsage","clientAuth, emailProtection, codeSigning",false], + ["keyUsage", "nonRepudiation, digitalSignature, keyEncipherment", true], + ["authorityKeyIdentifier", "keyid:always", false], + ["extendedKeyUsage", "clientAuth, emailProtection, codeSigning", false], ] - @ee1_cert = issue_cert(ee1, @rsa1024, 2, ee_exts, @ca_cert, @rsa2048) - @ee2_cert = issue_cert(ee2, @rsa1024, 3, ee_exts, @ca_cert, @rsa2048) + @ee1_cert = issue_cert(ee1, @ee1_key, 2, ee_exts, @ca_cert, @ca_key) + @ee2_cert = issue_cert(ee2, @ee2_key, 3, ee_exts, @ca_cert, @ca_key) end def test_signed store = OpenSSL::X509::Store.new store.add_cert(@ca_cert) + + data = "aaaaa\nbbbbb\nccccc\n" ca_certs = [@ca_cert] + tmp = OpenSSL::PKCS7.sign(@ee1_cert, @ee1_key, data, ca_certs) + # TODO: #data contains untranslated content + assert_equal("aaaaa\nbbbbb\nccccc\n", tmp.data) + assert_nil(tmp.error_string) - data = "aaaaa\r\nbbbbb\r\nccccc\r\n" - tmp = OpenSSL::PKCS7.sign(@ee1_cert, @rsa1024, data, ca_certs) p7 = OpenSSL::PKCS7.new(tmp.to_der) + assert_nil(p7.data) + assert_nil(p7.error_string) + + assert_true(p7.verify([], store)) + # AWS-LC does not appear to convert to CRLF automatically + assert_equal("aaaaa\r\nbbbbb\r\nccccc\r\n", p7.data) unless aws_lc? + assert_nil(p7.error_string) + certs = p7.certificates - signers = p7.signers - assert(p7.verify([], store)) - assert_equal(data, p7.data) assert_equal(2, certs.size) - assert_equal(@ee1_cert.subject.to_s, certs[0].subject.to_s) - assert_equal(@ca_cert.subject.to_s, certs[1].subject.to_s) + assert_equal(@ee1_cert.subject, certs[0].subject) + assert_equal(@ca_cert.subject, certs[1].subject) + + signers = p7.signers assert_equal(1, signers.size) assert_equal(@ee1_cert.serial, signers[0].serial) - assert_equal(@ee1_cert.issuer.to_s, signers[0].issuer.to_s) + assert_equal(@ee1_cert.issuer, signers[0].issuer) # AWS-LC does not generate authenticatedAttributes assert_in_delta(Time.now, signers[0].signed_time, 10) unless aws_lc? + assert_false(p7.verify([@ca_cert], OpenSSL::X509::Store.new)) + end + + def test_signed_flags + store = OpenSSL::X509::Store.new + store.add_cert(@ca_cert) + # Normally OpenSSL tries to translate the supplied content into canonical # MIME format (e.g. a newline character is converted into CR+LF). # If the content is a binary, PKCS7::BINARY flag should be used. - + # + # PKCS7::NOATTR flag suppresses authenticatedAttributes. data = "aaaaa\nbbbbb\nccccc\n" flag = OpenSSL::PKCS7::BINARY | OpenSSL::PKCS7::NOATTR - tmp = OpenSSL::PKCS7.sign(@ee1_cert, @rsa1024, data, ca_certs, flag) + tmp = OpenSSL::PKCS7.sign(@ee1_cert, @ee1_key, data, [@ca_cert], flag) p7 = OpenSSL::PKCS7.new(tmp.to_der) - certs = p7.certificates - signers = p7.signers - assert(p7.verify([], store)) + + assert_true(p7.verify([], store)) assert_equal(data, p7.data) + + certs = p7.certificates assert_equal(2, certs.size) - assert_equal(@ee1_cert.subject.to_s, certs[0].subject.to_s) - assert_equal(@ca_cert.subject.to_s, certs[1].subject.to_s) + assert_equal(@ee1_cert.subject, certs[0].subject) + assert_equal(@ca_cert.subject, certs[1].subject) + + signers = p7.signers assert_equal(1, signers.size) assert_equal(@ee1_cert.serial, signers[0].serial) - assert_equal(@ee1_cert.issuer.to_s, signers[0].issuer.to_s) + assert_equal(@ee1_cert.issuer, signers[0].issuer) assert_raise(OpenSSL::PKCS7::PKCS7Error) { signers[0].signed_time } + end + + def test_signed_multiple_signers + store = OpenSSL::X509::Store.new + store.add_cert(@ca_cert) # A signed-data which have multiple signatures can be created # through the following steps. # 1. create two signed-data # 2. copy signerInfo and certificate from one to another - - tmp1 = OpenSSL::PKCS7.sign(@ee1_cert, @rsa1024, data, [], flag) - tmp2 = OpenSSL::PKCS7.sign(@ee2_cert, @rsa1024, data, [], flag) + data = "aaaaa\r\nbbbbb\r\nccccc\r\n" + tmp1 = OpenSSL::PKCS7.sign(@ee1_cert, @ee1_key, data) + tmp2 = OpenSSL::PKCS7.sign(@ee2_cert, @ee2_key, data) tmp1.add_signer(tmp2.signers[0]) tmp1.add_certificate(@ee2_cert) p7 = OpenSSL::PKCS7.new(tmp1.to_der) - certs = p7.certificates - signers = p7.signers - assert(p7.verify([], store)) + assert_true(p7.verify([], store)) assert_equal(data, p7.data) + + certs = p7.certificates assert_equal(2, certs.size) + + signers = p7.signers assert_equal(2, signers.size) assert_equal(@ee1_cert.serial, signers[0].serial) - assert_equal(@ee1_cert.issuer.to_s, signers[0].issuer.to_s) + assert_equal(@ee1_cert.issuer, signers[0].issuer) assert_equal(@ee2_cert.serial, signers[1].serial) - assert_equal(@ee2_cert.issuer.to_s, signers[1].issuer.to_s) + assert_equal(@ee2_cert.issuer, signers[1].issuer) end def test_signed_add_signer data = "aaaaa\nbbbbb\nccccc\n" - psi = OpenSSL::PKCS7::SignerInfo.new(@ee1_cert, @rsa1024, "sha256") + psi = OpenSSL::PKCS7::SignerInfo.new(@ee1_cert, @ee1_key, "sha256") p7 = OpenSSL::PKCS7.new p7.type = :signed p7.add_signer(psi) @@ -113,27 +143,33 @@ def test_signed_add_signer def test_detached_sign store = OpenSSL::X509::Store.new store.add_cert(@ca_cert) - ca_certs = [@ca_cert] data = "aaaaa\nbbbbb\nccccc\n" + ca_certs = [@ca_cert] flag = OpenSSL::PKCS7::BINARY|OpenSSL::PKCS7::DETACHED - tmp = OpenSSL::PKCS7.sign(@ee1_cert, @rsa1024, data, ca_certs, flag) + tmp = OpenSSL::PKCS7.sign(@ee1_cert, @ee1_key, data, ca_certs, flag) p7 = OpenSSL::PKCS7.new(tmp.to_der) - assert_nothing_raised do - OpenSSL::ASN1.decode(p7) - end + assert_predicate(p7, :detached?) + assert_true(p7.detached) - certs = p7.certificates - signers = p7.signers - assert(!p7.verify([], store)) - assert(p7.verify([], store, data)) + assert_false(p7.verify([], store)) + # FIXME: Should it be nil? + assert_equal("", p7.data) + assert_match(/no content|NO_CONTENT/, p7.error_string) + + assert_true(p7.verify([], store, data)) assert_equal(data, p7.data) + assert_nil(p7.error_string) + + certs = p7.certificates assert_equal(2, certs.size) - assert_equal(@ee1_cert.subject.to_s, certs[0].subject.to_s) - assert_equal(@ca_cert.subject.to_s, certs[1].subject.to_s) + assert_equal(@ee1_cert.subject, certs[0].subject) + assert_equal(@ca_cert.subject, certs[1].subject) + + signers = p7.signers assert_equal(1, signers.size) assert_equal(@ee1_cert.serial, signers[0].serial) - assert_equal(@ee1_cert.issuer.to_s, signers[0].issuer.to_s) + assert_equal(@ee1_cert.issuer, signers[0].issuer) end def test_signed_authenticated_attributes @@ -181,6 +217,8 @@ def test_signed_authenticated_attributes end def test_enveloped + omit_on_fips # PKCS #1 v1.5 padding + certs = [@ee1_cert, @ee2_cert] cipher = OpenSSL::Cipher::AES.new("128-CBC") data = "aaaaa\nbbbbb\nccccc\n" @@ -191,15 +229,20 @@ def test_enveloped assert_equal(:enveloped, p7.type) assert_equal(2, recip.size) - assert_equal(@ca_cert.subject.to_s, recip[0].issuer.to_s) - assert_equal(2, recip[0].serial) - assert_equal(data, p7.decrypt(@rsa1024, @ee1_cert)) + assert_equal(@ca_cert.subject, recip[0].issuer) + assert_equal(@ee1_cert.serial, recip[0].serial) + assert_equal(16, @ee1_key.decrypt(recip[0].enc_key).size) + assert_equal(data, p7.decrypt(@ee1_key, @ee1_cert)) - assert_equal(@ca_cert.subject.to_s, recip[1].issuer.to_s) - assert_equal(3, recip[1].serial) - assert_equal(data, p7.decrypt(@rsa1024, @ee2_cert)) + assert_equal(@ca_cert.subject, recip[1].issuer) + assert_equal(@ee2_cert.serial, recip[1].serial) + assert_equal(data, p7.decrypt(@ee2_key, @ee2_cert)) - assert_equal(data, p7.decrypt(@rsa1024)) + assert_equal(data, p7.decrypt(@ee1_key)) + + assert_raise(OpenSSL::PKCS7::PKCS7Error) { + p7.decrypt(@ca_key, @ca_cert) + } # Default cipher has been removed in v3.3 assert_raise_with_message(ArgumentError, /RC2-40-CBC/) { @@ -232,7 +275,8 @@ def test_data # PKCS7#verify can't distinguish verification failure and other errors store = OpenSSL::X509::Store.new assert_equal(false, p7.verify([@ee1_cert], store)) - assert_raise(OpenSSL::PKCS7::PKCS7Error) { p7.decrypt(@rsa1024) } + assert_match(/wrong content type|WRONG_CONTENT_TYPE/, p7.error_string) + assert_raise(OpenSSL::PKCS7::PKCS7Error) { p7.decrypt(@ee1_key) } end def test_empty_signed_data_ruby_bug_19974 @@ -293,7 +337,7 @@ def test_smime ca_certs = [@ca_cert] data = "aaaaa\r\nbbbbb\r\nccccc\r\n" - tmp = OpenSSL::PKCS7.sign(@ee1_cert, @rsa1024, data, ca_certs) + tmp = OpenSSL::PKCS7.sign(@ee1_cert, @ee1_key, data, ca_certs) p7 = OpenSSL::PKCS7.new(tmp.to_der) smime = OpenSSL::PKCS7.write_smime(p7) assert_equal(true, smime.start_with?(< Date: Thu, 24 Jul 2025 13:01:04 -0400 Subject: [PATCH 13/16] ZJIT: A64: Add add_extended() which can add a register to sp --- zjit/src/asm/arm64/mod.rs | 67 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/zjit/src/asm/arm64/mod.rs b/zjit/src/asm/arm64/mod.rs index d1fa3b0d9f9c9b..ae14d561f65b16 100644 --- a/zjit/src/asm/arm64/mod.rs +++ b/zjit/src/asm/arm64/mod.rs @@ -13,6 +13,21 @@ use inst::*; pub use arg::*; pub use opnd::*; +/// The extend type for register operands in extended register instructions. +/// It's the reuslt size is determined by the the destination register and +/// the source size interpreted using the last letter. +#[derive(Clone, Copy)] +pub enum ExtendType { + UXTB = 0b000, // unsigned extend byte + UXTH = 0b001, // unsigned extend halfword + UXTW = 0b010, // unsigned extend word + UXTX = 0b011, // unsigned extend doubleword + SXTB = 0b100, // signed extend byte + SXTH = 0b101, // signed extend halfword + SXTW = 0b110, // signed extend word + SXTX = 0b111, // signed extend doubleword +} + /// Checks that a signed value fits within the specified number of bits. pub const fn imm_fits_bits(imm: i64, num_bits: u8) -> bool { let minimum = if num_bits == 64 { i64::MIN } else { -(2_i64.pow((num_bits as u32) - 1)) }; @@ -59,6 +74,42 @@ pub fn add(cb: &mut CodeBlock, rd: A64Opnd, rn: A64Opnd, rm: A64Opnd) { cb.write_bytes(&bytes); } +/// Encode ADD (extended register) +/// +/// +/// +/// 31 30 29 28 27 26 25 24 23 22 21 20 19 18 17 16 15 14 13 12 11 10 09 08 07 06 05 04 03 02 01 00 +/// 0 1 0 1 1 0 0 1 │ │ │ │ │ │ │ │ │ │ +/// sf op S └────rm─────┘ └option┘ └─imm3─┘ └────rn─────┘ └────rd─────┘ +fn encode_add_extend(rd: u8, rn: u8, rm: u8, extend_type: ExtendType, shift: u8, num_bits: u8) -> [u8; 4] { + assert!(shift <= 4, "shift must be 0-4"); + + ((Sf::from(num_bits) as u32) << 31 | + 0b0 << 30 | // op = 0 for add + 0b0 << 29 | // S = 0 for non-flag-setting + 0b01011001 << 21 | + (rm as u32) << 16 | + (extend_type as u32) << 13 | + (shift as u32) << 10 | + (rn as u32) << 5 | + rd as u32).to_le_bytes() +} + +/// ADD (extended register) - add rn and rm with UXTX extension (no extension for 64-bit registers) +/// This is equivalent to a regular ADD for 64-bit registers since UXTX with shift 0 means no modification. +/// For reg_no=31, rd and rn mean SP while with rm means the zero register. +pub fn add_extended(cb: &mut CodeBlock, rd: A64Opnd, rn: A64Opnd, rm: A64Opnd) { + let bytes: [u8; 4] = match (rd, rn, rm) { + (A64Opnd::Reg(rd), A64Opnd::Reg(rn), A64Opnd::Reg(rm)) => { + assert!(rd.num_bits == rn.num_bits, "rd and rn must be of the same size."); + encode_add_extend(rd.reg_no, rn.reg_no, rm.reg_no, ExtendType::UXTX, 0, rd.num_bits) + }, + _ => panic!("Invalid operand combination to add_extend instruction."), + }; + + cb.write_bytes(&bytes); +} + /// ADDS - add rn and rm, put the result in rd, update flags pub fn adds(cb: &mut CodeBlock, rd: A64Opnd, rn: A64Opnd, rm: A64Opnd) { let bytes: [u8; 4] = match (rd, rn, rm) { @@ -1142,6 +1193,7 @@ fn cbz_cbnz(num_bits: u8, op: bool, offset: InstructionOffset, rt: u8) -> [u8; 4 #[cfg(test)] mod tests { use super::*; + use crate::assertions::assert_disasm; /// Check that the bytes for an instruction sequence match a hex string fn check_bytes(bytes: &str, run: R) where R: FnOnce(&mut super::CodeBlock) { @@ -1675,4 +1727,19 @@ mod tests { fn test_tst_32b_immediate() { check_bytes("1f3c0072", |cb| tst(cb, W0, A64Opnd::new_uimm(0xffff))); } + + #[test] + fn test_add_extend_various_regs() { + let mut cb = CodeBlock::new_dummy(); + + add_extended(&mut cb, X10, X11, X9); + add_extended(&mut cb, X30, X30, X30); + add_extended(&mut cb, X31, X31, X31); + + assert_disasm!(cb, "6a61298bde633e8bff633f8b", " + 0x0: add x10, x11, x9, uxtx + 0x4: add x30, x30, x30, uxtx + 0x8: add sp, sp, xzr + "); + } } From 214d587a7761def8f422c217f60d89ce9680a553 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Wed, 30 Jul 2025 19:57:55 -0400 Subject: [PATCH 14/16] ZJIT: Only build the assembler for `target_arch` Fixes test error from running the ARM assembler on x86, but then trying to disassemble it as x86. --- zjit/src/asm/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/zjit/src/asm/mod.rs b/zjit/src/asm/mod.rs index 3b9b8a26f7778f..6c3e95546302ae 100644 --- a/zjit/src/asm/mod.rs +++ b/zjit/src/asm/mod.rs @@ -7,7 +7,9 @@ use crate::virtualmem::*; // Lots of manual vertical alignment in there that rustfmt doesn't handle well. #[rustfmt::skip] +#[cfg(target_arch = "x86_64")] pub mod x86_64; +#[cfg(target_arch = "aarch64")] pub mod arm64; /// Index to a label created by cb.new_label() From 0aabbbe31d14fa851ca466291cb77f44a59b2910 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Wed, 30 Jul 2025 19:02:22 -0400 Subject: [PATCH 15/16] ZJIT: Remove false comment [ci skip] --- zjit/src/asm/arm64/arg/shifted_imm.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/zjit/src/asm/arm64/arg/shifted_imm.rs b/zjit/src/asm/arm64/arg/shifted_imm.rs index 4602ac64ab9495..06daefdef7715d 100644 --- a/zjit/src/asm/arm64/arg/shifted_imm.rs +++ b/zjit/src/asm/arm64/arg/shifted_imm.rs @@ -16,7 +16,6 @@ pub struct ShiftedImmediate { impl TryFrom for ShiftedImmediate { type Error = (); - /// Attempt to convert a u64 into a BitmaskImm. fn try_from(value: u64) -> Result { let current = value; if current < 2_u64.pow(12) { From da0de3cb872ce212ce70a7a6f8516356e97bcb2c Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Wed, 30 Jul 2025 19:13:24 -0400 Subject: [PATCH 16/16] ZJIT: A64: Fix splitting for large memory displacements On the ruby side, this fixes a crash for methods with 39 or more parameters. We used to miscomp those entry points due to Insn::Lea picking ADDS which cannot reference SP: # set method params: 40 mov x0, #0xfee8 movk x0, #0xffff, lsl #16 movk x0, #0xffff, lsl #32 movk x0, #0xffff, lsl #48 adds x0, xzr, x0 Have Lea work for all i32 displacements and avoid involving the split pass. Previously, direct use of Insn::Lea directly from the user (as opposed to generated by the split pass for some memory operations) wasn't split, so being able to handle the whole range in arm64_emit() was implicitly required. Also, not going through split reduces register pressure. --- test/ruby/test_zjit.rb | 11 ++++ zjit/src/backend/arm64/mod.rs | 96 +++++++++++++++++++++-------------- 2 files changed, 68 insertions(+), 39 deletions(-) diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index 2d18759f50fbcb..684a4bb2d40723 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -826,6 +826,17 @@ def a(n1,n2,n3,n4,n5,n6,n7,n8) = [n8] } end + def test_forty_param_method + # This used to a trigger a miscomp on A64 due + # to a memory displacement larger than 9 bits. + assert_compiles '1', %Q{ + def foo(#{'_,' * 39} n40) = n40 + + foo(0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1) + } + end + + def test_opt_aref_with assert_compiles ':ok', %q{ def aref_with(hash) = hash["key"] diff --git a/zjit/src/backend/arm64/mod.rs b/zjit/src/backend/arm64/mod.rs index 42dc31c90fd5cc..a1243f45e17e58 100644 --- a/zjit/src/backend/arm64/mod.rs +++ b/zjit/src/backend/arm64/mod.rs @@ -219,29 +219,6 @@ impl Assembler /// have no memory operands. fn arm64_split(mut self) -> Assembler { - /// When we're attempting to load a memory address into a register, the - /// displacement must fit into the maximum number of bits for an Op::Add - /// immediate. If it doesn't, we have to load the displacement into a - /// register first. - fn split_lea_operand(asm: &mut Assembler, opnd: Opnd) -> Opnd { - match opnd { - Opnd::Mem(Mem { base, disp, num_bits }) => { - if disp >= 0 && ShiftedImmediate::try_from(disp as u64).is_ok() { - asm.lea(opnd) - } else { - let disp = asm.load(Opnd::Imm(disp.into())); - let reg = match base { - MemBase::Reg(reg_no) => Opnd::Reg(Reg { reg_no, num_bits }), - MemBase::VReg(idx) => Opnd::VReg { idx, num_bits } - }; - - asm.add(reg, disp) - } - }, - _ => unreachable!("Op::Lea only accepts Opnd::Mem operands.") - } - } - /// When you're storing a register into a memory location or loading a /// memory location into a register, the displacement from the base /// register of the memory location must fit into 9 bits. If it doesn't, @@ -252,7 +229,7 @@ impl Assembler if mem_disp_fits_bits(mem.disp) { opnd } else { - let base = split_lea_operand(asm, opnd); + let base = asm.lea(opnd); Opnd::mem(64, base, 0) } }, @@ -575,7 +552,7 @@ impl Assembler }, Insn::IncrCounter { mem, value } => { let counter_addr = match mem { - Opnd::Mem(_) => split_lea_operand(asm, *mem), + Opnd::Mem(_) => asm.lea(*mem), _ => *mem }; @@ -1113,22 +1090,24 @@ impl Assembler } }, Insn::Lea { opnd, out } => { - let opnd: A64Opnd = opnd.into(); + let &Opnd::Mem(Mem { num_bits: _, base: MemBase::Reg(base_reg_no), disp }) = opnd else { + panic!("Unexpected Insn::Lea operand in arm64_emit: {opnd:?}"); + }; + let out: A64Opnd = out.into(); + let base_reg = A64Opnd::Reg(A64Reg { num_bits: 64, reg_no: base_reg_no }); + assert_ne!(31, out.unwrap_reg().reg_no, "Insn::Lea sp, [sp, #imm] not always encodable. Use add/sub instead."); - match opnd { - A64Opnd::Mem(mem) => { - add( - cb, - out.into(), - A64Opnd::Reg(A64Reg { reg_no: mem.base_reg_no, num_bits: 64 }), - A64Opnd::new_imm(mem.disp.into()) - ); - }, - _ => { - panic!("Op::Lea only accepts Opnd::Mem operands."); - } + if ShiftedImmediate::try_from(disp.unsigned_abs() as u64).is_ok() { + // Use ADD/SUB if the displacement fits + add(cb, out, base_reg, A64Opnd::new_imm(disp.into())); + } else { + // Use add_extended() to interpret reg_no=31 as sp + // since the base register is never the zero register. + // Careful! Only the first two operands can refer to sp. + emit_load_value(cb, out, disp as u64); + add_extended(cb, out, base_reg, out); }; - }, + } Insn::LeaJumpTarget { out, target, .. } => { if let Target::Label(label_idx) = target { // Set output to the raw address of the label @@ -1607,6 +1586,45 @@ mod tests { asm.compile_with_num_regs(&mut cb, 0); } + #[test] + fn test_emit_lea() { + let (mut asm, mut cb) = setup_asm(); + + // Test values that exercise various types of immediates. + // - 9 bit displacement for Load/Store + // - 12 bit shifted immediate + // - bit mask immediates + for displacement in [i32::MAX, 0x10008, 0x1800, 0x208, -0x208, -0x1800, -0x1008, i32::MIN] { + let mem = Opnd::mem(64, NATIVE_STACK_PTR, displacement); + asm.lea_into(Opnd::Reg(X0_REG), mem); + } + + asm.compile_with_num_regs(&mut cb, 0); + assert_disasm!(cb, "e07b40b2e063208b000180d22000a0f2e063208b000083d2e063208be0230891e02308d100009dd2e0ffbff2e0ffdff2e0fffff2e063208b00ff9dd2e0ffbff2e0ffdff2e0fffff2e063208be08361b2e063208b", " + 0x0: orr x0, xzr, #0x7fffffff + 0x4: add x0, sp, x0 + 0x8: mov x0, #8 + 0xc: movk x0, #1, lsl #16 + 0x10: add x0, sp, x0 + 0x14: mov x0, #0x1800 + 0x18: add x0, sp, x0 + 0x1c: add x0, sp, #0x208 + 0x20: sub x0, sp, #0x208 + 0x24: mov x0, #0xe800 + 0x28: movk x0, #0xffff, lsl #16 + 0x2c: movk x0, #0xffff, lsl #32 + 0x30: movk x0, #0xffff, lsl #48 + 0x34: add x0, sp, x0 + 0x38: mov x0, #0xeff8 + 0x3c: movk x0, #0xffff, lsl #16 + 0x40: movk x0, #0xffff, lsl #32 + 0x44: movk x0, #0xffff, lsl #48 + 0x48: add x0, sp, x0 + 0x4c: orr x0, xzr, #0xffffffff80000000 + 0x50: add x0, sp, x0 + "); + } + /* #[test] fn test_emit_lea_label() {