[BoundsSafety] Move traps reasons for indexing to new trap reasons infrastructure

[delcypher]

This patch moves generation of the trap reason strings for indexing into
pointers into the new trap reason infrastructure. We will need to move
many other trap reasons over to the new infrastructure but this is the
first we are moving over.

Previously array indexing emitted these very unspecifc trap reason
messages:

• Dereferencing above bounds
• Deferencing below bounds

Now we emit trap reasons that look like

• indexing above upper bound in '<expr>'
• indexing below lower bound in '<expr>'
• indexing overflows address space in '<expr>'

where <expr> is the ArraySubscriptExpr printed as a string (see test
cases for example).

There are several improvements here:

1. We say indexing rather than dereferencing which is more specific.
2. We emit a specific trap reason for address space overflow. Previously
   there was no distinction between the upper bound trap and the address
   space overflow trap.
3. We emit the textual representation of the ArraySubscriptExpr that
   triggered the bounds check failed. This makes the message very
   specific.

This new approach to emitting trap reasons will likely increase the size
of debug info. To give users control a new flag
-fbounds-safety-debug-trap-trap-reasons has been added which is
analogous to -fsanitize-debug-trap-reasons for UBSan. The flag takes
three values:

• none - Dont' emit any trap reasons
• detailed - Emit the new more detailed trap reasons (the default)
• basic - Emit the less descriptive trap reasons using the legacy
  infrastructure.

While working on this it became clear that emission of trap diagnostics
is more complicated than for UBSan become some of the context for where
we are emitting the trap exists in a different stackframe than the
location where we can actually emit the trap diagnostic. In
EmitWidePtrArraySubscriptExpr we don't know if we are emitting a
lower/upper bound/address space check. In EmitBoundsSafetyBoundsCheck
we don't know we are emitting a check for an ArraySubscriptExpr so there
isn't a function where we can emit the trap diagnostic that has all the
necessary information. The solution used in this patch is to re-use the
PartialDiagnostic class which essentially lets us partially construct
a diagnostic in one function and then pass it along to another function
to the actual where the trap diagnostic can be fully constructed.

Note in this implementation the "detailed" trap reason is always
constructed even if it later gets thrown away. There are several reasons
for doing this:

• For clang's diagnostics normally we typically don't write guards
  around them to check they are enabled (e.g. the warning might be
  actually disabled).
• While technically we could write guards around all the code that
  builds the TrapReason objects this will become repetitive very
  quickly. It's cleaner to just put the guard in this function.
• I'm also planning to use these TrapReason objects for the upcoming
  soft trap mode and I didn't want to put guards around their creation
  until I've figured out exactly how this is going to be implemented.
• This is also how its implemented for UBSan's trapping diagnostics
  right now. This is not a particularly strong argument because I'm the
  one who implemented that but at least upstream didn't object to me
  doing it this way.

rdar://158623471

[delcypher]

@swift-ci test llvm

[rapidsna]

How multiple reasons are handled? It looks like the order of trap reason is upside down, it's in the overflow -> upper order, and then in actual checking it's upper -> overflow.

[delcypher]

Good catch. So the order of building traps reasons doesn't matter here. The only thing that matters is we fill in the OverflowTR and UpperBoundTR objects appropriately.

However, having the order not match that we codegen the checks is unnecessarily confusing. I'll fix that.

[rapidsna]

In your description:

emission of trap diagnostics
is more complicated than for UBSan become some of the context for where
we are emitting the trap exists in a different stackframe than the
location where we can actually emit the trap diagnostic. This uses the
PartialDiagnostic infrastructure to solve this problem.

By "stackframe", you meant in a different function in the compiler implementation, right? Is it fundamentally because we may emit multiple checks and potentially skip some checks, the decision for which may be generally handled by a function that generally handles wide pointer, and such?

[rapidsna]

LGTM. Approving assuming the couple of minor comments will be addressed.

[delcypher]

By "stackframe", you meant in a different function in the compiler implementation, right? Is it fundamentally because we may emit multiple checks and potentially skip some checks, the decision for which may be generally handled by a function that generally handles wide pointer, and such?

@rapidsna Yes different stackframes in the compiler implementation. In this concrete case the information about this being an array index access is in CodeGenFunction::EmitWidePtrArraySubscriptExpr but then further down the callstack we actually emit the lower/upper bound checks in CodeGenFunction::EmitBoundsSafetyBoundsCheck. CodeGenFunction::EmitBoundsSafetyBoundsCheck is more general and is handles lots of different cases. Our code being structured this way means there is no one place that we can build the trap diagnostic. In EmitWidePtrArraySubscriptExpr we don't know if we are emitting a lower/upper bound/address space check. In EmitBoundsSafetyBoundsCheck we don't know we are emitting a check for an ArraySubscriptExpr. The solution I came up with here is to re-use the PartialDiagnostic class which essentially let's you partially construct a diagnostic and then pass it along (e.g. through calls) to the actual place where the diagnostic can be fully constructed.

When I implemented better trap diagnostics for UBSan I didn't run into this problem because everything I needed to know for the diagnostic was available in the function where I wanted to emit the trap diagnostic.

I think your explanation of the fundamental problem is correct.