JosephTremoulet marked 4 inline comments as done.
JosephTremoulet added a comment.
Added Exception stream to minidump-basic.yaml as suggested.
================
Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:394
+ mapOptionalHex(IO, "Exception Address", Exception.ExceptionAddress, 0);
+ IO.mapOptional("Number Parameters", Exception.NumberParameters,
+ support::ulittle32_t(0u));
----------------
labath wrote:
> This file has a helper function for this (`mapOptional(IO, "name", value,
> 0)`. I'd consider changing the field name to "Number of Parameters" even
> though it does not match the field name, as it reads weird without that. I'm
> not sure why the microsoft naming is inconsistent here -- most of the other
> minidump structs have "of" in their name already (BaseOfImage, SizeOfImage,
> etc.), but at least we can be consistent.
Updated to use the helper, and changed the name in the YAML to "Number of
Parameters". Let me know if it's important to you to also change the name of
the field in the llvm::minidump::Exception type to `NumberOfParameters` -- I
wasn't sure if you were suggesting that, and regardless my preference would be
to leave that as-is to match breakpad aside from casing, as otherwise it's hard
to know where to stop (e.g. change "ExceptionInformation" to "Parameters" to
match "NumberOfParameters" and the YAML? Reconcile the several different ways
that alignment padding fields are named? etc.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
