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

Prevent clobbering of data in Result's tail padding internally and externally #407

Merged
merged 9 commits into from
Nov 25, 2023
34 changes: 17 additions & 17 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,22 @@ jobs:
fail-fast: false
matrix:
config:
#- {
# name: "Windows MSVC",
# artifact: "Windows-MSVC.tar.xz",
# # MSVC 17.6.3 can no longer build Subspace. 16 doesn't work either.
# # https://developercommunity.visualstudio.com/t/Update-to-1763-now-rejects-valid-C-i/10394500
# os: windows-latest,
# build_type: "Release",
# cc: "cl",
# cxx: "cl",
# environment_script: "C:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Auxiliary/Build/vcvars64.bat",
# }
#- {
# name: "Windows MinGW", artifact: "Windows-MinGW.tar.xz",
# os: windows-latest,
# build_type: "Release", cc: "gcc", cxx: "g++"
# }
# MSVC is broken: https://github.com/chromium/subspace/issues/267
#- {
# name: "Windows MSVC",
# artifact: "Windows-MSVC.tar.xz",
# os: windows-latest,
# build_type: "Release",
# cc: "cl",
# cxx: "cl",
# environment_script: "C:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Auxiliary/Build/vcvars64.bat",
# }

#- {
# name: "Windows MinGW", artifact: "Windows-MinGW.tar.xz",
# os: windows-latest,
# build_type: "Release", cc: "gcc", cxx: "g++"
# }

- {
name: "Ubuntu Clang-16 Debug",
Expand Down Expand Up @@ -75,7 +75,7 @@ jobs:
build_type: "Release",
cc: "gcc-13",
cxx: "g++-13",
clang_version: 18,
clang_version: 17,
installed_clang_version: 14
}

Expand Down
154 changes: 77 additions & 77 deletions .github/workflows/try.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,83 +15,83 @@ jobs:
fail-fast: false
matrix:
config:
#- {
# name: "Windows MSVC",
# artifact: "Windows-MSVC.tar.xz",
# # MSVC 17.6.3 can no longer build Subspace. 16 doesn't work either.
# # https://developercommunity.visualstudio.com/t/Update-to-1763-now-rejects-valid-C-i/10394500
# os: windows-latest,
# build_type: "Release",
# cc: "cl",
# cxx: "cl",
# environment_script: "C:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Auxiliary/Build/vcvars64.bat",
# }
#- {
# name: "Windows MinGW", artifact: "Windows-MinGW.tar.xz",
# os: windows-latest,
# build_type: "Release", cc: "gcc", cxx: "g++"
# }

- {
name: "Ubuntu Clang-16 Debug",
os: ubuntu-latest,
build_type: "Debug",
cc: "clang-16",
cxx: "clang++-16",
clang_version: 16,
installed_clang_version: 14
}

- {
name: "Ubuntu Clang-17 Debug",
os: ubuntu-latest,
build_type: "Debug",
cc: "clang-17",
cxx: "clang++-17",
clang_version: 17,
installed_clang_version: 14
}

- {
name: "Ubuntu Clang-17 Sanitizer",
os: ubuntu-latest,
build_type: "Release",
cc: "clang-17",
cxx: "clang++-17",
clang_version: 17,
installed_clang_version: 14
}

- {
name: "Ubuntu Clang-18 Debug",
os: ubuntu-latest,
build_type: "Debug",
cc: "clang-18",
cxx: "clang++-18",
clang_version: 18,
installed_clang_version: 14
}

- {
name: "Ubuntu GCC",
os: ubuntu-latest,
build_type: "Release",
cc: "gcc-13",
cxx: "g++-13",
clang_version: 18,
installed_clang_version: 14
}

# Clang doesn't have good enough C++20 support to compile this library
# yet.
#- {
# name: "macOS Latest Clang",
# artifact: "macOS.tar.xz",
# os: macos-latest,
# build_type: "Release",
# cc: "clang",
# cxx: "clang++",
# }
# MSVC is broken: https://github.com/chromium/subspace/issues/267
#- {
# name: "Windows MSVC",
# artifact: "Windows-MSVC.tar.xz",
# os: windows-latest,
# build_type: "Release",
# cc: "cl",
# cxx: "cl",
# environment_script: "C:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Auxiliary/Build/vcvars64.bat",
# }

#- {
# name: "Windows MinGW", artifact: "Windows-MinGW.tar.xz",
# os: windows-latest,
# build_type: "Release", cc: "gcc", cxx: "g++"
# }

- {
name: "Ubuntu Clang-16 Debug",
os: ubuntu-latest,
build_type: "Debug",
cc: "clang-16",
cxx: "clang++-16",
clang_version: 16,
installed_clang_version: 14
}

- {
name: "Ubuntu Clang-17 Debug",
os: ubuntu-latest,
build_type: "Debug",
cc: "clang-17",
cxx: "clang++-17",
clang_version: 17,
installed_clang_version: 14
}

- {
name: "Ubuntu Clang-17 Sanitizer",
os: ubuntu-latest,
build_type: "Release",
cc: "clang-17",
cxx: "clang++-17",
clang_version: 17,
installed_clang_version: 14
}

- {
name: "Ubuntu Clang-18 Debug",
os: ubuntu-latest,
build_type: "Debug",
cc: "clang-18",
cxx: "clang++-18",
clang_version: 18,
installed_clang_version: 14
}

- {
name: "Ubuntu GCC",
os: ubuntu-latest,
build_type: "Release",
cc: "gcc-13",
cxx: "g++-13",
clang_version: 17,
installed_clang_version: 14
}

# Clang doesn't have good enough C++20 support to compile this library
# yet.
#- {
# name: "macOS Latest Clang",
# artifact: "macOS.tar.xz",
# os: macos-latest,
# build_type: "Release",
# cc: "clang",
# cxx: "clang++",
# }

steps:
- uses: actions/checkout@v3
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ check out which compiler versions are used by the
|------------|---------|
| **Clang:** | 16 and up |
| **GCC**: | 13 and up |
| **MSVC**: | 17.4.2 (not [newer versions](https://github.com/chromium/subspace/issues/267)) |
| **MSVC**: | VS2022 17.8.1 (Build 17.8.34316.72) and up |

We attempt to work around bugs when reasonable, to widen compiler version
support. See [compiler_bugs.h](sus/macros/__private/compiler_bugs.h) for
Expand Down
23 changes: 17 additions & 6 deletions subdoc/lib/path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,28 @@ bool path_contains_namespace(clang::Decl* decl, Namespace n) noexcept {
}

bool path_is_private(clang::NamedDecl* decl) noexcept {
// clang::Linkage members were renamed in 18.
#if CLANG_VERSION_MAJOR >= 18
constexpr auto ModuleLinkage = clang::Linkage::Module;
constexpr auto ExternalLinkage = clang::Linkage::External;
constexpr auto NoneLinkage = clang::Linkage::None;
#else
constexpr auto ModuleLinkage = clang::Linkage::ModuleLinkage;
constexpr auto ExternalLinkage = clang::Linkage::ExternalLinkage;
constexpr auto NoneLinkage = clang::Linkage::NoLinkage;
#endif

clang::Linkage linkage = decl->getLinkageInternal();
if (linkage != clang::Linkage::ModuleLinkage &&
linkage != clang::Linkage::ExternalLinkage) {
// NoLinkage describes itself as "can only be referred to from within its
// scope".
if (linkage != ModuleLinkage &&
linkage != ExternalLinkage) {
// Linkage::None describes itself as "can only be referred to from within
// its scope".
//
// However `namespace a { using b::S; }` brings S into `a` in a way that is
// usable publicly from other scopes. So we accept `NoLinkage` for
// usable publicly from other scopes. So we accept `Linkage::None` for
// `UsingDecl` and `UsingEnumDecl` (aka `BaseUsingDecl`). Similar for
// type aliases.
if (!(linkage == clang::Linkage::NoLinkage &&
if (!(linkage == NoneLinkage &&
(clang::isa<clang::BaseUsingDecl>(decl) ||
clang::isa<clang::TypedefNameDecl>(decl)))) {
return true;
Expand Down
5 changes: 2 additions & 3 deletions sus/collections/array.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,8 @@ namespace __private {

template <class T, size_t N>
struct Storage final {
sus_msvc_bug_10416202_else(
[[sus_no_unique_address]]) sus::iter::IterRefCounter iter_refs_;
T data_[N];
[[sus_no_unique_address]] sus::iter::IterRefCounter iter_refs_;
[[sus_no_unique_address]] T data_[N];
};

template <class T>
Expand Down
4 changes: 1 addition & 3 deletions sus/collections/invalidation_off_size_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ using sus::Slice;

constexpr usize array_padding = 0u
// No support for no_unique_address in clang-cl.
sus_clang_bug_49358(+sizeof(i32))
// MSVC bug with no_unique_address and arrays, so not used here.
sus_clang_bug_49358_else(sus_msvc_bug_10416202(+sizeof(i32)));
sus_clang_bug_49358(+sizeof(i32));

static_assert(alignof(Array<i32, 5>) == alignof(i32));
static_assert(sizeof(Array<i32, 5>) == sizeof(i32) * 5 + array_padding);
Expand Down
2 changes: 1 addition & 1 deletion sus/env/var.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ struct VarError {

/// This struct acts as a proxy for the Reason enum, so it can be implicitly
/// constructed from the reason value.
VarError(Reason r) : reason(r) {}
constexpr VarError(Reason r) : reason(r) {}

Reason reason;

Expand Down
3 changes: 1 addition & 2 deletions sus/macros/__private/compiler_bugs.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,7 @@
// TODO: https://github.com/llvm/llvm-project/issues/49358
// Clang-cl doesn't support either [[no_unique_address]] nor
// [[msvc::no_unique_address]]
#if _MSC_VER && defined(__clang__) && \
__clang_major__ > 0 // TODO: Update when the bug is fixed.
#if _MSC_VER && defined(__clang__) && __clang_major__ <= 17
#define sus_clang_bug_49358(...) __VA_ARGS__
#define sus_clang_bug_49358_else(...)
#else
Expand Down
6 changes: 6 additions & 0 deletions sus/mem/copy.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,10 @@ concept TrivialCopy =
template <class T>
concept CopyOrRef = Copy<T> || std::is_reference_v<T>;

/// Matches types which are [`CopyOrRef`]($sus::mem::CopyOrRef) or are `void`.
///
/// A helper for genertic types which can hold void as a type.
template <class T>
concept CopyOrRefOrVoid = CopyOrRef<T> || std::is_void_v<T>;

} // namespace sus::mem
6 changes: 6 additions & 0 deletions sus/mem/move.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ concept Move =
template <class T>
concept MoveOrRef = Move<T> || std::is_reference_v<T>;

/// Matches types which are [`MoveOrRef`]($sus::mem::MoveOrRef) or are `void`.
///
/// A helper for genertic types which can hold void as a type.
template <class T>
concept MoveOrRefOrVoid = MoveOrRef<T> || std::is_void_v<T>;

/// Cast `t` to an r-value reference so that it can be used to construct or be
/// assigned to a (non-reference) object of type `T`.
///
Expand Down
5 changes: 5 additions & 0 deletions sus/option/__private/storage.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,11 @@ struct Storage<T, false> final {

private:
union {
// TODO: We can make this T [[no_unique_address]], however then
// we can not construct_at on it. Instead, we would need to only
// construct_at the Storage class, and have the ctor set them both.
//
// See also https://github.com/llvm/llvm-project/issues/70494
T val_;
};
State state_ = None;
Expand Down
4 changes: 4 additions & 0 deletions sus/option/option.h
Original file line number Diff line number Diff line change
Expand Up @@ -1991,6 +1991,10 @@ class Option final {
using StorageType =
std::conditional_t<std::is_reference_v<U>, Storage<StoragePointer<U>>,
Storage<U>>;
// TODO: We can make this no_unique_address, however... if StorageType<T> has
// tail padding and Option was marked with no_unique_address, then
// constructing T may clobber stuff OUTSIDE the Option. So this can only be
// no_unique_address when StorageType<T> has no tail padding.
StorageType<T> t_;

sus_class_trivially_relocatable_if_types(::sus::marker::unsafe_fn,
Expand Down
Loading
Loading