diff options
| author | Jay Kwak <82421531+jkwak-work@users.noreply.github.com> | 2025-06-06 10:48:06 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2025-06-06 17:48:06 +0000 |
| commit | 7f04adbfb9dc0c39c372809ea02cc740d484b291 (patch) | |
| tree | b845bb73d027bef240cc6841d87dbc7290980944 /source | |
| parent | 2203df78332f155374866b172887d2568422ed76 (diff) | |
Disable Link-Time-Optimization by default (#7345)
* Disable Link-Time-Optimization by default
LTO was requested for the release package a while ago.
When we added it, LTO was enabled by default although it was needed only for
the release packages.
Later we found that the Release build cannot be incrementally recompiled
when LTO is enabled. It sometimes works fine, but it required full recompilation
when it doesn't work. We added a new CMake option, `SLANG_ENABLE_RELEASE_LTO`,
to disable it for developers. But many Slang developers don't know the
option exists.
I was going to update the document, CONTRIBUTING.md, but I thought it
will be better to change the default behavior.
* Fix a compiler warning treated as an error on linux
A padding variable was uninitialized, which is fine, but the compiler
was complaining about it.
* Fix other gcc error for uninitialized variable
* Fix more compile warning treated as error
* Fix compiler warning from gcc 11
It appears that this is a valid warning that the `delete this` is done
on an offset 8 when the class uses multiple inheritance.
The compiler warning is following:
```
In file included from /home/runner/work/slang/slang/source/core/slang-memory-file-system.h:5,
from /home/runner/work/slang/slang/tools/slang-unit-test/unit-test-module-ptr.cpp:3:
In destructor ‘virtual Slang::ComBaseObject::~ComBaseObject()’,
inlined from ‘uint32_t Slang::ComBaseObject::_releaseImpl()’ at /home/runner/work/slang/slang/source/core/slang-com-object.h:49:16,
inlined from ‘virtual uint32_t Slang::MemoryFileSystem::release()’ at /home/runner/work/slang/slang/source/core/slang-memory-file-system.h:34:5:
/home/runner/work/slang/slang/source/core/slang-com-object.h:33:31: error: ‘void operator delete(void*, std::size_t)’ called on pointer ‘<unknown>’ with nonzero offset 8 [-Werror=free-nonheap-object]
33 | virtual ~ComBaseObject() {}
| ^
In destructor ‘virtual Slang::ComBaseObject::~ComBaseObject()’,
inlined from ‘uint32_t Slang::ComBaseObject::_releaseImpl()’ at /home/runner/work/slang/slang/source/core/slang-com-object.h:49:16,
inlined from ‘virtual uint32_t Slang::MemoryFileSystem::release()’ at /home/runner/work/slang/slang/source/core/slang-memory-file-system.h:34:5,
inlined from ‘Slang::ComPtr<T>::~ComPtr() [with T = ISlangMutableFileSystem]’ at /home/runner/work/slang/slang/include/slang-com-ptr.h:113:34,
inlined from ‘void _modulePtr_impl(UnitTestContext*)’ at /home/runner/work/slang/slang/tools/slang-unit-test/unit-test-module-ptr.cpp:92:1:
/home/runner/work/slang/slang/source/core/slang-com-object.h:33:31: error: ‘void operator delete(void*, std::size_t)’ called on pointer ‘<unknown>’ with nonzero offset 8 [-Werror=free-nonheap-object]
33 | virtual ~ComBaseObject() {}
| ^
/home/runner/work/slang/slang/tools/slang-unit-test/unit-test-module-ptr.cpp: In function ‘void _modulePtr_impl(UnitTestContext*)’:
/home/runner/work/slang/slang/tools/slang-unit-test/unit-test-module-ptr.cpp:36:69: note: returned from ‘void* operator new(std::size_t)’
36 | ComPtr<ISlangMutableFileSystem>(new Slang::MemoryFileSystem());
| ^
```
The problem is on the fact that `ComBaseObject` is not the first in the
multiple inheritance:
```
class MemoryFileSystem : public ISlangMutableFileSystem, public ComBaseObject
{
public:
// ISlangUnknown
SLANG_COM_BASE_IUNKNOWN_ALL
```
It should be:
```
class MemoryFileSystem : public ComBaseObject, public ISlangMutableFileSystem
```
The chain of ComObject release is little complicated and it is easy to
make a mistake. Here is summary with details,
1. `release()` is declared as a pure-virtual in ISlangUnknown, which is
one of the base classes of `ISlangMutableFileSystem`.
```
struct ISlangUnknown
{
virtual SLANG_NO_THROW uint32_t SLANG_MCALL release() = 0;
```
2. `release()` is implemented with the macro
`SLANG_COM_BASE_IUNKNOWN_RELEASE`.
```
SLANG_NO_THROW uint32_t SLANG_MCALL release() SLANG_OVERRIDE \
{ \
return _releaseImpl(); \
}
inline uint32_t ComBaseObject::_releaseImpl()
{
// Check there is a ref count to avoid underflow
SLANG_ASSERT(m_refCount != 0);
const uint32_t count = --m_refCount;
if (count == 0)
{
delete this;
}
return count;
}
```
3. The instance of `MemoryFileSystem` is handled by ComPtr. And
`ComPtr::~ComPtr()` calls the `release()`.
```
ComPtr<ISlangMutableFileSystem> memoryFileSystem =
ComPtr<ISlangMutableFileSystem>(new Slang::MemoryFileSystem());
SLANG_FORCE_INLINE ~ComPtr()
{
if (m_ptr)
((Ptr)m_ptr)->release();
}
```
4. When `delete this` is called, because ComBaseObject is not the first
in the multiple inheritance, `this` is 8 byte off from the actual
instance address.
A fix for this is to change the order of the inheritance and make
ComBaseObject to be the first in the order.
Diffstat (limited to 'source')
| -rw-r--r-- | source/core/slang-blob.h | 2 | ||||
| -rw-r--r-- | source/core/slang-file-system.h | 4 | ||||
| -rw-r--r-- | source/core/slang-memory-file-system.h | 2 | ||||
| -rw-r--r-- | source/core/slang-shared-library.h | 2 | ||||
| -rw-r--r-- | source/core/slang-zip-file-system.cpp | 6 | ||||
| -rw-r--r-- | source/slang-llvm/slang-llvm.cpp | 2 | ||||
| -rw-r--r-- | source/slang/slang-emit-vm.cpp | 2 | ||||
| -rw-r--r-- | source/slang/slang-serialize-ast.cpp | 6 | ||||
| -rw-r--r-- | source/slang/slang-serialize.h | 4 | ||||
| -rw-r--r-- | source/slang/slang-vm-bytecode.cpp | 8 | ||||
| -rw-r--r-- | source/slang/slang-workspace-version.h | 2 |
11 files changed, 20 insertions, 20 deletions
diff --git a/source/core/slang-blob.h b/source/core/slang-blob.h index af3206fb5..5277ef0cd 100644 --- a/source/core/slang-blob.h +++ b/source/core/slang-blob.h @@ -15,7 +15,7 @@ namespace Slang /** Base class for simple blobs. */ -class BlobBase : public ISlangBlob, public ICastable, public ComBaseObject +class BlobBase : public ComBaseObject, public ISlangBlob, public ICastable { public: // ISlangUnknown diff --git a/source/core/slang-file-system.h b/source/core/slang-file-system.h index 7ed500eb9..06d647c80 100644 --- a/source/core/slang-file-system.h +++ b/source/core/slang-file-system.h @@ -97,7 +97,7 @@ NOTE! That this behavior is the same as previously in that.... 1) calcRelativePath, just returns the path as processed by the Path:: methods 2) getUniqueIdentity behavior depends on the UniqueIdentityMode. */ -class CacheFileSystem : public ISlangFileSystemExt, public ComBaseObject +class CacheFileSystem : public ComBaseObject, public ISlangFileSystemExt { public: SLANG_CLASS_GUID(0x2f4d1d03, 0xa0d1, 0x434b, {0x87, 0x7a, 0x65, 0x5, 0xa4, 0xa0, 0x9a, 0x3b}) @@ -268,7 +268,7 @@ protected: OSPathKind m_osPathKind = OSPathKind::None; ///< OS path kind }; -class RelativeFileSystem : public ISlangMutableFileSystem, public ComBaseObject +class RelativeFileSystem : public ComBaseObject, public ISlangMutableFileSystem { public: SLANG_COM_BASE_IUNKNOWN_ALL diff --git a/source/core/slang-memory-file-system.h b/source/core/slang-memory-file-system.h index ebf6e239a..84ce6a244 100644 --- a/source/core/slang-memory-file-system.h +++ b/source/core/slang-memory-file-system.h @@ -27,7 +27,7 @@ TODO(JS): * We may want to make saveFile take a blob, or have a version that does. Doing so would allow the application to handle memory management around the blob. */ -class MemoryFileSystem : public ISlangMutableFileSystem, public ComBaseObject +class MemoryFileSystem : public ComBaseObject, public ISlangMutableFileSystem { public: // ISlangUnknown diff --git a/source/core/slang-shared-library.h b/source/core/slang-shared-library.h index fbc8a1d30..8128eee05 100644 --- a/source/core/slang-shared-library.h +++ b/source/core/slang-shared-library.h @@ -48,7 +48,7 @@ private: static DefaultSharedLibraryLoader s_singleton; }; -class DefaultSharedLibrary : public ISlangSharedLibrary, public ComBaseObject +class DefaultSharedLibrary : public ComBaseObject, public ISlangSharedLibrary { public: SLANG_CLASS_GUID(0xe7f2597b, 0xf803, 0x4b6e, {0xaf, 0x8b, 0xcb, 0xe3, 0xa2, 0x21, 0xfd, 0x5a}) diff --git a/source/core/slang-zip-file-system.cpp b/source/core/slang-zip-file-system.cpp index 9c805dc55..0d3503222 100644 --- a/source/core/slang-zip-file-system.cpp +++ b/source/core/slang-zip-file-system.cpp @@ -15,9 +15,9 @@ namespace Slang { -class ZipFileSystemImpl : public ISlangMutableFileSystem, - public IArchiveFileSystem, - public ComBaseObject +class ZipFileSystemImpl : public ComBaseObject, + public ISlangMutableFileSystem, + public IArchiveFileSystem { public: // ISlangUnknown diff --git a/source/slang-llvm/slang-llvm.cpp b/source/slang-llvm/slang-llvm.cpp index e645fb3c7..2860874b5 100644 --- a/source/slang-llvm/slang-llvm.cpp +++ b/source/slang-llvm/slang-llvm.cpp @@ -175,7 +175,7 @@ public: /* This implementation uses atomic ref counting to ensure the shared libraries lifetime can outlive the LLVMDownstreamCompileResult and the compilation that created it */ -class LLVMJITSharedLibrary : public ISlangSharedLibrary, public ComBaseObject +class LLVMJITSharedLibrary : public ComBaseObject, public ISlangSharedLibrary { public: // ISlangUnknown diff --git a/source/slang/slang-emit-vm.cpp b/source/slang/slang-emit-vm.cpp index fc0b4432e..772d9f84a 100644 --- a/source/slang/slang-emit-vm.cpp +++ b/source/slang/slang-emit-vm.cpp @@ -260,7 +260,7 @@ public: uint32_t extOp, ArrayView<VMOperand> operands) { - VMInstHeader instHeader; + VMInstHeader instHeader = {}; instHeader.opcode = op; instHeader.opcodeExtension = extOp; instHeader.operandCount = (uint16_t)operands.getCount(); diff --git a/source/slang/slang-serialize-ast.cpp b/source/slang/slang-serialize-ast.cpp index 02dd374c1..9fa338449 100644 --- a/source/slang/slang-serialize-ast.cpp +++ b/source/slang/slang-serialize-ast.cpp @@ -136,7 +136,7 @@ void serialize(Serializer const& serializer, SemanticVersion& value) void serialize(Serializer const& serializer, SyntaxClass<NodeBase>& value) { - ASTNodeType raw; + ASTNodeType raw = ASTNodeType(0); if (isWriting(serializer)) { raw = value.getTag(); @@ -277,7 +277,7 @@ void serialize(Serializer const& serializer, CapabilityAtomSet& value) { while (hasElements(serializer)) { - CapabilityAtom atom; + CapabilityAtom atom = CapabilityAtom(0); serialize(serializer, atom); value.add(UInt(atom)); } @@ -769,7 +769,7 @@ void ASTDecodingContext::handleASTNode(NodeBase*& outNode) { ASTSerializer serializer(this); - ASTNodeType typeTag; + ASTNodeType typeTag = ASTNodeType(0); serialize(serializer, typeTag); switch (_getPseudoASTNodeType(typeTag)) { diff --git a/source/slang/slang-serialize.h b/source/slang/slang-serialize.h index b962ee2b7..591f43139 100644 --- a/source/slang/slang-serialize.h +++ b/source/slang/slang-serialize.h @@ -859,7 +859,7 @@ void serialize(S const& serializer, Dictionary<K, V>& value) value.clear(); while (hasElements(serializer)) { - KeyValuePair<K, V> pair; + KeyValuePair<K, V> pair{K(), V()}; serialize(serializer, pair); value.add(pair.key, pair.value); } @@ -880,7 +880,7 @@ void serialize(S const& serializer, OrderedDictionary<K, V>& value) value.clear(); while (hasElements(serializer)) { - KeyValuePair<K, V> pair; + KeyValuePair<K, V> pair{K(), V()}; serialize(serializer, pair); value.add(pair.key, pair.value); } diff --git a/source/slang/slang-vm-bytecode.cpp b/source/slang/slang-vm-bytecode.cpp index 1eafa4e57..bf16805b8 100644 --- a/source/slang/slang-vm-bytecode.cpp +++ b/source/slang/slang-vm-bytecode.cpp @@ -328,28 +328,28 @@ void printVMInst(StringBuilder& sb, VMModuleView* moduleView, VMInstHeader* inst { case OperandDataType::Int32: { - int32_t val; + int32_t val = 0; moduleView->getConstant<int32_t>(operand, val); sb << "i32(" << val << ")"; continue; } case OperandDataType::Int64: { - int64_t val; + int64_t val = 0; moduleView->getConstant<int64_t>(operand, val); sb << "i64(" << val << ")"; continue; } case OperandDataType::Float32: { - float val; + float val = 0.f; moduleView->getConstant<float>(operand, val); sb << "f32(" << val << ")"; continue; } case OperandDataType::Float64: { - double val; + double val = 0.0; moduleView->getConstant<double>(operand, val); sb << "f32(" << val << ")"; continue; diff --git a/source/slang/slang-workspace-version.h b/source/slang/slang-workspace-version.h index 711504afb..1a12a2b42 100644 --- a/source/slang/slang-workspace-version.h +++ b/source/slang/slang-workspace-version.h @@ -159,7 +159,7 @@ struct OwnedPreprocessorMacroDefinition String name; String value; }; -class Workspace : public ISlangFileSystem, public ComObject +class Workspace : public ComObject, public ISlangFileSystem { private: RefPtr<WorkspaceVersion> currentVersion; |
