Skip to content

Ctf2 fixes #365

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MatthewKhouzam wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Ctf2 fixes #365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
215d5cc
ctf2: add missing types
MatthewKhouzam Dec 4, 2025
4882775
ctf: make an alias cache to auto-expand ctf2 aliases
MatthewKhouzam Dec 4, 2025
b347e64
ctf: fix variants can have integer selectors
MatthewKhouzam Dec 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 41 additions & 7 deletions ...g.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/ctf/core/trace/Metadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.UUID;

Expand Down Expand Up @@ -73,6 +75,9 @@

import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
import com.google.gson.JsonParser;
import com.google.gson.JsonSyntaxException;

/**
Expand Down Expand Up @@ -247,30 +252,42 @@ private static ICTFMetadataNode parseJsonToTree(String json) throws CTFException
Gson gson = builder.create();

String[] jsonBlocks = json.split("\u001e"); //$NON-NLS-1$
Map<String, JsonObject> metadata = new HashMap<>();

// Second pass: expand string references and parse
Copy link

@coderabbitai coderabbitai bot Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Clarify or correct the "Second pass" comment.

The comment states "Second pass" but there's no visible first pass in the code. The loop performs JSON parsing, alias expansion, and fragment creation in a single pass.

Consider updating the comment to accurately describe the operation:

-        // Second pass: expand string references and parse
+        // Parse fragments, expand aliases, and build metadata tree
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Second pass: expand string references and parse
// Parse fragments, expand aliases, and build metadata tree
🤖 Prompt for AI Agents 
In
ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/ctf/core/trace/Metadata.java
around line 257, the inline comment "Second pass: expand string references and
parse" is misleading because there is no prior "first pass" and the loop
performs parsing, alias expansion, and fragment creation in one pass; update the
comment to accurately describe the work being done (for example: "Parse metadata
entries, expand string references/aliases, and create fragments"), or split the
logic into clearly named helper methods with comments for each stage if you
prefer to retain multi-pass terminology.

for (int i = 1; i < jsonBlocks.length; i++) {
JsonElement element = JsonParser.parseString(jsonBlocks[i]);
if (element.isJsonObject()) {
JsonObject obj = element.getAsJsonObject();
expandAliases(obj, metadata);
if (obj.has(JsonMetadataStrings.TYPE) && obj.get(JsonMetadataStrings.TYPE).getAsString().equals(JsonMetadataStrings.FRAGMENT_FIELD_ALIAS)) {
metadata.put(obj.get(JsonMetadataStrings.NAME).getAsString(), obj.get(JsonMetadataStrings.FIELD_CLASS).getAsJsonObject());
}
Comment on lines +263 to +265
Copy link

@coderabbitai coderabbitai bot Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add null checks before accessing NAME and FIELD_CLASS.

Line 264 assumes that FRAGMENT_FIELD_ALIAS nodes always have both NAME and FIELD_CLASS fields. If either is missing, this will throw a NullPointerException.

Apply this diff to add defensive checks:

 if (obj.has(JsonMetadataStrings.TYPE) && obj.get(JsonMetadataStrings.TYPE).getAsString().equals(JsonMetadataStrings.FRAGMENT_FIELD_ALIAS)) {
-    metadata.put(obj.get(JsonMetadataStrings.NAME).getAsString(), obj.get(JsonMetadataStrings.FIELD_CLASS).getAsJsonObject());
+    if (obj.has(JsonMetadataStrings.NAME) && obj.has(JsonMetadataStrings.FIELD_CLASS)) {
+        metadata.put(obj.get(JsonMetadataStrings.NAME).getAsString(), obj.get(JsonMetadataStrings.FIELD_CLASS).getAsJsonObject());
+    }
 }
🤖 Prompt for AI Agents 
In
ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/ctf/core/trace/Metadata.java
around lines 263 to 265, the code assumes FRAGMENT_FIELD_ALIAS objects always
contain NAME and FIELD_CLASS, which can cause NullPointerException; update the
conditional to also check obj.has(JsonMetadataStrings.NAME) and
obj.has(JsonMetadataStrings.FIELD_CLASS) (and ensure the retrieved members are
non-null before calling getAsString()/getAsJsonObject()), and only call
metadata.put(...) when both checks pass (optionally log or skip entries when
fields are missing).

}

ICTFMetadataNode fragment;
try {
fragment = Objects.requireNonNull(gson.fromJson(jsonBlocks[i], CTFJsonMetadataNode.class));
fragment = Objects.requireNonNull(gson.fromJson(element, CTFJsonMetadataNode.class));
} catch (JsonSyntaxException e) {
throw new CTFException("Trace cannot be parsed as CTF2"); //$NON-NLS-1$
}

String type = fragment.getType();
if (type.equals(JsonMetadataStrings.FRAGMENT_PREAMBLE)) {
fragment = Objects.requireNonNull(gson.fromJson(jsonBlocks[i], JsonPreambleMetadataNode.class));
fragment = Objects.requireNonNull(gson.fromJson(element, JsonPreambleMetadataNode.class));
} else if (type.equals(JsonMetadataStrings.FRAGMENT_TRACE)) {
fragment = Objects.requireNonNull(gson.fromJson(jsonBlocks[i], JsonTraceMetadataNode.class));
fragment = Objects.requireNonNull(gson.fromJson(element, JsonTraceMetadataNode.class));
} else if (type.equals(JsonMetadataStrings.FRAGMENT_CLOCK)) {
fragment = Objects.requireNonNull(gson.fromJson(jsonBlocks[i], JsonClockMetadataNode.class));
fragment = Objects.requireNonNull(gson.fromJson(element, JsonClockMetadataNode.class));
} else if (type.equals(JsonMetadataStrings.FRAGMENT_EVENT_RECORD)) {
fragment = Objects.requireNonNull(gson.fromJson(jsonBlocks[i], JsonEventRecordMetadataNode.class));
fragment = Objects.requireNonNull(gson.fromJson(element, JsonEventRecordMetadataNode.class));
} else if (type.equals(JsonMetadataStrings.FRAGMENT_DATA_STREAM)) {
fragment = Objects.requireNonNull(gson.fromJson(jsonBlocks[i], JsonDataStreamMetadataNode.class));
fragment = Objects.requireNonNull(gson.fromJson(element, JsonDataStreamMetadataNode.class));
if (!jsonBlocks[i].contains("id:")) { //$NON-NLS-1$
((JsonDataStreamMetadataNode) fragment).setId(-1);
}
} else if (type.equals(JsonMetadataStrings.FRAGMENT_FIELD_ALIAS)) {
fragment = Objects.requireNonNull(gson.fromJson(jsonBlocks[i], JsonFieldClassAliasMetadataNode.class));
fragment = Objects.requireNonNull(gson.fromJson(element, JsonFieldClassAliasMetadataNode.class));
}

((CTFJsonMetadataNode) fragment).initialize();
Expand All @@ -281,6 +298,23 @@ private static ICTFMetadataNode parseJsonToTree(String json) throws CTFException
return root;
}

private static void expandAliases(JsonObject obj, Map<String, JsonObject> metadata) {
for (String key : obj.keySet()) {
JsonElement value = obj.get(key);
if (value.isJsonPrimitive() && value.getAsJsonPrimitive().isString() && metadata.containsKey(value.getAsString())) {
obj.add(key, metadata.get(value.getAsString()));
} else if (value.isJsonObject()) {
expandAliases(value.getAsJsonObject(), metadata);
} else if (value.isJsonArray()) {
for (JsonElement elem : value.getAsJsonArray()) {
if (elem.isJsonObject()) {
expandAliases(elem.getAsJsonObject(), metadata);
}
}
}
}
}

/**
* Checks the version of the CTF trace by reading the first JSON fragment if
* it is a CTF2 fragment it updates the major of the trace
Expand Down
19 changes: 12 additions & 7 deletions ...org/eclipse/tracecompass/internal/ctf/core/event/metadata/tsdl/variant/VariantParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.eclipse.tracecompass.ctf.core.event.metadata.DeclarationScope;
import org.eclipse.tracecompass.ctf.core.event.types.EnumDeclaration;
import org.eclipse.tracecompass.ctf.core.event.types.IDeclaration;
import org.eclipse.tracecompass.ctf.core.event.types.IntegerDeclaration;
import org.eclipse.tracecompass.ctf.core.event.types.VariantDeclaration;
import org.eclipse.tracecompass.ctf.core.trace.CTFTrace;
import org.eclipse.tracecompass.ctf.parser.CTFParser;
Expand Down Expand Up @@ -266,7 +267,7 @@ public VariantDeclaration parse(ICTFMetadataNode variant, ICommonTreeParserParam
if (variant instanceof JsonStructureFieldMemberMetadataNode) {
JsonObject fieldClass = ((JsonStructureFieldMemberMetadataNode) variant).getFieldClass().getAsJsonObject();
if (fieldClass.has(SELECTOR_FIELD_LOCATION)) {
JsonArray location = fieldClass.get(SELECTOR_FIELD_LOCATION).getAsJsonArray();
JsonArray location = (fieldClass.get(SELECTOR_FIELD_LOCATION).getAsJsonObject().get(JsonMetadataStrings.PATH)).getAsJsonArray();
Copy link

@coderabbitai coderabbitai bot Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add null safety checks for nested JSON navigation.

The chained method calls assume that SELECTOR_FIELD_LOCATION is a JsonObject containing a PATH field that is a JsonArray. If the structure is malformed, this will throw an exception.

Consider adding intermediate checks or using a try-catch block:

 if (fieldClass.has(SELECTOR_FIELD_LOCATION)) {
-    JsonArray location = (fieldClass.get(SELECTOR_FIELD_LOCATION).getAsJsonObject().get(JsonMetadataStrings.PATH)).getAsJsonArray();
+    JsonElement selectorElement = fieldClass.get(SELECTOR_FIELD_LOCATION);
+    if (selectorElement != null && selectorElement.isJsonObject()) {
+        JsonElement pathElement = selectorElement.getAsJsonObject().get(JsonMetadataStrings.PATH);
+        if (pathElement != null && pathElement.isJsonArray()) {
+            JsonArray location = pathElement.getAsJsonArray();
-    variantTag = location.get(location.size() - 1).getAsString();
-    hasTag = true;
+            variantTag = location.get(location.size() - 1).getAsString();
+            hasTag = true;
+        }
+    }
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In
ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/internal/ctf/core/event/metadata/tsdl/variant/VariantParser.java
around line 270, the code directly chains
JsonElement.get/getAsJsonObject/getAsJsonArray calls which will throw NPEs or
IllegalStateExceptions if SELECTOR_FIELD_LOCATION or PATH are missing or of the
wrong type; add explicit null/type checks (e.g., check that
fieldClass.has(SELECTOR_FIELD_LOCATION) and that the retrieved element
isJsonObject, that it has PATH and that PATH isJsonArray) before calling
getAsJsonArray, or wrap the access in a try-catch and produce a clear
error/return path; ensure you handle the malformed case by logging or throwing a
descriptive exception rather than letting a chained call blow up.

variantTag = location.get(location.size() - 1).getAsString();
hasTag = true;
}
Expand Down Expand Up @@ -361,13 +362,17 @@ public VariantDeclaration parse(ICTFMetadataNode variant, ICommonTreeParserParam
if (decl == null) {
throw new ParseException("Variant tag not found: " + variantTag); //$NON-NLS-1$
}
if (!(decl instanceof EnumDeclaration)) {
throw new ParseException("Variant tag must be an enum: " + variantTag); //$NON-NLS-1$
}
EnumDeclaration tagDecl = (EnumDeclaration) decl;
if (!intersects(tagDecl.getLabels(), variantDeclaration.getFields().keySet())) {
throw new ParseException("Variant contains no values of the tag, impossible to use: " + variantName); //$NON-NLS-1$
if (decl instanceof EnumDeclaration) {
EnumDeclaration tagDecl = (EnumDeclaration) decl;
if (!intersects(tagDecl.getLabels(), variantDeclaration.getFields().keySet())) {
throw new ParseException("Variant contains no values of the tag, impossible to use: " + variantName); //$NON-NLS-1$
}
} else if (decl instanceof IntegerDeclaration) {
// do nothing
} else {
throw new ParseException("Variant tag must be an enum: " + variantTag);//$NON-NLS-1$
}

}

return variantDeclaration;
Expand Down
16 changes: 16 additions & 0 deletions ...ss.ctf.core/src/org/eclipse/tracecompass/internal/ctf/core/utils/JsonMetadataStrings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,17 @@ private JsonMetadataStrings() {
*/
public static final String FRAGMENT_FIELD_ALIAS = "field-class-alias"; //$NON-NLS-1$

/**
* The field class
*/
public static final String FIELD_CLASS = "field-class"; //$NON-NLS-1$

/**
* The name of the fragment
*/
public static final String NAME = "name"; //$NON-NLS-1$


/**
* Type string for a CTF2 clock class fragment
*/
Expand Down Expand Up @@ -162,6 +173,11 @@ private JsonMetadataStrings() {
*/
public static final String VARIANT = "variant"; //$NON-NLS-1$

/**
* Type string for a structure field class
*/
public static final String PATH = "path"; //$NON-NLS-1$

/**
* Type string for a structure field class
*/
Expand Down
Loading