This is an archive of the discontinued LLVM Phabricator instance.

Differential D65206

[analyzer] Fix text range end columns in SARIF to be exclusive
ClosedPublic

Authored by jranieri-grammatech on Jul 24 2019, 6:22 AM.

Details

Reviewers
george.karpenkov
NoQ
aaron.ballman
Commits
rL370060: Fix text range end columns in SARIF to be exclusive
rC370060: Fix text range end columns in SARIF to be exclusive
rG68a6a28ef835: Fix text range end columns in SARIF to be exclusive
Summary

According to the SARIF specification, "a text region does not include the character specified by endColumn".

Diff Detail

Event Timeline

jranieri-grammatech created this revision.Jul 24 2019, 6:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 24 2019, 6:22 AM
Herald added subscribers: cfe-commits, Charusso, dkrupp and 7 others. · View Herald Transcript
jranieri-grammatech added a child revision: D65209: [analyzer] Fix a SARIF exporter crash with macro expansions.Jul 24 2019, 7:02 AM
aaron.ballman accepted this revision.Jul 30 2019, 5:29 AM

This LGTM, but I'd appreciate a second reviewer chiming in only because Joe is a coworker.

This revision is now accepted and ready to land.Jul 30 2019, 5:29 AM
aaron.ballman added a comment.Aug 20 2019, 12:36 PM
In D65206#1606072, @aaron.ballman wrote:

This LGTM, but I'd appreciate a second reviewer chiming in only because Joe is a coworker.

Sufficient time has passed that any concerns other reviewers have can be addressed post-commit. Continues to LGTM.

Closed by commit rG68a6a28ef835: Fix text range end columns in SARIF to be exclusive (authored by jranieri-grammatech). · Explain WhyAug 27 2019, 6:55 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
clang/
lib/
StaticAnalyzer/
Core/
12 lines
test/
Analysis/
diagnostics/
Inputs/
expected-sarif/
6 lines
19 lines

Diff 217390

clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp

Show First 20 Lines • Show All 137 Lines • ▼ Show 20 Linesstatic json::Object createFileLocation(const FileEntry &FE,
auto Index = static_cast<unsigned>(std::distance(Files.begin(), I)); auto Index = static_cast<unsigned>(std::distance(Files.begin(), I));
if (I == Files.end()) if (I == Files.end())
Files.push_back(createFile(FE)); Files.push_back(createFile(FE));
return json::Object{{"uri", FileURI}, {"fileIndex", Index}}; return json::Object{{"uri", FileURI}, {"fileIndex", Index}};
} }
static json::Object createTextRegion(SourceRange R, const SourceManager &SM) { static json::Object createTextRegion(SourceRange R, const SourceManager &SM) {
return json::Object{ json::Object Region{
{"startLine", SM.getExpansionLineNumber(R.getBegin())}, {"startLine", SM.getExpansionLineNumber(R.getBegin())},
{"endLine", SM.getExpansionLineNumber(R.getEnd())},
{"startColumn", SM.getExpansionColumnNumber(R.getBegin())}, {"startColumn", SM.getExpansionColumnNumber(R.getBegin())},
{"endColumn", SM.getExpansionColumnNumber(R.getEnd())}}; };
if (R.getBegin() == R.getEnd()) {
Region["endColumn"] = SM.getExpansionColumnNumber(R.getBegin());
} else {
Region["endLine"] = SM.getExpansionLineNumber(R.getEnd());
Region["endColumn"] = SM.getExpansionColumnNumber(R.getEnd()) + 1;
}
return Region;
} }
static json::Object createPhysicalLocation(SourceRange R, const FileEntry &FE, static json::Object createPhysicalLocation(SourceRange R, const FileEntry &FE,
const SourceManager &SMgr, const SourceManager &SMgr,
json::Array &Files) { json::Array &Files) {
return json::Object{{{"fileLocation", createFileLocation(FE, Files)}, return json::Object{{{"fileLocation", createFileLocation(FE, Files)},
{"region", createTextRegion(R, SMgr)}}}; {"region", createTextRegion(R, SMgr)}}};
} }
▲ Show 20 Lines • Show All 191 Lines • Show Last 20 Lines

clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif

Show All 38 Lines "runs": [
"message": { "message": {
"text": "Calling 'f'" "text": "Calling 'f'"
}, },
"physicalLocation": { "physicalLocation": {
"fileLocation": { "fileLocation": {
"fileIndex": 0, "fileIndex": 0,
}, },
"region": { "region": {
"endColumn": 5, "endColumn": 6,
"endLine": 13, "endLine": 13,
"startColumn": 3, "startColumn": 3,
"startLine": 13 "startLine": 13
} }
} }
} }
}, },
{ {
"importance": "essential", "importance": "essential",
"location": { "location": {
"message": { "message": {
"text": "tainted" "text": "tainted"
}, },
"physicalLocation": { "physicalLocation": {
"fileLocation": { "fileLocation": {
"fileIndex": 0, "fileIndex": 0,
}, },
"region": { "region": {
"endColumn": 17, "endColumn": 18,
"endLine": 9, "endLine": 9,
"startColumn": 11, "startColumn": 11,
"startLine": 9 "startLine": 9
} }
} }
} }
} }
] ]
} }
] ]
} }
], ],
"locations": [ "locations": [
{ {
"physicalLocation": { "physicalLocation": {
"fileLocation": { "fileLocation": {
"fileIndex": 0, "fileIndex": 0,
}, },
"region": { "region": {
"endColumn": 17, "endColumn": 18,
"endLine": 9, "endLine": 9,
"startColumn": 11, "startColumn": 11,
"startLine": 9 "startLine": 9
} }
} }
} }
], ],
"message": { "message": {
Show All 14 Lines

clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif

Show First 20 Lines • Show All 58 Lines • ▼ Show 20 Lines "runs": [
"message": { "message": {
"text": "Calling 'f'" "text": "Calling 'f'"
}, },
"physicalLocation": { "physicalLocation": {
"fileLocation": { "fileLocation": {
"fileIndex": 0, "fileIndex": 0,
}, },
"region": { "region": {
"endColumn": 5, "endColumn": 6,
"endLine": 24, "endLine": 24,
"startColumn": 3, "startColumn": 3,
"startLine": 24 "startLine": 24
} }
} }
} }
}, },
{ {
"importance": "essential", "importance": "essential",
"location": { "location": {
"message": { "message": {
"text": "tainted" "text": "tainted"
}, },
"physicalLocation": { "physicalLocation": {
"fileLocation": { "fileLocation": {
"fileIndex": 0, "fileIndex": 0,
}, },
"region": { "region": {
"endColumn": 17, "endColumn": 18,
"endLine": 9, "endLine": 9,
"startColumn": 11, "startColumn": 11,
"startLine": 9 "startLine": 9
} }
} }
} }
} }
] ]
} }
] ]
} }
], ],
"locations": [ "locations": [
{ {
"physicalLocation": { "physicalLocation": {
"fileLocation": { "fileLocation": {
"fileIndex": 0, "fileIndex": 0,
}, },
"region": { "region": {
"endColumn": 17, "endColumn": 18,
"endLine": 9, "endLine": 9,
"startColumn": 11, "startColumn": 11,
"startLine": 9 "startLine": 9
} }
} }
} }
], ],
"message": { "message": {
Show All 14 Lines "runs": [
"message": { "message": {
"text": "Calling 'g'" "text": "Calling 'g'"
}, },
"physicalLocation": { "physicalLocation": {
"fileLocation": { "fileLocation": {
"fileIndex": 0, "fileIndex": 0,
}, },
"region": { "region": {
"endColumn": 5, "endColumn": 6,
"endLine": 25, "endLine": 25,
"startColumn": 3, "startColumn": 3,
"startLine": 25 "startLine": 25
} }
} }
} }
}, },
{ {
"importance": "essential", "importance": "essential",
"location": { "location": {
"message": { "message": {
"text": "'fp' declared without an initial value" "text": "'fp' declared without an initial value"
}, },
"physicalLocation": { "physicalLocation": {
"fileLocation": { "fileLocation": {
"fileIndex": 0, "fileIndex": 0,
}, },
"region": { "region": {
"endColumn": 10, "endColumn": 11,
"endLine": 13, "endLine": 13,
"startColumn": 3, "startColumn": 3,
"startLine": 13 "startLine": 13
} }
} }
} }
}, },
{ {
"importance": "essential", "importance": "essential",
"location": { "location": {
"message": { "message": {
"text": "Called function pointer is an uninitialized pointer value" "text": "Called function pointer is an uninitialized pointer value"
}, },
"physicalLocation": { "physicalLocation": {
"fileLocation": { "fileLocation": {
"fileIndex": 0, "fileIndex": 0,
}, },
"region": { "region": {
"endColumn": 8, "endColumn": 9,
"endLine": 14, "endLine": 14,
"startColumn": 3, "startColumn": 3,
"startLine": 14 "startLine": 14
} }
} }
} }
} }
] ]
} }
] ]
} }
], ],
"locations": [ "locations": [
{ {
"physicalLocation": { "physicalLocation": {
"fileLocation": { "fileLocation": {
"fileIndex": 0, "fileIndex": 0,
}, },
"region": { "region": {
"endColumn": 8, "endColumn": 9,
"endLine": 14, "endLine": 14,
"startColumn": 3, "startColumn": 3,
"startLine": 14 "startLine": 14
} }
} }
} }
], ],
"message": { "message": {
Show All 14 Lines "runs": [
"message": { "message": {
"text": "Assuming 'i' is equal to 0" "text": "Assuming 'i' is equal to 0"
}, },
"physicalLocation": { "physicalLocation": {
"fileLocation": { "fileLocation": {
"fileIndex": 0, "fileIndex": 0,
}, },
"region": { "region": {
"endColumn": 12, "endColumn": 13,
"endLine": 18, "endLine": 18,
"startColumn": 7, "startColumn": 7,
"startLine": 18 "startLine": 18
} }
} }
} }
}, },
{ {
"importance": "unimportant", "importance": "unimportant",
"location": { "location": {
"message": { "message": {
"text": "Taking true branch" "text": "Taking true branch"
}, },
"physicalLocation": { "physicalLocation": {
"fileLocation": { "fileLocation": {
"fileIndex": 0, "fileIndex": 0,
}, },
"region": { "region": {
"endColumn": 3, "endColumn": 3,
"endLine": 18,
"startColumn": 3, "startColumn": 3,
"startLine": 18 "startLine": 18
} }
} }
} }
}, },
{ {
"importance": "essential", "importance": "essential",
"location": { "location": {
"message": { "message": {
"text": "Division by zero" "text": "Division by zero"
}, },
"physicalLocation": { "physicalLocation": {
"fileLocation": { "fileLocation": {
"fileIndex": 0, "fileIndex": 0,
}, },
"region": { "region": {
"endColumn": 14, "endColumn": 14,
"endLine": 19,
"startColumn": 14, "startColumn": 14,
"startLine": 19 "startLine": 19
} }
} }
} }
} }
] ]
} }
] ]
} }
], ],
"locations": [ "locations": [
{ {
"physicalLocation": { "physicalLocation": {
"fileLocation": { "fileLocation": {
"fileIndex": 0, "fileIndex": 0,
}, },
"region": { "region": {
"endColumn": 14, "endColumn": 14,
"endLine": 19,
"startColumn": 14, "startColumn": 14,
"startLine": 19 "startLine": 19
} }
} }
} }
], ],
"message": { "message": {
"text": "Division by zero" "text": "Division by zero"
Show All 13 Lines