|
* 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.
|
