commit 0b824324bc8dd21003e6c028df940f0399d88a03
author Mike <Mango@Live.cn> Thu Jun 30 00:05:00 2022 +0800
committer GitHub <noreply@github.com> Wed Jun 29 12:05:00 2022 -0400
tree 88b56e2c1d37fff01244cdf3059586be801ed5cd
parent 66d88508dd31831664b234ec59e6a5e2d0095a10

Fix segfault in `SpirvTools::Disassemble` when printing (#4833)

When the `SPV_BINARY_TO_TEXT_OPTION_PRINT` option is specified, `spvtext` will not be created (see https://github.com/KhronosGroup/SPIRV-Tools/blob/37d2396cabe56b29d37551ea55d0d745d5748ded/source/disassemble.cpp#L117), and the attempt to dereference into its members (`spvtext->str` and `spvtext->length`) results in segmentation fault.

This is fixed by first checking if `spvtext` is nulll.

Co-authored-by: Steven Perron <stevenperron@google.com>

source/disassemble.h

diff --git a/source/disassemble.h b/source/disassemble.h
index 8eacb10..b520a1e 100644
--- a/source/disassemble.h
+++ b/source/disassemble.h
@@ -25,9 +25,10 @@
 
 // Decodes the given SPIR-V instruction binary representation to its assembly
 // text. The context is inferred from the provided module binary. The options
-// parameter is a bit field of spv_binary_to_text_options_t. Decoded text will
-// be stored into *text. Any error will be written into *diagnostic if
-// diagnostic is non-null.
+// parameter is a bit field of spv_binary_to_text_options_t (note: the option
+// SPV_BINARY_TO_TEXT_OPTION_PRINT will be ignored). Decoded text will be
+// stored into *text. Any error will be written into *diagnostic if diagnostic
+// is non-null.
 std::string spvInstructionBinaryToText(const spv_target_env env,
                                        const uint32_t* inst_binary,
                                        const size_t inst_word_count,

source/libspirv.cpp

diff --git a/source/libspirv.cpp b/source/libspirv.cpp
index 0bc0935..be76caa 100644
--- a/source/libspirv.cpp
+++ b/source/libspirv.cpp
@@ -99,7 +99,9 @@
   spv_text spvtext = nullptr;
   spv_result_t status = spvBinaryToText(impl_->context, binary, binary_size,
                                         options, &spvtext, nullptr);
-  if (status == SPV_SUCCESS) {
+  if (status == SPV_SUCCESS &&
+      (options & SPV_BINARY_TO_TEXT_OPTION_PRINT) == 0) {
+    assert(spvtext);
     text->assign(spvtext->str, spvtext->str + spvtext->length);
   }
   spvTextDestroy(spvtext);

test/binary_to_text_test.cpp

diff --git a/test/binary_to_text_test.cpp b/test/binary_to_text_test.cpp
index df703e5..44705f2 100644
--- a/test/binary_to_text_test.cpp
+++ b/test/binary_to_text_test.cpp
@@ -101,6 +101,17 @@
   spvTextDestroy(text);
 }
 
+TEST_F(BinaryToText, Print) {
+  spv_text text = nullptr;
+  spv_diagnostic diagnostic = nullptr;
+  ASSERT_EQ(
+      SPV_SUCCESS,
+      spvBinaryToText(context, binary->code, binary->wordCount,
+                      SPV_BINARY_TO_TEXT_OPTION_PRINT, &text, &diagnostic));
+  ASSERT_EQ(text, nullptr);
+  spvTextDestroy(text);
+}
+
 TEST_F(BinaryToText, MissingModule) {
   spv_text text;
   spv_diagnostic diagnostic = nullptr;