Skip to content

Commit 7420835

Browse files
zbjornsongibfahn
zbjornson
authored and
gibfahn
committed
src: fix build for older clang
Removes use of builtins that are unavailable for older clang. Per benchmarks, only uses builtins on Windows, where speedup is significant. Also adds test for unaligned ucs2 buffer write. Between #3410 and #7645, bytes were swapped twice on bigendian platforms if buffer was not two-byte aligned. See comment in #7645. PR-URL: #7645 Fixes: #7618 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent d33c4bf commit 7420835

5 files changed

Lines changed: 119 additions & 115 deletions

File tree

‎src/node_buffer.cc‎

Lines changed: 3 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212

1313
#include <string.h>
1414
#include <limits.h>
15-
#include <utility>
1615

1716
#define BUFFER_ID 0xB0E4
1817

@@ -49,38 +48,6 @@
4948
THROW_AND_RETURN_IF_OOB(end <= end_max); \
5049
size_t length = end - start;
5150

52-
#if defined(__GNUC__) || defined(__clang__)
53-
#define BSWAP_INTRINSIC_2(x) __builtin_bswap16(x)
54-
#define BSWAP_INTRINSIC_4(x) __builtin_bswap32(x)
55-
#define BSWAP_INTRINSIC_8(x) __builtin_bswap64(x)
56-
#elif defined(__linux__)
57-
#include <byteswap.h>
58-
#define BSWAP_INTRINSIC_2(x) bswap_16(x)
59-
#define BSWAP_INTRINSIC_4(x) bswap_32(x)
60-
#define BSWAP_INTRINSIC_8(x) bswap_64(x)
61-
#elif defined(_MSC_VER)
62-
#include <intrin.h>
63-
#define BSWAP_INTRINSIC_2(x) _byteswap_ushort(x);
64-
#define BSWAP_INTRINSIC_4(x) _byteswap_ulong(x);
65-
#define BSWAP_INTRINSIC_8(x) _byteswap_uint64(x);
66-
#else
67-
#define BSWAP_INTRINSIC_2(x) ((x) << 8) | ((x) >> 8)
68-
#define BSWAP_INTRINSIC_4(x) \
69-
(((x) & 0xFF) << 24) | \
70-
(((x) & 0xFF00) << 8) | \
71-
(((x) >> 8) & 0xFF00) | \
72-
(((x) >> 24) & 0xFF)
73-
#define BSWAP_INTRINSIC_8(x) \
74-
(((x) & 0xFF00000000000000ull) >> 56) | \
75-
(((x) & 0x00FF000000000000ull) >> 40) | \
76-
(((x) & 0x0000FF0000000000ull) >> 24) | \
77-
(((x) & 0x000000FF00000000ull) >> 8) | \
78-
(((x) & 0x00000000FF000000ull) << 8) | \
79-
(((x) & 0x0000000000FF0000ull) << 24) | \
80-
(((x) & 0x000000000000FF00ull) << 40) | \
81-
(((x) & 0x00000000000000FFull) << 56)
82-
#endif
83-
8451
namespace node {
8552

8653
// if true, all Buffer and SlowBuffer instances will automatically zero-fill
@@ -1204,23 +1171,7 @@ void Swap16(const FunctionCallbackInfo<Value>& args) {
12041171
Environment* env = Environment::GetCurrent(args);
12051172
THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
12061173
SPREAD_ARG(args[0], ts_obj);
1207-
1208-
CHECK_EQ(ts_obj_length % 2, 0);
1209-
1210-
int align = reinterpret_cast<uintptr_t>(ts_obj_data) % sizeof(uint16_t);
1211-
1212-
if (align == 0) {
1213-
uint16_t* data16 = reinterpret_cast<uint16_t*>(ts_obj_data);
1214-
size_t len16 = ts_obj_length / 2;
1215-
for (size_t i = 0; i < len16; i++) {
1216-
data16[i] = BSWAP_INTRINSIC_2(data16[i]);
1217-
}
1218-
} else {
1219-
for (size_t i = 0; i < ts_obj_length; i += 2) {
1220-
std::swap(ts_obj_data[i], ts_obj_data[i + 1]);
1221-
}
1222-
}
1223-
1174+
SwapBytes16(ts_obj_data, ts_obj_length);
12241175
args.GetReturnValue().Set(args[0]);
12251176
}
12261177

@@ -1229,24 +1180,7 @@ void Swap32(const FunctionCallbackInfo<Value>& args) {
12291180
Environment* env = Environment::GetCurrent(args);
12301181
THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
12311182
SPREAD_ARG(args[0], ts_obj);
1232-
1233-
CHECK_EQ(ts_obj_length % 4, 0);
1234-
1235-
int align = reinterpret_cast<uintptr_t>(ts_obj_data) % sizeof(uint32_t);
1236-
1237-
if (align == 0) {
1238-
uint32_t* data32 = reinterpret_cast<uint32_t*>(ts_obj_data);
1239-
size_t len32 = ts_obj_length / 4;
1240-
for (size_t i = 0; i < len32; i++) {
1241-
data32[i] = BSWAP_INTRINSIC_4(data32[i]);
1242-
}
1243-
} else {
1244-
for (size_t i = 0; i < ts_obj_length; i += 4) {
1245-
std::swap(ts_obj_data[i], ts_obj_data[i + 3]);
1246-
std::swap(ts_obj_data[i + 1], ts_obj_data[i + 2]);
1247-
}
1248-
}
1249-
1183+
SwapBytes32(ts_obj_data, ts_obj_length);
12501184
args.GetReturnValue().Set(args[0]);
12511185
}
12521186

@@ -1255,26 +1189,7 @@ void Swap64(const FunctionCallbackInfo<Value>& args) {
12551189
Environment* env = Environment::GetCurrent(args);
12561190
THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
12571191
SPREAD_ARG(args[0], ts_obj);
1258-
1259-
CHECK_EQ(ts_obj_length % 8, 0);
1260-
1261-
int align = reinterpret_cast<uintptr_t>(ts_obj_data) % sizeof(uint64_t);
1262-
1263-
if (align == 0) {
1264-
uint64_t* data64 = reinterpret_cast<uint64_t*>(ts_obj_data);
1265-
size_t len32 = ts_obj_length / 8;
1266-
for (size_t i = 0; i < len32; i++) {
1267-
data64[i] = BSWAP_INTRINSIC_8(data64[i]);
1268-
}
1269-
} else {
1270-
for (size_t i = 0; i < ts_obj_length; i += 8) {
1271-
std::swap(ts_obj_data[i], ts_obj_data[i + 7]);
1272-
std::swap(ts_obj_data[i + 1], ts_obj_data[i + 6]);
1273-
std::swap(ts_obj_data[i + 2], ts_obj_data[i + 5]);
1274-
std::swap(ts_obj_data[i + 3], ts_obj_data[i + 4]);
1275-
}
1276-
}
1277-
1192+
SwapBytes64(ts_obj_data, ts_obj_length);
12781193
args.GetReturnValue().Set(args[0]);
12791194
}
12801195

‎src/string_bytes.cc‎

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -309,27 +309,13 @@ size_t StringBytes::Write(Isolate* isolate,
309309
if (chars_written != nullptr)
310310
*chars_written = nchars;
311311

312-
if (!IsBigEndian())
313-
break;
314-
315312
// Node's "ucs2" encoding wants LE character data stored in
316313
// the Buffer, so we need to reorder on BE platforms. See
317314
// http://nodejs.org/api/buffer.html regarding Node's "ucs2"
318315
// encoding specification
316+
if (IsBigEndian())
317+
SwapBytes16(buf, nbytes);
319318

320-
const bool is_aligned =
321-
reinterpret_cast<uintptr_t>(buf) % sizeof(uint16_t);
322-
if (is_aligned) {
323-
uint16_t* const dst = reinterpret_cast<uint16_t*>(buf);
324-
SwapBytes(dst, dst, nchars);
325-
}
326-
327-
ASSERT_EQ(sizeof(uint16_t), 2);
328-
for (size_t i = 0; i < nchars; i++) {
329-
char tmp = buf[i * 2];
330-
buf[i * 2] = buf[i * 2 + 1];
331-
buf[i * 2 + 1] = tmp;
332-
}
333319
break;
334320
}
335321

@@ -705,17 +691,19 @@ Local<Value> StringBytes::Encode(Isolate* isolate,
705691
Local<Value> StringBytes::Encode(Isolate* isolate,
706692
const uint16_t* buf,
707693
size_t buflen) {
708-
Local<String> val;
694+
// Node's "ucs2" encoding expects LE character data inside a
695+
// Buffer, so we need to reorder on BE platforms. See
696+
// http://nodejs.org/api/buffer.html regarding Node's "ucs2"
697+
// encoding specification
709698
std::vector<uint16_t> dst;
710699
if (IsBigEndian()) {
711-
// Node's "ucs2" encoding expects LE character data inside a
712-
// Buffer, so we need to reorder on BE platforms. See
713-
// http://nodejs.org/api/buffer.html regarding Node's "ucs2"
714-
// encoding specification
715-
dst.resize(buflen);
716-
SwapBytes(&dst[0], buf, buflen);
700+
dst.assign(buf, buf + buflen);
701+
size_t nbytes = buflen * sizeof(dst[0]);
702+
SwapBytes16(reinterpret_cast<char*>(&dst[0]), nbytes);
717703
buf = &dst[0];
718704
}
705+
706+
Local<String> val;
719707
if (buflen < EXTERN_APEX) {
720708
val = String::NewFromTwoByte(isolate,
721709
buf,

‎src/util-inl.h‎

Lines changed: 94 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,30 @@
44
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
55

66
#include "util.h"
7+
#include <cstring>
8+
9+
#if defined(_MSC_VER)
10+
#include <intrin.h>
11+
#define BSWAP_2(x) _byteswap_ushort(x)
12+
#define BSWAP_4(x) _byteswap_ulong(x)
13+
#define BSWAP_8(x) _byteswap_uint64(x)
14+
#else
15+
#define BSWAP_2(x) ((x) << 8) | ((x) >> 8)
16+
#define BSWAP_4(x) \
17+
(((x) & 0xFF) << 24) | \
18+
(((x) & 0xFF00) << 8) | \
19+
(((x) >> 8) & 0xFF00) | \
20+
(((x) >> 24) & 0xFF)
21+
#define BSWAP_8(x) \
22+
(((x) & 0xFF00000000000000ull) >> 56) | \
23+
(((x) & 0x00FF000000000000ull) >> 40) | \
24+
(((x) & 0x0000FF0000000000ull) >> 24) | \
25+
(((x) & 0x000000FF00000000ull) >> 8) | \
26+
(((x) & 0x00000000FF000000ull) << 8) | \
27+
(((x) & 0x0000000000FF0000ull) << 24) | \
28+
(((x) & 0x000000000000FF00ull) << 40) | \
29+
(((x) & 0x00000000000000FFull) << 56)
30+
#endif
731

832
namespace node {
933

@@ -200,9 +224,76 @@ TypeName* Unwrap(v8::Local<v8::Object> object) {
200224
return static_cast<TypeName*>(pointer);
201225
}
202226

203-
void SwapBytes(uint16_t* dst, const uint16_t* src, size_t buflen) {
204-
for (size_t i = 0; i < buflen; i += 1)
205-
dst[i] = (src[i] << 8) | (src[i] >> 8);
227+
void SwapBytes16(char* data, size_t nbytes) {
228+
CHECK_EQ(nbytes % 2, 0);
229+
230+
#if defined(_MSC_VER)
231+
int align = reinterpret_cast<uintptr_t>(data) % sizeof(uint16_t);
232+
if (align == 0) {
233+
// MSVC has no strict aliasing, and is able to highly optimize this case.
234+
uint16_t* data16 = reinterpret_cast<uint16_t*>(data);
235+
size_t len16 = nbytes / sizeof(*data16);
236+
for (size_t i = 0; i < len16; i++) {
237+
data16[i] = BSWAP_2(data16[i]);
238+
}
239+
return;
240+
}
241+
#endif
242+
243+
uint16_t temp;
244+
for (size_t i = 0; i < nbytes; i += sizeof(temp)) {
245+
memcpy(&temp, &data[i], sizeof(temp));
246+
temp = BSWAP_2(temp);
247+
memcpy(&data[i], &temp, sizeof(temp));
248+
}
249+
}
250+
251+
void SwapBytes32(char* data, size_t nbytes) {
252+
CHECK_EQ(nbytes % 4, 0);
253+
254+
#if defined(_MSC_VER)
255+
int align = reinterpret_cast<uintptr_t>(data) % sizeof(uint32_t);
256+
// MSVC has no strict aliasing, and is able to highly optimize this case.
257+
if (align == 0) {
258+
uint32_t* data32 = reinterpret_cast<uint32_t*>(data);
259+
size_t len32 = nbytes / sizeof(*data32);
260+
for (size_t i = 0; i < len32; i++) {
261+
data32[i] = BSWAP_4(data32[i]);
262+
}
263+
return;
264+
}
265+
#endif
266+
267+
uint32_t temp;
268+
for (size_t i = 0; i < nbytes; i += sizeof(temp)) {
269+
memcpy(&temp, &data[i], sizeof(temp));
270+
temp = BSWAP_4(temp);
271+
memcpy(&data[i], &temp, sizeof(temp));
272+
}
273+
}
274+
275+
void SwapBytes64(char* data, size_t nbytes) {
276+
CHECK_EQ(nbytes % 8, 0);
277+
278+
#if defined(_MSC_VER)
279+
int align = reinterpret_cast<uintptr_t>(data) % sizeof(uint64_t);
280+
if (align == 0) {
281+
// MSVC has no strict aliasing, and is able to highly optimize this case.
282+
uint64_t* data64 = reinterpret_cast<uint64_t*>(data);
283+
size_t len64 = nbytes / sizeof(*data64);
284+
for (size_t i = 0; i < len64; i++) {
285+
data64[i] = BSWAP_8(data64[i]);
286+
}
287+
return;
288+
}
289+
#endif
290+
291+
uint64_t temp;
292+
for (size_t i = 0; i < nbytes; i += sizeof(temp)) {
293+
memcpy(&temp, &data[i], sizeof(temp));
294+
temp = BSWAP_8(temp);
295+
memcpy(&data[i], &temp, sizeof(temp));
296+
}
206297
}
207298

208299
char ToLower(char c) {

‎src/util.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,11 @@ inline void ClearWrap(v8::Local<v8::Object> object);
254254
template <typename TypeName>
255255
inline TypeName* Unwrap(v8::Local<v8::Object> object);
256256

257-
inline void SwapBytes(uint16_t* dst, const uint16_t* src, size_t buflen);
257+
// Swaps bytes in place. nbytes is the number of bytes to swap and must be a
258+
// multiple of the word size (checked by function).
259+
inline void SwapBytes16(char* data, size_t nbytes);
260+
inline void SwapBytes32(char* data, size_t nbytes);
261+
inline void SwapBytes64(char* data, size_t nbytes);
258262

259263
// tolower() is locale-sensitive. Use ToLower() instead.
260264
inline char ToLower(char c);

‎test/parallel/test-buffer-alloc.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,12 @@ assert.strictEqual('<Buffer 81 a3 66 6f 6f a3 62 61 72>', x.inspect());
585585
assert.strictEqual(b.toString(encoding), 'あいうえお');
586586
});
587587

588+
['ucs2', 'ucs-2', 'utf16le', 'utf-16le'].forEach((encoding) => {
589+
const b = Buffer.allocUnsafe(11);
590+
b.write('あいうえお', 1, encoding);
591+
assert.strictEqual(b.toString(encoding, 1), 'あいうえお');
592+
});
593+
588594
{
589595
// latin1 encoding should write only one byte per character.
590596
const b = Buffer.from([0xde, 0xad, 0xbe, 0xef]);

0 commit comments

Comments
 (0)