RESOLVED FIXED 237572
[libpas] Report more actionable crash in pas_enumerator
https://bugs.webkit.org/show_bug.cgi?id=237572
Summary [libpas] Report more actionable crash in pas_enumerator
Yusuke Suzuki
Reported 2022-03-07 18:21:06 PST
[libpas] Report more actionable crash in pas_enumerator
Attachments
Patch (34.94 KB, patch)
2022-03-07 18:34 PST, Yusuke Suzuki
no flags
Yusuke Suzuki
Comment 1 2022-03-07 18:34:50 PST
Mark Lam
Comment 2 2022-03-07 18:52:20 PST
Comment on attachment 454060 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=454060&action=review > Source/bmalloc/libpas/src/libpas/pas_utils.c:60 > + pas_panic("%s:%d: %s: assertion %s failed.\n", filename, line, function, expression); It looks like pas_panic will trample on the incoming args: ``` void pas_panic(const char* format, ...) { static const bool fast_panic = false; if (!fast_panic) { ... pas_vlog(format, arg_list); } __builtin_trap(); } ``` Can you also change pas_panic to not call __builtin_trap() directly, but call a NEVER_INLINE pas_trap(const char* filename, int line, const char* function, const char* expression) (similar to WTFCrashWithInfoImpl()). This way we can at least see the line number in crash logs as well. I think this will be useful.
Mark Lam
Comment 3 2022-03-07 20:54:05 PST
Comment on attachment 454060 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=454060&action=review >> Source/bmalloc/libpas/src/libpas/pas_utils.c:60 >> + pas_panic("%s:%d: %s: assertion %s failed.\n", filename, line, function, expression); > > It looks like pas_panic will trample on the incoming args: > ``` > void pas_panic(const char* format, ...) > { > static const bool fast_panic = false; > if (!fast_panic) { > ... > pas_vlog(format, arg_list); > } > __builtin_trap(); > } > ``` > > Can you also change pas_panic to not call __builtin_trap() directly, but call a NEVER_INLINE pas_trap(const char* filename, int line, const char* function, const char* expression) (similar to WTFCrashWithInfoImpl()). This way we can at least see the line number in crash logs as well. I think this will be useful. Never mind. Yusuke reminded me offline that the caller of pas_assertion_failed_no_inline will be on the stack trace in the crash log. So, the needed information is captured after all.
EWS
Comment 4 2022-03-08 12:49:42 PST
Committed r291007 (248183@main): <https://commits.webkit.org/248183@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 454060 [details].
Radar WebKit Bug Importer
Comment 5 2022-03-08 12:50:18 PST
Yusuke Suzuki
Comment 6 2022-03-11 16:00:22 PST
Note You need to log in before you can comment on or make changes to this bug.