Skip to content
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

Apply audit mode to TerminalInput/Adapter/Parser libraries #4005

Merged
28 commits merged into from
Jan 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
7461678
Adjust the public methods to the state machine to use more modern con…
miniksa Dec 12, 2019
169bfb3
State Machine, Output State Machine Engine, and Tracing classes use r…
miniksa Dec 13, 2019
64d1496
InputStateMachine and associated fallout
miniksa Dec 13, 2019
fee4b54
Dropping hungarians, using unique_ptr constructors, BOOL to bool, usi…
miniksa Dec 16, 2019
dcf0e8f
Fix logical conversion errors discovered by running test suite.
miniksa Dec 16, 2019
d97a4da
Remove many but probably not all of the std::wstring intermediates be…
miniksa Dec 16, 2019
68bc729
found the other one.
miniksa Dec 17, 2019
e73d516
Fix a few things I found in self-review.
miniksa Dec 17, 2019
3a92383
Merge branch 'master' into dev/miniksa/gardening3
miniksa Dec 17, 2019
a2c234a
Merge branch 'master' into dev/miniksa/gardening3
miniksa Dec 17, 2019
576da99
code formatting run.
miniksa Dec 17, 2019
1616801
TerminalParser is now audit clean. Noexcepts propagated. Removed C-ca…
miniksa Dec 17, 2019
e769ad3
TerminalInput is now audited too. Most of it was moving the tables to…
miniksa Dec 17, 2019
608e10c
Enable audit on TerminalAdapter too.
miniksa Dec 17, 2019
f78f328
Oops, setting noexcept had effects in the children. Propagate as appr…
miniksa Dec 18, 2019
cfca853
Merge branch 'master' into dev/miniksa/gardening3_audit
miniksa Dec 31, 2019
9d8da4f
Add TIL library for header-based utilities we need to manage audit mo…
miniksa Jan 2, 2020
fc49c35
Comments from Dustin about references, function level try blocks, and…
miniksa Jan 2, 2020
fc05775
Drop constexpr suppressions and just make them static constexprs in e…
miniksa Jan 2, 2020
d28c8c1
Revert adaptDispatch.cpp
miniksa Jan 2, 2020
8853d1a
Restore overzealous revert.
miniksa Jan 2, 2020
796aea3
Correct SA failures but still using safemath. Ensure unit tests still…
miniksa Jan 2, 2020
5cc2943
suppress warning on clearly incorrect c26496 detection.
miniksa Jan 2, 2020
fdd4dcc
fix the code format
miniksa Jan 2, 2020
8a82b2c
tinker with this some more to avoid x86 signed/unsigned mismatch.
miniksa Jan 2, 2020
a009ef1
a few more comments on til::at, safe math, and a missed rename opport…
miniksa Jan 2, 2020
916a9de
Merge branch 'master' into dev/miniksa/gardening3_audit
miniksa Jan 2, 2020
e4115ee
Merge branch 'master' into dev/miniksa/gardening3_audit
miniksa Jan 2, 2020
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
11 changes: 6 additions & 5 deletions OpenConsole.sln
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,8 @@ Global
{2FD12FBB-1DDB-46D8-B818-1023C624CACA}.Release|x86.Build.0 = Release|Win32
{3AE13314-1939-4DFA-9C14-38CA0834050C}.AuditMode|Any CPU.ActiveCfg = AuditMode|Win32
{3AE13314-1939-4DFA-9C14-38CA0834050C}.AuditMode|ARM64.ActiveCfg = Release|ARM64
{3AE13314-1939-4DFA-9C14-38CA0834050C}.AuditMode|x64.ActiveCfg = Release|x64
{3AE13314-1939-4DFA-9C14-38CA0834050C}.AuditMode|x64.Build.0 = Release|x64
{3AE13314-1939-4DFA-9C14-38CA0834050C}.AuditMode|x64.ActiveCfg = AuditMode|x64
{3AE13314-1939-4DFA-9C14-38CA0834050C}.AuditMode|x64.Build.0 = AuditMode|x64
{3AE13314-1939-4DFA-9C14-38CA0834050C}.AuditMode|x86.ActiveCfg = Release|Win32
{3AE13314-1939-4DFA-9C14-38CA0834050C}.Debug|Any CPU.ActiveCfg = Debug|Win32
{3AE13314-1939-4DFA-9C14-38CA0834050C}.Debug|ARM64.ActiveCfg = Debug|ARM64
Expand All @@ -372,7 +372,8 @@ Global
{3AE13314-1939-4DFA-9C14-38CA0834050C}.Release|x86.Build.0 = Release|Win32
{DCF55140-EF6A-4736-A403-957E4F7430BB}.AuditMode|Any CPU.ActiveCfg = AuditMode|Win32
{DCF55140-EF6A-4736-A403-957E4F7430BB}.AuditMode|ARM64.ActiveCfg = Release|ARM64
{DCF55140-EF6A-4736-A403-957E4F7430BB}.AuditMode|x64.ActiveCfg = Release|x64
{DCF55140-EF6A-4736-A403-957E4F7430BB}.AuditMode|x64.ActiveCfg = AuditMode|x64
{DCF55140-EF6A-4736-A403-957E4F7430BB}.AuditMode|x64.Build.0 = AuditMode|x64
{DCF55140-EF6A-4736-A403-957E4F7430BB}.AuditMode|x86.ActiveCfg = Release|Win32
{DCF55140-EF6A-4736-A403-957E4F7430BB}.Debug|Any CPU.ActiveCfg = Debug|Win32
{DCF55140-EF6A-4736-A403-957E4F7430BB}.Debug|ARM64.ActiveCfg = Debug|ARM64
Expand All @@ -390,8 +391,8 @@ Global
{DCF55140-EF6A-4736-A403-957E4F7430BB}.Release|x86.Build.0 = Release|Win32
{1CF55140-EF6A-4736-A403-957E4F7430BB}.AuditMode|Any CPU.ActiveCfg = AuditMode|Win32
{1CF55140-EF6A-4736-A403-957E4F7430BB}.AuditMode|ARM64.ActiveCfg = Release|ARM64
{1CF55140-EF6A-4736-A403-957E4F7430BB}.AuditMode|x64.ActiveCfg = Release|x64
{1CF55140-EF6A-4736-A403-957E4F7430BB}.AuditMode|x64.Build.0 = Release|x64
{1CF55140-EF6A-4736-A403-957E4F7430BB}.AuditMode|x64.ActiveCfg = AuditMode|x64
{1CF55140-EF6A-4736-A403-957E4F7430BB}.AuditMode|x64.Build.0 = AuditMode|x64
{1CF55140-EF6A-4736-A403-957E4F7430BB}.AuditMode|x86.ActiveCfg = Release|Win32
{1CF55140-EF6A-4736-A403-957E4F7430BB}.Debug|Any CPU.ActiveCfg = Debug|Win32
{1CF55140-EF6A-4736-A403-957E4F7430BB}.Debug|ARM64.ActiveCfg = Debug|ARM64
Expand Down
28 changes: 14 additions & 14 deletions src/cascadia/TerminalCore/TerminalDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ void TerminalDispatch::PrintString(const std::wstring_view string)
}

bool TerminalDispatch::CursorPosition(const size_t line,
const size_t column)
const size_t column) noexcept
{
const auto columnInBufferSpace = column - 1;
const auto lineInBufferSpace = line - 1;
Expand All @@ -40,33 +40,33 @@ bool TerminalDispatch::CursorPosition(const size_t line,
return _terminalApi.SetCursorPosition(x, y);
}

bool TerminalDispatch::CursorForward(const size_t distance)
bool TerminalDispatch::CursorForward(const size_t distance) noexcept
{
const auto cursorPos = _terminalApi.GetCursorPosition();
const COORD newCursorPos{ cursorPos.X + gsl::narrow<short>(distance), cursorPos.Y };
return _terminalApi.SetCursorPosition(newCursorPos.X, newCursorPos.Y);
}

bool TerminalDispatch::CursorBackward(const size_t distance)
bool TerminalDispatch::CursorBackward(const size_t distance) noexcept
{
const auto cursorPos = _terminalApi.GetCursorPosition();
const COORD newCursorPos{ cursorPos.X - gsl::narrow<short>(distance), cursorPos.Y };
return _terminalApi.SetCursorPosition(newCursorPos.X, newCursorPos.Y);
}

bool TerminalDispatch::CursorUp(const size_t distance)
bool TerminalDispatch::CursorUp(const size_t distance) noexcept
{
const auto cursorPos = _terminalApi.GetCursorPosition();
const COORD newCursorPos{ cursorPos.X, cursorPos.Y + gsl::narrow<short>(distance) };
return _terminalApi.SetCursorPosition(newCursorPos.X, newCursorPos.Y);
}

bool TerminalDispatch::EraseCharacters(const size_t numChars)
bool TerminalDispatch::EraseCharacters(const size_t numChars) noexcept
{
return _terminalApi.EraseCharacters(numChars);
}

bool TerminalDispatch::SetWindowTitle(std::wstring_view title)
bool TerminalDispatch::SetWindowTitle(std::wstring_view title) noexcept
{
return _terminalApi.SetWindowTitle(title);
}
Expand All @@ -79,12 +79,12 @@ bool TerminalDispatch::SetWindowTitle(std::wstring_view title)
// Return Value:
// True if handled successfully. False otherwise.
bool TerminalDispatch::SetColorTableEntry(const size_t tableIndex,
const DWORD color)
const DWORD color) noexcept
{
return _terminalApi.SetColorTableEntry(tableIndex, color);
}

bool TerminalDispatch::SetCursorStyle(const DispatchTypes::CursorStyle cursorStyle)
bool TerminalDispatch::SetCursorStyle(const DispatchTypes::CursorStyle cursorStyle) noexcept
{
return _terminalApi.SetCursorStyle(cursorStyle);
}
Expand All @@ -95,7 +95,7 @@ bool TerminalDispatch::SetCursorStyle(const DispatchTypes::CursorStyle cursorSty
// - color: The new RGB color value to use, in 0x00BBGGRR form
// Return Value:
// True if handled successfully. False otherwise.
bool TerminalDispatch::SetDefaultForeground(const DWORD color)
bool TerminalDispatch::SetDefaultForeground(const DWORD color) noexcept
{
return _terminalApi.SetDefaultForeground(color);
}
Expand All @@ -106,7 +106,7 @@ bool TerminalDispatch::SetDefaultForeground(const DWORD color)
// - color: The new RGB color value to use, in 0x00BBGGRR form
// Return Value:
// True if handled successfully. False otherwise.
bool TerminalDispatch::SetDefaultBackground(const DWORD color)
bool TerminalDispatch::SetDefaultBackground(const DWORD color) noexcept
{
return _terminalApi.SetDefaultBackground(color);
}
Expand All @@ -117,7 +117,7 @@ bool TerminalDispatch::SetDefaultBackground(const DWORD color)
// - eraseType: the erase type (from beginning, to end, or all)
// Return Value:
// True if handled successfully. False otherwise.
bool TerminalDispatch::EraseInLine(const DispatchTypes::EraseType eraseType)
bool TerminalDispatch::EraseInLine(const DispatchTypes::EraseType eraseType) noexcept
{
return _terminalApi.EraseInLine(eraseType);
}
Expand All @@ -128,7 +128,7 @@ bool TerminalDispatch::EraseInLine(const DispatchTypes::EraseType eraseType)
// - count, the number of characters to delete
// Return Value:
// True if handled successfully. False otherwise.
bool TerminalDispatch::DeleteCharacter(const size_t count)
bool TerminalDispatch::DeleteCharacter(const size_t count) noexcept
{
return _terminalApi.DeleteCharacter(count);
}
Expand All @@ -139,7 +139,7 @@ bool TerminalDispatch::DeleteCharacter(const size_t count)
// - count, the number of spaces to add
// Return Value:
// True if handled successfully, false otherwise
bool TerminalDispatch::InsertCharacter(const size_t count)
bool TerminalDispatch::InsertCharacter(const size_t count) noexcept
{
return _terminalApi.InsertCharacter(count);
}
Expand All @@ -150,7 +150,7 @@ bool TerminalDispatch::InsertCharacter(const size_t count)
// - eraseType: the desired erase type
// Return Value:
// True if handled successfully. False otherwise
bool TerminalDispatch::EraseInDisplay(const DispatchTypes::EraseType eraseType)
bool TerminalDispatch::EraseInDisplay(const DispatchTypes::EraseType eraseType) noexcept
{
return _terminalApi.EraseInDisplay(eraseType);
}
30 changes: 15 additions & 15 deletions src/cascadia/TerminalCore/TerminalDispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,27 @@ class TerminalDispatch : public Microsoft::Console::VirtualTerminal::TermDispatc
virtual void Print(const wchar_t wchPrintable) override;
virtual void PrintString(const std::wstring_view string) override;

bool SetGraphicsRendition(const std::basic_string_view<::Microsoft::Console::VirtualTerminal::DispatchTypes::GraphicsOptions> options) override;
bool SetGraphicsRendition(const std::basic_string_view<::Microsoft::Console::VirtualTerminal::DispatchTypes::GraphicsOptions> options) noexcept override;

virtual bool CursorPosition(const size_t line,
const size_t column) override; // CUP
const size_t column) noexcept override; // CUP

bool CursorForward(const size_t distance) override;
bool CursorBackward(const size_t distance) override;
bool CursorUp(const size_t distance) override;
bool CursorForward(const size_t distance) noexcept override;
bool CursorBackward(const size_t distance) noexcept override;
bool CursorUp(const size_t distance) noexcept override;

bool EraseCharacters(const size_t numChars) override;
bool SetWindowTitle(std::wstring_view title) override;
bool EraseCharacters(const size_t numChars) noexcept override;
bool SetWindowTitle(std::wstring_view title) noexcept override;

bool SetColorTableEntry(const size_t tableIndex, const DWORD color) override;
bool SetCursorStyle(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::CursorStyle cursorStyle) override;
bool SetColorTableEntry(const size_t tableIndex, const DWORD color) noexcept override;
bool SetCursorStyle(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::CursorStyle cursorStyle) noexcept override;

bool SetDefaultForeground(const DWORD color) override;
bool SetDefaultBackground(const DWORD color) override;
bool EraseInLine(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::EraseType eraseType) override; // ED
bool DeleteCharacter(const size_t count) override;
bool InsertCharacter(const size_t count) override;
bool EraseInDisplay(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::EraseType eraseType) override;
bool SetDefaultForeground(const DWORD color) noexcept override;
bool SetDefaultBackground(const DWORD color) noexcept override;
bool EraseInLine(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::EraseType eraseType) noexcept override; // ED
bool DeleteCharacter(const size_t count) noexcept override;
bool InsertCharacter(const size_t count) noexcept override;
bool EraseInDisplay(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::EraseType eraseType) noexcept override;

private:
::Microsoft::Terminal::Core::ITerminalApi& _terminalApi;
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ void TerminalDispatch::_SetGraphicsOptionHelper(const DispatchTypes::GraphicsOpt
}
}

bool TerminalDispatch::SetGraphicsRendition(const std::basic_string_view<DispatchTypes::GraphicsOptions> options)
bool TerminalDispatch::SetGraphicsRendition(const std::basic_string_view<DispatchTypes::GraphicsOptions> options) noexcept
{
bool success = false;
// Run through the graphics options and apply them
Expand Down
3 changes: 3 additions & 0 deletions src/inc/ITerminalOutputConnection.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ namespace Microsoft::Console
class ITerminalOutputConnection
{
public:
#pragma warning(push)
#pragma warning(disable : 26432) // suppress rule of 5 violation on interface because tampering with this is fraught with peril
virtual ~ITerminalOutputConnection() = 0;

[[nodiscard]] virtual HRESULT WriteTerminalUtf8(const std::string_view str) = 0;
[[nodiscard]] virtual HRESULT WriteTerminalW(const std::wstring_view wstr) = 0;
};

inline Microsoft::Console::ITerminalOutputConnection::~ITerminalOutputConnection() {}
#pragma warning(pop)
}
3 changes: 3 additions & 0 deletions src/inc/LibraryIncludes.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@
// WRL
#include <wrl.h>

// TIL - Terminal Implementation Library
#include "til.h"

#pragma warning(pop)

// clang-format on
45 changes: 45 additions & 0 deletions src/inc/til.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

#pragma once

namespace til // Terminal Implementation Library. Also: "Today I Learned"
{
// The at function declares that you've already sufficiently checked that your array access
// is in range before retrieving an item inside it at an offset.
// This is to save double/triple/quadruple testing in circumstances where you are already
// pivoting on the length of a set and now want to pull elements out of it by offset
// without checking again.
// gsl::at will do the check again. As will .at(). And using [] will have a warning in audit.
template<class T>
constexpr auto at(T& cont, const size_t i) -> decltype(cont[cont.size()])
{
#pragma warning(suppress : 26482) // Suppress bounds.2 check for indexing with constant expressions
#pragma warning(suppress : 26446) // Suppress bounds.4 check for subscript operator.
return cont[i];
}
}

// These sit outside the namespace because they sit outside for WIL too.

// Inspired from RETURN_IF_WIN32_BOOL_FALSE
// WIL doesn't include a RETURN_BOOL_IF_FALSE, and RETURN_IF_WIN32_BOOL_FALSE
// will actually return the value of GLE.
#define RETURN_BOOL_IF_FALSE(b) \
do \
{ \
const bool __boolRet = wil::verify_bool(b); \
if (!__boolRet) \
{ \
return __boolRet; \
} \
} while (0, 0)

// Due to a bug (DevDiv 441931), Warning 4297 (function marked noexcept throws exception) is detected even when the throwing code is unreachable, such as the end of scope after a return, in function-level catch.
#define CATCH_LOG_RETURN_FALSE() \
catch (...) \
{ \
__pragma(warning(suppress : 4297)); \
LOG_CAUGHT_EXCEPTION(); \
return false; \
}
24 changes: 12 additions & 12 deletions src/terminal/adapter/DispatchCommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,17 @@ bool DispatchCommon::s_ResizeWindow(ConGetSet& conApi,
SHORT sRows = 0;

// We should do nothing if 0 is passed in for a size.
bool fSuccess = SUCCEEDED(SizeTToShort(width, &sColumns)) &&
SUCCEEDED(SizeTToShort(height, &sRows)) &&
(width > 0 && height > 0);
bool success = SUCCEEDED(SizeTToShort(width, &sColumns)) &&
SUCCEEDED(SizeTToShort(height, &sRows)) &&
(width > 0 && height > 0);

if (fSuccess)
if (success)
{
CONSOLE_SCREEN_BUFFER_INFOEX csbiex = { 0 };
csbiex.cbSize = sizeof(CONSOLE_SCREEN_BUFFER_INFOEX);
fSuccess = !!conApi.GetConsoleScreenBufferInfoEx(csbiex);
success = conApi.GetConsoleScreenBufferInfoEx(csbiex);

if (fSuccess)
if (success)
{
const Viewport oldViewport = Viewport::FromInclusive(csbiex.srWindow);
const Viewport newViewport = Viewport::FromDimensions(oldViewport.Origin(),
Expand All @@ -51,20 +51,20 @@ bool DispatchCommon::s_ResizeWindow(ConGetSet& conApi,
}

// SetConsoleWindowInfo expect inclusive rects
SMALL_RECT sri = newViewport.ToInclusive();
const auto sri = newViewport.ToInclusive();

// SetConsoleScreenBufferInfoEx however expects exclusive rects
SMALL_RECT sre = newViewport.ToExclusive();
const auto sre = newViewport.ToExclusive();
csbiex.srWindow = sre;

fSuccess = conApi.SetConsoleScreenBufferInfoEx(csbiex);
if (fSuccess)
success = conApi.SetConsoleScreenBufferInfoEx(csbiex);
if (success)
{
fSuccess = conApi.SetConsoleWindowInfo(true, sri);
success = conApi.SetConsoleWindowInfo(true, sri);
}
}
}
return fSuccess;
return success;
}

// Routine Description:
Expand Down
3 changes: 3 additions & 0 deletions src/terminal/adapter/IInteractDispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ namespace Microsoft::Console::VirtualTerminal
class IInteractDispatch
{
public:
#pragma warning(push)
#pragma warning(disable : 26432) // suppress rule of 5 violation on interface because tampering with this is fraught with peril
virtual ~IInteractDispatch() = default;
#pragma warning(pop)

virtual bool WriteInput(std::deque<std::unique_ptr<IInputEvent>>& inputEvents) = 0;

Expand Down
4 changes: 4 additions & 0 deletions src/terminal/adapter/ITermDispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ namespace Microsoft::Console::VirtualTerminal
class Microsoft::Console::VirtualTerminal::ITermDispatch
{
public:
#pragma warning(push)
#pragma warning(disable : 26432) // suppress rule of 5 violation on interface because tampering with this is fraught with peril
virtual ~ITermDispatch() = 0;

virtual void Execute(const wchar_t wchControl) = 0;
virtual void Print(const wchar_t wchPrintable) = 0;
virtual void PrintString(const std::wstring_view string) = 0;
Expand Down Expand Up @@ -97,3 +100,4 @@ class Microsoft::Console::VirtualTerminal::ITermDispatch
const std::basic_string_view<size_t> parameters) = 0;
};
inline Microsoft::Console::VirtualTerminal::ITermDispatch::~ITermDispatch() {}
#pragma warning(pop)
2 changes: 1 addition & 1 deletion src/terminal/adapter/InteractDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ bool InteractDispatch::WindowManipulation(const DispatchTypes::WindowManipulatio
// the ConGetSet interface, that specifically handles a conpty resize.
if (parameters.size() == 2)
{
success = DispatchCommon::s_ResizeWindow(*_pConApi, parameters[1], parameters[0]);
success = DispatchCommon::s_ResizeWindow(*_pConApi, til::at(parameters, 1), til::at(parameters, 0));
if (success)
{
DispatchCommon::s_SuppressResizeRepaint(*_pConApi);
Expand Down
2 changes: 0 additions & 2 deletions src/terminal/adapter/InteractDispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ namespace Microsoft::Console::VirtualTerminal
public:
InteractDispatch(std::unique_ptr<ConGetSet> pConApi);

~InteractDispatch() = default;

bool WriteInput(std::deque<std::unique_ptr<IInputEvent>>& inputEvents) override;
bool WriteCtrlC() override;
bool WriteString(const std::wstring_view string) override;
Expand Down
Loading