[PATCH] D73820: [llvm-strip][WebAssembly] Support strip flags

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/wasm/WasmObjcopy.cpp:67
+    RemovePred = [RemovePred](const Section &Sec) {
+      return RemovePred(Sec) || Sec.SectionType == llvm::wasm::WASM_SEC_CUSTOM;
+    };
----------------
```
       -S
       --strip-all
           Do not copy relocation and symbol information from the source file.

       -g
       --strip-debug
           Do not copy debugging symbols or sections from the source file.
```

Are you sure we want to remove all custom sections.. seems like strip-all relates mostly to symbols/relocations/debugging, not general custom sections.


================
Comment at: llvm/tools/llvm-objcopy/wasm/WasmObjcopy.cpp:74
+      // Explictily keep debug sections regardless of previous removes.
+      if (isDebugSection(Sec))
+        return false;
----------------
Just `return !isDebugSection(Sec))`?


================
Comment at: llvm/tools/llvm-objcopy/wasm/WasmObjcopy.cpp:86
+        return false;
+      // Remove everything else, inluding known sections.
+      return true;
----------------
Ditto.. or is this some kind of pattern being followed from ELF?


================
Comment at: llvm/tools/llvm-objcopy/wasm/WasmObjcopy.cpp:139
       !Config.SymbolsToAdd.empty() || !Config.RPathToAdd.empty() ||
-      !Config.OnlySection.empty() || !Config.SymbolsToGlobalize.empty() ||
-      !Config.SymbolsToKeep.empty() || !Config.SymbolsToLocalize.empty() ||
+      !Config.SymbolsToGlobalize.empty() || !Config.SymbolsToLocalize.empty() ||
       !Config.SymbolsToRemove.empty() ||
----------------
Did we loose SymbolsToKeep by accident here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73820