From patchwork Tue Feb 27 15:35:02 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Carlos O'Donell X-Patchwork-Id: 86456 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id AE96B3858284 for ; Tue, 27 Feb 2024 15:35:58 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id A06573858C35 for ; Tue, 27 Feb 2024 15:35:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A06573858C35 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A06573858C35 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709048134; cv=none; b=td7jRJ+ExEHlefE+IYnhtjN5FwucLCwSF+WzdY1pIa+YbFtFLc+waHoANbRU+MfzzUHfIxOhgBdQSaIcHXe9dS8dA7lMy6O4xN03f+n1pI69oBAw9ac6piHWuoYk0+VTG24ScRQ+5p/ZfWVOozAXwepotnaygov+1yHiF28RtCE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709048134; c=relaxed/simple; bh=QhX+9QuvWA7zfW7jBh1E/ReC+mY5mONsZZJCUz95GpM=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=wBfUwJZp9UM4I+FpYOm+i3CK2/wi/zm6dsp5qW/KY0VSCWOzxiKtmnVzpwCkLESwhB6Jz6rC6JivPMQmHcFfXXKocUtAtKBAb2XAREVFMUJrdask09NcwJy20oOzk1VISFkAVym/qMfOitbwFBvbadAGX/JjUQo9Xynh4tja75c= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1709048131; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=TmeQ5+ii7YE+UU9Vi6MemmonxI5Mn6NW8uvfv9ikMV0=; b=KD8sYmiC+tP32GAWkQ7R56gY+m01z3Mnxk0nD/QhLXelJR1T5+WITc3UoGIYFQxOZcRcgY mi49BFDwxdppB16C5Fa/aZBILGs3kLESJPRohi/vLK4yju9d7mabLIQaEFNtUWoqYsHKmv kLDwaHay2bEGB4IbVbbTsJC2R/IbOVo= Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-623-Xmn4o3B_NYqV8vjmWUGxhw-1; Tue, 27 Feb 2024 10:35:29 -0500 X-MC-Unique: Xmn4o3B_NYqV8vjmWUGxhw-1 Received: by mail-qk1-f200.google.com with SMTP id af79cd13be357-787e165b205so98842185a.0 for ; Tue, 27 Feb 2024 07:35:29 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709048129; x=1709652929; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=TmeQ5+ii7YE+UU9Vi6MemmonxI5Mn6NW8uvfv9ikMV0=; b=LPPoSIBfTu43TQ42jH/e8/vKjMYvjNNOq1Etwkm9t6SXClzEsvhvieAFCjRUW8lNSq dj1yOrywWcxXDAhvI66/xyYv2TyB304olb4ySIIRSIaUkTYLmZetjqispZWvvJOzTMTY S2ZDWiU1aRXy/h8/bYse6db7KAn03cfTTwibHqoYC96jt0jqINgvqf3KVkLngbEU0q/8 X1xgakNGokRo4kwWf6AsoeJryhhTTXwJLGHdK8imuCV5E6dhkIbEhIWAAKUj18U4pRgC m/7TajM3WnUtMd/oVGeawP9Oi7Znl/neA4VkCV1Qc8sNNKA7Up9MDBayGJ3OGsKFLsCE 4Dsw== X-Gm-Message-State: AOJu0Yw9OHqsOiLHvkx4A3RZMGl5tuxaj/l4yvod9Vh1jtvyXjWE8HWP ozh9saNMJHF0jCWChSvNoS0l0Kr65IvJv29X97wPKRyXNTKCRL6vlWZhcXqdc4hbz5apOkHgFUI avoqUi1U0CSMB3ukqImRnJs28r+Wn3VV7JHBBxoKwCoZXd/2BxS3aVmmfDrXaNW2h6olyqPzWrU hMGH+ziPDSfcyta2MaxPRdGzh+8tScyqd4MEIsRAQ= X-Received: by 2002:a05:620a:3903:b0:787:90f6:c8d2 with SMTP id qr3-20020a05620a390300b0078790f6c8d2mr3106576qkn.37.1709048128608; Tue, 27 Feb 2024 07:35:28 -0800 (PST) X-Google-Smtp-Source: AGHT+IEXrrZk3rKh6ignIYPs7ahhrgepg5KGqT3kcauXB0zaBchlPies7mPXGL/KuqHuTJ68v6JdNQ== X-Received: by 2002:a05:620a:3903:b0:787:90f6:c8d2 with SMTP id qr3-20020a05620a390300b0078790f6c8d2mr3106533qkn.37.1709048127945; Tue, 27 Feb 2024 07:35:27 -0800 (PST) Received: from athas.localdomain ([198.48.244.52]) by smtp.gmail.com with ESMTPSA id k14-20020a05620a0b8e00b00787b7732c0csm3694168qkh.4.2024.02.27.07.35.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 27 Feb 2024 07:35:27 -0800 (PST) From: Carlos O'Donell To: libc-alpha@sourceware.org, fweimer@redhat.com Cc: Carlos O'Donell Subject: [PATCH] locale: Handle loading a missing locale twice (Bug 14247) Date: Tue, 27 Feb 2024 10:35:02 -0500 Message-ID: <20240227153513.1790813-1-carlos@redhat.com> X-Mailer: git-send-email 2.43.2 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-type: text/plain; charset=UTF-8 X-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Fedora has been carrying a downstream-only patch for 13 years that changes the error handling for newlocale() and other locale related APIs. The patch was likely never upstreamed because it was incomplete and needed further fixes. In an attempt to cleanup our downstream patch collection I've debugged the remaining failures and resolved bug 14247 in the process. 8< --- 8< --- 8< Delay setting file->decided until the data has been successfully loaded by _nl_load_locale(). If the function fails to load the data then we must return and error and leave decided untouched to allow the caller to attempt to load the data again at a later time. We should not set decided to 1 early in the function since doing so may prevent attempting to load it again. We want to try loading it again because that allows an open to fail and set errno correctly. On the other side of this problem is that if we are called again with the same inputs we will fetch the cached version of the object and carry out no open syscalls and that fails to set errno so we must set errno to ENOENT in that case. There is a second code path that has to be handled where the name of the locale matches but the codeset doesn't match. These changes ensure that errno is correctly set on failure in all the return paths in _nl_find_locale(). Adds tst-locale-loadlocale to cover the bug. No regressions on x86_64. Co-authored-by: Jeff Law --- gen-locales.mk | 13 +++- locale/findlocale.c | 19 ++++- locale/loadlocale.c | 2 +- localedata/Makefile | 4 ++ localedata/gen-locale.sh | 24 +++++-- localedata/tst-locale-loadlocale.c | 109 +++++++++++++++++++++++++++++ 6 files changed, 161 insertions(+), 10 deletions(-) create mode 100644 localedata/tst-locale-loadlocale.c diff --git a/gen-locales.mk b/gen-locales.mk index 9c523d2a05..96add65c05 100644 --- a/gen-locales.mk +++ b/gen-locales.mk @@ -1,8 +1,19 @@ # defines target $(gen-locales) that generates the locales given in $(LOCALES) LOCALE_SRCS := $(shell echo "$(LOCALES)"|sed 's/\([^ .]*\)[^@ ]*\(@[^ ]*\)\?/\1\2/g') +# The CHARMAPS dependency handling must be able to process: +# 1. No character map e.g. eo, en_US +# 2. Character maps e.g. en_US.UTF-8 +# 3. Character maps with modifier e.g. tt_RU.UTF-8@iqtelif +# This complicates the processing slightly so we do it multiple edits, +# the first captures the character map with the anchoring period while +# the rest of the edits remove the period to get a valid file or adjust +# the name to match the true name. CHARMAPS := $(shell echo "$(LOCALES)" | \ - sed -e 's/[^ .]*[.]\([^@ ]*\)\(@[^@ ]*\)*/\1/g' -e s/SJIS/SHIFT_JIS/g) + sed -e 's/\([^ .]*\)\([^@ ]*\)\(@[^@ ]*\)*/\2/g' \ + -e 's/^\./ /g' \ + -e 's/ \./ /g' \ + -e s/SJIS/SHIFT_JIS/g) CTYPE_FILES = $(addsuffix /LC_CTYPE,$(LOCALES)) gen-locales := $(addprefix $(common-objpfx)localedata/,$(CTYPE_FILES)) diff --git a/locale/findlocale.c b/locale/findlocale.c index 8d6e4e33e3..43ff7201c1 100644 --- a/locale/findlocale.c +++ b/locale/findlocale.c @@ -244,7 +244,14 @@ _nl_find_locale (const char *locale_path, size_t locale_path_len, locale_file = locale_file->successor[cnt]; if (locale_file == NULL) - return NULL; + { + /* If this is the second time we tried to load a failed + locale then the locale_file value comes from the cache + and we will not carry out any actual filesystem + operations so we must set ENOENT here. */ + __set_errno (ENOENT); + return NULL; + } } /* The LC_CTYPE category allows to check whether a locale is really @@ -291,8 +298,14 @@ _nl_find_locale (const char *locale_path, size_t locale_path_len, if (__gconv_compare_alias (upstr (ccodeset, ccodeset), upstr (clocale_codeset, clocale_codeset)) != 0) - /* The codesets are not identical, don't use the locale. */ - return NULL; + { + /* The codesets are not identical, don't use the locale. + If this is the second time we tried to load a locale + whose codeset doesn't match then the result came from + the cache and must set ENOENT here. */ + __set_errno (ENOENT); + return NULL; + } } /* Determine the locale name for which loading succeeded. This diff --git a/locale/loadlocale.c b/locale/loadlocale.c index 1e5f93e927..991c0591e9 100644 --- a/locale/loadlocale.c +++ b/locale/loadlocale.c @@ -237,7 +237,6 @@ _nl_load_locale (struct loaded_l10nfile *file, int category) int save_err; int alloc = ld_mapped; - file->decided = 1; file->data = NULL; fd = __open_nocancel (file->filename, O_RDONLY | O_CLOEXEC); @@ -345,6 +344,7 @@ _nl_load_locale (struct loaded_l10nfile *file, int category) newdata->alloc = alloc; file->data = newdata; + file->decided = 1; } void diff --git a/localedata/Makefile b/localedata/Makefile index 713e7aebad..6d0bac225b 100644 --- a/localedata/Makefile +++ b/localedata/Makefile @@ -239,6 +239,7 @@ tests = \ tst-iconv-emojis-trans \ tst-iconv-math-trans \ tst-leaks \ + tst-locale-loadlocale \ tst-mbswcs1 \ tst-mbswcs2 \ tst-mbswcs3 \ @@ -325,6 +326,7 @@ LOCALES := \ dsb_DE.UTF-8 \ dz_BT.UTF-8 \ en_GB.UTF-8 \ + en_US \ en_US.ANSI_X3.4-1968 \ en_US.ISO-8859-1\ en_US.UTF-8 \ @@ -406,6 +408,8 @@ include ../gen-locales.mk $(objpfx)tst-iconv-emojis-trans.out: $(gen-locales) $(objpfx)tst-iconv-math-trans.out: $(gen-locales) +# tst-locale-loadlocale: Needs an en_US-named locale for the test. +$(objpfx)tst-locale-loadlocale.out: $(gen-locales) endif include ../Rules diff --git a/localedata/gen-locale.sh b/localedata/gen-locale.sh index b8c422ad13..e400d02b82 100644 --- a/localedata/gen-locale.sh +++ b/localedata/gen-locale.sh @@ -48,9 +48,9 @@ generate_locale () } locfile=`echo $locfile|sed 's|.*/\([^/]*/LC_CTYPE\)|\1|'` -locale=`echo $locfile|sed 's|\([^.]*\)[.].*/LC_CTYPE|\1|'` -charmap=`echo $locfile|sed 's|[^.]*[.]\([^@ ]*\)\(@[^ ]*\)\?/LC_CTYPE|\1|'` -modifier=`echo $locfile|sed 's|[^.]*[.]\([^@ ]*\)\(@[^ ]*\)\?/LC_CTYPE|\2|'` +locale=`echo $locfile|sed 's|\([^.]*\).*/LC_CTYPE|\1|'` +charmap=`echo $locfile|sed -e 's|[^.]*\([^@ ]*\)\(@[^ ]*\)\?/LC_CTYPE|\1|' -e 's|^\.||g'` +modifier=`echo $locfile|sed 's|[^.]*\([^@ ]*\)\(@[^ ]*\)\?/LC_CTYPE|\2|'` echo "Generating locale $locale.$charmap: this might take a while..." @@ -63,10 +63,16 @@ echo "Generating locale $locale.$charmap: this might take a while..." # you to develop in-progress locales. flags="" +charmap_real="$charmap" + +# If no character map is specified then we fall back to UTF-8. +if [ -z "$charmap" ]; then + charmap_real="UTF-8" +fi + # For SJIS the charmap is SHIFT_JIS. We just want the locale to have # a slightly nicer name instead of using "*.SHIFT_SJIS", but that # means we need a mapping here. -charmap_real="$charmap" if [ "$charmap" = "SJIS" ]; then charmap_real="SHIFT_JIS" fi @@ -80,4 +86,12 @@ if [ "$charmap_real" = 'SHIFT_JIS' ] \ flags="$flags --no-warnings=ascii" fi -generate_locale $charmap_real $locale$modifier $locale.$charmap$modifier "$flags" +# If the character map is not specified then we output a locale +# with the just the name of the locale e.g. eo, en_US. This is +# used for test locales that act as fallbacks. +output_file="$locale.$charmap$modifier" +if [ -z "$charmap" ]; then + output_file="$locale" +fi + +generate_locale $charmap_real $locale$modifier $output_file "$flags" diff --git a/localedata/tst-locale-loadlocale.c b/localedata/tst-locale-loadlocale.c new file mode 100644 index 0000000000..14f394d395 --- /dev/null +++ b/localedata/tst-locale-loadlocale.c @@ -0,0 +1,109 @@ +/* Test for locale loading error handling (Bug 14247) + Copyright (C) 2023 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include + +static int +do_test(void) +{ + locale_t loc; + int ret; + int saved_errno; + + /* We must load an en_US-based locale for the final test to fail. + The locale loading code will find the en_US locale already loaded + but the codesets won't match. */ + xsetlocale(LC_ALL, "en_US.UTF-8"); + + /* Call newlocale with an invalid locale. We expect the test system + does not have "invalidlocale" locale. */ + errno = 0; + ret = 0; + loc = newlocale(LC_COLLATE_MASK | LC_CTYPE_MASK, + "invalidlocale.utf8", + 0); + saved_errno = errno; + if (errno != 0) + printf ("PASS: First call to newlocale failed as expected.\n"); + else + { + printf ("FAIL: First call to newlocale unexpectedly succeeded.\n"); + ret = 1; + } + printf("result = %p errno = %d\n", loc, saved_errno); + + /* Call newlocale again with the same name. This triggers bug 14247 where + the second call fails to set errno correctly. */ + errno = 0; + loc = newlocale(LC_COLLATE_MASK | LC_CTYPE_MASK, + "invalidlocale.utf8", + 0); + saved_errno = errno; + if (errno != 0) + printf ("PASS: Second call to newlocale failed as expected.\n"); + else + { + printf ("FAIL: Second call to newlocale unexpectedly succeeded.\n"); + ret = 1; + } + printf("result = %p errno = %d\n", loc, saved_errno); + + /* Next we attempt to load a locale that exists but whose codeset + does not match the codeset of the locale on the system. + This is more difficult to test but we rely on the fact that locale + processing will normalize the locale name and attempt to open + "en_US" with no codeset as a fallback and this will allow us to + compare a loaded "en_US" locale with a UTF-8 codeset to the + ficiticious "en_US.utf99" and get a codeset match failure. */ + errno = 0; + loc = newlocale(LC_COLLATE_MASK | LC_CTYPE_MASK, + "en_US.utf99", + 0); + saved_errno = errno; + if (errno != 0) + printf ("PASS: Third call to newlocale failed as expected.\n"); + else + { + printf ("FAIL: Third call to newlocale unexpectedly succeeded.\n"); + ret = 1; + } + printf("result = %p errno = %d\n", loc, saved_errno); + + /* Call newlocale again with the same name. This triggers bug 14247 where + the second call fails to set errno correctly. */ + errno = 0; + loc = newlocale(LC_COLLATE_MASK | LC_CTYPE_MASK, + "en_US.utf99", + 0); + saved_errno = errno; + if (errno != 0) + printf ("PASS: Fourth call to newlocale failed as expected.\n"); + else + { + printf ("FAIL: Fourth call to newlocale unexpectedly succeeded.\n"); + ret = 1; + } + printf("result = %p errno = %d\n", loc, saved_errno); + + return ret; +} + +#include