languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-09-05 00:13:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

Reland "[platform] Fix Utils::IsAbsoluteUint and rename to MagnitudeIsUint."

Browse source 
datastream.h is included in places where we cannot import the compiler
namespace, so we can't make compiler::target::word the type of
WriteTargetWord. That also would possibly silently truncate a
host-word sized value that was passed in instead of alerting to a
possible issue if those bits were important.

However, just using IsInt fails if the value being passed in is
a compiler::target::uword that is larger than the max value
that fits in a compiler::target::word. Instead, check for
truncation by checking the bit length of the value, which works
for both signed and unsigned target word values.

Original description:

Also remove previous (incorrect) uses in datastream.cc and
image_snapshot.cc, now that Utils::IsInt<T>(N, value) can now
be run for values of N >= sizeof(T).

Fixes https://github.com/dart-lang/sdk/issues/46572

TEST=language/generic/super_bounded_types_test passes on NNBD simarm,
     added value that triggered failure to vm/cc/MagnitudeIsUint

Cq-Include-Trybots: luci.dart.try:vm-kernel-nnbd-linux-release-simarm-try,vm-kernel-precomp-linux-debug-simarm_x64-try
Change-Id: Idbfdda9f28243d206d482a1d9ac7ae76a5022ad2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206201
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
This commit is contained in:
Tess Strickland 2021-07-08 14:22:54 +00:00 committed by commit-bot@chromium.org
parent f36c55501a
commit f709bf12f0
8 changed files with 47 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

18
runtime/platform/utils.h
View file

@ -229,12 +229,22 @@ class Utils {
return value <= limit;
}
// Check whether the magnitude of value fits in N bits, i.e., whether an
// (N+1)-bit sign-magnitude representation can hold value.
// Check whether the magnitude of value fits in N bits. This differs from
// IsInt(N + 1, value) only in that this returns false for the minimum value
// of a N+1 bit two's complement value.
//
// Primarily used for testing whether a two's complement value can be used in
// a place where the sign is replaced with a marker that says whether the
// magnitude is added or subtracted, e.g., the U bit (bit 23) in some ARM7
// instructions.
template <typename T>
static inline bool IsAbsoluteUint(intptr_t N, T value) {
static inline bool MagnitudeIsUint(intptr_t N, T value) {
ASSERT(N >= 1);
return IsInt(N + 1, value);
if constexpr (std::is_signed<T>::value) {
using Unsigned = typename std::make_unsigned<T>::type;
if (value < 0) return IsUint<Unsigned>(N, -value);
}
return IsUint(N, value);
}
static inline int32_t Low16Bits(int32_t value) {

14
runtime/vm/compiler/assembler/assembler_arm.cc
View file

@ -2336,19 +2336,20 @@ bool Address::CanHoldLoadOffset(OperandSize size,
case kUnsignedTwoBytes:
case kWordPair: {
*offset_mask = 0xff;
return Utils::IsAbsoluteUint(8, offset); // Addressing mode 3.
return Utils::MagnitudeIsUint(8, offset); // Addressing mode 3.
}
case kUnsignedByte:
case kFourBytes:
case kUnsignedFourBytes: {
*offset_mask = 0xfff;
return Utils::IsAbsoluteUint(12, offset); // Addressing mode 2.
return Utils::MagnitudeIsUint(12, offset); // Addressing mode 2.
}
case kSWord:
case kDWord: {
*offset_mask = 0x3fc; // Multiple of 4.
// VFP addressing mode.
return (Utils::IsAbsoluteUint(10, offset) && Utils::IsAligned(offset, 4));
return (Utils::MagnitudeIsUint(10, offset) &&
Utils::IsAligned(offset, 4));
}
case kRegList: {
*offset_mask = 0x0;
@ -2369,20 +2370,21 @@ bool Address::CanHoldStoreOffset(OperandSize size,
case kUnsignedTwoBytes:
case kWordPair: {
*offset_mask = 0xff;
return Utils::IsAbsoluteUint(8, offset); // Addressing mode 3.
return Utils::MagnitudeIsUint(8, offset); // Addressing mode 3.
}
case kByte:
case kUnsignedByte:
case kFourBytes:
case kUnsignedFourBytes: {
*offset_mask = 0xfff;
return Utils::IsAbsoluteUint(12, offset); // Addressing mode 2.
return Utils::MagnitudeIsUint(12, offset); // Addressing mode 2.
}
case kSWord:
case kDWord: {
*offset_mask = 0x3fc; // Multiple of 4.
// VFP addressing mode.
return (Utils::IsAbsoluteUint(10, offset) && Utils::IsAligned(offset, 4));
return (Utils::MagnitudeIsUint(10, offset) &&
Utils::IsAligned(offset, 4));
}
case kRegList: {
*offset_mask = 0x0;

2
runtime/vm/compiler/assembler/assembler_arm.h
View file

@ -238,7 +238,7 @@ class Address : public ValueObject {
}
explicit Address(Register rn, int32_t offset = 0, Mode am = Offset) {
ASSERT(Utils::IsAbsoluteUint(12, offset));
ASSERT(Utils::MagnitudeIsUint(12, offset));
kind_ = Immediate;
if (offset < 0) {
encoding_ = (am ^ (1 << kUShift)) | -offset; // Flip U to adjust sign.

2
runtime/vm/compiler/backend/flow_graph_compiler_arm.cc
View file

@ -712,7 +712,7 @@ void FlowGraphCompiler::EmitDispatchTableCall(
} else {
__ add(LR, DISPATCH_TABLE_REG,
compiler::Operand(cid_reg, LSL, compiler::target::kWordSizeLog2));
if (!Utils::IsAbsoluteUint(12, offset)) {
if (!Utils::MagnitudeIsUint(12, offset)) {
const intptr_t adjust = offset & -(1 << 12);
__ AddImmediate(LR, LR, adjust);
offset -= adjust;

2
runtime/vm/compiler/backend/il_arm.cc
View file

@ -1855,7 +1855,7 @@ static bool CanBeImmediateIndex(Value* value,
const intptr_t base_offset =
(is_external ? 0 : (Instance::DataOffsetFor(cid) - kHeapObjectTag));
const int64_t offset = index * scale + base_offset;
if (!Utils::IsAbsoluteUint(12, offset)) {
if (!Utils::MagnitudeIsUint(12, offset)) {
return false;
}
if (compiler::Address::CanHoldImmediateOffset(is_load, cid, offset)) {

3
runtime/vm/datastream.cc
View file

@ -10,8 +10,7 @@
namespace dart {
void BaseWriteStream::WriteTargetWord(word value) {
ASSERT(compiler::target::kBitsPerWord == kBitsPerWord ||
Utils::IsAbsoluteUint(compiler::target::kBitsPerWord, value));
ASSERT(Utils::BitLength(value) <= compiler::target::kBitsPerWord);
WriteFixed(static_cast<compiler::target::word>(value));
}

3
runtime/vm/image_snapshot.cc
View file

@ -1207,8 +1207,7 @@ void AssemblyImageWriter::ExitSection(ProgramSection name,
}
intptr_t AssemblyImageWriter::WriteTargetWord(word value) {
ASSERT(compiler::target::kBitsPerWord == kBitsPerWord ||
Utils::IsAbsoluteUint(compiler::target::kBitsPerWord, value));
ASSERT(Utils::BitLength(value) <= compiler::target::kBitsPerWord);
// Padding is helpful for comparing the .S with --disassemble.
assembly_stream_->Printf("%s 0x%.*" Px "\n", kWordDirective,
2 * compiler::target::kWordSize, value);

36
runtime/vm/utils_test.cc
View file

@ -290,22 +290,26 @@ VM_UNIT_TEST_CASE(IsUint) {
EXPECT(!Utils::IsUint(32, 4294967296LL));
}
VM_UNIT_TEST_CASE(IsAbsoluteUint) {
EXPECT(Utils::IsAbsoluteUint(8, 16));
EXPECT(Utils::IsAbsoluteUint(8, 0));
EXPECT(Utils::IsAbsoluteUint(8, -128));
EXPECT(Utils::IsAbsoluteUint(8, 255));
EXPECT(!Utils::IsAbsoluteUint(8, 256));
EXPECT(Utils::IsAbsoluteUint(16, 16));
EXPECT(Utils::IsAbsoluteUint(16, 0));
EXPECT(Utils::IsAbsoluteUint(16, 65535));
EXPECT(Utils::IsAbsoluteUint(16, -32768));
EXPECT(!Utils::IsAbsoluteUint(16, 65536));
EXPECT(Utils::IsAbsoluteUint(32, 16LL));
EXPECT(Utils::IsAbsoluteUint(32, 0LL));
EXPECT(Utils::IsAbsoluteUint(32, -2147483648LL));
EXPECT(Utils::IsAbsoluteUint(32, 4294967295LL));
EXPECT(!Utils::IsAbsoluteUint(32, 4294967296LL));
VM_UNIT_TEST_CASE(MagnitudeIsUint) {
EXPECT(Utils::MagnitudeIsUint(8, 16));
EXPECT(Utils::MagnitudeIsUint(8, 0));
EXPECT(Utils::MagnitudeIsUint(8, -128));
EXPECT(Utils::MagnitudeIsUint(8, 255));
EXPECT(!Utils::MagnitudeIsUint(8, 256));
EXPECT(Utils::MagnitudeIsUint(12, 4095));
EXPECT(Utils::MagnitudeIsUint(12, -4095));
EXPECT(!Utils::MagnitudeIsUint(12, 4096));
EXPECT(!Utils::MagnitudeIsUint(12, -4096));
EXPECT(Utils::MagnitudeIsUint(16, 16));
EXPECT(Utils::MagnitudeIsUint(16, 0));
EXPECT(Utils::MagnitudeIsUint(16, 65535));
EXPECT(Utils::MagnitudeIsUint(16, -32768));
EXPECT(!Utils::MagnitudeIsUint(16, 65536));
EXPECT(Utils::MagnitudeIsUint(32, 16LL));
EXPECT(Utils::MagnitudeIsUint(32, 0LL));
EXPECT(Utils::MagnitudeIsUint(32, -2147483648LL));
EXPECT(Utils::MagnitudeIsUint(32, 4294967295LL));
EXPECT(!Utils::MagnitudeIsUint(32, 4294967296LL));
}
VM_UNIT_TEST_CASE(LowBits) {