Skip to content
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

Fix aliasing-based undefined behavior in string functions. #11215

Merged
merged 1 commit into from
May 26, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 3 additions & 0 deletions system/include/libc/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,7 @@
#define _Noreturn
#endif

#define weak_alias(old, new) \
extern __typeof(old) new __attribute__((__weak__, __alias__(#old)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks like its already defined in src/internal/libc.h.. And in upstream musl it moved to the internal header src/include/features.h (as opposed to this features.h header which is externally visible).

Is this a necessary part of the change? Do the sources in string not already include src/internal/libc.h?

I wonder if its worth bringing over the new src/include/features.h wrapper header?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, you nerd-sniped me. I am seeing how much v1.2.0 I can pull in here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If possible I'd recommend keeping this change focused, unless you want to do a full musl upgrade .. which is quite a big endeavor. (I think it took weeks or maybe months to completely last time it was done).

Copy link
Member

@kripken kripken May 26, 2020

Choose a reason for hiding this comment

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

I agree, let's land this now, but @zekoz please submit more PRs with more 1.2 pieces or even the whole upgrade!


#endif
1 change: 1 addition & 0 deletions system/lib/libc/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ Some changes have been made to the version that was taken from upstream, includi
* Setting `_POSIX_REALTIME_SIGNALS` and `_POSIX_SPAWN` macros to -1, to exclude unsupported functions.

Backported src/stdio/vswprintf.c from 1.1.23 to fix #9305.
Backported src/string/{memccpy,memchr,memmove,stpcpy,stpncpy,strchrnul,strlcpy,strlen}.c from 1.2.0 to fix #7279.
9 changes: 6 additions & 3 deletions system/lib/libc/musl/src/string/memccpy.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,22 @@ void *memccpy(void *restrict dest, const void *restrict src, int c, size_t n)
{
unsigned char *d = dest;
const unsigned char *s = src;
size_t *wd, k;
const size_t *ws;

c = (unsigned char)c;
#ifdef __GNUC__
typedef size_t __attribute__((__may_alias__)) word;
word *wd;
const word *ws;
if (((uintptr_t)s & ALIGN) == ((uintptr_t)d & ALIGN)) {
for (; ((uintptr_t)s & ALIGN) && n && (*d=*s)!=c; n--, s++, d++);
if ((uintptr_t)s & ALIGN) goto tail;
k = ONES * c;
size_t k = ONES * c;
wd=(void *)d; ws=(const void *)s;
for (; n>=sizeof(size_t) && !HASZERO(*ws^k);
n-=sizeof(size_t), ws++, wd++) *wd = *ws;
d=(void *)wd; s=(const void *)ws;
}
#endif
for (; n && (*d=*s)!=c; n--, s++, d++);
tail:
if (*s==c) return d+1;
Expand Down
8 changes: 6 additions & 2 deletions system/lib/libc/musl/src/string/memchr.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,16 @@ void *memchr(const void *src, int c, size_t n)
{
const unsigned char *s = src;
c = (unsigned char)c;
#ifdef __GNUC__
for (; ((uintptr_t)s & ALIGN) && n && *s != c; s++, n--);
if (n && *s != c) {
const size_t *w;
typedef size_t __attribute__((__may_alias__)) word;
const word *w;
size_t k = ONES * c;
for (w = (const void *)s; n>=SS && !HASZERO(*w^k); w++, n-=SS);
for (s = (const void *)w; n && *s != c; s++, n--);
s = (const void *)w;
}
#endif
for (; n && *s != c; s++, n--);
return n ? (void *)s : 0;
}
10 changes: 8 additions & 2 deletions system/lib/libc/musl/src/string/memmove.c
Original file line number Diff line number Diff line change
@@ -1,34 +1,40 @@
#include <string.h>
#include <stdint.h>

#define WT size_t
#ifdef __GNUC__
typedef __attribute__((__may_alias__)) size_t WT;
#define WS (sizeof(WT))
#endif

void *memmove(void *dest, const void *src, size_t n)
{
char *d = dest;
const char *s = src;

if (d==s) return d;
if (s+n <= d || d+n <= s) return memcpy(d, s, n);
if ((uintptr_t)s-(uintptr_t)d-n <= -2*n) return memcpy(d, s, n);

if (d<s) {
#ifdef __GNUC__
if ((uintptr_t)s % WS == (uintptr_t)d % WS) {
while ((uintptr_t)d % WS) {
if (!n--) return dest;
*d++ = *s++;
}
for (; n>=WS; n-=WS, d+=WS, s+=WS) *(WT *)d = *(WT *)s;
}
#endif
for (; n; n--) *d++ = *s++;
} else {
#ifdef __GNUC__
if ((uintptr_t)s % WS == (uintptr_t)d % WS) {
while ((uintptr_t)(d+n) % WS) {
if (!n--) return dest;
d[n] = s[n];
}
while (n>=WS) n-=WS, *(WT *)(d+n) = *(WT *)(s+n);
}
#endif
while (n) n--, d[n] = s[n];
}

Expand Down
9 changes: 5 additions & 4 deletions system/lib/libc/musl/src/string/stpcpy.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#include <string.h>
#include <stdint.h>
#include <limits.h>
#include "libc.h"

#define ALIGN (sizeof(size_t))
#define ONES ((size_t)-1/UCHAR_MAX)
Expand All @@ -10,16 +9,18 @@

char *__stpcpy(char *restrict d, const char *restrict s)
{
size_t *wd;
const size_t *ws;

#ifdef __GNUC__
typedef size_t __attribute__((__may_alias__)) word;
word *wd;
const word *ws;
if ((uintptr_t)s % ALIGN == (uintptr_t)d % ALIGN) {
for (; (uintptr_t)s % ALIGN; s++, d++)
if (!(*d=*s)) return d;
wd=(void *)d; ws=(const void *)s;
for (; !HASZERO(*ws); *wd++ = *ws++);
d=(void *)wd; s=(const void *)ws;
}
#endif
for (; (*d=*s); s++, d++);

return d;
Expand Down
10 changes: 5 additions & 5 deletions system/lib/libc/musl/src/string/stpncpy.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#include <string.h>
#include <stdint.h>
#include <limits.h>
#include "libc.h"

#define ALIGN (sizeof(size_t)-1)
#define ONES ((size_t)-1/UCHAR_MAX)
Expand All @@ -10,9 +9,10 @@

char *__stpncpy(char *restrict d, const char *restrict s, size_t n)
{
size_t *wd;
const size_t *ws;

#ifdef __GNUC__
typedef size_t __attribute__((__may_alias__)) word;
word *wd;
const word *ws;
if (((uintptr_t)s & ALIGN) == ((uintptr_t)d & ALIGN)) {
for (; ((uintptr_t)s & ALIGN) && n && (*d=*s); n--, s++, d++);
if (!n || !*s) goto tail;
Expand All @@ -21,11 +21,11 @@ char *__stpncpy(char *restrict d, const char *restrict s, size_t n)
n-=sizeof(size_t), ws++, wd++) *wd = *ws;
d=(void *)wd; s=(const void *)ws;
}
#endif
for (; n && (*d=*s); n--, s++, d++);
tail:
memset(d, 0, n);
return d;
}

weak_alias(__stpncpy, stpncpy);

12 changes: 7 additions & 5 deletions system/lib/libc/musl/src/string/strchrnul.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#include <string.h>
#include <stdint.h>
#include <limits.h>
#include "libc.h"

#define ALIGN (sizeof(size_t))
#define ONES ((size_t)-1/UCHAR_MAX)
Expand All @@ -10,16 +9,19 @@

char *__strchrnul(const char *s, int c)
{
size_t *w, k;

c = (unsigned char)c;
if (!c) return (char *)s + strlen(s);

#ifdef __GNUC__
typedef size_t __attribute__((__may_alias__)) word;
const word *w;
for (; (uintptr_t)s % ALIGN; s++)
if (!*s || *(unsigned char *)s == c) return (char *)s;
k = ONES * c;
size_t k = ONES * c;
for (w = (void *)s; !HASZERO(*w) && !HASZERO(*w^k); w++);
for (s = (void *)w; *s && *(unsigned char *)s != c; s++);
s = (void *)w;
#endif
for (; *s && *(unsigned char *)s != c; s++);
return (char *)s;
}

Expand Down
8 changes: 6 additions & 2 deletions system/lib/libc/musl/src/string/strlen.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,13 @@
size_t strlen(const char *s)
{
const char *a = s;
const size_t *w;
#ifdef __GNUC__
typedef size_t __attribute__((__may_alias__)) word;
const word *w;
for (; (uintptr_t)s % ALIGN; s++) if (!*s) return s-a;
for (w = (const void *)s; !HASZERO(*w); w++);
for (s = (const void *)w; *s; s++);
s = (const void *)w;
#endif
for (; *s; s++);
return s-a;
}
4 changes: 2 additions & 2 deletions tests/code_size/random_printf_wasm2js.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"a.html": 19929,
"a.html": 19899,
"a.html.gz": 8344,
"total": 19929,
"total": 19899,
"total_gz": 8344
}