Skip to content

Commit 2707880

Browse files
rmacnak-googlecommit-bot@chromium.org
rmacnak-google
authored and
commit-bot@chromium.org
committed
[vm] Fix various UBSan failures.
Bug: #39427 Change-Id: I74e0eee623d88005fb2893d03e284a87daa09260 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/146696 Reviewed-by: Alexander Markov <alexmarkov@google.com> Commit-Queue: Ryan Macnak <rmacnak@google.com>
1 parent 8c79102 commit 2707880

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+278
-151
lines changed

‎runtime/bin/file_android.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,7 @@ int64_t File::LengthFromPath(Namespace* namespc, const char* name) {
484484
static void MillisecondsToTimespec(int64_t millis, struct timespec* t) {
485485
ASSERT(t != NULL);
486486
t->tv_sec = millis / kMillisecondsPerSecond;
487-
t->tv_nsec = (millis - (t->tv_sec * kMillisecondsPerSecond)) * 1000L;
487+
t->tv_nsec = (millis % kMillisecondsPerSecond) * 1000L;
488488
}
489489

490490
void File::Stat(Namespace* namespc, const char* name, int64_t* data) {

‎runtime/bin/file_fuchsia.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,7 @@ static int64_t TimespecToMilliseconds(const struct timespec& t) {
480480
static void MillisecondsToTimespec(int64_t millis, struct timespec* t) {
481481
ASSERT(t != NULL);
482482
t->tv_sec = millis / kMillisecondsPerSecond;
483-
t->tv_nsec = (millis - (t->tv_sec * kMillisecondsPerSecond)) * 1000L;
483+
t->tv_nsec = (millis % kMillisecondsPerSecond) * 1000L;
484484
}
485485

486486
void File::Stat(Namespace* namespc, const char* name, int64_t* data) {

‎runtime/bin/file_linux.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,7 @@ static int64_t TimespecToMilliseconds(const struct timespec& t) {
489489
static void MillisecondsToTimespec(int64_t millis, struct timespec* t) {
490490
ASSERT(t != NULL);
491491
t->tv_sec = millis / kMillisecondsPerSecond;
492-
t->tv_nsec = (millis - (t->tv_sec * kMillisecondsPerSecond)) * 1000L;
492+
t->tv_nsec = (millis % kMillisecondsPerSecond) * 1000L;
493493
}
494494

495495
void File::Stat(Namespace* namespc, const char* name, int64_t* data) {

‎runtime/bin/hashmap_test.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,12 @@ void TestSet(IntKeyHash hash, int size) {
123123
EXPECT_EQ(0u, set.occupancy());
124124

125125
// Insert a long series of values.
126-
const int start = 453;
127-
const int factor = 13;
128-
const int offset = 7;
126+
const uint32_t start = 453;
127+
const uint32_t factor = 13;
128+
const uint32_t offset = 7;
129129
const uint32_t n = size;
130130

131-
int x = start;
131+
uint32_t x = start;
132132
for (uint32_t i = 0; i < n; i++) {
133133
EXPECT_EQ(i, set.occupancy());
134134
set.Insert(x);

‎runtime/bin/process_test.cc

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,14 @@
66
#include <stdlib.h>
77
#include <string.h>
88

9+
#if defined(__GNUC__) || defined(__Clang__)
10+
__attribute__((no_sanitize("undefined")))
11+
#endif
12+
void Crash() {
13+
int* segfault = NULL;
14+
*segfault = 1;
15+
}
16+
917
/*
1018
* Run ./process_test <outstream> <echocount> <exitcode> <crash>
1119
* <outstream>: 0 = stdout, 1 = stderr, 2 = stdout and stderr
@@ -32,8 +40,7 @@ int main(int argc, char* argv[]) {
3240
int crash = atoi(argv[4]);
3341

3442
if (crash == 1) {
35-
int* segfault = NULL;
36-
*segfault = 1;
43+
Crash();
3744
}
3845

3946
const int kLineSize = 128;

‎runtime/lib/double.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,8 @@ DEFINE_NATIVE_ENTRY(Double_hashCode, 0, 1) {
8989
if (FLAG_trace_intrinsified_natives) {
9090
OS::PrintErr("Double_hashCode %f\n", val);
9191
}
92-
if (val >= static_cast<double>(kMinInt64) &&
93-
val <= static_cast<double>(kMaxInt64)) {
92+
if ((val >= kMinInt64RepresentableAsDouble) &&
93+
(val <= kMaxInt64RepresentableAsDouble)) {
9494
int64_t ival = static_cast<int64_t>(val);
9595
if (static_cast<double>(ival) == val) {
9696
return Integer::New(ival);

‎runtime/platform/globals.h

Lines changed: 2 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,8 @@ const int32_t kMaxInt32 = 0x7FFFFFFF;
459459
const uint32_t kMaxUint32 = 0xFFFFFFFF;
460460
const int64_t kMinInt64 = DART_INT64_C(0x8000000000000000);
461461
const int64_t kMaxInt64 = DART_INT64_C(0x7FFFFFFFFFFFFFFF);
462+
const int64_t kMinInt64RepresentableAsDouble = kMinInt64;
463+
const int64_t kMaxInt64RepresentableAsDouble = DART_INT64_C(0x7FFFFFFFFFFFFC00);
462464
const uint64_t kMaxUint64 = DART_2PART_UINT64_C(0xFFFFFFFF, FFFFFFFF);
463465
const int64_t kSignBitDouble = DART_INT64_C(0x8000000000000000);
464466

@@ -636,36 +638,6 @@ inline D bit_copy(const S& source) {
636638
return destination;
637639
}
638640

639-
#if defined(HOST_ARCH_ARM) || defined(HOST_ARCH_ARM64)
640-
// Similar to bit_copy and bit_cast, but does take the type from the argument.
641-
template <typename T>
642-
static inline T ReadUnaligned(const T* ptr) {
643-
T value;
644-
memcpy(reinterpret_cast<void*>(&value), reinterpret_cast<const void*>(ptr),
645-
sizeof(value));
646-
return value;
647-
}
648-
649-
// Similar to bit_copy and bit_cast, but does take the type from the argument.
650-
template <typename T>
651-
static inline void StoreUnaligned(T* ptr, T value) {
652-
memcpy(reinterpret_cast<void*>(ptr), reinterpret_cast<const void*>(&value),
653-
sizeof(value));
654-
}
655-
#else // !(HOST_ARCH_ARM || HOST_ARCH_ARM64)
656-
// Similar to bit_copy and bit_cast, but does take the type from the argument.
657-
template <typename T>
658-
static inline T ReadUnaligned(const T* ptr) {
659-
return *ptr;
660-
}
661-
662-
// Similar to bit_copy and bit_cast, but does take the type from the argument.
663-
template <typename T>
664-
static inline void StoreUnaligned(T* ptr, T value) {
665-
*ptr = value;
666-
}
667-
#endif // !(HOST_ARCH_ARM || HOST_ARCH_ARM64)
668-
669641
// On Windows the reentrent version of strtok is called
670642
// strtok_s. Unify on the posix name strtok_r.
671643
#if defined(HOST_OS_WINDOWS)

‎runtime/platform/growable_array.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,9 @@ class BaseGrowableArray : public B {
205205
template <typename T, typename B, typename Allocator>
206206
inline void BaseGrowableArray<T, B, Allocator>::Sort(int compare(const T*,
207207
const T*)) {
208+
// Avoid calling qsort with a null array.
209+
if (length_ == 0) return;
210+
208211
typedef int (*CompareFunction)(const void*, const void*);
209212
qsort(data_, length_, sizeof(T), reinterpret_cast<CompareFunction>(compare));
210213
}

‎runtime/platform/safe_stack.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55
#ifndef RUNTIME_PLATFORM_SAFE_STACK_H_
66
#define RUNTIME_PLATFORM_SAFE_STACK_H_
77

8-
#include "platform/globals.h"
9-
108
#if defined(__has_feature)
119
#if __has_feature(safe_stack)
1210
#define USING_SAFE_STACK

‎runtime/platform/unaligned.h

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
#ifndef RUNTIME_PLATFORM_UNALIGNED_H_
6+
#define RUNTIME_PLATFORM_UNALIGNED_H_
7+
8+
#include "platform/globals.h"
9+
#include "platform/undefined_behavior_sanitizer.h"
10+
11+
namespace dart {
12+
13+
#if defined(HOST_ARCH_ARM) || defined(HOST_ARCH_ARM64)
14+
template <typename T>
15+
static inline T LoadUnaligned(const T* ptr) {
16+
T value;
17+
memcpy(reinterpret_cast<void*>(&value), reinterpret_cast<const void*>(ptr),
18+
sizeof(value));
19+
return value;
20+
}
21+
22+
template <typename T>
23+
static inline void StoreUnaligned(T* ptr, T value) {
24+
memcpy(reinterpret_cast<void*>(ptr), reinterpret_cast<const void*>(&value),
25+
sizeof(value));
26+
}
27+
#else // !(HOST_ARCH_ARM || HOST_ARCH_ARM64)
28+
template <typename T>
29+
NO_SANITIZE_UNDEFINED("alignment")
30+
static inline T LoadUnaligned(const T* ptr) {
31+
return *ptr;
32+
}
33+
34+
template <typename T>
35+
NO_SANITIZE_UNDEFINED("alignment")
36+
static inline void StoreUnaligned(T* ptr, T value) {
37+
*ptr = value;
38+
}
39+
#endif // !(HOST_ARCH_ARM || HOST_ARCH_ARM64)
40+
41+
} // namespace dart
42+
43+
#endif // RUNTIME_PLATFORM_UNALIGNED_H_

‎runtime/platform/undefined_behavior_sanitizer.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
#ifndef RUNTIME_PLATFORM_UNDEFINED_BEHAVIOR_SANITIZER_H_
6+
#define RUNTIME_PLATFORM_UNDEFINED_BEHAVIOR_SANITIZER_H_
7+
8+
#if defined(__has_feature)
9+
#if __has_feature(undefined_behavior_sanitizer)
10+
#define USING_UNDEFINED_BEHAVIOR_SANITIZER
11+
#endif
12+
#endif
13+
14+
#if defined(USING_UNDEFINED_BEHAVIOR_SANITIZER)
15+
#define NO_SANITIZE_UNDEFINED(check) __attribute__((no_sanitize(check)))
16+
#else
17+
#define NO_SANITIZE_UNDEFINED(check)
18+
#endif
19+
20+
#endif // RUNTIME_PLATFORM_UNDEFINED_BEHAVIOR_SANITIZER_H_

‎runtime/platform/unicode.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include "platform/allocation.h"
99
#include "platform/globals.h"
10+
#include "platform/unaligned.h"
1011

1112
namespace dart {
1213

@@ -132,9 +133,9 @@ class Utf16 : AllStatic {
132133
// Returns the character at i and advances i to the next character
133134
// boundary.
134135
static int32_t Next(const uint16_t* characters, intptr_t* i, intptr_t len) {
135-
int32_t ch = characters[*i];
136+
int32_t ch = LoadUnaligned(&characters[*i]);
136137
if (Utf16::IsLeadSurrogate(ch) && (*i < (len - 1))) {
137-
int32_t ch2 = characters[*i + 1];
138+
int32_t ch2 = LoadUnaligned(&characters[*i + 1]);
138139
if (Utf16::IsTrailSurrogate(ch2)) {
139140
ch = Utf16::Decode(ch, ch2);
140141
*i += 1;

‎runtime/platform/utils.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ class Utils {
215215
}
216216

217217
static inline int64_t LowHighTo64Bits(uint32_t low, int32_t high) {
218-
return (static_cast<int64_t>(high) << 32) | (low & 0x0ffffffffLL);
218+
return (static_cast<uint64_t>(high) << 32) | (low & 0x0ffffffffLL);
219219
}
220220

221221
static inline constexpr bool IsAlphaNumeric(uint32_t c) {
@@ -347,7 +347,7 @@ class Utils {
347347
"Unexpected uword size");
348348
return std::numeric_limits<uword>::max();
349349
}
350-
return (1ll << n) - 1;
350+
return (static_cast<uword>(1) << n) - 1;
351351
}
352352

353353
static word SignedNBitMask(uint32_t n) {
@@ -373,22 +373,22 @@ class Utils {
373373
static ValueType DecodeSLEB128(const uint8_t* data,
374374
const intptr_t data_length,
375375
intptr_t* byte_index) {
376+
using Unsigned = typename std::make_unsigned<ValueType>::type;
376377
ASSERT(*byte_index < data_length);
377378
uword shift = 0;
378-
ValueType value = 0;
379+
Unsigned value = 0;
379380
uint8_t part = 0;
380381
do {
381382
part = data[(*byte_index)++];
382-
value |= static_cast<ValueType>(part & 0x7f) << shift;
383+
value |= static_cast<Unsigned>(part & 0x7f) << shift;
383384
shift += 7;
384385
} while ((part & 0x80) != 0);
385386

386387
if ((shift < (sizeof(ValueType) * CHAR_BIT)) && ((part & 0x40) != 0)) {
387-
using Unsigned = typename std::make_unsigned<ValueType>::type;
388388
const Unsigned kMax = std::numeric_limits<Unsigned>::max();
389-
value |= static_cast<ValueType>(kMax << shift);
389+
value |= static_cast<Unsigned>(kMax << shift);
390390
}
391-
return value;
391+
return static_cast<ValueType>(value);
392392
}
393393

394394
static char* StrError(int err, char* buffer, size_t bufsize);

‎runtime/vm/code_patcher_ia32.cc

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "vm/globals.h" // Needed here to get TARGET_ARCH_IA32.
66
#if defined(TARGET_ARCH_IA32)
77

8+
#include "platform/unaligned.h"
89
#include "vm/code_patcher.h"
910
#include "vm/cpu.h"
1011
#include "vm/dart_entry.h"
@@ -89,21 +90,18 @@ class InstanceCall : public UnoptimizedCall {
8990
#endif // DEBUG
9091
}
9192

92-
ObjectPtr data() const { return *reinterpret_cast<ObjectPtr*>(start_ + 1); }
93+
ObjectPtr data() const {
94+
return LoadUnaligned(reinterpret_cast<ObjectPtr*>(start_ + 1));
95+
}
9396
void set_data(const Object& data) const {
94-
uword* cache_addr = reinterpret_cast<uword*>(start_ + 1);
95-
uword imm = static_cast<uword>(data.raw());
96-
*cache_addr = imm;
97+
StoreUnaligned(reinterpret_cast<ObjectPtr*>(start_ + 1), data.raw());
9798
}
9899

99100
CodePtr target() const {
100-
const uword imm = *reinterpret_cast<uword*>(start_ + 6);
101-
return static_cast<CodePtr>(imm);
101+
return LoadUnaligned(reinterpret_cast<CodePtr*>(start_ + 6));
102102
}
103103
void set_target(const Code& target) const {
104-
uword* target_addr = reinterpret_cast<uword*>(start_ + 6);
105-
uword imm = static_cast<uword>(target.raw());
106-
*target_addr = imm;
104+
StoreUnaligned(reinterpret_cast<CodePtr*>(start_ + 6), target.raw());
107105
}
108106

109107
private:

‎runtime/vm/compiler/assembler/assembler_base.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ class PatchCodeWithHandle : public AssemblerFixup {
179179
// Patch the handle into the code. Once the instructions are installed into
180180
// a raw code object and the pointer offsets are setup, the handle is
181181
// resolved.
182-
region.Store<const Object*>(position, &object_);
182+
region.StoreUnaligned<const Object*>(position, &object_);
183183
pointer_offsets_->Add(position);
184184
}
185185

‎runtime/vm/compiler/assembler/assembler_base.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#endif // defined(DART_PRECOMPILED_RUNTIME)
1111

1212
#include "platform/assert.h"
13+
#include "platform/unaligned.h"
1314
#include "vm/allocation.h"
1415
#include "vm/compiler/assembler/object_pool_builder.h"
1516
#include "vm/compiler/runtime_api.h"
@@ -164,7 +165,13 @@ class AssemblerBuffer : public ValueObject {
164165
template <typename T>
165166
void Emit(T value) {
166167
ASSERT(HasEnsuredCapacity());
168+
#if defined(TARGET_ARCH_IA32) || defined(TARGET_ARCH_X64)
169+
// Variable-length instructions in ia32/x64 have unaligned immediates.
170+
StoreUnaligned(reinterpret_cast<T*>(cursor_), value);
171+
#else
172+
// Other architecture have aligned, fixed-length instructions.
167173
*reinterpret_cast<T*>(cursor_) = value;
174+
#endif
168175
cursor_ += sizeof(T);
169176
}
170177

@@ -181,14 +188,26 @@ class AssemblerBuffer : public ValueObject {
181188
T Load(intptr_t position) {
182189
ASSERT(position >= 0 &&
183190
position <= (Size() - static_cast<intptr_t>(sizeof(T))));
191+
#if defined(TARGET_ARCH_IA32) || defined(TARGET_ARCH_X64)
192+
// Variable-length instructions in ia32/x64 have unaligned immediates.
193+
return LoadUnaligned(reinterpret_cast<T*>(contents_ + position));
194+
#else
195+
// Other architecture have aligned, fixed-length instructions.
184196
return *reinterpret_cast<T*>(contents_ + position);
197+
#endif
185198
}
186199

187200
template <typename T>
188201
void Store(intptr_t position, T value) {
189202
ASSERT(position >= 0 &&
190203
position <= (Size() - static_cast<intptr_t>(sizeof(T))));
204+
#if defined(TARGET_ARCH_IA32) || defined(TARGET_ARCH_X64)
205+
// Variable-length instructions in ia32/x64 have unaligned immediates.
206+
StoreUnaligned(reinterpret_cast<T*>(contents_ + position), value);
207+
#else
208+
// Other architecture have aligned, fixed-length instructions.
191209
*reinterpret_cast<T*>(contents_ + position) = value;
210+
#endif
192211
}
193212

194213
const ZoneGrowableArray<intptr_t>& pointer_offsets() const {

‎runtime/vm/compiler/assembler/disassembler.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
#include "vm/compiler/assembler/disassembler.h"
66

7+
#include "platform/unaligned.h"
78
#include "vm/code_patcher.h"
89
#include "vm/deopt_instructions.h"
910
#include "vm/globals.h"
@@ -232,7 +233,7 @@ void Disassembler::DisassembleCodeHelper(const char* function_fullname,
232233
Object& obj = Object::Handle(zone);
233234
for (intptr_t i = code.pointer_offsets_length() - 1; i >= 0; i--) {
234235
const uword addr = code.GetPointerOffsetAt(i) + code.PayloadStart();
235-
obj = *reinterpret_cast<ObjectPtr*>(addr);
236+
obj = LoadUnaligned(reinterpret_cast<ObjectPtr*>(addr));
236237
THR_Print(" %d : %#" Px " '%s'\n", code.GetPointerOffsetAt(i), addr,
237238
obj.ToCString());
238239
}

0 commit comments

Comments
 (0)