[PATCH] D84606: [llvm-readelf] Symbol index in symbol table printing is not reset

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

Generally looks good. A few comments inlined.
But please do not upload patches without a context, see https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface.



================
Comment at: llvm/test/tools/llvm-readobj/ELF/section-symbols-index.test:1
+## This test verifies that the Num index starts from zero at every new Symbol table.
+
----------------
Probably this test should be a part of `symbols.test`.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/section-symbols-index.test:4
+# RUN: yaml2obj %s -o %t1
+# RUN: llvm-readelf -s %t1 %t1 | FileCheck %s
+
----------------
We prefer long form for options in tests I think. I.e. `-s` -> `--symbols`.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/section-symbols-index.test:18
+# CHECK-NEXT:      1: {{.*}}
+# CHECK-NEXT:      2: {{.*}}
+
----------------
I'd expect to have just 6 lines here probably. E.g:

```
# CHECK:      Symbol table '.symtab' contains 3 entries:
# CHECK-NEXT:  Num: {{.*}}
# CHECK-NEXT:    0: {{.*}}
# CHECK:      Symbol table '.symtab' contains 3 entries:
# CHECK-NEXT:  Num: {{.*}}
# CHECK-NEXT:    0: {{.*}}
```



================
Comment at: llvm/test/tools/llvm-readobj/ELF/section-symbols-index.test:30
+  - Name:  bar
+    Value: 0x2
----------------
You can just have `Symbols: []` here. It is enough for the test.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:3932
                                  bool NonVisibilityBitsUsed) {
-  static int Idx = 0;
-  static bool Dynamic = true;
-
-  // If this function was called with a different value from IsDynamic
-  // from last call, happens when we move from dynamic to static symbol
-  // table, "Num" field should be reset.
-  if (!Dynamic != !IsDynamic) {
-    Idx = 0;
-    Dynamic = false;
-  }
+  int Idx = Symbol - FirstSym;
 
----------------
You can inline it:

```
Fields[0].Str = to_string(format_decimal(Symbol - FirstSym, 6)) + ":";
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84606/new/

https://reviews.llvm.org/D84606