catapult-project / catapult

Deprecated Catapult GitHub. Please instead use http://crbug.com "Speed>Benchmarks" component for bugs and https://chromium.googlesource.com/catapult for downloading and editing source code..
https://chromium.googlesource.com/catapult
BSD 3-Clause "New" or "Revised" License
1.91k stars 563 forks source link

Catapult doesn't handle version-4 cswitch events, breaks system tracing on Windows 10 #4582

Closed randomascii closed 5 years ago

randomascii commented 5 years ago

In third_party\catapult\tracing\tracing\extras\importer\etw\thread_parser.html we attempt to parse context-switch events but the code has not been updated and will actually cause failures when recording traces.

In decodeFields() the code only handles up to header.version==3, while Windows 10 16299 actually generates header.version == 4. In decodeCSwitchFields() the code only handles header.version==2.

Both functions (well, decodeCSwitchFields() at least) need to handle up to level 4 or else fail gracefully or else we need to disable this feature.

This bug was preventing a trace from being recorded in crbug.com/872930.

benshayden commented 5 years ago

I'd be happy to review a CL. Would you be able to draft it?

randomascii commented 5 years ago

I can't find any documentation on what the format is for a version 4 cswitch event. The documentation doesn't seem to acknowledge anything after version 2. That said, if I just remove the version check then it seems to work fine - the data seems entirely plausible. I'll chat with some other experts...

randomascii commented 5 years ago

I tested with today's canary (71.0.3552.2) and I can now record a system trace with context switch events (when running chrome as administrator) so I am closing this as fixed.