[mlir][bytecode] Error if requested bytecode version is unsupported

[GleasonK]

Summary

Currently desired bytecode version is clamped to the maximum. This allows requesting bytecode versions that do not exist. We have added callsite validation for this in StableHLO to ensure we don't pass an invalid version number, probably better if this is managed upstream. If a user wants to use the current version, then omitting setDesiredBytecodeVersion is the best way to do that (as opposed to providing a large number).

Adding this check will also properly error on older version numbers as we increment the minimum supported version. Silently claming on minimum version would likely lead to unintentional forward incompatibilities.

Separately, due to bytecode version being int64_t and using methods to read/write uints, we can generate payloads with invalid version numbers:

mlir-opt file.mlir --emit-bytecode --emit-bytecode-version=-1 | mlir-opt
<stdin>:0:0: error: bytecode version 18446744073709551615 is newer than the current version 5

This is fixed with version bounds checking as well.

[mehdi_amini]

Thanks!

[GleasonK]

Thanks for the quick review Mehdi! I don't have commit access, so would appreciate some help there when you get a chance!

mlir/include/mlir/Bytecode/BytecodeWriter.h

public:
 
   /// An internal implementation class that contains the state of the
   /// configuration.
   struct Impl;
 
   /// Return an instance of the internal implementation.
   const Impl &getImpl() const { return *impl; }
 
-  /// Set the desired bytecode version to emit. This function clamps the version
-  /// to the existing version if larger than existing. The desired version may
-  /// not be used depending on the features used and the actual version required
-  /// is returned by bytecode writer entry point.
+  /// Set the desired bytecode version to emit. This method does not validate
+  /// the desired version. The bytecode writer entry point will return failure
+  /// if it cannot emit the desired version.
   void setDesiredBytecodeVersion(int64_t bytecodeVersion);
 
   /// Get the set desired bytecode version to emit.
   int64_t getDesiredBytecodeVersion() const;
 
   //===--------------------------------------------------------------------===//
   // Resources
   //===--------------------------------------------------------------------===//

mlir/lib/Bytecode/Writer/BytecodeWriter.cpp

 BytecodeWriterConfig::~BytecodeWriterConfig() = default;
 
 void BytecodeWriterConfig::attachResourcePrinter(
     std::unique_ptr<AsmResourcePrinter> printer) {
   impl->externalResourcePrinters.emplace_back(std::move(printer));
 }
 
 void BytecodeWriterConfig::setDesiredBytecodeVersion(int64_t bytecodeVersion) {
-  // Clamp to current version.
-  impl->bytecodeVersion =
-      std::min<int64_t>(bytecodeVersion, bytecode::kVersion);
+  impl->bytecodeVersion = bytecodeVersion;
 }
 
 int64_t BytecodeWriterConfig::getDesiredBytecodeVersion() const {
   return impl->bytecodeVersion;
 }
 
 //===----------------------------------------------------------------------===//
 // EncodingEmitter

 LogicalResult BytecodeWriter::write(Operation *rootOp, raw_ostream &os) {
   EncodingEmitter emitter;
 
   // Emit the bytecode file header. This is how we identify the output as a
   // bytecode file.
   emitter.emitString("ML\xefR");
 
   // Emit the bytecode version.
+  if (config.bytecodeVersion < bytecode::kMinSupportedVersion ||
+      config.bytecodeVersion > bytecode::kVersion)
+    return rootOp->emitError()
+           << "unsupported version requested " << config.bytecodeVersion
+           << ", must be in range ["
+           << static_cast<int64_t>(bytecode::kMinSupportedVersion) << ", "
+           << static_cast<int64_t>(bytecode::kVersion) << ']';
   emitter.emitVarInt(config.bytecodeVersion);
 
   // Emit the producer.
   emitter.emitNulTerminatedString(config.producer);
 
   // Emit the dialect section.
   writeDialectSection(emitter);
 

mlir/test/Bytecode/versioning/versioned_bytecode.mlir

 // This file contains test cases related to roundtripping.
 
 // Bytecode currently does not support big-endian platforms
 // UNSUPPORTED: target=s390x-{{.*}}
 
 //===--------------------------------------------------------------------===//
 // Test roundtrip
 //===--------------------------------------------------------------------===//
 
 // RUN: mlir-opt %S/versioned-op-1.12.mlirbc -emit-bytecode \
 // RUN:   -emit-bytecode-version=0 | mlir-opt -o %t.1 && \
 // RUN: mlir-opt %S/versioned-op-1.12.mlirbc -o %t.2 && \
 // RUN: diff %t.1 %t.2
 
+//===--------------------------------------------------------------------===//
+// Test invalid versions
+//===--------------------------------------------------------------------===//
+
+// RUN: not mlir-opt %S/versioned-op-1.12.mlirbc -emit-bytecode \
+// RUN:   -emit-bytecode-version=-1 2>&1 | FileCheck %s --check-prefix=ERR_VERSION_NEGATIVE
+// ERR_VERSION_NEGATIVE: unsupported version requested -1, must be in range [{{[0-9]+}}, {{[0-9]+}}]
+
+// RUN: not mlir-opt %S/versioned-op-1.12.mlirbc -emit-bytecode \
+// RUN:   -emit-bytecode-version=999 2>&1 | FileCheck %s --check-prefix=ERR_VERSION_FUTURE
+// ERR_VERSION_FUTURE: unsupported version requested 999, must be in range [{{[0-9]+}}, {{[0-9]+}}]