[haiku-commits] Re: haiku: hrev51274 - src/system/kernel/fs

  • From: Jérôme Duval <jerome.duval@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Tue, 11 Jul 2017 17:50:40 +0200

Am 11.07.2017 5:30 nachm. schrieb "Rene Gollent" <anevilyak@xxxxxxxxx>:

Hi,

On Tue, Jul 11, 2017 at 11:25 AM,  <jerome.duval@xxxxxxxxx> wrote:

diff --git a/src/system/kernel/fs/vfs.cpp b/src/system/kernel/fs/vfs.cpp
index ea47773..f3d562d 100644
--- a/src/system/kernel/fs/vfs.cpp
+++ b/src/system/kernel/fs/vfs.cpp
@@ -7275,7 +7275,9 @@ fs_mount(char* path, const char* device, const
char* fsName, uint32 flags,
                FileDeviceDeleter() : id(-1) {}
                ~FileDeviceDeleter()
                {
-                       KDiskDeviceManager::Default()-
DeleteFileDevice(id);
+                       KDiskDeviceManager* ddm =
KDiskDeviceManager::Default();
+                       if (ddm != NULL)
+                               ddm->DeleteFileDevice(id);
                }

I don't quite follow why this is a gcc6 problem? Wouldn't the disk
device manager not existing when fs_mount() is called signify a bigger
underlying issue to be solved? AFAICT from the related code, the above
should only happen if we try to mount/unmount before the disk device
manager was ever created, or we ignored that creating it failed.


Well, I forgot to explain the crash: the Autolocker class checks if the
Lockable object is NULL before actually trying to lock. When the object is
null nothing happens. But this is C++ and such a behaviour is undefined
(this being null in a method called on this). gcc6 optimizes such null
checks by default, so the Autolocker doesn't check anymore that "this" is
NULL, and crashes.
Now why is the ddm NULL at this place? Good question.
Also I'm suggesting to change the Autolocker behaviour to panic on a NULL
Lockable in the kernel, to detect such issues.
See #13285 for the stack trace.
Bye
Jerome

Other related posts: