Skip to content

Commit

Permalink
Fix more MSVC warnings, courgette/ edition.
Browse files Browse the repository at this point in the history
This is mostly about changing types and inserting casts so as to avoid implicit
value truncations.

BUG=81439
TEST=none

Review URL: https://codereview.chromium.org/613893002

Cr-Commit-Position: refs/heads/master@{#298069}
  • Loading branch information
pkasting authored and Commit bot committed Oct 3, 2014
1 parent 1bd4f75 commit 8e3a26a
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 62 deletions.
27 changes: 13 additions & 14 deletions courgette/adjustment_method_2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ LabelInfo::~LabelInfo() {
// position of one of the occurrences in the Trace.
class Shingle {
public:
static const size_t kWidth = 5;
static const uint8 kWidth = 5;

struct InterningLess {
bool operator()(const Shingle& a, const Shingle& b) const;
Expand Down Expand Up @@ -430,7 +430,7 @@ class Shingle {
std::string ToString(const Shingle* instance) {
std::string s;
const char* sep = "<";
for (size_t i = 0; i < Shingle::kWidth; ++i) {
for (uint8 i = 0; i < Shingle::kWidth; ++i) {
// base::StringAppendF(&s, "%s%x ", sep, instance.at(i)->label_->rva_);
s += sep;
s += ToString(instance->at(i));
Expand All @@ -446,7 +446,7 @@ std::string ToString(const Shingle* instance) {
bool Shingle::InterningLess::operator()(
const Shingle& a,
const Shingle& b) const {
for (size_t i = 0; i < kWidth; ++i) {
for (uint8 i = 0; i < kWidth; ++i) {
LabelInfo* info_a = a.at(i);
LabelInfo* info_b = b.at(i);
if (info_a->label_->rva_ < info_b->label_->rva_)
Expand Down Expand Up @@ -526,7 +526,7 @@ std::string ToString(const ShinglePattern::Index* index) {
} else {
base::StringAppendF(&s, "<%d: ", index->variables_);
const char* sep = "";
for (size_t i = 0; i < Shingle::kWidth; ++i) {
for (uint8 i = 0; i < Shingle::kWidth; ++i) {
s += sep;
sep = ", ";
uint32 kind = index->kinds_[i];
Expand Down Expand Up @@ -622,7 +622,7 @@ struct ShinglePatternIndexLess {
if (a.hash_ < b.hash_) return true;
if (a.hash_ > b.hash_) return false;

for (size_t i = 0; i < Shingle::kWidth; ++i) {
for (uint8 i = 0; i < Shingle::kWidth; ++i) {
if (a.kinds_[i] < b.kinds_[i]) return true;
if (a.kinds_[i] > b.kinds_[i]) return false;
if ((a.kinds_[i] & ShinglePattern::kVariable) == 0) {
Expand All @@ -647,11 +647,11 @@ ShinglePattern::Index::Index(const Shingle* instance) {
unique_variables_ = 0;
first_variable_index_ = 255;

for (uint32 i = 0; i < Shingle::kWidth; ++i) {
for (uint8 i = 0; i < Shingle::kWidth; ++i) {
LabelInfo* info = instance->at(i);
uint32 kind = 0;
uint8 kind = 0;
int code = -1;
size_t j = 0;
uint8 j = 0;
for ( ; j < i; ++j) {
if (info == instance->at(j)) { // Duplicate LabelInfo
kind = kinds_[j];
Expand Down Expand Up @@ -889,7 +889,7 @@ class AssignmentProblem {


void AddShingles(size_t begin, size_t end) {
for (size_t i = begin; i + Shingle::kWidth - 1 < end; ++i) {
for (size_t i = begin; i + Shingle::kWidth - 1 < end; ++i) {
instances_[i] = Shingle::Find(trace_, i, &shingle_instances_);
}
}
Expand Down Expand Up @@ -942,17 +942,16 @@ class AssignmentProblem {

// For the positions in |info|, find the shingles that overlap that position.
void AddAffectedPositions(LabelInfo* info, ShingleSet* affected_shingles) {
const size_t kWidth = Shingle::kWidth;
const uint8 kWidth = Shingle::kWidth;
for (size_t i = 0; i < info->positions_.size(); ++i) {
size_t position = info->positions_[i];
// Find bounds to the subrange of |trace_| we are in.
size_t start = position < model_end_ ? 0 : model_end_;
size_t end = position < model_end_ ? model_end_ : trace_.size();

// Clip [position-kWidth+1, position+1)
size_t low = position > start + kWidth - 1
? position - kWidth + 1
: start;
size_t low =
position > start + kWidth - 1 ? position - kWidth + 1 : start;
size_t high = position + kWidth < end ? position + 1 : end - kWidth + 1;

for (size_t shingle_position = low;
Expand Down Expand Up @@ -1060,7 +1059,7 @@ class AssignmentProblem {
// int score = p1; // ? weigh all equally??
int score = std::min(p1, m1);

for (size_t i = 0; i < Shingle::kWidth; ++i) {
for (uint8 i = 0; i < Shingle::kWidth; ++i) {
LabelInfo* program_info = program_instance->at(i);
LabelInfo* model_info = model_instance->at(i);
if ((model_info->assignment_ == NULL) !=
Expand Down
23 changes: 5 additions & 18 deletions courgette/assembly_program.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,16 +90,16 @@ class ByteInstruction : public Instruction {
// Emits a single byte.
class BytesInstruction : public Instruction {
public:
BytesInstruction(const uint8* values, uint32 len)
BytesInstruction(const uint8* values, size_t len)
: Instruction(DEFBYTES, 0),
values_(values),
len_(len) {}
const uint8* byte_values() const { return values_; }
uint32 len() const { return len_; }
size_t len() const { return len_; }

private:
const uint8* values_;
uint32 len_;
size_t len_;
};

// A ABS32 to REL32 instruction emits a reference to a label's address.
Expand Down Expand Up @@ -179,7 +179,7 @@ CheckBool AssemblyProgram::EmitByteInstruction(uint8 byte) {
}

CheckBool AssemblyProgram::EmitBytesInstruction(const uint8* values,
uint32 len) {
size_t len) {
return Emit(new(std::nothrow) BytesInstruction(values, len));
}

Expand Down Expand Up @@ -421,7 +421,7 @@ EncodedProgram* AssemblyProgram::Encode() const {
case DEFBYTES: {
const uint8* byte_values =
static_cast<BytesInstruction*>(instruction)->byte_values();
uint32 len = static_cast<BytesInstruction*>(instruction)->len();
size_t len = static_cast<BytesInstruction*>(instruction)->len();

if (!encoded->AddCopy(len, byte_values))
return NULL;
Expand Down Expand Up @@ -547,19 +547,6 @@ CheckBool AssemblyProgram::TrimLabels() {
return true;
}

void AssemblyProgram::PrintLabelCounts(RVAToLabel* labels) {
for (RVAToLabel::const_iterator p = labels->begin(); p != labels->end();
++p) {
Label* current = p->second;
if (current->index_ != Label::kNoIndex)
printf("%d\n", current->count_);
}
}

void AssemblyProgram::CountRel32ARM() {
PrintLabelCounts(&rel32_labels_);
}

////////////////////////////////////////////////////////////////////////////////

Status TrimLabels(AssemblyProgram* program) {
Expand Down
5 changes: 1 addition & 4 deletions courgette/assembly_program.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class AssemblyProgram {
CheckBool EmitByteInstruction(uint8 byte) WARN_UNUSED_RESULT;

// Generates multiple bytes of data or machine instructions.
CheckBool EmitBytesInstruction(const uint8* value, uint32 len)
CheckBool EmitBytesInstruction(const uint8* value, size_t len)
WARN_UNUSED_RESULT;

// Generates 4-byte relative reference to address of 'label'.
Expand Down Expand Up @@ -129,9 +129,6 @@ class AssemblyProgram {
// Trim underused labels
CheckBool TrimLabels();

void PrintLabelCounts(RVAToLabel* labels);
void CountRel32ARM();

private:
ExecutableType kind_;

Expand Down
10 changes: 3 additions & 7 deletions courgette/disassembler_elf_32.cc
Original file line number Diff line number Diff line change
Expand Up @@ -405,16 +405,12 @@ CheckBool DisassemblerElf32::ParseSimpleRegion(
size_t start_file_offset,
size_t end_file_offset,
AssemblyProgram* program) {

const uint8* start = OffsetToPointer(start_file_offset);
const uint8* end = OffsetToPointer(end_file_offset);

// Callers don't guarantee start < end
if (start >= end) return true;
if (start_file_offset >= end_file_offset) return true;

const ptrdiff_t len = end - start; // Works because vars are byte pointers
const size_t len = end_file_offset - start_file_offset;

if (!program->EmitBytesInstruction(start, len))
if (!program->EmitBytesInstruction(OffsetToPointer(start_file_offset), len))
return false;

return true;
Expand Down
8 changes: 4 additions & 4 deletions courgette/disassembler_elf_32_arm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ CheckBool DisassemblerElf32ARM::Compress(ARM_RVA type, uint32 arm_op, RVA rva,
fflush(stdout);

(*addr) = temp;
(*c_op) = (arm_op >> 8) | 0x1000;
(*c_op) = static_cast<uint16>(arm_op >> 8) | 0x1000;
break;
}
case ARM_OFF11: {
Expand All @@ -50,7 +50,7 @@ CheckBool DisassemblerElf32ARM::Compress(ARM_RVA type, uint32 arm_op, RVA rva,
temp += 4; // Offset from _next_ PC.

(*addr) = temp;
(*c_op) = (arm_op >> 11) | 0x2000;
(*c_op) = static_cast<uint16>(arm_op >> 11) | 0x2000;
break;
}
case ARM_OFF24: {
Expand Down Expand Up @@ -101,7 +101,7 @@ CheckBool DisassemblerElf32ARM::Compress(ARM_RVA type, uint32 arm_op, RVA rva,
temp2 |= (arm_op & (1 << 15)) >> 13;
temp2 |= (arm_op & 0xF8000000) >> 24;
temp2 |= (prefetch & 0x0000000F) << 8;
(*c_op) = temp2;
(*c_op) = static_cast<uint16>(temp2);
break;
}
case ARM_OFF21: {
Expand All @@ -122,7 +122,7 @@ CheckBool DisassemblerElf32ARM::Compress(ARM_RVA type, uint32 arm_op, RVA rva,

uint32 temp2 = 0x5000;
temp2 |= (arm_op & 0x03C00000) >> 22; // just save the cond
(*c_op) = temp2;
(*c_op) = static_cast<uint16>(temp2);
break;
}
default:
Expand Down
4 changes: 3 additions & 1 deletion courgette/disassembler_win32_x64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,9 @@ CheckBool DisassemblerWin32X64::ParseFileRegion(

if (abs32_pos != abs32_locations_.end() && *abs32_pos == current_rva) {
uint32 target_address = Read32LittleEndian(p);
RVA target_rva = target_address - image_base();
// TODO(wfh): image_base() can be larger than 32 bits, so this math can
// underflow. Figure out the right solution here.
RVA target_rva = target_address - static_cast<uint32>(image_base());
// TODO(sra): target could be Label+offset. It is not clear how to guess
// which it might be. We assume offset==0.
if (!program->EmitAbs32(program->FindOrMakeAbs32Label(target_rva)))
Expand Down
18 changes: 8 additions & 10 deletions courgette/encoded_program.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ CheckBool WriteVector(const V& items, SinkStream* buffer) {
size_t count = items.size();
bool ok = buffer->WriteSizeVarint32(count);
for (size_t i = 0; ok && i < count; ++i) {
COMPILE_ASSERT(sizeof(items[0]) <= sizeof(uint32), // NOLINT
T_must_fit_in_uint32);
ok = buffer->WriteSizeVarint32(items[i]);
}
return ok;
Expand Down Expand Up @@ -196,7 +194,7 @@ CheckBool EncodedProgram::AddOrigin(RVA origin) {
return ops_.push_back(ORIGIN) && origins_.push_back(origin);
}

CheckBool EncodedProgram::AddCopy(uint32 count, const void* bytes) {
CheckBool EncodedProgram::AddCopy(size_t count, const void* bytes) {
const uint8* source = static_cast<const uint8*>(bytes);

bool ok = true;
Expand All @@ -214,7 +212,7 @@ CheckBool EncodedProgram::AddCopy(uint32 count, const void* bytes) {
}
if (ok && ops_.back() == COPY) {
copy_counts_.back() += count;
for (uint32 i = 0; ok && i < count; ++i) {
for (size_t i = 0; ok && i < count; ++i) {
ok = copy_bytes_.push_back(source[i]);
}
return ok;
Expand All @@ -226,7 +224,7 @@ CheckBool EncodedProgram::AddCopy(uint32 count, const void* bytes) {
ok = ops_.push_back(COPY1) && copy_bytes_.push_back(source[0]);
} else {
ok = ops_.push_back(COPY) && copy_counts_.push_back(count);
for (uint32 i = 0; ok && i < count; ++i) {
for (size_t i = 0; ok && i < count; ++i) {
ok = copy_bytes_.push_back(source[i]);
}
}
Expand Down Expand Up @@ -427,7 +425,7 @@ CheckBool EncodedProgram::EvaluateRel32ARM(OP op,
&decompressed_op)) {
return false;
}
uint16 op16 = decompressed_op;
uint16 op16 = static_cast<uint16>(decompressed_op);
if (!output->Write(&op16, 2))
return false;
current_rva += 2;
Expand All @@ -447,7 +445,7 @@ CheckBool EncodedProgram::EvaluateRel32ARM(OP op,
&decompressed_op)) {
return false;
}
uint16 op16 = decompressed_op;
uint16 op16 = static_cast<uint16>(decompressed_op);
if (!output->Write(&op16, 2))
return false;
current_rva += 2;
Expand Down Expand Up @@ -556,19 +554,19 @@ CheckBool EncodedProgram::AssembleTo(SinkStream* final_buffer) {
}

case COPY: {
uint32 count;
size_t count;
if (!VectorAt(copy_counts_, ix_copy_counts, &count))
return false;
++ix_copy_counts;
for (uint32 i = 0; i < count; ++i) {
for (size_t i = 0; i < count; ++i) {
uint8 b;
if (!VectorAt(copy_bytes_, ix_copy_bytes, &b))
return false;
++ix_copy_bytes;
if (!output->Write(&b, 1))
return false;
}
current_rva += count;
current_rva += static_cast<RVA>(count);
break;
}

Expand Down
5 changes: 3 additions & 2 deletions courgette/encoded_program.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class EncodedProgram {
// NOTE: If any of these methods ever fail, the EncodedProgram instance
// has failed and should be discarded.
CheckBool AddOrigin(RVA rva) WARN_UNUSED_RESULT;
CheckBool AddCopy(uint32 count, const void* bytes) WARN_UNUSED_RESULT;
CheckBool AddCopy(size_t count, const void* bytes) WARN_UNUSED_RESULT;
CheckBool AddRel32(int label_index) WARN_UNUSED_RESULT;
CheckBool AddRel32ARM(uint16 op, int label_index) WARN_UNUSED_RESULT;
CheckBool AddAbs32(int label_index) WARN_UNUSED_RESULT;
Expand Down Expand Up @@ -87,6 +87,7 @@ class EncodedProgram {
};

typedef NoThrowBuffer<RVA> RvaVector;
typedef NoThrowBuffer<size_t> SizeTVector;
typedef NoThrowBuffer<uint32> UInt32Vector;
typedef NoThrowBuffer<uint8> UInt8Vector;
typedef NoThrowBuffer<OP> OPVector;
Expand All @@ -109,7 +110,7 @@ class EncodedProgram {
RvaVector abs32_rva_;
OPVector ops_;
RvaVector origins_;
UInt32Vector copy_counts_;
SizeTVector copy_counts_;
UInt8Vector copy_bytes_;
UInt32Vector rel32_ix_;
UInt32Vector abs32_ix_;
Expand Down
4 changes: 2 additions & 2 deletions courgette/streams.cc
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,10 @@ const uint8* Varint::Parse32WithLimit(const uint8* source,
// have the high bit set to indicate more digits.
inline uint8* Varint::Encode32(uint8* destination, uint32 value) {
while (value >= 128) {
*(destination++) = value | 128;
*(destination++) = static_cast<uint8>(value) | 128;
value = value >> 7;
}
*(destination++) = value;
*(destination++) = static_cast<uint8>(value);
return destination;
}

Expand Down

0 comments on commit 8e3a26a

Please sign in to comment.