New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Runtime safety (panic interface, slices) #19764
Open
amp-59
wants to merge
125
commits into
ziglang:master
Choose a base branch
from
amp-59:runtime_safety
base: master
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Helper functions require standard library substitutes.
require the extreme slice edge-case: slice of comptime-known pointer with runtime-known end.
…gate of type `ty` permits sentinels.
'convenience' declaration of `runtime_safety`.
* Added convenience declaration `runtime_safety`. * This value modified directly from the command line in `main.zig`. * This value is used to initialize `Builtin` options field `runtime_safety`.
* Updated command line parser to parse controls for each panic cause compile command: `-funwrapped-error`, `-funwrapped-null`, `-freturned-noreturn`, `-freached-unreachable`, `-faccessed-out-of-bounds`, `-faccessed-out-of-order`, `-faccessed-inactive-field`, `-fdivided-by-zero`, `-fmemcpy-argument-aliasing`, `-fmismatched-memcpy-argument-lengths`, `-fmismatched-for-loop-capture-lengths`, `-fmismatched-sentinel`, `-fshift-amt-overflowed`, `-farith-overflowed`, `-fcast-truncated-data`, `-fcast-to-enum-from-invalid`, `-fcast-to-error-from-invalid`, `-fcast-to-pointer-from-invalid`, `-fcast-to-int-from-invalid`, `-fcast-to-unsigned-from-negative`. * Updated command line parser to parse convenience/test flags: `-fruntime-safety`, `-f[no-]analyze-slice2`.
…Safety`. When testing is complete this namespace will be removed and all analogous functions in the file scope will be replaced. * Added helper functions: `preparePanicCause`, `failBlock`, `preparePanicExtraType`, `callBuiltinHelper`, `wantPanicCause`, `bestGenericIntType`, `resolveArithOverflowedPanicImpl`, `resolveArithOverflowedPanicCause`, `abiSizeOfContainingDecl`. * Added panic preparation functions: `panicWithMsg`, `panicReachedUnreachable`, `panicCastToEnumFromInvalid`, `panicUnwrappedError`, `checkAccessNullValue`, `checkUnwrappedError`, `checkDivisionByZero`, `checkAccessOutOfBounds`, `checkAccessOutOfOrder`, `checkAccessOutOfOrderExtra`, `checkAliasingMemcpyArguments`, `checkMismatchedMemcpyArgumentLengths`, `checkMismatchedForLoopCaptureLengths`, `checkAccessInactiveUnionField`, `checkMismatchedSentinel`, `checkMismatchedNullTerminator`, `checkShiftAmountOverflow`, `checkArithmeticOverflow`, `checkCastToEnumFromInvalid`, `checkCastToEnumFromInvalidHelper`, `checkCastToErrorFromInvalid`, `checkCastToErrorFromInvalidHelper`, `checkCastToPointerFromInvalid`, `checkCastToIntFromInvalid`, `checkCastTruncatedData`, `checkCastToUnsignedFromNegative`. * Added primary functions: `zirSliceStart`, `zirSliceEnd`, `zirSliceSentinel`, `zirSliceLength`, `analyzeSlice2`.
`zirReify`, `zirArrayTypeSentinel`, `zirPtrType`, `analyzeSlice`.
`zirSliceStart`, `zirSliceEnd`, `zirSliceSentinel`, and `zirSliceLength`.
`Block.addUnreachable`.
by `zirErrorFromInt`.
`zirEnumFromInt`.
`analyzeOptionalPayloadPtr`.
`zirOptionalPayload`.
`analyzeErrUnionPayload`.
`analyzeErrUnionPayloadPtr`.
`checkCastToUnsignedFromNegative` by `intCast`.
`zirSwitchBlock`. This is potentially the wrong panic cause, but aligns more closely with the backend requirement.
`panicReachedUnreachable` by `analyzeSwitchRuntimeBlock`.
`checkArithmeticOverflow` by `zirShl`.
`checkArithmeticOverflow` by `zirShr`.
* Improved general usability of panic preparation functions by preventing usage by backends which do not support the feature. Note: The x86-64 backend actually does support all but three of the current panic causes. It does not support `checkCastToEnumFromInvalid` and `checkCastToErrorFromInvalid`, nor does it support the former `corrupt_switch` panic cause (except in one unconditional case)--but this might be unreachable for other reasons. * Updated `reinterpretLength` to allow recursively reinterpreting arrays of arrays, arrays of vectors, and vectors of arrays of the child type. * Relocated helper functions `reinterpretLength`, `reinterpretLengthOfContainingDecl`, and `abiSizeOfContainingDecl`, which may be suitable for general use. * Removed extra definition of `Sema.analyzeSlice2` from the sub-namespace. * Added experimental `addSubSafeOptimized`. This function has been tested to produce a small optimisation to arithmetic overflow checks, which are currently very expensive when generated by LLVM. The soundness of this function has not been totally verified, so it remains unused.
a small effect overall, because the applicable variants are not very common (in empty main). However the effect will likely be much larger for functions in `std.crypto`, where the variant is frequently seen in inline while loops.
…nic` parameter count.
precisely. Note that mismatches have not been ruled out, but none are known.
'checked vector arithmetic'.
`panicIndexOutOfBounds`, `panicInactiveUnionField`, `panicSentinelMismatch`, `safetyCheckFormatted`, and `safetyPanic`.
these are unreachable due to the preceding if-expression.
…ed`. This is more efficient for the standard library, because all of these are likely to be compiled before reaching `main`.
`riscv64` + `-fno-llvm`, `-fno-lld`.
etc. for tests using `-ofmt=c`.
For 7e617d2, feel free to regress the RISC-V backend. I do not mind fixing it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This initial section is only intended as a high-level explanation of the obvious features of the new panic interface.
This will be the overarching order of discussion (including later installments):
Note: names of functions, function parameters, fields, types, and compile commands, and texts of compile error messages and panic error messages will be changed if exact substitutes are provided. These changes will be applied last.
New panic interface
Resolves #17969
Below is the design suggested by @andrewrk in the linked issue:
Below is the final design, implemented by this patch:
Changes to the panic function signature
Exclusion of return address
?usize
The purpose of the
?usize
parameter is to allow the special handler functions to forward their return addresses throughdebug.panicExtra
, which calls back tobuiltin.default_panic
to get todebug.panicImpl
. See commit 694fab4.Usage in detail ...
inactive_union_field
,sentinel_mismatch
,index_out_of_bounds
,start_index_greater_than_end
, andunwrap_error
. Every other panic ID emits a call tobuiltin.default_panic
with a message frompanic_messages
.?usize
is non-null
.builtin.default_panic
throughout the standard library. The sole caller isdebug.panicExtra
.debug.panicExtra
throughout the standard library. Five are from the special handler functions. The final caller isdebug.panic
, which calls with both?usize
and?*builtin.StackTrace
asnull
.Exclusion of stack trace pointer
?*builtin.StackTrace
The only possible non-
null
value for the stack trace parameter tobuiltin.default_panic
originates frombuiltin.panicUnwrapError
viadebug.panicExtra
.Usage in detail ...
@errorReturnTrace()
throughout the standard library; this is relevant because it reveals every potential origin of a non-null
stack trace parameter in a call tobuiltin.default_panic
. All instances are found in startup code (where errors thrown back to entry or thread-main are caught) wheredebug.dumpStackTrace
is called with the unwrapped, dereferenced return value.debug.dumpStackTrace
throughout the standard library is fromdebug.panicImpl
.debug.panicImpl
throughout the standard library. Three are exclusive to the Windows fault handler (which does not compile) and each call with?*builtin.StackTrace
asnull
. The only remaining caller isbuiltin.default_panic
.All remaining callers call with
null
for both valuesThe special handler functions will be removed by this patch, meaning that neither parameter will ever be defined at the interface with the compiler, except by panic cause
unwrapped_error
where it (the stack trace pointer) is part of the panic data and not an independent parameter.The excluded parameters only have the appearance of a purpose due to the standard library overusing
builtin.default_panic
. See commit 754ea11.Comptime-known panic cause
builtin.PanicCause
Resolves #18995
Two of the existing five special handler functions are generic, because they needed to be so that the feature would work. However for this interface there are significant costs if the interface is not generic.
If the panic cause is runtime-known:
Certain types of context data can not and will never allow reporting, either because the data size exceeds the maximum permitted by any reasonable common type, or because the type ID(s) relevant to the panic condition has unlimited variety and there is no possible common type (such as with pointers).
Multiple additional causes (where data type is the only variable, like
sentinel_mismatch_usize: usize
,sentinel_mismatch_isize: isize
in the suggested interface) would be required to support some of the current variety of panic data, and the logic required to determine which (sub-)causes should be used for which type(s) is complex.Every writer for every panic cause must be compiled regardless of whether it is used. This would mean that programs compiling
std.builtin.panicImpl
would includeErrol3
tables for the sole purpose of representing float sentinels. The ability to overridebuiltin.panic
does not excuse this requirement, because the user should not be forced to compile any (potentially unused) handler defined by their own implementation either.Some data types for some panic causes become substantially more expensive to report when a compromise type is used. The significance of this point is demonstrated by a later example.
A fixed interface is a downgrade for existing and future runtime safety features (1). It is more complex to maintain (2), comes at a greater upfront cost to the user (3), and is innately less efficient in the fail block for reliance on compromise types (4).
If the panic cause is compile-time-known, meaning the interface is generic:
The user pays for precisely what they use, regardless of whether
panic
is their definition or the standard library's.Users are able to define compile errors and other compile-time behaviour for specific panic causes.
Every panic data payload type for every panic cause is supported. This patch adds support for panic data for every panic cause with relevant data:
unwrapped_error
: Discarded error and the error return trace.accessed_out_of_bounds
: Out-of-bounds index and the upper bound.accessed_out_of_order
: Start and end operands.accessed_out_of_order_extra
: Start and end operands and the upper bound of the pointer operand.accessed_inactive_field
: Requested field and the active field.alias_of_noalias_parameter
: Destination and source addresses and the length.mismatched_memcpy_arguments
: Destination and source lengths.mismatched_for_loop_capture_lengths
: Expected length and the length of the first mismatch.mismatched_null_terminator
: Value of the element which should be null.mismatched_sentinel
: Expected value and the actual value.mul_overflowed
: LHS and RHS operands.add_overflowed
: LHS and RHS operands.sub_overflowed
: LHS and RHS operands.div_with_remainder
: LHS and RHS operands.shl_overflow
: Integer value and the shift amount.shr_overflow
: Integer value and the shift amount.shift_amt_overflowed
: Shift amount.cast_to_ptr_from_invalid
: Address.cast_to_int_from_invalid
: Float value.cast_truncated_data
: Integer value.cast_to_unsigned_from_negative
: Signed integer value.cast_to_enum_from_invalid
: Integer value.cast_to_error_from_invalid
: Integer value or error set element.Notes:
builtin.default_panic
is non-null
for errors unwrapped likeswitch (err) { error.A => @panic("message") }
. This functionality could be implemented in the new interface with an additional panic causeunwrapped_error_extra
, with an extra data field for the panic message. Whether this will be added to this patch is undecided. The current behaviour for this 'switch-prong-panic-unwrap' syntax is panic causemessage
like any other@panic
.usize
being used as the payload type for all unsigned integers (like the suggested sentinel data type) because it can accommodate most of them, with the exceptions being so infrequent that it is no great loss.@tagName
to convert all types of enumeration to strings and usingstruct { active: [:0]const u8, accessed: [:0]const u8 }
to report inactive field accesses for tagged unions.Panic examples from the compiler safety testsuite
Spoiler warning.
Slice syntax
This patch includes changes to slice semantic analysis. For this and future sections, the implementation offered by this patch will be referred to as
Sema.analyzeSlice2
, though in reality it will remain in the sub-namespace (Sema.RuntimeSafety
) while the pull request is in draft.This section is only intended to address changes which directly or indirectly contribute to correctness.
Differences compared to status quo
These differences do not directly contribute to the correctness of
Sema.analyzeSlice2
.1. Slice operation
slice_start
does not preserve sentinel value forMany
pointer operandsThis difference exists because
Sema.analyzeSlice2
ignores the values of sentinels for pointer operand types without explicit lengths, because (they mean different things) these can not participate in the same proofs as sentinels for pointer operand types with explicit lengths.The original behaviour can be restored if that is preferred. The cost of doing so is unknown.
2. Slicing
*T
uses the same core logic and error messages as every other pointer operandStatus quo allows slicing pointers-to-one like
*T
using theslice_end
syntax. This variant has a separate set of compile errors which are arguably more instructive.This implementation only provides special compile errors for slicing
*T
when start or end operands are runtime-known. This is done to adhere to the stricter standard (status quo), which requires that start and end operands are known at compile time. If runtime start and end operands were permitted the behaviour of slicing*T
would be indistinguishable from the behaviour of slicing*[1]T
.Special compile errors similar to status quo may be added if that is necessary. The cost of adding these special error messages would be high in relation to how often they would be seen.
Changes to slice behaviour
These changes result in differences in behaviour significant to these categories of outcome:
Added sentinel runtime safety check for compile-time-known pointer operands referencing runtime-known memory
Fixes #19792
Compile and run example program with
zig-runtime_safety run omit_sentinel_safety_check.zig
omit_sentinel_safety_check.zig
:Output:
This difference exists because
Sema.analyzeSlice
incorrectly assumes that a compile-time-check will always perform the test for this combination of inputs. Instead, the pointer load fails (because the memory is runtime-known) and there is no mechanism in place to try again at runtime.Corrected computation of upper bound used by runtime safety check
Fixes #19794
Compile and run example program with
zig-runtime_safety run underestimate_upper_bound.zig
underestimate_upper_bound.zig
:Output:
This will be explained in future comments or issues on slicing to consume sentinel.
Corrected computation of upper bound used by compile-time assertion
Fixes #19795
Compile example program with
zig-runtime_safety build-obj overestimate_upper_bound.zig
overestimate_upper_bound.zig
:Output:
This will be explained in future comments or issues on slicing to consume sentinel.
Substituted runtime crash with compile error for
slice_start
(with sentinel) for certain pointer operandsFixes #19796
Compile example program with
zig-runtime_safety build-obj slice_start_sentinel_always_out_of_bounds.zig
slice_start_sentinel_always_out_of_bounds.zig
:Output:
Added compile-time assertions for slices of reinterpreted memory for certain
comptime
pointer operandsCloses #19797
Compile example program with
zig-runtime_safety build-obj demo_various_unusable_results.zig
demo_various_unusable_results.zig
:Output:
The functions used to compute these lengths are currently:
Sema.RuntimeSafety.abiSizeOfContainingDecl
for element types with runtime bits.Sema.RuntimeSafety.reinterpretLengthOfContainingDecl
for element types without runtime bits.Reduced variety of outcome when slicing compile-time-known undefined pointers, etc.
Fixes #19798
less_various_behaviour_of_slice_of_const_undefined.zig
:This new behaviour reflects the third option presented in the linked issue. This was done to avoid adding the complexity of breakages (potentially at runtime in the case of the second option) to what is likely to be a time-consuming merge.
Implementation progress
Sema
panic_fn
,panic_unwrap_error
, andsafety_check_formatted
.panicUnwrapError
panicIndexOutOfBounds
panicInactiveUnionField
panicSentinelMismatch
safetyCheckFormatted
safetyPanic
analyzeSlice
zirReify
,zirArrayTypeSentinel
,zirPtrType
,analyzeSlice
.checkAccessInactiveUnionField
byunionFieldPtr
checkAccessInactiveUnionField
byunionFieldVal
checkAccessNullValue
byanalyzeOptionalPayloadPtr
checkAccessNullValue
byzirOptionalPayload
checkAccessNullValue
,checkAccessOutOfBounds
,checkAccessOutOfOrder
, andcheckMismatchedSentinel
byanalyzeSlice
checkAccessOutOfBounds
byelemPtrArray
checkAccessOutOfBounds
byelemPtrSlice
checkAccessOutOfBounds
byelemValArray
checkAccessOutOfBounds
byelemValSlice
checkAliasingMemcpyArguments
,checkMismatchedMemcpyArgumentLengths
, andanalyzeSlice2
byzirMemcpy
checkArithmeticOverflow
byaddDivIntOverflowSafety
checkArithmeticOverflow
byanalyzeArithmetic
checkArithmeticOverflow
byzirDivExact
checkCastToEnumFromInvalid
andpanicReachedUnreachable
byanalyzeSwitchRuntimeBlock
checkCastToEnumFromInvalid
byzirEnumFromInt
checkCastToEnumFromInvalid
byzirSwitchBlock
checkCastToEnumFromInvalid
byzirTagName
checkCastToErrorFromInvalid
byzirErrorCast
checkCastToErrorFromInvalid
byzirErrorFromInt
checkCastToIntFromInvalid
byzirIntFromFloat
checkCastToPointerFromInvalid
bycoerceCompatiblePtrs
checkCastToPointerFromInvalid
byptrCastFull
checkCastToPointerFromInvalid
byzirPtrFromInt
checkCastTruncatedData
andcheckCastToUnsignedFromNegative
byintCast
checkDivisionByZero
byaddDivByZeroSafety
checkMismatchedForLoopCaptureLengths
byzirForLen
checkShiftAmountOverflow
andcheckArithmeticOverflow
byzirShl
checkShiftAmountOverflow
andcheckArithmeticOverflow
byzirShr
checkUnwrappedError
byanalyzeErrUnionPayloadPtr
checkUnwrappedError
byanalyzeErrUnionPayload
panicReachedUnreachable
byBlock.addUnreachable
panicReachedUnreachable
byanalyzeCall
panicUnwrappedError
bymaybeErrorUnwrap
panicWithMsg
byzirPanic
analyzeSlice2
byzirSliceStart
,zirSliceEnd
,zirSliceSentinel
, andzirSliceLength
Sema.RuntimeSafety
check*
preparation functions.preparePanicCause
failBlock
preparePanicExtraType
callBuiltinHelper
wantPanicCause
resolveArithOverflowedPanicImpl
resolveArithOverflowedPanicCause
abiSizeOfContainingDecl
reinterpretLengthOfContainingDecl
panicWithMsg
panicReachedUnreachable
panicCastToEnumFromInvalid
panicUnwrappedError
checkAccessNullValue
checkUnwrappedError
checkDivisionByZero
checkAccessOutOfBounds
checkAccessOutOfOrder
checkAccessOutOfOrderExtra
checkAliasingMemcpyArguments
checkMismatchedMemcpyArgumentLengths
checkMismatchedForLoopCaptureLengths
checkAccessInactiveUnionField
checkMismatchedSentinel
checkMismatchedNullTerminator
checkShiftAmountOverflow
checkArithmeticOverflow
checkCastToEnumFromInvalid
checkCastToErrorFromInvalid
checkCastToPointerFromInvalid
checkCastToIntFromInvalid
checkCastTruncatedData
checkCastToUnsignedFromNegative
zirSliceStart
zirSliceEnd
zirSliceSentinel
zirSliceLength
analyzeSlice2
Module
c
panicNew
topanic2
.panicNew
function to replacepanic
.compiler_rt
panicNew
topanic2
.panicNew
function to replacepanic
.main
-f[no-]runtime-safety
and-f[no-]analyze-slice2
when testing is complete.-f[no-]runtime-safety
and-f[no-]analyze-slice2
.-funwrapped-error
-funwrapped-null
-freturned-noreturn
-freached-unreachable
-faccessed-out-of-bounds
-faccessed-out-of-order
-faccessed-inactive-field
-fdivided-by-zero
-fmemcpy-argument-aliasing
-fmismatched-memcpy-argument-lengths
-fmismatched-for-loop-capture-lengths
-fmismatched-sentinel
-fshift-amt-overflowed
-farith-overflowed
(
add_overflowed
,sub_overflowed
,inc_overflowed
,dec_overflowed
,div_overflowed
,div_with_remainder
)-fcast-truncated-data
-fcast-to-enum-from-invalid
-fcast-to-error-from-invalid
-fcast-to-pointer-from-invalid
-fcast-to-int-from-invalid
-fcast-to-unsigned-from-negative
main.RuntimeSafety
setAll
parseSafety
parseAllSafety
std.builtin
format
fromStackTrace
.panicNew
.default_panic
.std.debug
panicImpl
topanicImplDefault
, and replacedpanicImpl
with definition of formerbuiltin.default_panic
.panicExtra
to callpanicImpl
instead ofbuiltin.panic
.std.math.nextafter
std.multi_array_list
MultiArrayList.Slice.items
to avoid slice ofcomptime
-knownundefined
pointer with runtime-known end operand.test
Sema.analyzeSlice2
runtime panic variants.Sema.analyzeSlice2
compile error variants.Sema.analyzeSlice2
success variants.test.tests
panicNew
topanic2
.panicNew
to Compiler Explorer program.type
Type.allowSentinel
for determining whether an aggregate of typety
permits sentinels.