From 50902e72d72ff1a0ed6982e765bdc58585a9de41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Thu, 18 Jul 2019 10:09:15 +0200 Subject: [PATCH] Platform: check for non-null GL_VENDOR before comparing it. Otherwise, when context creation fails *really bad*, this crashes somewhere deep in __strncmp_sse42 or so. --- doc/changelog.dox | 6 ++++++ src/Magnum/Platform/GlfwApplication.cpp | 7 +++++-- src/Magnum/Platform/Sdl2Application.cpp | 7 +++++-- src/Magnum/Platform/WindowlessGlxApplication.cpp | 5 ++++- src/Magnum/Platform/WindowlessWglApplication.cpp | 5 ++++- 5 files changed, 24 insertions(+), 6 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 854d7b194..198b85344 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -432,6 +432,12 @@ See also: [mosra/magnum-bootstrap#15](https://github.com/mosra/magnum-bootstrap/pull/15)) - @ref Trade::TgaImporter "TgaImporter" now properly handles files with too short data (see [mosra/magnum#359](https://github.com/mosra/magnum/issues/359)) +- When context creation fails *really bad* with + @ref Platform::Sdl2Application, @ref Platform::GlfwApplication, + @ref Platform::WindowlessGlxApplication or + @ref Platform::WindowlessWglApplication, driver detection for the + @cpp "no-forward-compatible-core-context" @ce now properly handles + @fn_gl{GetString} returning a @cpp nullptr @ce @subsection changelog-latest-docs Documentation diff --git a/src/Magnum/Platform/GlfwApplication.cpp b/src/Magnum/Platform/GlfwApplication.cpp index 124abb4f1..a6b4ad3ab 100644 --- a/src/Magnum/Platform/GlfwApplication.cpp +++ b/src/Magnum/Platform/GlfwApplication.cpp @@ -401,9 +401,12 @@ bool GlfwApplication::tryCreate(const Configuration& configuration, const GLConf #endif if(glConfiguration.version() == GL::Version::None && (!_window #ifndef CORRADE_TARGET_APPLE - /* Sorry about the UGLY code, HOPEFULLY THERE WON'T BE MORE WORKAROUNDS */ + /* If context creation fails *really bad*, glGetString() may actually + return nullptr. Check for that to avoid crashes deep inside + strncmp(). Sorry about the UGLY code, HOPEFULLY THERE WON'T BE MORE + WORKAROUNDS */ || (vendorString = reinterpret_cast(glGetString(GL_VENDOR)), - (std::strncmp(vendorString, nvidiaVendorString, sizeof(nvidiaVendorString)) == 0 || + vendorString && (std::strncmp(vendorString, nvidiaVendorString, sizeof(nvidiaVendorString)) == 0 || #ifdef CORRADE_TARGET_WINDOWS std::strncmp(vendorString, intelVendorString, sizeof(intelVendorString)) == 0 || #endif diff --git a/src/Magnum/Platform/Sdl2Application.cpp b/src/Magnum/Platform/Sdl2Application.cpp index 85e6c8d05..51b7c4ad7 100644 --- a/src/Magnum/Platform/Sdl2Application.cpp +++ b/src/Magnum/Platform/Sdl2Application.cpp @@ -410,9 +410,12 @@ bool Sdl2Application::tryCreate(const Configuration& configuration, const GLConf #endif if(glConfiguration.version() == GL::Version::None && (!_glContext #ifndef CORRADE_TARGET_APPLE - /* Sorry about the UGLY code, HOPEFULLY THERE WON'T BE MORE WORKAROUNDS */ + /* If context creation fails *really bad*, glGetString() may actually + return nullptr. Check for that to avoid crashes deep inside + strncmp(). Sorry about the UGLY code, HOPEFULLY THERE WON'T BE MORE + WORKAROUNDS */ || (vendorString = reinterpret_cast(glGetString(GL_VENDOR)), - (std::strncmp(vendorString, nvidiaVendorString, sizeof(nvidiaVendorString)) == 0 || + vendorString && (std::strncmp(vendorString, nvidiaVendorString, sizeof(nvidiaVendorString)) == 0 || #ifdef CORRADE_TARGET_WINDOWS std::strncmp(vendorString, intelVendorString, sizeof(intelVendorString)) == 0 || #endif diff --git a/src/Magnum/Platform/WindowlessGlxApplication.cpp b/src/Magnum/Platform/WindowlessGlxApplication.cpp index 63fd950be..0e2402419 100644 --- a/src/Magnum/Platform/WindowlessGlxApplication.cpp +++ b/src/Magnum/Platform/WindowlessGlxApplication.cpp @@ -123,7 +123,10 @@ WindowlessGlxContext::WindowlessGlxContext(const WindowlessGlxContext::Configura constexpr static const char nvidiaVendorString[] = "NVIDIA Corporation"; constexpr static const char amdVendorString[] = "ATI Technologies Inc."; const char* const vendorString = reinterpret_cast(glGetString(GL_VENDOR)); - if((std::strncmp(vendorString, nvidiaVendorString, sizeof(nvidiaVendorString)) == 0 || + /* If context creation fails *really bad*, glGetString() may actually + return nullptr. Check for that to avoid crashes deep inside + strncmp() */ + if(vendorString && (std::strncmp(vendorString, nvidiaVendorString, sizeof(nvidiaVendorString)) == 0 || std::strncmp(vendorString, amdVendorString, sizeof(amdVendorString)) == 0) && (!magnumContext || !magnumContext->isDriverWorkaroundDisabled("no-forward-compatible-core-context"))) { diff --git a/src/Magnum/Platform/WindowlessWglApplication.cpp b/src/Magnum/Platform/WindowlessWglApplication.cpp index 321c2740f..7669b1107 100644 --- a/src/Magnum/Platform/WindowlessWglApplication.cpp +++ b/src/Magnum/Platform/WindowlessWglApplication.cpp @@ -176,7 +176,10 @@ WindowlessWglContext::WindowlessWglContext(const Configuration& configuration, G constexpr static const char intelVendorString[] = "Intel"; constexpr static const char amdVendorString[] = "ATI Technologies Inc."; const char* const vendorString = reinterpret_cast(glGetString(GL_VENDOR)); - if((std::strncmp(vendorString, nvidiaVendorString, sizeof(nvidiaVendorString)) == 0 || + /* If context creation fails *really bad*, glGetString() may actually + return nullptr. Check for that to avoid crashes deep inside + strncmp() */ + if(vendorString && (std::strncmp(vendorString, nvidiaVendorString, sizeof(nvidiaVendorString)) == 0 || std::strncmp(vendorString, intelVendorString, sizeof(intelVendorString)) == 0 || std::strncmp(vendorString, amdVendorString, sizeof(amdVendorString)) == 0) && (!magnumContext || !magnumContext->isDriverWorkaroundDisabled("no-forward-compatible-core-context")))