Restore line in newlocale.c that was lost in #4813#13584
Conversation
When musl was last upgraded this malloc line removed from newlocale.c, perhaps by accident. This restores the behaviour of musl which is to allow `newlocale` with arbitrary names to succeed. I verified that this is the case using musl on my desktop. The reason that test_locale_wrong was passing prior to #4813 is that the musl older version of musl we were using prior to #4813 rejected all locales except for "C" and "POSIX". Upstream version that we are currently based on: https://github.com/emscripten-core/musl/blob/v1.1.15/src/locale/newlocale.c#L44 Upstream version of musl that we were previously using: https://github.com/emscripten-core/musl/blob/0b44a0315b47dd8eced9f3b7f31580cf14bbfc01/src/locale/newlocale.c#L5
|
|
||
| def test_setlocale(self): | ||
| self.do_run_in_out_file_test('tests', 'core', 'test_setlocale.c') | ||
|
|
There was a problem hiding this comment.
these feel excessive to me in core as I don't think opt levels matter for them? how about other?
There was a problem hiding this comment.
Sure, but I think there are a whole lot of tests that fall into that category no? For example this existing test_locale was in the wrong place according your assessment? How should we make this decision about which test suite a give test belongs in? (I'm sure I filed a bug about this discussion at some point but I can't seem to find it now).
There was a problem hiding this comment.
My reasoning is that core runs on various opt levels, so tests that want that should be there. Otherwise, it can be in other.
A codegen test for exceptions would be in core, for example, to see the optimizer can handle it. A libc issue like this could be in other.
|
Ah I didn't notice the auto-merge. Anyhow, no worries, if you agree on the comment it can be a followup (or not if not). |
|
But core also has other configurations too (asan, SAFE_HEAP, STACK_OVERFLOW_CHECK, MINIMAL_RUNTIME etc..). Sometimes when I add a new feature of setting I add a new core test config in the hope I can run a "big enough" set of general tests to find most bugs. Also, core is currently has a lot of general stdlib tests. For example test_stdlibs, test_strtoll_hex, test_time_c. It looks like there are tons of them. Should we move all them to other? |
|
I'm not disagreeing with you, but rather trying to decide on a policy. |
|
I think there won't ever be a simple rule. Some libc tests should run in various modes to see the optimizer doesn't do bad things to strtoll, say, which might have interesting irreducible control flow. And asan can find libc bugs, good point. But when we don't have a reasonable reason to think of wanting to run multiple modes, I'd say |
When musl was last upgraded this malloc line removed from newlocale.c,
perhaps by accident.
This restores the behaviour of musl which is to allow
newlocalewitharbitrary names to succeed. I verified that this is the case using musl
on my desktop.
The reason that test_locale_wrong was passing prior to #4813 is that
the musl older version of musl we were using prior to #4813 rejected
all locales except for "C" and "POSIX".
Upstream version that we are currently based on:
https://github.com/emscripten-core/musl/blob/v1.1.15/src/locale/newlocale.c#L44
Upstream version of musl that we were previously using:
https://github.com/emscripten-core/musl/blob/0b44a0315b47dd8eced9f3b7f31580cf14bbfc01/src/locale/newlocale.c#L5