string.c: drop a case walk's check that nothing reaches - #7191

mrb_str_case_convert_unicode(), the walk String#upcase, #downcase, #capitalize and #swapcase reach for where a string holds a character the case tables speak about, asks twice whether there is anything to walk:

  if (RSTR_BINARY_P(s) || str_ascii_p(s)) return -1;

  str_modify_keep_cr(mrb, s);
  if (RSTR_LEN(s) == 0 || RSTR_PTR(s) == NULL) return -1;

The second question has no answer left to give, and this drops it.

Why the first line has already answered

str_ascii_p() answers FALSE in one place only, out of its scan:

static mrb_bool
str_ascii_p(struct RString *s)
{
  if (RSTR_CODERANGE(s) == MRB_STR_CODERANGE_7BIT) return TRUE;

  const char *p = RSTR_PTR(s);
  const char *e = p + RSTR_LEN(s);
  if (search_nonascii(p, e) != e) return FALSE;
  RSTR_CODERANGE_SET(s, MRB_STR_CODERANGE_7BIT);
  return TRUE;
}

and search_nonascii() hands back something other than e only where it read a byte with the high bit set. So a string that gets past the first line has a length above zero and a pointer that was read through, which is both halves of the check below it. str_modify_keep_cr() changes neither: it unshares the buffer, which allocates, and clears a coderange that says broken.

An empty string leaves through the first line rather than the second. search_nonascii(p, e) does not enter its loop when p == e, so it hands back e, str_ascii_p() records the string as holding nothing but ASCII and answers TRUE, and the -1 sends the caller back to the byte loop it has of its own, where the same question waits:

  str_modify_keep_cr(mrb, s);
  char *p = RSTR_PTR(s);
  char *pend = RSTR_PTR(s) + len;
  if (len == 0 || p == NULL) return mrb_nil_value();

That is where the line came from: it arrived with the function, in dcddbd4, written beside those loops.

Generated code

gcc -O3, ci/gcc-clang bintest, against the same object built from master. The check is emitted, since nothing tells the compiler what str_ascii_p() answers: one test where the string is embedded and two where it is on the heap, all three jumping to the return -1 at a4a8 that the first line has already reached.

master, on return from str_modify_keep_cr():

    a50a:	8b 51 08             	mov    0x8(%rcx),%edx
    a50d:	89 d0                	mov    %edx,%eax
    a50f:	c1 e8 0c             	shr    $0xc,%eax
    a512:	80 e6 80             	and    $0x80,%dh
    a515:	0f 84 72 03 00 00    	je     a88d                <- on the heap
    a51b:	a9 f0 01 00 00       	test   $0x1f0,%eax         <- embedded length
    a520:	74 86                	je     a4a8                <- to return -1

    a88d:	4c 8b 71 10          	mov    0x10(%rcx),%r14     <- heap length
    a891:	4d 85 f6             	test   %r14,%r14
    a894:	0f 84 0e fc ff ff    	je     a4a8
    a89a:	48 8b 69 20          	mov    0x20(%rcx),%rbp     <- heap pointer
    a89e:	48 89 4c 24 08       	mov    %rcx,0x8(%rsp)
    a8a3:	48 85 ed             	test   %rbp,%rbp
    a8a6:	0f 84 fc fb ff ff    	je     a4a8

this branch, where the same two paths walk straight into the conversion:

    a4f7:	8b 51 08             	mov    0x8(%rcx),%edx
    a4fa:	89 d0                	mov    %edx,%eax
    a4fc:	c1 e8 0c             	shr    $0xc,%eax
    a4ff:	80 e6 80             	and    $0x80,%dh
    a502:	0f 84 75 03 00 00    	je     a87d
    a508:	c1 f8 04             	sar    $0x4,%eax           <- embedded length, used

    a87d:	4c 8b 71 10          	mov    0x10(%rcx),%r14     <- heap length, used
    a881:	49 8b 53 50          	mov    0x50(%r11),%rdx
    a885:	4c 89 df             	mov    %r11,%rdi
    a888:	be 12 00 00 00       	mov    $0x12,%esi
    a88d:	48 8b 69 20          	mov    0x20(%rcx),%rbp

Size

.text, master then this branch:

The byte-indexed build is unchanged byte for byte: the walk is behind MRB_UTF8_STRING and is not compiled there at all.

Testing

rake -m test, all green with 0 KO, 0 crash, and no new warnings:

Behaviour is compared directly as well. 1160 case conversions, over 29 receivers spanning empty, ASCII, multi-byte, title case, one-to-many ("ß", "fi"), astral, broken and truncated UTF-8, each in five forms (literal, dup, + "", [0, length], * 1) and through upcase, downcase, capitalize, swapcase and their ! forms, with the length, valid_encoding? and bytesize read afterwards: the output is identical to master byte for byte, and identical again under address,undefined with no report.

Relation to #7190

None, in either direction. #7190 adds an #elif defined(MRB_UTF8_STRING) arm after this function's closing brace, three lines below the line dropped here, and leaves the line itself alone. The two merge cleanly whichever lands first; this branch is on master.

Environment

# ci/gcc-clang, full-debug
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -g3 -O0 -DMRB_GC_STRESS -DMRB_USE_DEBUG_HOOK -DMRB_UNICODE_CASE -DMRB_DEBUG -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER

# ci/gcc-clang, bintest
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRB_GC_FIXED_ARENA -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER -DMRB_USE_DEBUG_HOOK

# ci/gcc-clang, cxx_abi
gcc -g -O3 -Wall -Wundef -Wwrite-strings -x c++ -std=gnu++03 -DMRB_GC_FIXED_ARENA -DMRB_USE_CXX_EXCEPTION -DMRB_USE_CXX_ABI -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER

# ci/gcc-clang, byte-string
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER

# build_config/asan.rb
clang -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -Wzero-length-array -fsanitize=address,undefined -g3 -O0 -DMRB_DEBUG -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER

Summary by CodeRabbit

• Bug Fixes
  • Improved Unicode case conversion handling for strings, including empty and null-related edge cases.