[EH] Make tag's attribute encoding detail

This removes `attribute` field from `Tag` class, making the reserved and unused field known only to binary encoder and decoder. Suggested in WebAssembly#3946 (review).
aheejin · Jun 20, 2021 · 52b4fb4 · 52b4fb4
1 parent 9fc2762
commit 52b4fb4
Show file tree

Hide file tree

Showing 10 changed files with 10 additions and 36 deletions.
diff --git a/src/binaryen-c.cpp b/src/binaryen-c.cpp
@@ -3352,12 +3352,10 @@ BinaryenGlobalRef BinaryenGetGlobalByIndex(BinaryenModuleRef module,
 
 BinaryenTagRef BinaryenAddTag(BinaryenModuleRef module,
                               const char* name,
-                              uint32_t attribute,
                               BinaryenType params,
                               BinaryenType results) {
   auto* ret = new Tag();
   ret->setExplicitName(name);
-  ret->attribute = attribute;
   ret->sig = Signature(Type(params), Type(results));
   ((Module*)module)->addTag(ret);
   return ret;
@@ -3423,7 +3421,6 @@ void BinaryenAddTagImport(BinaryenModuleRef module,
                           const char* internalName,
                           const char* externalModuleName,
                           const char* externalBaseName,
-                          uint32_t attribute,
                           BinaryenType params,
                           BinaryenType results) {
   auto* ret = new Tag();
@@ -4152,9 +4149,6 @@ BinaryenExpressionRef BinaryenGlobalGetInitExpr(BinaryenGlobalRef global) {
 const char* BinaryenTagGetName(BinaryenTagRef tag) {
   return ((Tag*)tag)->name.c_str();
 }
-uint32_t BinaryenTagGetAttribute(BinaryenTagRef tag) {
-  return ((Tag*)tag)->attribute;
-}
 BinaryenType BinaryenTagGetParams(BinaryenTagRef tag) {
   return ((Tag*)tag)->sig.params.getID();
 }

diff --git a/src/binaryen-c.h b/src/binaryen-c.h
@@ -2072,7 +2072,6 @@ BINARYEN_API void BinaryenAddTagImport(BinaryenModuleRef module,
                                        const char* internalName,
                                        const char* externalModuleName,
                                        const char* externalBaseName,
-                                       uint32_t attribute,
                                        BinaryenType params,
                                        BinaryenType results);
 
@@ -2142,7 +2141,6 @@ BINARYEN_REF(Tag);
 // Adds a tag to the module.
 BINARYEN_API BinaryenTagRef BinaryenAddTag(BinaryenModuleRef module,
                                            const char* name,
-                                           uint32_t attribute,
                                            BinaryenType params,
                                            BinaryenType results);
 // Gets a tag reference by name. Returns NULL if the tag does not exist.
@@ -2555,8 +2553,6 @@ BinaryenGlobalGetInitExpr(BinaryenGlobalRef global);
 
 // Gets the name of the specified `Tag`.
 BINARYEN_API const char* BinaryenTagGetName(BinaryenTagRef tag);
-// Gets the attribute of the specified `Tag`.
-BINARYEN_API uint32_t BinaryenTagGetAttribute(BinaryenTagRef tag);
 // Gets the parameters type of the specified `Tag`.
 BINARYEN_API BinaryenType BinaryenTagGetParams(BinaryenTagRef tag);
 // Gets the results type of the specified `Tag`.

diff --git a/src/ir/module-splitting.cpp b/src/ir/module-splitting.cpp
@@ -650,7 +650,6 @@ void ModuleSplitter::shareImportableItems() {
 
   for (auto& tag : primary.tags) {
     auto secondaryTag = std::make_unique<Tag>();
-    secondaryTag->attribute = tag->attribute;
     secondaryTag->sig = tag->sig;
     makeImportExport(*tag, *secondaryTag, "tag", ExternalKind::Tag);
     secondary.addTag(std::move(secondaryTag));

diff --git a/src/ir/module-utils.h b/src/ir/module-utils.h
@@ -66,7 +66,6 @@ inline Global* copyGlobal(Global* global, Module& out) {
 inline Tag* copyTag(Tag* tag, Module& out) {
   auto* ret = new Tag();
   ret->name = tag->name;
-  ret->attribute = tag->attribute;
   ret->sig = tag->sig;
   out.addTag(ret);
   return ret;

diff --git a/src/js/binaryen.js-post.js b/src/js/binaryen.js-post.js
@@ -2396,8 +2396,8 @@ function wrapModule(module, self = {}) {
   self['removeElementSegment'] = function(name) {
     return preserveStack(() => Module['_BinaryenRemoveElementSegment'](module, strToStack(name)));
   };
-  self['addTag'] = function(name, attribute, params, results) {
-    return preserveStack(() => Module['_BinaryenAddTag'](module, strToStack(name), attribute, params, results));
+  self['addTag'] = function(name, params, results) {
+    return preserveStack(() => Module['_BinaryenAddTag'](module, strToStack(name), params, results));
   };
   self['getTag'] = function(name) {
     return preserveStack(() => Module['_BinaryenGetTag'](module, strToStack(name)));
@@ -2425,9 +2425,9 @@ function wrapModule(module, self = {}) {
       Module['_BinaryenAddGlobalImport'](module, strToStack(internalName), strToStack(externalModuleName), strToStack(externalBaseName), globalType, mutable)
     );
   };
-  self['addTagImport'] = function(internalName, externalModuleName, externalBaseName, attribute, params, results) {
+  self['addTagImport'] = function(internalName, externalModuleName, externalBaseName, params, results) {
     return preserveStack(() =>
-      Module['_BinaryenAddTagImport'](module, strToStack(internalName), strToStack(externalModuleName), strToStack(externalBaseName), attribute, params, results)
+      Module['_BinaryenAddTagImport'](module, strToStack(internalName), strToStack(externalModuleName), strToStack(externalBaseName), params, results)
     );
   };
   self['addExport'] = // deprecated
@@ -3212,7 +3212,6 @@ Module['getTagInfo'] = function(tag) {
     'name': UTF8ToString(Module['_BinaryenTagGetName'](tag)),
     'module': UTF8ToString(Module['_BinaryenTagImportGetModule'](tag)),
     'base': UTF8ToString(Module['_BinaryenTagImportGetBase'](tag)),
-    'attribute': Module['_BinaryenTagGetAttribute'](tag),
     'params': Module['_BinaryenTagGetParams'](tag),
     'results': Module['_BinaryenTagGetResults'](tag)
   };

diff --git a/src/tools/fuzzing.h b/src/tools/fuzzing.h
@@ -499,7 +499,6 @@ class TranslateToFuzzReader {
     Index num = upTo(3);
     for (size_t i = 0; i < num; i++) {
       auto tag = builder.makeTag(Names::getValidTagName(wasm, "tag$"),
-                                 WASM_TAG_ATTRIBUTE_EXCEPTION,
                                  Signature(getControlFlowType(), Type::none));
       wasm.addTag(std::move(tag));
     }

diff --git a/src/wasm-builder.h b/src/wasm-builder.h
@@ -126,10 +126,9 @@ class Builder {
   }
 
   static std::unique_ptr<Tag>
-  makeTag(Name name, uint32_t attribute, Signature sig) {
+  makeTag(Name name, Signature sig) {
     auto tag = std::make_unique<Tag>();
     tag->name = name;
-    tag->attribute = attribute;
     tag->sig = sig;
     return tag;
   }

diff --git a/src/wasm.h b/src/wasm.h
@@ -1801,13 +1801,8 @@ class Global : public Importable {
   bool mutable_ = false;
 };
 
-// Kinds of tag attributes.
-enum WasmTagAttribute : unsigned { WASM_TAG_ATTRIBUTE_EXCEPTION = 0x0 };
-
 class Tag : public Importable {
 public:
-  // Kind of tag. Currently only WASM_TAG_ATTRIBUTE_EXCEPTION is possible.
-  uint32_t attribute = WASM_TAG_ATTRIBUTE_EXCEPTION;
   Signature sig;
 };
 

diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
@@ -282,7 +282,7 @@ void WasmBinaryWriter::writeImports() {
     BYN_TRACE("write one tag\n");
     writeImportHeader(tag);
     o << U32LEB(int32_t(ExternalKind::Tag));
-    o << U32LEB(tag->attribute);
+    o << U32LEB(0); // Reserved 'attribute' field
     o << U32LEB(getTypeIndex(tag->sig));
   });
   if (wasm->memory.imported()) {
@@ -656,7 +656,7 @@ void WasmBinaryWriter::writeTags() {
   o << U32LEB(num);
   ModuleUtils::iterDefinedTags(*wasm, [&](Tag* tag) {
     BYN_TRACE("write one\n");
-    o << U32LEB(tag->attribute);
+    o << U32LEB(0); // Reserved 'attribute' field. Always 0.
     o << U32LEB(getTypeIndex(tag->sig));
   });
 
@@ -2071,10 +2071,9 @@ void WasmBinaryBuilder::readImports() {
       }
       case ExternalKind::Tag: {
         Name name(std::string("eimport$") + std::to_string(tagCounter++));
-        auto attribute = getU32LEB();
+        getU32LEB(); // Reserved 'attribute' field
         auto index = getU32LEB();
-        auto curr =
-          builder.makeTag(name, attribute, getSignatureByTypeIndex(index));
+        auto curr = builder.makeTag(name, getSignatureByTypeIndex(index));
         curr->module = module;
         curr->base = base;
         wasm.addTag(std::move(curr));
@@ -2908,10 +2907,9 @@ void WasmBinaryBuilder::readTags() {
   BYN_TRACE("num: " << numTags << std::endl);
   for (size_t i = 0; i < numTags; i++) {
     BYN_TRACE("read one\n");
-    auto attribute = getU32LEB();
+    getU32LEB(); // Reserved 'attribute' field. Always 0.
     auto typeIndex = getU32LEB();
     wasm.addTag(Builder::makeTag("tag$" + std::to_string(i),
-                                 attribute,
                                  getSignatureByTypeIndex(typeIndex)));
   }
 }

diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp
@@ -2984,10 +2984,6 @@ static void validateTags(Module& module, ValidationInfo& info) {
                       "Module has tags (exception-handling is disabled)");
   }
   for (auto& curr : module.tags) {
-    info.shouldBeEqual(curr->attribute,
-                       (unsigned)0,
-                       curr->attribute,
-                       "Currently only attribute 0 is supported");
     info.shouldBeEqual(curr->sig.results,
                        Type(Type::none),
                        curr->name,