Stop writing strings in issue_instances and trace_frames

Differential Revision: D15016189

fbshipit-source-id: 6d558bc7ee411f989ea3a60b776ed31848e3a750

Author: fahndrich

sapp/sapp/model_generator.py

@@ -147,8 +147,6 @@ def _generate_issue(self, run, entry, callablesCount):
             id=IssueDBID(),
             code=entry["code"],
             handle=handle,
-            callable=callable,
-            filename=entry["filename"],
             status=IssueStatus.UNCATEGORIZED,
             first_seen=run.date,
             run_id=run.id,
@@ -174,7 +172,6 @@ def _generate_issue(self, run, entry, callablesCount):
             id=DBID(),
             issue_id=issue.id,
             location=self.get_location(entry),
-            filename=entry["filename"],
             filename_id=filename_record.id,
             callable_id=callable_record.id,
             run_id=run.id,
@@ -187,7 +184,7 @@ def _generate_issue(self, run, entry, callablesCount):
             min_trace_length_to_sinks=self._get_minimum_trace_length(
                 entry["preconditions"]
             ),
-            callable_count=callablesCount[issue.callable],
+            callable_count=callablesCount[callable],
         )
 
         for sink in final_sinks:
@@ -226,20 +223,21 @@ def _generate_issue_postcondition(self, run, issue, callinfo):
             callinfo["leaves"],  # sources
             callinfo["type_interval"],
         )
-        keys = [(callee, callee_port)]
+        keys = [(call_tf.callee_id, callee_port)]
         while len(keys) > 0:
             key = keys.pop()
             if self.graph.has_postconditions_with_caller(key[0], key[1]):
                 continue
 
+            key = (self.graph.get_text(key[0]), key[1])
             new = [
                 self._generate_postcondition(run, e)
                 for e in self.summary["postcondition_entries"].pop(key, [])
             ]
             if len(new) == 0 and key[1] != "source":
                 self.summary["missing_postconditions"].add(key)
 
-            keys.extend([(tf.callee, tf.callee_port) for tf in new])
+            keys.extend([(tf.callee_id, tf.callee_port) for tf in new])
 
         return call_tf
 
@@ -280,16 +278,13 @@ def _generate_raw_postcondition(
         trace_frame = TraceFrame.Record(
             id=DBID(),
             kind=TraceKind.POSTCONDITION,
-            caller=caller,
             caller_id=caller_record.id,
-            callee=callee,
             callee_id=callee_record.id,
             callee_location=SourceLocation(
                 callee_location["line"],
                 callee_location["start"],
                 callee_location["end"],
             ),
-            filename=filename,
             filename_id=filename_record.id,
             run_id=run.id,
             caller_port=caller_port,
@@ -329,20 +324,21 @@ def _generate_issue_precondition(self, run, issue, callinfo):
             callinfo["type_interval"],
             callinfo["features"],
         )
-        keys = [(callee, callee_port)]
+        keys = [(call_tf.callee_id, callee_port)]
         while len(keys) > 0:
             key = keys.pop()
             if self.graph.has_preconditions_with_caller(key[0], key[1]):
                 continue
 
+            key = (self.graph.get_text(key[0]), key[1])
             new = [
                 self._generate_precondition(run, e)
                 for e in self.summary["precondition_entries"].pop(key, [])
             ]
             if len(new) == 0 and key[1] != "sink":
                 self.summary["missing_preconditions"].add(key)
 
-            keys.extend([(tf.callee, tf.callee_port) for tf in new])
+            keys.extend([(tf.callee_id, tf.callee_port) for tf in new])
 
         return call_tf
 
@@ -399,18 +395,15 @@ def _generate_raw_precondition(
         trace_frame = TraceFrame.Record(
             id=DBID(),
             kind=TraceKind.PRECONDITION,
-            caller=caller,
             caller_id=caller_record.id,
             caller_port=caller_port,
-            callee=callee,
             callee_id=callee_record.id,
             callee_port=callee_port,
             callee_location=SourceLocation(
                 callee_location["line"],
                 callee_location["start"],
                 callee_location["end"],
             ),
-            filename=filename,
             filename_id=filename_record.id,
             titos=titos,
             run_id=run.id,
@@ -467,11 +460,10 @@ def _get_issue_handle(self, entry):
         return entry["handle"]
 
     def _get_shared_text(self, kind, name):
+        name = name[:SHARED_TEXT_LENGTH]
         shared_text = self.graph.get_shared_text(kind, name)
         if shared_text is None:
-            shared_text = SharedText.Record(
-                id=DBID(), contents=name[:SHARED_TEXT_LENGTH], kind=kind
-            )
+            shared_text = SharedText.Record(id=DBID(), contents=name, kind=kind)
             self.graph.add_shared_text(shared_text)
         return shared_text
 

sapp/sapp/models.py

@@ -635,17 +635,22 @@ class IssueInstance(Base, PrepareMixin, MutableRecordMixin):  # noqa
         doc="Location (possibly a range) of the issue",
     )
 
-    filename = Column(
-        String(length=767),
-        doc="Filename containing the issue",
-        nullable=True,
-        index=True,
-    )
-
     filename_id = Column(BIGDBIDType, nullable=False, server_default="0", default=0)
 
+    filename = relationship(
+        "SharedText",
+        primaryjoin="foreign(SharedText.id) == IssueInstance.filename_id",
+        uselist=False,
+    )
+
     callable_id = Column(BIGDBIDType, nullable=False, server_default="0", default=0)
 
+    callable = relationship(
+        "SharedText",
+        primaryjoin="foreign(SharedText.id) == IssueInstance.callable_id",
+        uselist=False,
+    )
+
     is_new_issue = Column(
         Boolean,
         index=True,
@@ -795,13 +800,6 @@ class Issue(Base, PrepareMixin, MutableRecordMixin):  # noqa
         Integer, doc="Code identifiying the issue type", nullable=False, index=True
     )
 
-    callable = Column(
-        String(length=INNODB_MAX_INDEX_LENGTH),
-        doc="Callable containing the issue",
-        nullable=False,
-        index=True,
-    )
-
     instances = relationship(
         "IssueInstance",
         primaryjoin="Issue.id == foreign(IssueInstance.issue_id)",
@@ -1090,12 +1088,6 @@ class TraceFrame(Base, PrepareMixin, RecordMixin):  # noqa
 
     kind = Column(Enum(TraceKind), nullable=False, index=False)
 
-    caller: str = Column(
-        String(length=INNODB_MAX_INDEX_LENGTH),
-        nullable=False,
-        doc="The function/method that produces the tainted trace",
-    )
-
     caller_id = Column(BIGDBIDType, nullable=False, server_default="0", default=0)
 
     caller_port: str = Column(
@@ -1105,12 +1097,6 @@ class TraceFrame(Base, PrepareMixin, RecordMixin):  # noqa
         doc="The caller port of this call edge",
     )
 
-    callee: str = Column(
-        String(length=INNODB_MAX_INDEX_LENGTH),
-        nullable=False,
-        doc="The function/method within the caller that produces the tainted trace.",
-    )
-
     callee_id = Column(BIGDBIDType, nullable=False, server_default="0", default=0)
 
     callee_location = Column(
@@ -1126,10 +1112,6 @@ class TraceFrame(Base, PrepareMixin, RecordMixin):  # noqa
         doc="The callee port of this call edge'",
     )
 
-    filename = Column(
-        String(length=4096), doc="Filename containing the call", nullable=False
-    )
-
     filename_id = Column(BIGDBIDType, nullable=False, server_default="0", default=0)
 
     run_id = Column("run_id", BIGDBIDType, nullable=False, index=False)

sapp/sapp/tests/fake_object_generator.py

@@ -82,14 +82,11 @@ def precondition(
         trace_frame = TraceFrame.Record(
             id=DBID(),
             kind=TraceKind.PRECONDITION,
-            caller=caller,
             caller_id=caller_record.id,
             caller_port=caller_port,
-            callee=callee,
             callee_id=callee_record.id,
             callee_port=callee_port,
             callee_location=SourceLocation(location[0], location[1], location[2]),
-            filename=filename,
             filename_id=filename_record.id,
             titos=[],
             run_id=self.run_id,
@@ -119,14 +116,11 @@ def postcondition(
         trace_frame = TraceFrame.Record(
             id=DBID(),
             kind=TraceKind.POSTCONDITION,
-            caller=caller,
             caller_id=caller_record.id,
             caller_port=caller_port,
-            callee=callee,
             callee_id=callee_record.id,
             callee_port=callee_port,
             callee_location=SourceLocation(location[0], location[1], location[2]),
-            filename=filename,
             filename_id=filename_record.id,
             titos=[],
             run_id=self.run_id,
@@ -142,6 +136,11 @@ def postcondition(
         return trace_frame
 
     def shared_text(self, contents, kind):
+        if self.graph:
+            shared_text = self.graph.get_shared_text(kind, contents)
+            if shared_text is not None:
+                return shared_text
+
         result = SharedText.Record(id=DBID(), contents=contents, kind=kind)
         if self.graph:
             self.graph.add_shared_text(result)
@@ -194,7 +193,6 @@ def instance(
         result = IssueInstance.Record(
             id=DBID(),
             location=SourceLocation(6, 7, 8),
-            filename=filename.contents,
             filename_id=filename.id,
             message_id=message.id,
             callable_id=callable.id,

sapp/sapp/trace_graph.py

@@ -31,17 +31,17 @@ def __init__(self) -> None:
         self._issue_instances: Dict[int, IssueInstance] = {}
         self._trace_annotations: Dict[int, TraceFrameAnnotation] = {}
 
-        # Create a mapping of (caller, caller_port) to the corresponding
+        # Create a mapping of (caller_id, caller_port) to the corresponding
         # trace frame's id.
         self._trace_frames_map: DefaultDict[  # pyre-ignore: T41307149
-            Tuple[str, str], Set[int]
+            Tuple[int, str], Set[int]
         ] = defaultdict(set)
 
         # Similar to _trace_frames_map, but maps the reverse direction
-        # of the trace graph, i.e. (callee[, callee_port]) to the
+        # of the trace graph, i.e. (callee_id, callee_port) to the
         # trace_frame_id.
         self._trace_frames_rev_map: DefaultDict[  # pyre-ignore: T41307149
-            Tuple[str, str], Set[int]
+            Tuple[int, str], Set[int]
         ] = defaultdict(set)
 
         self._trace_frames: Dict[int, TraceFrame] = {}
@@ -105,6 +105,9 @@ def add_issue_instance_fix_info(
         ), "Instance fix info already exists"
         self._issue_instance_fix_info[instance.id.local_id] = fix_info
 
+    def get_text(self, shared_text_id: DBID) -> str:
+        return self._shared_texts[shared_text_id.local_id].contents
+
     def get_shared_text(
         self, kind: SharedTextKind, content: str
     ) -> Optional[SharedText]:
@@ -114,17 +117,17 @@ def get_shared_text(
                 return self._shared_texts[contents[content]]
         return None
 
-    def has_postconditions_with_caller(self, caller: str, caller_port: str) -> bool:
-        key = (caller, caller_port)
+    def has_postconditions_with_caller(self, caller_id: DBID, caller_port: str) -> bool:
+        key = (caller_id.local_id, caller_port)
         post_ids = {
             tf_id
             for tf_id in self._trace_frames_map[key]
             if self._trace_frames[tf_id].kind == TraceKind.POSTCONDITION
         }
         return len(post_ids) != 0
 
-    def has_preconditions_with_caller(self, caller: str, caller_port: str) -> bool:
-        key = (caller, caller_port)
+    def has_preconditions_with_caller(self, caller_id: DBID, caller_port: str) -> bool:
+        key = (caller_id.local_id, caller_port)
         pre_ids = {
             tf_id
             for tf_id in self._trace_frames_map[key]
@@ -143,21 +146,21 @@ def get_precondition_annotations(self, pre_id: int) -> List[TraceFrameAnnotation
         ]
 
     def add_trace_frame(self, trace_frame: TraceFrame) -> None:
-        key = (trace_frame.caller, trace_frame.caller_port)
-        rev_key = (trace_frame.callee, trace_frame.callee_port)
+        key = (trace_frame.caller_id.local_id, trace_frame.caller_port)
+        rev_key = (trace_frame.callee_id.local_id, trace_frame.callee_port)
         self._trace_frames_map[key].add(trace_frame.id.local_id)
         self._trace_frames_rev_map[rev_key].add(trace_frame.id.local_id)
         self._trace_frames[trace_frame.id.local_id] = trace_frame
 
-    def has_trace_frame_with_caller(self, caller: str, caller_port: str) -> bool:
-        key = (caller, caller_port)
+    def has_trace_frame_with_caller(self, caller_id: DBID, caller_port: str) -> bool:
+        key = (caller_id.local_id, caller_port)
         return key in self._trace_frames_map
 
     def get_trace_frames_from_caller(
-        self, caller: str, caller_port: str
+        self, caller_id: DBID, caller_port: str
     ) -> List[TraceFrame]:
-        if self.has_trace_frame_with_caller(caller, caller_port):
-            key = (caller, caller_port)
+        if self.has_trace_frame_with_caller(caller_id, caller_port):
+            key = (caller_id.local_id, caller_port)
             return [
                 self._trace_frames[trace_frame_id]
                 for trace_frame_id in self._trace_frames_map[key]
@@ -172,6 +175,10 @@ def add_shared_text(self, shared_text: SharedText) -> None:
         assert (
             shared_text.id.local_id not in self._shared_texts
         ), "Shared text already exists"
+        assert (
+            shared_text.kind not in self._shared_text_lookup
+            or shared_text.contents not in self._shared_text_lookup[shared_text.kind]
+        ), "Shared text with same kind, contents exists"
 
         self._shared_texts[shared_text.id.local_id] = shared_text
 

sapp/sapp/trimmed_trace_graph.py

@@ -87,7 +87,9 @@ def _populate_affected_issues(self, graph: TraceGraph) -> None:
         affected_instance_ids = [
             instance.id.local_id
             for instance in graph._issue_instances.values()
-            if self._is_filename_prefixed_with(instance.filename, self._affected_files)
+            if self._is_filename_prefixed_with(
+                graph.get_text(instance.filename_id), self._affected_files
+            )
         ]
 
         for instance_id in affected_instance_ids:
@@ -128,7 +130,7 @@ def _populate_issues_from_affected_trace_frames(self, graph: TraceGraph) -> None
             trace_frame
             for trace_frame in graph._trace_frames.values()
             if self._is_filename_prefixed_with(
-                trace_frame.filename, self._affected_files
+                graph.get_text(trace_frame.filename_id), self._affected_files
             )
         ]
 
@@ -142,7 +144,7 @@ def _populate_issues_from_affected_trace_frames(self, graph: TraceGraph) -> None
                 [
                     graph._trace_frames[trace_frame_id]
                     for trace_frame_id in graph._trace_frames_rev_map[
-                        (trace_frame.caller, trace_frame.caller_port)
+                        (trace_frame.caller_id.local_id, trace_frame.caller_port)
                     ]
                     if graph._trace_frames[trace_frame_id].kind == trace_frame.kind
                 ]
@@ -274,7 +276,7 @@ def _populate_issue_trace(
         filtered_ids = []
         for trace_frame_id in trace_frame_ids:
             frame = graph._trace_frames[trace_frame_id]
-            if not kind or kind == frame.kind:
+            if kind is None or kind == frame.kind:
                 self.add_issue_instance_trace_frame_assoc(instance, frame)
                 filtered_ids.append(trace_frame_id)
         self._populate_trace(graph, filtered_ids)
@@ -318,7 +320,7 @@ def _populate_trace(self, graph: TraceGraph, trace_frame_ids: List[int]) -> None
             self._add_trace_frame(graph, trace_frame)
             self._visited_trace_frame_ids.add(trace_frame_id)
 
-            key = (trace_frame.callee, trace_frame.callee_port)
+            key = (trace_frame.callee_id.local_id, trace_frame.callee_port)
             trace_frame_ids.extend(
                 [
                     trace_frame_id