Skip to content

Commit

Permalink
Fix position on annotation subtrace
Browse files Browse the repository at this point in the history 
Summary: This diff starts to use subtrace position when emitting first traceframe of the subtrace, instead of what was used before: annotation's position. For taint transform subtraces these two positions are equal, but in general case they are not.

Differential Revision: D50207798

fbshipit-source-id: f7739c57c1bc90df48e949225c7278ac17ca2776
  • Loading branch information
@marat-turaev @facebook-github-bot
marat-turaev authored and facebook-github-bot committed Oct 25, 2023
1 parent 3450285 commit b1a795f
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 63 deletions.
12 changes: 9 additions & 3 deletions sapp/pipeline/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,14 @@ class ParseTypeInterval(NamedTuple):
preserves_type_context: bool


class ParseTraceAnnotationSubtrace(NamedTuple):
callee: str
port: str
position: SourceLocation
features: List["ParseTraceFeature"] = []
annotations: List["ParseTraceAnnotation"] = []


class ParseTraceAnnotation(NamedTuple):
location: SourceLocation
kind: str
Expand All @@ -117,9 +125,7 @@ class ParseTraceAnnotation(NamedTuple):
link: Optional[str]
trace_key: Optional[str]
titos: List[SourceLocation]
subtraces: List[
Dict[str, Any]
] # callee, port, features, annotations (TODO figure what exactly this shape is)
subtraces: List[ParseTraceAnnotationSubtrace]

@staticmethod
def from_json(j: Dict[str, Any]) -> "ParseTraceAnnotation":
Expand Down
10 changes: 5 additions & 5 deletions sapp/pipeline/mariana_trench_parser.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,11 +342,11 @@ def from_taint_json(caller: Method, extra_trace: Dict[str, Any]) -> "ExtraTrace"
def to_sapp(self) -> sapp.ParseTraceAnnotation:
subtraces = (
[
{
"callee": self.callee.method.name,
"port": self.callee.port.value,
"position": self.callee.position.to_sapp(),
}
sapp.ParseTraceAnnotationSubtrace(
callee=self.callee.method.name,
port=self.callee.port.value,
position=self.callee.position.to_sapp(),
)
]
if not self.callee.method.is_leaf()
else []
Expand Down
16 changes: 9 additions & 7 deletions sapp/pipeline/model_generator.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import json
import logging
from collections import defaultdict
from typing import Any, Dict, Iterable, List, Optional, Set, Tuple, Union
from typing import Dict, Iterable, List, Optional, Set, Tuple, Union

from ..models import (
DBID,
Expand Down Expand Up @@ -39,6 +39,7 @@
ParseIssueTuple,
ParseLeaf,
ParseTraceAnnotation,
ParseTraceAnnotationSubtrace,
ParseTraceFeature,
ParseTypeInterval,
PipelineStep,
Expand Down Expand Up @@ -578,17 +579,18 @@ def _generate_annotation_trace(
run: Run,
parent_filename: str,
parent_caller: str,
trace: Dict[str, Any],
trace: ParseTraceAnnotationSubtrace,
annotation: ParseTraceAnnotation,
) -> TraceFrame:
# Generates the first-hop trace frames from the annotation and
# all dependencies of these sub traces. If this gets called, it is
# assumed that the annotation leads to traces, and that the leaf kind
# and depth are specified.
callee = trace["callee"]
callee_port = trace["port"]
features = trace.get("features", [])
nested_annotations = trace.get("annotations", [])
callee = trace.callee
callee_port = trace.port
features = trace.features
nested_annotations = trace.annotations
position = trace.position
titos = self._generate_tito(parent_filename, annotation, parent_caller)
call_tf = self._generate_raw_trace_frame(
trace_kind,
Expand All @@ -598,7 +600,7 @@ def _generate_annotation_trace(
"root",
callee,
callee_port,
annotation.location,
position,
titos,
[(annotation.leaf_kind or "", annotation.leaf_depth)],
annotation.type_interval,
Expand Down
13 changes: 8 additions & 5 deletions sapp/pipeline/pysa_taint_parser.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
ParseIssueTuple,
ParsePosition,
ParseTraceAnnotation,
ParseTraceAnnotationSubtrace,
ParseTypeInterval,
SourceLocation,
)
Expand Down Expand Up @@ -444,11 +445,13 @@ def _parse_extra_traces(self, trace: Dict[str, Any]) -> List[ParseTraceAnnotatio
if "call" in extra_trace:
call = extra_trace["call"]
first_hops = [
{
"callee": resolved,
"port": call["port"],
"position": self._adjust_location(call["position"]),
}
ParseTraceAnnotationSubtrace(
callee=resolved,
port=call["port"],
position=SourceLocation.from_typed_dict(
self._adjust_location(call["position"])
),
)
for resolved in call["resolves_to"]
]
if len(first_hops) == 0:
Expand Down
97 changes: 54 additions & 43 deletions sapp/pipeline/tests/test_pysa_taint_parser.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
ParseIssueConditionTuple,
ParseIssueTuple,
ParseTraceAnnotation,
ParseTraceAnnotationSubtrace,
ParseTraceFeature,
ParseTypeInterval,
SourceLocation,
Expand Down Expand Up @@ -241,15 +242,15 @@ def testIssueV3(self) -> None:
trace_key=None,
titos=[],
subtraces=[
{
"callee": "extra_trace.transform_yz",
"port": "formal(arg)",
"position": {
"line": 117,
"start": 23,
"end": 24,
},
}
ParseTraceAnnotationSubtrace(
callee="extra_trace.transform_yz",
port="formal(arg)",
position=SourceLocation(
line_no=117,
begin_column=23,
end_column=24,
),
)
],
)
],
Expand Down Expand Up @@ -288,15 +289,15 @@ def testIssueV3(self) -> None:
trace_key=None,
titos=[],
subtraces=[
{
"callee": "extra_trace.transform_yz",
"port": "formal(arg)",
"position": {
"line": 117,
"start": 23,
"end": 24,
},
}
ParseTraceAnnotationSubtrace(
callee="extra_trace.transform_yz",
port="formal(arg)",
position=SourceLocation(
line_no=117,
begin_column=23,
end_column=24,
),
)
],
)
],
Expand Down Expand Up @@ -1593,11 +1594,13 @@ def testSourceModelV3(self) -> None:
trace_key=None,
titos=[],
subtraces=[
{
"callee": "extra_trace.nested_transform_x",
"port": "formal(arg)",
"position": {"line": 59, "start": 33, "end": 34},
}
ParseTraceAnnotationSubtrace(
callee="extra_trace.nested_transform_x",
port="formal(arg)",
position=SourceLocation(
line_no=59, begin_column=33, end_column=34
),
)
],
)
],
Expand Down Expand Up @@ -1637,11 +1640,13 @@ def testSourceModelV3(self) -> None:
trace_key=None,
titos=[],
subtraces=[
{
"callee": "extra_trace.nested_transform_x",
"port": "formal(arg)",
"position": {"line": 59, "start": 33, "end": 34},
}
ParseTraceAnnotationSubtrace(
callee="extra_trace.nested_transform_x",
port="formal(arg)",
position=SourceLocation(
line_no=59, begin_column=33, end_column=34
),
)
],
)
],
Expand Down Expand Up @@ -1681,11 +1686,13 @@ def testSourceModelV3(self) -> None:
trace_key=None,
titos=[],
subtraces=[
{
"callee": "extra_trace.nested_transform_x",
"port": "formal(arg)",
"position": {"line": 59, "start": 33, "end": 34},
}
ParseTraceAnnotationSubtrace(
callee="extra_trace.nested_transform_x",
port="formal(arg)",
position=SourceLocation(
line_no=59, begin_column=33, end_column=34
),
),
],
)
],
Expand Down Expand Up @@ -1725,11 +1732,13 @@ def testSourceModelV3(self) -> None:
trace_key=None,
titos=[],
subtraces=[
{
"callee": "extra_trace.nested_transform_x",
"port": "formal(arg)",
"position": {"line": 59, "start": 33, "end": 34},
}
ParseTraceAnnotationSubtrace(
callee="extra_trace.nested_transform_x",
port="formal(arg)",
position=SourceLocation(
line_no=59, begin_column=33, end_column=34
),
)
],
)
],
Expand Down Expand Up @@ -2253,11 +2262,13 @@ def testSinkModelV3(self) -> None:
trace_key=None,
titos=[],
subtraces=[
{
"callee": "extra_trace.nested_transform_x",
"port": "formal(arg)",
"position": {"line": 59, "start": 33, "end": 34},
}
ParseTraceAnnotationSubtrace(
callee="extra_trace.nested_transform_x",
port="formal(arg)",
position=SourceLocation(
line_no=59, begin_column=33, end_column=34
),
)
],
)
],
Expand Down

0 comments on commit b1a795f

Please sign in to comment.