Improve SuperPMI missing data asserts (#52129)
authorBruce Forstall <brucefo@microsoft.com>
Tue, 4 May 2021 22:31:30 +0000 (15:31 -0700)
committerGitHub <noreply@github.com>
Tue, 4 May 2021 22:31:30 +0000 (15:31 -0700)
commit4129f80b2f3e98b63befa8a54dc2444a706fa7fc
treed0bada6dbfd3b65c8a8d2eaed152e399652c6ac9
parent576be0dde8c3e267c9e247ec17f2f4af8491ce0c
Improve SuperPMI missing data asserts (#52129)

* Improve SuperPMI missing data asserts

SuperPMI has many asserts for missing data that are specially handled
to return a "missing data" error code. We typically ignore these, except
to note that a new collection might be needed. In particular, we prefer a
"missing data" error to an unidentified (caught) crash, which might indicate a JIT crash.

A recent change exposed a case where we were missing an assert, leading
to a crash. I went through all the methodcontext.cpp "rep" functions and added
appropriate asserts to all cases where they were missing.

To do this, add new `AssertMapExists`, `AssertKeyExists`, and `AssertMapAndKeyExist`
macros to make this consistent, easy, and compact, and converted all existing map and key
asserts to use these new forms.

In addition,
1. Rewrite the code to be much more regular and consistent.
2. Add many missing `DEBUG_REC` and `DEBUG_REP` cases, and fix some that were wrong.
3. Create a `key` variable almost everywhere, to avoid repeated `CastHandle` calls.
4. Simplify/commonize the `ZeroMemory` calls.
5. Remove all BOOL; use bool as appropriate.

There are no asm diffs (as expected). I verified the new asserts properly fire by using
`superpmi.py replay -jitoption JitStress=2`.

(Re-revert change, with fix)

* Fix clang compilation failures, more

clang is less permissive about empty variatic macros `__VA_ARGS__`,
so add `AssertMapExistsNoMessage`, `AssertKeyExistsNoMessage`, and
`AssertMapAndKeyExistNoMessage` for cases where we haven't bothered
to write a failure message, to avoid compiler problems. It's also more
clear in the code.

Change all the functions to explicity compute and use a `value` variable
for the key=>value map, to reduce duplicate casting, and make the code
more clear.

* clang build fix
src/coreclr/ToolBox/superpmi/superpmi-shared/errorhandling.h
src/coreclr/ToolBox/superpmi/superpmi-shared/methodcontext.cpp