From 352f329d92f762dde8d1e86448b866738af40b71 Mon Sep 17 00:00:00 2001
From: Shivam Raikundalia <sraikund@meta.com>
Date: Wed, 16 Oct 2024 12:55:55 -0700
Subject: [PATCH] Prevent Negative TIDs in Trace

Summary:
Right now we take the TIDs returned from CUPTI and convert them from int64_t to int32_t which results in negative tids. Because Perfetto uses int32_t in its protobuf files (according to https://github.com/google/perfetto/issues/886) we can't just convert to uint32_t because perfetto will just convert values back into their negative counterparts. To fix this, we convert to only positive values in int32_t by finding the absolute value of the original int32_t representation of the tid. We also make a special case for INT_MIN to map to 0 so that it is apparent how we handle overflow.

We add this sanitization to all tid outputs for hygiene purposes instead of just CUPTI output.

Differential Revision: D64490195
---
 libkineto/src/output_json.cpp | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/libkineto/src/output_json.cpp b/libkineto/src/output_json.cpp
index ab70ab4b..203c3689 100644
--- a/libkineto/src/output_json.cpp
+++ b/libkineto/src/output_json.cpp
@@ -77,6 +77,15 @@ void ChromeTraceLogger::sanitizeStrForJSON(std::string& value) {
   value.erase(std::remove(value.begin(), value.end(), '\n'), value.end());
 }
 
+static inline int32_t sanitizeTid(int32_t tid) {
+  // Convert all negative tids to its positive value. Create a specific case
+  // for INT_MIN so it is obvious how it is being handled.
+  if (tid == INT_MIN) {
+    return 0;
+  }
+  return std::abs(tid);
+}
+
 void ChromeTraceLogger::metadataToJSON(
     const std::unordered_map<std::string, std::string>& metadata) {
   for (auto [k, v] : metadata) {
@@ -197,9 +206,9 @@ void ChromeTraceLogger::handleResourceInfo(
       "sort_index": {}
     }}
   }},)JSON",
-      time/1000, time%1000, info.deviceId, info.id,
+      time/1000, time%1000, info.deviceId, sanitizeTid(info.id),
       info.name,
-      time/1000, time%1000, info.deviceId, info.id,
+      time/1000, time%1000, info.deviceId, sanitizeTid(info.id),
       info.sortIndex);
   // clang-format on
 }
@@ -312,7 +321,7 @@ void ChromeTraceLogger::handleGenericInstantEvent(
       toString(op.type()),
       op.name(),
       op.deviceId(),
-      op.resourceId(),
+      sanitizeTid(op.resourceId()),
       ts / 1000,
       ts % 1000,
       op.metadataJson());
@@ -483,7 +492,7 @@ void ChromeTraceLogger::handleActivity(const libkineto::ITraceActivity& op) {
     "ph": "X", "cat": "{}", "name": "{}", "pid": {}, "tid": {},
     "ts": {}.{:03}, "dur": {}.{:03}{}
   }},)JSON",
-          toString(op.type()), op_name, device, resource,
+          toString(op.type()), op_name, device, sanitizeTid(resource),
           ts/1000, ts %1000, duration/1000, duration %1000, args);
   // clang-format on
   if (op.flowId() > 0) {
@@ -537,7 +546,7 @@ void ChromeTraceLogger::handleLink(
     "ph": "{}", "id": {}, "pid": {}, "tid": {}, "ts": {}.{:03},
     "cat": "{}", "name": "{}"{}
   }},)JSON",
-      type, id, e.deviceId(), e.resourceId(), ts/1000, ts%1000, name, name, binding);
+      type, id, e.deviceId(), sanitizeTid(e.resourceId()), ts/1000, ts%1000, name, name, binding);
   // clang-format on
 }