Skip to content

[libcxx] Work around picolibc argv handling in tests. #127662

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

Merged
merged 6 commits into from
Feb 21, 2025
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion libcxx/utils/libcxx/test/dsl.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,14 +274,39 @@ def hasAnyLocale(config, locales):
%{exec} -- this means that the command may be executed on a remote host
depending on the %{exec} substitution.
"""

# Some locale names contain spaces, and some embedded use cases
# (in particular picolibc under Arm semihosting) can't support
# spaces in argv words. Work around this by applying URL-style
# %-encoding to the locale names, and undoing it in our test
# program.
percent_encode = lambda s: "".join(
"%{:02x}".format(c) if c in b" %\\\"'" or c >= 128 else chr(c)
for c in s.encode()
)

program = """
#include <stddef.h>
#if defined(_LIBCPP_VERSION) && !_LIBCPP_HAS_LOCALIZATION
int main(int, char**) { return 1; }
#else
#include <stdlib.h>
#include <locale.h>
int main(int argc, char** argv) {
for (int i = 1; i < argc; i++) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead, perhaps we could hardcode the locales we're testing in the program itself, and then run that program without any argument?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. I've done that, and updated the PR description to describe the alternative fix (which I hope means that the updated description will be used as the final commit message after squash-and-merge).

// %-decode argv[i] in place
for (char *p = argv[i], *q = argv[i]; *p; p++) {
if (*p == '%') {
char hex[3];
hex[0] = *++p;
hex[1] = *++p;
hex[2] = '\\0';
*q++ = (char)strtoul(hex, NULL, 0);
} else {
*q++ = *p;
}
}
// Try to set the locale
if (::setlocale(LC_ALL, argv[i]) != NULL) {
return 0;
}
Expand All @@ -290,7 +315,9 @@ def hasAnyLocale(config, locales):
}
#endif
"""
return programSucceeds(config, program, args=[shlex.quote(l) for l in locales])
return programSucceeds(
config, program, args=[shlex.quote(percent_encode(l)) for l in locales]
)


@_memoizeExpensiveOperation(lambda c, flags="": (c.substitutions, c.environment, flags))
Expand Down
Loading