Skip to content

[OSLogOptimization] Improve SIL-level diagnostics generated for the os log APIs. #31120

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions include/swift/AST/DiagnosticsSIL.def
Original file line number Diff line number Diff line change
Expand Up @@ -580,14 +580,15 @@ NOTE(try_branch_doesnt_yield, none, "missing yield when error is "

// OS log optimization diagnostics.

ERROR(oslog_non_const_interpolation_options, none, "interpolation arguments "
"like format and privacy options must be constants", ())
ERROR(oslog_constant_eval_trap, none, "%0", (StringRef))

ERROR(oslog_const_evaluable_fun_error, none, "evaluation of constant-evaluable "
"function '%0' failed", (StringRef))
ERROR(oslog_too_many_instructions, none, "interpolated expression and arguments "
"are too complex", ())

ERROR(oslog_fail_stop_error, none, "constant evaluation of log call failed "
"with fatal error", ())
ERROR(oslog_invalid_log_message, none, "invalid log message; do not define "
"extensions to types defined in the os module", ())

NOTE(oslog_const_evaluable_fun_error, none, "'%0' failed evaluation", (StringRef))

ERROR(oslog_non_constant_message, none, "'OSLogMessage' instance passed to the "
"log call is not a constant", ())
Expand All @@ -598,8 +599,11 @@ ERROR(oslog_non_constant_interpolation, none, "'OSLogInterpolation' instance "
ERROR(oslog_property_not_constant, none, "'OSLogInterpolation.%0' is not a "
"constant", (StringRef))

ERROR(oslog_message_alive_after_opts, none, "OSLogMessage instance must not "
"be explicitly created and must be deletable", ())
ERROR(oslog_message_alive_after_opts, none, "os log string interpolation cannot "
"be used in this context", ())

ERROR(oslog_message_explicitly_created, none, "'OSLogMessage' must be "
" created from a string interpolation or string literal", ())

WARNING(oslog_call_in_unreachable_code, none, "os log call will never be "
"executed and may have undiagnosed errors", ())
Expand Down
177 changes: 118 additions & 59 deletions lib/SILOptimizer/Mandatory/OSLogOptimization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
// Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
Expand All @@ -11,16 +11,16 @@
//===----------------------------------------------------------------------===//
///
/// This pass implements SIL-level optimizations and diagnostics for the
/// os log APIs based on string interpolations. The APIs are implemented
/// in the files: OSLogMessage.swift, OSLog.swift. This pass constant evaluates
/// the log calls along with the auto-generated calls to the custom string
/// interpolation methods, which processes the string interpolation
/// os log APIs based on string interpolations. A mock version of the APIs
/// are available in the private module: OSLogTestHelper. This pass constant
/// evaluates the log calls along with the auto-generated calls to the custom
/// string interpolation methods, which processes the string interpolation
/// passed to the log calls, and folds the constants found during the
/// evaluation. The constants that are folded include the C format string that
/// is constructed by the custom string interpolation methods from the string
/// interpolation, and the size and headers of the byte buffer into which
/// arguments are packed. This pass is closely tied to the implementation of
/// the log APIs.
/// evaluation. The constants that are folded include the printf-style format
/// string that is constructed by the custom string interpolation methods from
/// the string interpolation, and the size and headers of the byte buffer into
/// which arguments are packed. This pass is closely tied to the implementation
/// of the log APIs.
///
/// Pass Dependencies: This pass depends on MandatoryInlining and Mandatory
/// Linking happening before this pass and ConstantPropagation happening after
Expand Down Expand Up @@ -51,7 +51,6 @@
/// The errors discovered here may arise from the implementation of the
/// log APIs in the overlay or could be because of wrong usage of the
/// log APIs.
/// TODO: these errors will be diagnosed by a separate, dedicated pass.
///
/// 4. The constant instructions that were found in step 2 are folded by
/// generating SIL code that produces the constants. This also removes
Expand Down Expand Up @@ -160,6 +159,8 @@ class StringSILInfo {
this->stringMetatype = stringMetatypeInst->getType();
}

bool isInitialized() { return stringInitIntrinsic != nullptr; }

SILFunction *getStringInitIntrinsic() const {
assert(stringInitIntrinsic);
return stringInitIntrinsic;
Expand Down Expand Up @@ -353,59 +354,70 @@ static bool isSILValueFoldable(SILValue value) {
isFoldableArray(value, astContext) || isFoldableClosure(value)));
}

/// Diagnose traps and instruction-limit exceeded errors. These have customized
/// error messages. \returns true if the given error is diagnosed. Otherwise,
/// returns false.
static bool diagnoseSpecialErrors(SILInstruction *unevaluableInst,
SymbolicValue errorInfo) {
SourceLoc sourceLoc = unevaluableInst->getLoc().getSourceLoc();
ASTContext &ctx = unevaluableInst->getFunction()->getASTContext();
UnknownReason unknownReason = errorInfo.getUnknownReason();

if (unknownReason.getKind() == UnknownReason::Trap) {
// We have an assertion failure or fatal error.
const char *message = unknownReason.getTrapMessage();
diagnose(ctx, sourceLoc, diag::oslog_constant_eval_trap,
StringRef(message));
return true;
}
if (unknownReason.getKind() == UnknownReason::TooManyInstructions) {
// This should not normally happen. But could be because of extensions
// defined by users, or very rarely due to unknown bugs in the os_log API
// implementation. These errors may get hidden during testing as it is input
// specific.
diagnose(ctx, sourceLoc, diag::oslog_too_many_instructions);
return true;
}
return false;
}

/// Diagnose failure during evaluation of a call to a constant-evaluable
/// function. Note that all auto-generated 'appendInterpolation' calls are
/// constant evaluable. This function detects and specially handles such
/// functions to present better diagnostic messages.
/// function that is not a specially-handled error. These are errors that
/// happen within 'appendInterpolation' calls, which must be constant
/// evaluable by the definition of APIs.
static void diagnoseErrorInConstantEvaluableFunction(ApplyInst *call,
SymbolicValue errorInfo) {
SILNode *unknownNode = errorInfo.getUnknownNode();
UnknownReason unknownReason = errorInfo.getUnknownReason();

SILFunction *callee = call->getCalleeFunction();
assert(callee);
SILLocation loc = call->getLoc();
SourceLoc sourceLoc = loc.getSourceLoc();
ASTContext &astContext = callee->getASTContext();

// Here, we know very little about what actually went wrong. It could be due
// to bugs in the library implementation or in extensions created by users.
// Emit a general message here and some diagnostic notes.
std::string demangledCalleeName = Demangle::demangleSymbolAsString(
callee->getName(),
Demangle::DemangleOptions::SimplifiedUIDemangleOptions());

// If an 'appendInterpolation' evaluation failed, it is probably due to
// invalid privacy or format specifiers. These are the only possible errors
// that the users of the log API could make. The rest are for library authors
// or users who extend the log APIs.
if (unknownReason.getKind() == UnknownReason::CallArgumentUnknown &&
dyn_cast<ApplyInst>(unknownNode) == call) {
if (StringRef(demangledCalleeName)
.contains(astContext.Id_appendInterpolation.str())) {
// TODO: extract and report the label of the parameter that is not a
// constant.
diagnose(astContext, sourceLoc,
diag::oslog_non_const_interpolation_options);
return;
}
}
diagnose(astContext, sourceLoc, diag::oslog_invalid_log_message);
diagnose(astContext, sourceLoc, diag::oslog_const_evaluable_fun_error,
demangledCalleeName);
errorInfo.emitUnknownDiagnosticNotes(loc);
return;
}

/// Detect and emit diagnostics for errors found during evaluation. Errors
/// can happen due to incorrect implementation of the os log API in the
/// overlay or due to incorrect use of the os log API.
/// TODO: errors due to incorrect use of the API should be diagnosed by a
/// dedicated diagnostics pass that will happen before this optimization starts.
/// can happen due to bugs in the implementation of the os log API, or
/// due to incorrect use of the os log API.
static bool detectAndDiagnoseErrors(SymbolicValue errorInfo,
SILInstruction *unevaluableInst) {
// TODO: fix the globalStrinTableBuiltin error after emitting diagnostics.
SILFunction *parentFun = unevaluableInst->getFunction();
ASTContext &astContext = parentFun->getASTContext();

// If evaluation of any other constant_evaluable function call fails, point
// to that failed function along with a reason: such as that a parameter is
// non-constant parameter or that body is not constant evaluable.
if (diagnoseSpecialErrors(unevaluableInst, errorInfo))
return true;
// If evaluation of any constant_evaluable function call fails, point
// to that failed function along with a reason.
ApplyInst *call = dyn_cast<ApplyInst>(unevaluableInst);
if (call) {
SILFunction *callee = call->getCalleeFunction();
Expand All @@ -414,17 +426,14 @@ static bool detectAndDiagnoseErrors(SymbolicValue errorInfo,
return true; // abort evaluation.
}
}

// Every other error must happen in the body of the os_log function which
// is inlined in the 'parentFun' before this pass. In this case, if we have a
// Every other error must happen in the top-level code containing the string
// interpolation construction and body of the log methods. If we have a
// fail-stop error, point to the error and abort evaluation. Otherwise, just
// ignore the error and continue evaluation as this error might not affect the
// constant value of the OSLogMessage instance.
if (isFailStopError(errorInfo)) {
assert(errorInfo.getKind() == SymbolicValue::Unknown);
SILLocation loc = unevaluableInst->getLoc();
SourceLoc sourceLoc = loc.getSourceLoc();
diagnose(astContext, sourceLoc, diag::oslog_fail_stop_error);
diagnose(astContext, loc.getSourceLoc(), diag::oslog_invalid_log_message);
errorInfo.emitUnknownDiagnosticNotes(loc);
return true;
}
Expand All @@ -444,7 +453,8 @@ static Optional<SymbolicValue> collectConstants(FoldState &foldState) {
auto &endInstructions = foldState.endInstructions;

// The loop will break when it sees a return instruction or an instruction in
// endInstructions.
// endInstructions or when the next instruction to evaluate cannot be
// determined (which may happend due to non-constant branches).
while (true) {
SILInstruction *currInst = &(*currI);
if (endInstructions.count(currInst))
Expand Down Expand Up @@ -1009,7 +1019,7 @@ static void substituteConstants(FoldState &foldState) {

/// Check whether OSLogMessage and OSLogInterpolation instances and all their
/// stored properties are constants. If not, it indicates errors that are due to
/// incorrect implementation of OSLogMessage either in the overlay or in the
/// incorrect implementation of OSLogMessage either in the os module or in the
/// extensions created by users. Detect and emit diagnostics for such errors.
/// The diagnostics here are for os log library authors.
static bool checkOSLogMessageIsConstant(SingleValueInstruction *osLogMessage,
Expand Down Expand Up @@ -1058,6 +1068,8 @@ static bool checkOSLogMessageIsConstant(SingleValueInstruction *osLogMessage,
osLogInterpolationValue.getAggregateMembers();
auto propValueI = propertyValues.begin();
bool errorDetected = false;
// Also, track if there is a string-valued property.
bool hasStringValuedProperty = false;

for (auto *propDecl : propertyDecls) {
SymbolicValue propertyValue = *(propValueI++);
Expand All @@ -1067,6 +1079,15 @@ static bool checkOSLogMessageIsConstant(SingleValueInstruction *osLogMessage,
errorDetected = true;
break;
}
hasStringValuedProperty = propertyValue.getKind() == SymbolicValue::String;
}

// If we have a string-valued property but don't have the stringInfo
// initialized here, it means the initializer OSLogInterpolation is explicitly
// called, which should be diagnosed.
if (hasStringValuedProperty && !foldState.stringInfo.isInitialized()) {
diagnose(astContext, sourceLoc, diag::oslog_message_explicitly_created);
errorDetected = true;
}
return errorDetected;
}
Expand Down Expand Up @@ -1185,9 +1206,7 @@ static void deleteInstructionWithUsersAndFixLifetimes(

/// Try to dead-code eliminate the OSLogMessage instance \c oslogMessage passed
/// to the os log call and clean up its dependencies. If the instance cannot be
/// eliminated, it implies that either the instance is not auto-generated or the
/// implementation of the os log overlay is incorrect. Therefore emit
/// diagnostics in such cases.
/// eliminated, emit diagnostics.
static void tryEliminateOSLogMessage(SingleValueInstruction *oslogMessage) {
InstructionDeleter deleter;
// List of instructions that are possibly dead.
Expand All @@ -1214,17 +1233,26 @@ static void tryEliminateOSLogMessage(SingleValueInstruction *oslogMessage) {
});
}
deleter.cleanUpDeadInstructions();
// If the OSLogMessage instance is not deleted, the overlay implementation
// (or its extensions by users) is incorrect.
// If the OSLogMessage instance is not deleted, either we couldn't see the
// body of the log call or there is a bug in the library implementation.
// Assuming that the library implementation is correct, it means that either
// OSLogMessage is used in a context where it is not supposed to be used, or
// we somehow saw a conditional branch with a non-constant argument before
// completing evaluation (this can happen with the os_log(_:log:type)
// overload, when log or type is an optional unwrapping). Report an error
// that covers both contexts. (Note that it is very hard to distinguish these
// error cases in the current state.)
if (!deletedInstructions.count(oslogMessage)) {
SILFunction *fun = oslogMessage->getFunction();
diagnose(fun->getASTContext(), oslogMessage->getLoc().getSourceLoc(),
diag::oslog_message_alive_after_opts);
}
}

/// Constant evaluate instructions starting from 'start' and fold the uses
/// of the value 'oslogMessage'. Stop when oslogMessageValue is released.
/// Constant evaluate instructions starting from \p start and fold the uses
/// of the SIL value \p oslogMessage.
/// \returns true if the body of the function containing \p oslogMessage is
/// modified. Returns false otherwise.
static bool constantFold(SILInstruction *start,
SingleValueInstruction *oslogMessage,
unsigned assertConfig) {
Expand Down Expand Up @@ -1394,6 +1422,30 @@ static SILInstruction *beginOfInterpolation(ApplyInst *oslogInit) {
return startInst;
}

/// Replace every _globalStringTablePointer builtin in the transitive users of
/// oslogMessage with an empty string literal. This would suppress the errors
/// emitted by a later pass on _globalStringTablePointerBuiltins. This utility
/// shoud be called only when this pass emits diagnostics.
static void
suppressGlobalStringTablePointerError(SingleValueInstruction *oslogMessage) {
SmallVector<SILInstruction *, 8> users;
getTransitiveUsers(oslogMessage, users);

for (SILInstruction *user : users) {
BuiltinInst *bi = dyn_cast<BuiltinInst>(user);
if (!bi ||
bi->getBuiltinInfo().ID != BuiltinValueKind::GlobalStringTablePointer)
continue;
// Replace this builtin by a string_literal instruction for an empty string.
SILBuilderWithScope builder(bi);
StringLiteralInst *stringLiteral = builder.createStringLiteral(
bi->getLoc(), StringRef(""), StringLiteralInst::Encoding::UTF8);
bi->replaceAllUsesWith(stringLiteral);
// Here, the bulitin instruction is dead, so clean it up.
eliminateDeadInstruction(bi);
}
}

/// If the SILInstruction is an initialization of OSLogMessage, return the
/// initialization call as an ApplyInst. Otherwise, return nullptr.
static ApplyInst *getAsOSLogMessageInit(SILInstruction *inst) {
Expand Down Expand Up @@ -1484,10 +1536,17 @@ class OSLogOptimization : public SILFunctionTransform {
// The log call is in unreachable code here.
continue;
}
madeChange |= constantFold(interpolationStart, oslogInit, assertConfig);
bool bodyModified =
constantFold(interpolationStart, oslogInit, assertConfig);
// If body was not modified, it implies that an error was diagnosed.
// However, this will also trigger a diagnostics later on since
// _globalStringTablePointerBuiltin would not be passed a string literal.
// Suppress this error by synthesizing a dummy string literal for the
// builtin.
if (!bodyModified)
suppressGlobalStringTablePointerError(oslogInit);
madeChange = true;
}

// TODO: Can we be more conservative here with our invalidation?
if (madeChange) {
invalidateAnalysis(SILAnalysis::InvalidationKind::FunctionBody);
}
Expand Down
12 changes: 4 additions & 8 deletions test/SILOptimizer/OSLogCompilerDiagnosticsTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,12 @@
// performs compile-time analysis and optimization of the new os log APIs.
// Note that many usage errors are caught by the Sema check: ConstantnessSemaDiagnostics.
// The tests here check only those diagnostics that are enforced at the SIL level.
// TODO: diagnostics must be improved, and globalStringTablePointer builtin error must be
// suppressed.

import OSLogTestHelper

func testNonDecimalFormatOptionOnIntegers() {
_osLogTestHelper("Minimum integer value: \(Int.min, format: .hex)")
// expected-error @-1 {{evaluation of constant-evaluable function 'OSLogInterpolation.appendInterpolation(_:format:align:privacy:)' failed}}
// expected-note @-2 {{Fatal error: Signed integers must be formatted using .decimal}}
// expected-error @-3 {{globalStringTablePointer builtin must used only on string literals}}
// expected-error @-1 {{Fatal error: Signed integers must be formatted using .decimal}}
}

// Extending OSLogInterpolation (without the constant_evaluable attribute) would be an
Expand All @@ -31,9 +27,9 @@ extension OSLogInterpolation {

func testOSLogInterpolationExtension(a: A) {
_osLogTestHelper("Error at line: \(a: a)")
// expected-error @-1 {{evaluation of constant-evaluable function 'OSLogInterpolation.appendLiteral(_:)' failed}}
// expected-note @-2 {{value mutable by an unevaluated instruction is not a constant}}
// expected-error @-3 {{globalStringTablePointer builtin must used only on string literals}}
// expected-error @-1 {{invalid log message; do not define extensions to types defined in the os module}}
// expected-note @-2 {{'OSLogInterpolation.appendLiteral(_:)' failed evaluation}}
// expected-note @-3 {{value mutable by an unevaluated instruction is not a constant}}
}

internal enum Color {
Expand Down