[WebAssembly] Distinguish debug/symbol names in the Wasm structs. NFC

[ncw]

Summary

The need for this was flagged up in review for D44440; this change is needed for that review to land

[sbc100]

lib/Object/WasmObjectFile.cpp

282 ↗

(On Diff #140216)

Another way of saying it, maybe we can just drop these two lines of code?

[ncw]

Updated: removed redundant comment

[ncw]

There are no outstanding review comments, and this is blocking the already-approved D44440. Is it good to land?

[sbc100]

One comment, otherwise lgtm

include/llvm/BinaryFormat/Wasm.h

104 ↗

(On Diff #140769)

What do you think about dropping DebugName completely here? Since the name section is not really used in the object format right now this would make sense to me. If you really want to get to the debug names we have a method on the object to get them right?

[ncw]

include/llvm/BinaryFormat/Wasm.h

104 ↗

(On Diff #140769)

I've kept it, since we don't entirely ignore it. When reading in a relocatable Wasm file, we will propagate the DebugName if it's specified, which I think is a useful feature to keep hold of for now.

llvm/trunk/include/llvm/BinaryFormat/Wasm.h

struct WasmGlobalType {
   uint8_t Type;
   bool Mutable;
 };
 
 struct WasmGlobal {
   uint32_t Index;
   WasmGlobalType Type;
   WasmInitExpr InitExpr;
-  StringRef Name; // from the "linking" or "names" section
+  StringRef SymbolName; // from the "linking" section
 };
 
 struct WasmImport {
   StringRef Module;
   StringRef Field;
   uint8_t Kind;
   union {
     uint32_t SigIndex;

 };
 
 struct WasmFunction {
   uint32_t Index;
   std::vector<WasmLocalDecl> Locals;
   ArrayRef<uint8_t> Body;
   uint32_t CodeSectionOffset;
   uint32_t Size;
-  StringRef Name; // from the "linking" or "names" section
+  StringRef SymbolName; // from the "linking" section
+  StringRef DebugName; // from the "name" section
   uint32_t Comdat; // from the "comdat info" section
 };
 
 struct WasmDataSegment {
   uint32_t MemoryIndex;
   WasmInitExpr Offset;
   ArrayRef<uint8_t> Content;
-  StringRef Name;
+  StringRef Name; // from the "segment info" section
   uint32_t Alignment;
   uint32_t Flags;
   uint32_t Comdat; // from the "comdat info" section
 };
 
 struct WasmElemSegment {
   uint32_t TableIndex;
   WasmInitExpr Offset;
struct WasmInitFunc {
   uint32_t Symbol;
 };
 
 struct WasmSymbolInfo {
   StringRef Name;
   uint8_t Kind;
   uint32_t Flags;
   union {
-    // For function or global symbols, the index in function of global index
+    // For function or global symbols, the index in function or global index
     // space.
     uint32_t ElementIndex;
     // For a data symbols, the address of the data relative to segment.
     WasmDataReference DataRef;
   };
 };
 
 struct WasmFunctionName {

llvm/trunk/lib/Object/WasmObjectFile.cpp

case wasm::WASM_NAMES_FUNCTION: {
         if (!Seen.insert(Index).second)
           return make_error<GenericBinaryError>("Function named more than once",
                                                 object_error::parse_failed);
         StringRef Name = readString(Ptr);
         if (!isValidFunctionIndex(Index) || Name.empty())
           return make_error<GenericBinaryError>("Invalid name entry",
                                                 object_error::parse_failed);
         DebugNames.push_back(wasm::WasmFunctionName{Index, Name});
-        if (isDefinedFunctionIndex(Index)) {
-          // Override any existing name; the name specified by the "names"
-          // section is the Function's canonical name.
-          getDefinedFunction(Index).Name = Name;
-        }
+        if (isDefinedFunctionIndex(Index))
+          getDefinedFunction(Index).DebugName = Name;
       }
       break;
     }
     // Ignore local names for now
     case wasm::WASM_NAMES_LOCAL:
     default:
       Ptr += Size;
       break;
case wasm::WASM_SYMBOL_TYPE_FUNCTION:
           IsDefined != isDefinedFunctionIndex(Info.ElementIndex))
         return make_error<GenericBinaryError>("invalid function symbol index",
                                               object_error::parse_failed);
       if (IsDefined) {
         Info.Name = readString(Ptr);
         unsigned FuncIndex = Info.ElementIndex - NumImportedFunctions;
         FunctionType = &Signatures[FunctionTypes[FuncIndex]];
         wasm::WasmFunction &Function = Functions[FuncIndex];
-        if (Function.Name.empty()) {
-          // Use the symbol's name to set a name for the Function, but only if
-          // one hasn't already been set.
-          Function.Name = Info.Name;
-        }
+        if (Function.SymbolName.empty())
+          Function.SymbolName = Info.Name;
       } else {
         wasm::WasmImport &Import = *ImportedFunctions[Info.ElementIndex];
         FunctionType = &Signatures[Import.SigIndex];
         Info.Name = Import.Field;
       }
       break;
 
     case wasm::WASM_SYMBOL_TYPE_GLOBAL:
       Info.ElementIndex = readVaruint32(Ptr);
       if (!isValidGlobalIndex(Info.ElementIndex) ||
           IsDefined != isDefinedGlobalIndex(Info.ElementIndex))
         return make_error<GenericBinaryError>("invalid global symbol index",
                                               object_error::parse_failed);
       if (!IsDefined &&
           (Info.Flags & wasm::WASM_SYMBOL_BINDING_MASK) ==
               wasm::WASM_SYMBOL_BINDING_WEAK)
         return make_error<GenericBinaryError>("undefined weak global symbol",
                                               object_error::parse_failed);
       if (IsDefined) {
         Info.Name = readString(Ptr);
         unsigned GlobalIndex = Info.ElementIndex - NumImportedGlobals;
         wasm::WasmGlobal &Global = Globals[GlobalIndex];
         GlobalType = &Global.Type;
-        if (Global.Name.empty()) {
-          // Use the symbol's name to set a name for the Global, but only if
-          // one hasn't already been set.
-          Global.Name = Info.Name;
-        }
+        if (Global.SymbolName.empty())
+          Global.SymbolName = Info.Name;
       } else {
         wasm::WasmImport &Import = *ImportedGlobals[Info.ElementIndex];
         Info.Name = Import.Field;
         GlobalType = &Import.Global;
       }
       break;
 
     case wasm::WASM_SYMBOL_TYPE_DATA: