Skip to content

Commit 4c0b98b

Browse files
committed
Replace undefined shift operations by multiplications
Shift operations are undefined for negative numbers, but at least on Intel they return the same value as a multiplication with 2 ^ shift value. This fixes runtime errors reported by sanitizers and OSS-Fuzz: intmatcher.cpp:821:59: runtime error: left shift of negative value -14 intmatcher.cpp:823:75: runtime error: left shift of negative value -512 intmatcher.cpp:820:50: runtime error: left shift of negative value -80 See issue #2297 and https://oss-fuzz.com/testcase-detail/4845195990925312 for details. Signed-off-by: Stefan Weil <[email protected]>
1 parent 896698a commit 4c0b98b

File tree

1 file changed

+9
-16
lines changed

1 file changed

+9
-16
lines changed

src/classify/intmatcher.cpp

+9-16
Original file line numberDiff line numberDiff line change
@@ -774,13 +774,6 @@ int IntegerMatcher::UpdateTablesForFeature(
774774
uint32_t XFeatureAddress;
775775
uint32_t YFeatureAddress;
776776
uint32_t ThetaFeatureAddress;
777-
int ProtoIndex;
778-
uint8_t Temp;
779-
int* IntPointer;
780-
int ConfigNum;
781-
int32_t M3;
782-
int32_t A3;
783-
uint32_t A4;
784777

785778
tables->ClearFeatureEvidence(ClassTemplate);
786779

@@ -816,10 +809,10 @@ int IntegerMatcher::UpdateTablesForFeature(
816809
proto_byte = next_table[proto_byte];
817810
Proto = &(ProtoSet->Protos[ProtoNum + proto_offset]);
818811
ConfigWord = Proto->Configs[0];
819-
A3 = (((Proto->A * (Feature->X - 128)) << 1)
820-
- (Proto->B * (Feature->Y - 128)) + (Proto->C << 9));
821-
M3 =
822-
(((int8_t) (Feature->Theta - Proto->Angle)) * kIntThetaFudge) << 1;
812+
int32_t A3 = (((Proto->A * (Feature->X - 128)) * 2)
813+
- (Proto->B * (Feature->Y - 128)) + (Proto->C * 512));
814+
int32_t M3 = ((static_cast<int8_t>(Feature->Theta - Proto->Angle)) *
815+
kIntThetaFudge) * 2;
823816

824817
if (A3 < 0)
825818
A3 = ~A3;
@@ -832,7 +825,7 @@ int IntegerMatcher::UpdateTablesForFeature(
832825
if (static_cast<uint32_t>(M3) > evidence_mult_mask_)
833826
M3 = evidence_mult_mask_;
834827

835-
A4 = (A3 * A3) + (M3 * M3);
828+
uint32_t A4 = (A3 * A3) + (M3 * M3);
836829
A4 >>= table_trunc_shift_bits_;
837830
if (A4 > evidence_table_mask_)
838831
Evidence = 0;
@@ -863,11 +856,11 @@ int IntegerMatcher::UpdateTablesForFeature(
863856

864857
uint8_t* UINT8Pointer =
865858
&(tables->proto_evidence_[ActualProtoNum + proto_offset][0]);
866-
for (ProtoIndex =
859+
for (int ProtoIndex =
867860
ClassTemplate->ProtoLengths[ActualProtoNum + proto_offset];
868861
ProtoIndex > 0; ProtoIndex--, UINT8Pointer++) {
869862
if (Evidence > *UINT8Pointer) {
870-
Temp = *UINT8Pointer;
863+
uint8_t Temp = *UINT8Pointer;
871864
*UINT8Pointer = Evidence;
872865
Evidence = Temp;
873866
}
@@ -884,10 +877,10 @@ int IntegerMatcher::UpdateTablesForFeature(
884877
ClassTemplate->NumConfigs);
885878
}
886879

887-
IntPointer = tables->sum_feature_evidence_;
880+
int* IntPointer = tables->sum_feature_evidence_;
888881
uint8_t* UINT8Pointer = tables->feature_evidence_;
889882
int SumOverConfigs = 0;
890-
for (ConfigNum = ClassTemplate->NumConfigs; ConfigNum > 0; ConfigNum--) {
883+
for (int ConfigNum = ClassTemplate->NumConfigs; ConfigNum > 0; ConfigNum--) {
891884
int evidence = *UINT8Pointer++;
892885
SumOverConfigs += evidence;
893886
*IntPointer++ += evidence;

0 commit comments

Comments
 (0)