[GDB] Fix pretty-printer crash for sycl::item MOffset #19683
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a crash in the GDB pretty-printer for sycl::item objects when the MOffset field is not available. The fix adds exception handling to gracefully skip offset printing when the offset feature has been disabled (as deprecated in SYCL 2020).
- Adds try-catch block around offset field access and printing logic
- Prevents crashes when
MOffsetfield is not present insycl::itemimpl - Maintains backward compatibility for items with offset enabled
| offset = SYCLIdPrinter(offset_id).value_as_string() | ||
| if offset not in ["0", "{0, 0}", "{0, 0, 0}"]: | ||
| string += ", offset " + offset | ||
| except: |
There was a problem hiding this comment.
Using a bare 'except:' clause catches all exceptions including system errors like KeyboardInterrupt. Consider catching specific exceptions like AttributeError or KeyError, or use 'except Exception:' to avoid catching system exceptions.
| except: | |
| except Exception: |
Copilot uses AI. Check for mistakes.
| if offset not in ["0", "{0, 0}", "{0, 0, 0}"]: | ||
| string += ", offset " + offset | ||
| except: | ||
| pass # device offset disabled |
There was a problem hiding this comment.
The comment 'device offset disabled' is misleading. Based on the PR description, this handles cases where offset has been disabled/deprecated in SYCL 2020, not specifically device offset. Consider updating to '# offset feature disabled (deprecated in SYCL 2020)' for clarity.
| pass # device offset disabled | |
| pass # offset feature disabled (deprecated in SYCL 2020) |
Copilot uses AI. Check for mistakes.
|
Do you know if it's feasible to add a test for this change? I don't recall seeing tests for |
The pretty printer looks for an
MOffsetfield insycl::itemimpl without considering whether or not offset has been enabled (Offset was deprecated in SYCL 2020): Add a check to avoid printingMOffsetif offset has been disabled.