tp: work around clang compiler vectorization bug#170
Open
Conversation
When the trace processor shell returns a protocol error response, the engine communicating with it just hangs. The engine must accurately detect the protocol error response and then flush its pending state. This then lets the error be thrown to Sokatoa for reporting to the user. Signed-off-by: Christian W. Damus <cdamus@eclipsesource.com>
Apple Clang 21 (macOS 26.4 / Xcode 26) has a vectorization bug that miscompiles the WebSocket unmasking loop in http_server.cc. The pattern mask[i % sizeof(mask)] with a 4-byte mask is compiled into a vector where every other group of 4 bytes is XORed with zero: bytes 0–3 are correctly unmasked, bytes 4–7 stay masked, bytes 8–11 correct, 12–15 masked, etc. This caused the RPC request field (bytes 4–5 of the inner proto) to be left at its masked value (0x52 instead of 0x10=field-2-varint-tag), which decoded as field_id=10/wire_type=2 instead of field_id=2/varint, making req.request() return 0 (TPM_UNSPECIFIED). The server responded with invalid_request=0. For android-graphics/sokatoa#4933 Signed-off-by: Christian W. Damus <cdamus@eclipsesource.com>
colin-grant-work
approved these changes
Mar 30, 2026
Collaborator
colin-grant-work
left a comment
colin-grant-work
left a comment
There was a problem hiding this comment.
Without this change, I see the load failure reported; with this change, traces load successfully.
Comment on lines
+692
to
+725
| // Reject all pending operations so callers don't hang forever when the | ||
| // engine enters a failed state (e.g. after receiving an invalid_request). | ||
| for (const p of this.pendingParses) p.reject(error); | ||
| this.pendingParses = []; | ||
| for (const p of this.pendingEOFs) p.reject(error); | ||
| this.pendingEOFs = []; | ||
| for (const p of this.pendingResetTraceProcessors) p.reject(error); | ||
| this.pendingResetTraceProcessors = []; | ||
| for (const p of this.pendingRestoreTables) p.reject(error); | ||
| this.pendingRestoreTables = []; | ||
| for (const p of this.pendingComputeMetrics) p.reject(error); | ||
| this.pendingComputeMetrics = []; | ||
| this.pendingReadMetatrace?.reject(error); | ||
| this.pendingReadMetatrace = undefined; | ||
| this.pendingRegisterSqlPackage?.reject(error); | ||
| this.pendingRegisterSqlPackage = undefined; | ||
| this.pendingAnalyzeStructuredQueries?.reject(error); | ||
| this.pendingAnalyzeStructuredQueries = undefined; | ||
| this.pendingTraceSummary?.reject(error); | ||
| this.pendingTraceSummary = undefined; | ||
| // Complete any pending streaming queries with an error result. | ||
| // WritableQueryResult has no reject() method — errors are signalled by | ||
| // calling appendResultBatch() with a QueryResult proto that has `error` | ||
| // set and a final batch with `is_last_batch=true`. | ||
| if (this.pendingQueries.length > 0) { | ||
| const errBatch = protos.QueryResult.encode( | ||
| protos.QueryResult.create({ | ||
| error: reason, | ||
| batch: [protos.QueryResult.CellsBatch.create({isLastBatch: true})], | ||
| }), | ||
| ).finish(); | ||
| for (const q of this.pendingQueries) q.appendResultBatch(errBatch); | ||
| this.pendingQueries = []; | ||
| } |
Collaborator
There was a problem hiding this comment.
Breaking this out into something like a rejectAllPending function would make its goal clear without the opening comment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Apple Clang 21 (macOS 26.4 / Xcode 26) has a vectorization bug that miscompiles the WebSocket unmasking loop in
http_server.cc. Thepattern mask[i % sizeof(mask)]with a 4-byte mask is compiled into a vector where every other group of 4 bytes is XORed with zero: bytes 0–3 are correctly unmasked, bytes 4–7 stay masked, bytes 8–11 correct,12–15 masked, etc.
This caused the RPC request field (bytes 4–5 of the inner proto) to be left at its masked value (
0x52instead of0x10=field-2-varint-tag), which decoded asfield_id=10/wire_type=2instead offield_id=2/varint, makingreq.request()return0 (TPM_UNSPECIFIED). The server responded withinvalid_request=0.Also included in this PR is a fix in the Perfetto UI side of the
Engineclass to fix the application hanging on unexpected results like this from the trace processor shell. So with the broken trace processor that commit 1b1b439 lets the error be shown in the usual way in the Sokatoa Capture Widget instead of it showing a loading spinner forever.These changes should, of course, work just fine with any other version of the or clang compiler or gcc.
@wpaul-samsung this is the problem I reported initially on your
yarn upgradePR in Sokatoa, which I hadn't realized at the time coincided with an OS/Xcode upgrade on my system.