This is an archive of the discontinued LLVM Phabricator instance.

[test-suite] Fix signed integer overflow error for pc1cod.c
ClosedPublic

Authored by Shrill on Nov 10 2021, 6:55 PM.

Details

Summary

The patch fixes a test fail when compiling the benchmark using GCC 9.3.0. The benchmark was failing because of a signed integer overflow runtime error in the pc1code.c. This caused the file to have undefined behaviour and the output generated didn't pass the verification test.

The patch induces the modification of the data type of the variables causing the signed integer overflow to prevent the error.

Co-Author: @adata111

Diff Detail

Repository
rT test-suite

Event Timeline

Shrill requested review of this revision.Nov 10 2021, 6:55 PM
Shrill created this revision.
Shrill edited the summary of this revision. (Show Details)Nov 10 2021, 7:06 PM
Shrill edited the summary of this revision. (Show Details)Nov 11 2021, 8:40 AM
Shrill changed the edit policy from "All Users" to "Custom Policy".
Shrill added a subscriber: adata111.

Does the benchmark still produces the same values when going from int to unsigned? No change to *.reference_output necessary?

Does the benchmark still produces the same values when going from int to unsigned? No change to *.reference_output necessary?

No, no change to *.reference_output is necessary, the *.reference_output produced after changing to unsigned is the same as the previous one
The benchmark was failing for the GCC compiler and this change just fixes that.

adata111 retitled this revision from [test-suits] Fix signed integer overflow error for pc1cod.c to [test-suite] Fix signed integer overflow error for pc1cod.c.Nov 11 2021, 12:30 PM
adata111 edited the summary of this revision. (Show Details)
MatzeB accepted this revision.Nov 11 2021, 12:47 PM

LGTM! I'll land it.

This revision is now accepted and ready to land.Nov 11 2021, 12:47 PM

It seems something is off with the line endings of the patch (or arc patch D113639 is failing on my end). The changed lines get a CR+LF instead of a unix LF line ending. How did you produce and upload the patch?

It seems something is off with the line endings of the patch (or arc patch D113639 is failing on my end). The changed lines get a CR+LF instead of a unix LF line ending. How did you produce and upload the patch?

I generated a patch file using: git show HEAD -U999999 > mypatch.patch as mentioned in the code review webpage and uploaded the file using differential.

I wasn't aware it generated the patch file with CR+LF line ending. I will change the line endings to LF and upload the updated patch file.

Shrill updated this revision to Diff 386641.Nov 11 2021, 1:26 PM

Change line endings from CR+LF to LF.

Just double checked and it seems you got it "right" the first time. Turns out the source file for this benchmark has CR+LF line endings, so your patch having them was correct. (My git pager view or the git diff output somehow produce odd output marking the changed lines with different line endings, but this was on my end).

I committed the change now.