Bug 265871 - lib.libc.locale.c16rtomb_test.c16rtomb_utf_8_test failed after iconv changes on 2022-08-11
Summary: lib.libc.locale.c16rtomb_test.c16rtomb_utf_8_test failed after iconv changes ...
Status: In Progress
Alias: None
Product: Base System
Classification: Unclassified
Component: tests (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Kyle Evans
URL: https://reviews.freebsd.org/D41513
Keywords:
Depends on:
Blocks:
 
Reported: 2022-08-16 05:13 UTC by Li-Wen Hsu
Modified: 2023-08-28 17:24 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Li-Wen Hsu freebsd_committer freebsd_triage 2022-08-16 05:13:23 UTC
https://ci.freebsd.org/job/FreeBSD-main-amd64-test/21834/testReport/junit/lib.libc.locale/c16rtomb_test/c16rtomb_utf_8_test/


/usr/src/lib/libc/tests/locale/c16rtomb_test.c:145: c16rtomb(buf, L'A', &s) == (size_t)-1 not met
Comment 1 commit-hook freebsd_committer freebsd_triage 2022-08-16 05:21:11 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=b80572fe3b3a93e0490c90901c7e5336f0210303

commit b80572fe3b3a93e0490c90901c7e5336f0210303
Author:     Li-Wen Hsu <lwhsu@FreeBSD.org>
AuthorDate: 2022-08-16 05:18:15 +0000
Commit:     Li-Wen Hsu <lwhsu@FreeBSD.org>
CommitDate: 2022-08-16 05:18:15 +0000

    libc/locale tests: temporarily skip lib.libc.locale.c16rtomb_test.c16rtomb_utf_8_test

    This test case starts failing after iconv changes on 2022-08-11.

    PR:             265871
    Sponsored by:   The FreeBSD Foundation

 lib/libc/tests/locale/c16rtomb_test.c | 3 +++
 1 file changed, 3 insertions(+)
Comment 2 Kyle Evans freebsd_committer freebsd_triage 2022-08-16 05:22:55 UTC
Already marked as skip, taking this PR for the fix.
Comment 3 Mark Johnston freebsd_committer freebsd_triage 2023-03-14 17:02:10 UTC
(In reply to Kyle Evans from comment #2)
Do you have a patch for this?  I'm seeing the same test failure locally.
Comment 4 Kyle Evans freebsd_committer freebsd_triage 2023-03-20 01:59:24 UTC
(In reply to Mark Johnston from comment #3)

Sorry, I kind of forgot about this (not sure why this one doesn't show up in my weekly nag-mail from bz). I looked at it a bit tonight, but I'm still considering the options a bit.

What's happening is that it's passing through \xd8\x3d, an incomplete UTF-8 sequence, followed by 'A'. With a functional //IGNORE, we're doing the right thing and just dropping the invalid sequence on the floor and keeping the 'A'.

I don't know that that's the right thing for the interface, though; I think this might be worth another flag that you get when you specify //IGNORE while we retain the legacy behavior for ci_discard_ilseq.
Comment 5 commit-hook freebsd_committer freebsd_triage 2023-08-23 03:41:47 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=e2030ca246a777c50a7aacb3a35f88addded21cf

commit e2030ca246a777c50a7aacb3a35f88addded21cf
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2023-08-23 03:40:45 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2023-08-23 03:40:45 +0000

    libc: fix c*rtomb/mbrtoc*

    In 693f88c9da8d ("iconv_std: complete the //IGNORE support"), we
    more completely implemented //IGNORE, which changed the semantics of
    ci_discard_ilseq. DISCARD_ILSEQ semantics are supposed to match
    //IGNORE, so we really can't do much about that particular
    incompatibility.  This broke c*rtomb and mbrtoc* handling of invalid
    sequences, but it turns out they don't want DISCARD_ILSEQ semantics at
    all; they really want the subset that we call
    _CITRUS_ICONV_F_HIDE_INVALID.

    This restores the exact flow in iconv_std to precisely how it happened
    prior to 693f88c9da8d.

    PR:     265871
    Fixes:  693f88c9da8d ("iconv_std: complete the //IGNORE support")
    Sponsored by:   Klara, Inc.
    Differential Revision:  https://reviews.freebsd.org/D41513

 lib/libc/locale/cXXrtomb_iconv.h | 4 ++--
 lib/libc/locale/mbrtocXX_iconv.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
Comment 6 Mark Johnston freebsd_committer freebsd_triage 2023-08-23 13:38:19 UTC
Thank you!  Should we revert b80572fe3b3a93e0490c90901c7e5336f0210303 now?
Comment 7 Kyle Evans freebsd_committer freebsd_triage 2023-08-23 15:29:35 UTC
(In reply to Mark Johnston from comment #6)

Yeah, we should... I was hoping to see a run in CI to confirm my belief that everything is OK now, but then I realized it was marked as skip instead of expected fail, which is perhaps not ideal for things that are just currently broken rather than actively sabotaging the test system in some way (e.g., inducing panics).
Comment 8 commit-hook freebsd_committer freebsd_triage 2023-08-28 17:19:18 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=e0752f431b01f44c50df01f54f1c2c278304a899

commit e0752f431b01f44c50df01f54f1c2c278304a899
Author:     Li-Wen Hsu <lwhsu@FreeBSD.org>
AuthorDate: 2023-08-28 17:18:42 +0000
Commit:     Li-Wen Hsu <lwhsu@FreeBSD.org>
CommitDate: 2023-08-28 17:18:42 +0000

    Revert "libc/locale tests: temporarily skip lib.libc.locale.c16rtomb_test.c16rtomb_utf_8_test"

    This reverts commit b80572fe3b3a93e0490c90901c7e5336f0210303.

    This has been fixed in e2030ca246a777c50a7aacb3a35f88addded21cf.

    PR:             265871

 lib/libc/tests/locale/c16rtomb_test.c | 3 ---
 1 file changed, 3 deletions(-)
Comment 9 Li-Wen Hsu freebsd_committer freebsd_triage 2023-08-28 17:24:58 UTC
(In reply to Kyle Evans from comment #7)
Yes this should have been marked as expected failure. The skip should only be marked for flaky tests or tests caused panic, etc.

I've tested this with VM image from https://artifact.ci.freebsd.org/snapshot/main/7ea28254ec5376b5deb86c136e1838d0134dbb22/amd64/amd64/disk-test.img.zst

and:

kyua --variable test_suites.FreeBSD.ci=false test c16rtomb_test:c16rtomb_utf_8_test