[llvm-nm][WebAssembly] Report the size of data symbols

[sbc100]

Summary

Fixes: https://github.com/llvm/llvm-project/issues/58839

[sbc100]

• add test

[jhenderson]

Makes sense to me, but perhaps one additional test case needed. I'll leave @dschuff to give final approval though.

llvm/test/tools/llvm-nm/wasm/print-size.test

29

Perhaps worth showing that a symbol that isn't in the data segment doesn't have a size printed too?

llvm/tools/llvm-nm/llvm-nm.cpp

1822

Nit: not clang-formatted.

[dschuff]

Otherwise this looks good.

llvm/test/tools/llvm-nm/wasm/print-size.test

29

+1, showing other symbol kinds seems like a good idea as long as we're creating this test. Maybe get one BSS and a function symbol?

[sbc100]

• Add a function symbol, demonstrating that function symbols are unsized

[sbc100]

In D158799#4618705, @dschuff wrote:

Otherwise this looks good.

I added a function symbol.

I'm not sure its worth adding a BSS symbol. They are only distinguished by the segment in which they live (name starting with .bss... otherwise there is nothing special about them. )

[dschuff]

In D158799#4618789, @sbc100 wrote:

In D158799#4618705, @dschuff wrote:

Otherwise this looks good.

I added a function symbol.

I'm not sure its worth adding a BSS symbol. They are only distinguished by the segment in which they live (name starting with .bss... otherwise there is nothing special about them. )

Do they have a size?
Actually, what is the size supposed to represent? The size in the file or the initialized size in wasm memory? I guess in other platforms its the virtual memory size, right?

[sbc100]

In D158799#4618817, @dschuff wrote:

In D158799#4618789, @sbc100 wrote:

In D158799#4618705, @dschuff wrote:

Otherwise this looks good.

I added a function symbol.

I'm not sure its worth adding a BSS symbol. They are only distinguished by the segment in which they live (name starting with .bss... otherwise there is nothing special about them. )

Do they have a size?
Actually, what is the size supposed to represent? The size in the file or the initialized size in wasm memory? I guess in other platforms its the virtual memory size, right?

yes they have a size just like any other data symbol.

[sbc100]

• clang-format

[dschuff]

actually, what about globals, e.g. __stack_pointer? Do they also have a size?

[sbc100]

In D158799#4618931, @dschuff wrote:

actually, what about globals, e.g. __stack_pointer? Do they also have a size?

No, they are of a different type. WASM_SYMBOL_TYPE_GLOBAL vs WASM_SYMBOL_TYPE_DATA.

[sbc100]

In D158799#4618946, @sbc100 wrote:

In D158799#4618931, @dschuff wrote:

actually, what about globals, e.g. __stack_pointer? Do they also have a size?

No, they are of a different type. WASM_SYMBOL_TYPE_GLOBAL vs WASM_SYMBOL_TYPE_DATA.

So in the context of this patch they would return false in the WasmSym.isTypeData() check.

[dschuff]

Ah, got it, makes sense.

llvm/lib/Object/WasmObjectFile.cpp

case wasm::WASM_SYMBOL_TYPE_TABLE:
       }
       break;
 
     case wasm::WASM_SYMBOL_TYPE_DATA:
       Info.Name = readString(Ctx);
       if (IsDefined) {
         auto Index = readVaruint32(Ctx);
         if (Index >= DataSegments.size())
-          return make_error<GenericBinaryError>("invalid data symbol index",
+          return make_error<GenericBinaryError>("invalid data segment index",
                                                 object_error::parse_failed);
         auto Offset = readVaruint64(Ctx);
         auto Size = readVaruint64(Ctx);
         size_t SegmentSize = DataSegments[Index].Data.Content.size();
         if (Offset > SegmentSize)
           return make_error<GenericBinaryError>(
               "invalid data symbol offset: `" + Info.Name + "` (offset: " +
                   Twine(Offset) + " segment size: " + Twine(SegmentSize) + ")",

llvm/test/tools/llvm-nm/wasm/print-size.test

+# RUN: yaml2obj %s -o %t.o
+# RUN: llvm-nm --print-size %t.o | FileCheck %s --strict-whitespace
+# RUN: llvm-nm -S %t.o | FileCheck %s --strict-whitespace
+
+--- !WASM
+FileHeader:
+  Version:         0x1
+Sections:
+  - Type:            TYPE
+    Signatures:
+      - Index:           0
+        ParamTypes:       []
+        ReturnTypes:      []
+  - Type:            FUNCTION
+    FunctionTypes:   [ 0 ]
+  - Type:            CODE
+    Functions:
+      - Index:           0
+        Locals:
+        Body:            200008808080800041000B
+  - Type:            DATA
+    Segments:
+      - SectionOffset:   6
+        InitFlags:       0
+        Offset:
+          Opcode:          I32_CONST
+          Value:           0
+        Content:         '00000000'
+  - Type:            CUSTOM
+    Name:            linking
+    Version:         2
+    SymbolTable:
+      - Index:           0
+        Kind:            FUNCTION
+        Name:            a_func
+        Flags:           [  ]
+        Function:        0
+      - Index:           1
+        Kind:            DATA
+        Name:            a_data_symbol
+        Flags:           [  ]
+        Segment:         0
+        Size:            32
+
+# CHECK: 00000000 00000020 D a_data_symbol
+# CHECK: 00000001 00000000 T a_func

llvm/tools/llvm-nm/llvm-nm.cpp

for (BasicSymbolRef Sym : Symbols) {
       S.Address = 0;
       if (isa<ELFObjectFileBase>(&Obj))
         S.Size = ELFSymbolRef(Sym).getSize();
 
       if (const XCOFFObjectFile *XCOFFObj =
               dyn_cast<const XCOFFObjectFile>(&Obj))
         S.Size = XCOFFObj->getSymbolSize(Sym.getRawDataRefImpl());
 
+      if (const WasmObjectFile *WasmObj = dyn_cast<WasmObjectFile>(&Obj)) {
+        const WasmSymbol &WasmSym = WasmObj->getWasmSymbol(Sym);
+        if (WasmSym.isTypeData())
+          S.Size = WasmSym.Info.DataRef.Size;
+      }
+
       if (PrintAddress && isa<ObjectFile>(Obj)) {
         SymbolRef SymRef(Sym);
         Expected<uint64_t> AddressOrErr = SymRef.getAddress();
         if (!AddressOrErr) {
           consumeError(AddressOrErr.takeError());
           break;
         }
         S.Address = *AddressOrErr;