Skip to content

G
glslang
Commit 8004d365 by LoopDawg
Options

Remapper: make remapper robust against non-exiting error handlers

Remapper errors are generally fatal: there has been some unexpected situation while parsing the SPV binary, and there is no reasonable way to carry on. The errorHandler() function is called in this case, which by default exits, but it is possible to submit a handler which does not. In that case the remapper would carry on in a bad state. This change ensures a graceful termination of the remap() function. While a try {} catch {} construct would be the ideal and safe way to do this, that's off limits for certain environments, so this tries to do the same thing with explicit code, to catch all the bailout paths.
parent 31365afa
Showing with 26 additions and 3 deletions
CMakeLists.txt
...@@ -52,7 +52,7 @@ endif(WIN32) ...@@ -52,7 +52,7 @@ endif(WIN32)
if(${CMAKE_CXX_COMPILER_ID} MATCHES "GNU") if(${CMAKE_CXX_COMPILER_ID} MATCHES "GNU")
add_compile_options(-Wall -Wmaybe-uninitialized -Wuninitialized -Wunused -Wunused-local-typedefs add_compile_options(-Wall -Wmaybe-uninitialized -Wuninitialized -Wunused -Wunused-local-typedefs
-Wunused-parameter -Wunused-value -Wunused-variable -Wunused-but-set-parameter -Wunused-but-set-variable) -Wunused-parameter -Wunused-value -Wunused-variable -Wunused-but-set-parameter -Wunused-but-set-variable -fno-exceptions)
add_compile_options(-Wno-reorder) # disable this from -Wall, since it happens all over. add_compile_options(-Wno-reorder) # disable this from -Wall, since it happens all over.
elseif(${CMAKE_CXX_COMPILER_ID} MATCHES "Clang") elseif(${CMAKE_CXX_COMPILER_ID} MATCHES "Clang")
add_compile_options(-Wall -Wuninitialized -Wunused -Wunused-local-typedefs add_compile_options(-Wall -Wuninitialized -Wunused -Wunused-local-typedefs
......
SPIRV/SPVRemapper.h
...@@ -39,6 +39,7 @@ ...@@ -39,6 +39,7 @@
#include <string> #include <string>
#include <vector> #include <vector>
#include <cstdlib> #include <cstdlib>
#include <exception>
namespace spv { namespace spv {
...@@ -111,7 +112,9 @@ namespace spv { ...@@ -111,7 +112,9 @@ namespace spv {
class spirvbin_t : public spirvbin_base_t class spirvbin_t : public spirvbin_base_t
{ {
public: public:
spirvbin_t(int verbose = 0) : entryPoint(spv::NoResult), largestNewId(0), verbose(verbose) { } spirvbin_t(int verbose = 0) : entryPoint(spv::NoResult), largestNewId(0), verbose(verbose), errorLatch(false)
{ }
virtual ~spirvbin_t() { } virtual ~spirvbin_t() { }
// remap on an existing binary in memory // remap on an existing binary in memory
...@@ -165,7 +168,7 @@ private: ...@@ -165,7 +168,7 @@ private:
typedef std::unordered_map<spv::Id, unsigned> typesize_map_t; typedef std::unordered_map<spv::Id, unsigned> typesize_map_t;
// handle error // handle error
void error(const std::string& txt) const { errorHandler(txt); } void error(const std::string& txt) const { errorLatch = true; errorHandler(txt); }
bool isConstOp(spv::Op opCode) const; bool isConstOp(spv::Op opCode) const;
bool isTypeOp(spv::Op opCode) const; bool isTypeOp(spv::Op opCode) const;
...@@ -286,6 +289,11 @@ private: ...@@ -286,6 +289,11 @@ private:
std::uint32_t options; std::uint32_t options;
int verbose; // verbosity level int verbose; // verbosity level
// Error latch: this is set if the error handler is ever executed. It would be better to
// use a try/catch block and throw, but that's not desired for certain environments, so
// this is the alternative.
mutable bool errorLatch;
static errorfn_t errorHandler; static errorfn_t errorHandler;
static logfn_t logHandler; static logfn_t logHandler;
}; };
......
Test/runtests
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
TARGETDIR=localResults TARGETDIR=localResults
BASEDIR=baseResults BASEDIR=baseResults
EXE=../build/install/bin/glslangValidator EXE=../build/install/bin/glslangValidator
REMAPEXE=../build/install/bin/spirv-remap
HASERROR=0 HASERROR=0
mkdir -p localResults mkdir -p localResults
...@@ -141,6 +142,7 @@ diff -b $BASEDIR/hlsl.dashI.vert.out $TARGETDIR/hlsl.dashI.vert.out || HASERROR= ...@@ -141,6 +142,7 @@ diff -b $BASEDIR/hlsl.dashI.vert.out $TARGETDIR/hlsl.dashI.vert.out || HASERROR=
# #
# Testing -D and -U # Testing -D and -U
# #
echo "Testing -D and -U"
$EXE -DUNDEFED -UIN_SHADER -DFOO=200 -i -l -UUNDEFED -DMUL=FOO*2 glsl.-D-U.frag > $TARGETDIR/glsl.-D-U.frag.out $EXE -DUNDEFED -UIN_SHADER -DFOO=200 -i -l -UUNDEFED -DMUL=FOO*2 glsl.-D-U.frag > $TARGETDIR/glsl.-D-U.frag.out
diff -b $BASEDIR/glsl.-D-U.frag.out $TARGETDIR/glsl.-D-U.frag.out || HASERROR=1 diff -b $BASEDIR/glsl.-D-U.frag.out $TARGETDIR/glsl.-D-U.frag.out || HASERROR=1
$EXE -D -e main -V -i -DUNDEFED -UIN_SHADER -DFOO=200 -UUNDEFED hlsl.-D-U.frag > $TARGETDIR/hlsl.-D-U.frag.out $EXE -D -e main -V -i -DUNDEFED -UIN_SHADER -DFOO=200 -UUNDEFED hlsl.-D-U.frag > $TARGETDIR/hlsl.-D-U.frag.out
...@@ -149,6 +151,7 @@ diff -b $BASEDIR/hlsl.-D-U.frag.out $TARGETDIR/hlsl.-D-U.frag.out || HASERROR=1 ...@@ -149,6 +151,7 @@ diff -b $BASEDIR/hlsl.-D-U.frag.out $TARGETDIR/hlsl.-D-U.frag.out || HASERROR=1
# #
# Test --client and --target-env # Test --client and --target-env
# #
echo "Testing --client and --target-env"
$EXE --client vulkan100 spv.targetVulkan.vert || HASERROR=1 $EXE --client vulkan100 spv.targetVulkan.vert || HASERROR=1
$EXE --client opengl100 spv.targetOpenGL.vert || HASERROR=1 $EXE --client opengl100 spv.targetOpenGL.vert || HASERROR=1
$EXE --target-env vulkan1.0 spv.targetVulkan.vert || HASERROR=1 $EXE --target-env vulkan1.0 spv.targetVulkan.vert || HASERROR=1
...@@ -159,6 +162,7 @@ $EXE -G100 spv.targetOpenGL.vert || HASERROR=1 ...@@ -159,6 +162,7 @@ $EXE -G100 spv.targetOpenGL.vert || HASERROR=1
# #
# Testing GLSL entry point rename # Testing GLSL entry point rename
# #
echo "Testing GLSL entry point rename"
$EXE -H -e foo --source-entrypoint main glsl.entryPointRename.vert > $TARGETDIR/glsl.entryPointRename.vert.out $EXE -H -e foo --source-entrypoint main glsl.entryPointRename.vert > $TARGETDIR/glsl.entryPointRename.vert.out
diff -b $BASEDIR/glsl.entryPointRename.vert.out $TARGETDIR/glsl.entryPointRename.vert.out || HASERROR=1 diff -b $BASEDIR/glsl.entryPointRename.vert.out $TARGETDIR/glsl.entryPointRename.vert.out || HASERROR=1
$EXE -H -e foo --source-entrypoint bar glsl.entryPointRename.vert > $TARGETDIR/glsl.entryPointRename.vert.bad.out $EXE -H -e foo --source-entrypoint bar glsl.entryPointRename.vert > $TARGETDIR/glsl.entryPointRename.vert.bad.out
...@@ -167,6 +171,15 @@ $EXE -H -e foo --source-entrypoint main glsl.entryPointRename2.vert > $TARGETDIR ...@@ -167,6 +171,15 @@ $EXE -H -e foo --source-entrypoint main glsl.entryPointRename2.vert > $TARGETDIR
diff -b $BASEDIR/glsl.entryPointRename2.vert.out $TARGETDIR/glsl.entryPointRename2.vert.out || HASERROR=1 diff -b $BASEDIR/glsl.entryPointRename2.vert.out $TARGETDIR/glsl.entryPointRename2.vert.out || HASERROR=1
# #
# Testing remapper error handling
#
echo "Testing remapper error handling"
$REMAPEXE --do-everything -i remap.invalid-spirv-1.spv -o $TARGETDIR > $TARGETDIR/remap.invalid-spirv-1.out && HASERROR=1
diff -b $BASEDIR/remap.invalid-spirv-1.out $TARGETDIR/remap.invalid-spirv-1.out || HASERROR=1
$REMAPEXE --do-everything -i remap.invalid-spirv-2.spv -o $TARGETDIR > $TARGETDIR/remap.invalid-spirv-2.out && HASERROR=1
diff -b $BASEDIR/remap.invalid-spirv-2.out $TARGETDIR/remap.invalid-spirv-2.out || HASERROR=1
#
# Final checking # Final checking
# #
if [ $HASERROR -eq 0 ] if [ $HASERROR -eq 0 ]
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment