[PATCH] D69093: [llvm-objcopy] --add-symbol: fix crash if SHT_SYMTAB does not exist

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: test/tools/llvm-objcopy/ELF/add-symbol-no-symtab.test:5
+## If .strtab exists but .symtab does not, create .symtab and set its sh_link to .strtab .
+# RUN: llvm-objcopy --remove-section=.symtab %t %t1
+# RUN: llvm-objcopy --add-symbol='abs1=1' %t1 %t2
----------------
Will we want to remove this call when yaml2obj patch that supports omiting ".strtab" be landed?
If so - worth adding a TODOs, probably, I am not sure it is critical though.


================
Comment at: test/tools/llvm-objcopy/ELF/add-symbol-no-symtab.test:30
+
+## Create both .strtab and .symtab.
+# RUN: llvm-objcopy --add-symbol='abs1=1' %t1 %t2
----------------
I'd say - "Check we create ... "


================
Comment at: test/tools/llvm-objcopy/ELF/add-symbol-no-symtab.test:36
+# SEC2:      Index: 1
+# SEC2-NEXT: Name: .shstrtab
+# SEC2-NEXT: Type: SHT_STRTAB
----------------
".shstrtab" != ".strtab"


================
Comment at: tools/llvm-objcopy/ELF/Object.cpp:1554
+      }
+    if (StrTab == nullptr)
+      StrTab = &Obj.addSection<StringTableSection>();
----------------
Perhaps `if (!StrTab)`? Note that code above doesn't do `if (Obj.SymbolTable != null` for example.


================
Comment at: tools/llvm-objcopy/ELF/Object.cpp:1557
+
+    auto &SymTab = Obj.addSection<SymbolTableSection>();
+    SymTab.Name = ".symtab";
----------------
Avoid auto?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D69093