[PATCH] D46628: [ELF] Add --strip-debug-non-line option

[Lucian Adrian Grijincu via Phabricator via llvm-commits]

luciang marked 4 inline comments as done.
luciang added a comment.

`



================
Comment at: lld/ELF/Driver.cpp:1851
+          .EndsWith("debug_line", false)
+          .EndsWith("debug_ranges", false)
+          .EndsWith("debug_str", false)
----------------
MaskRay wrote:
> Do you need .debug_rnglists?
I'll add a test for dwarf 5. 

The gdb I have locally didn't print line number info with dwarf 5. I'll try a newer version of gdb.


================
Comment at: lld/ELF/OutputSections.h:116
+  // Reduced .debug_info/.debug_abbrev when using --strip-debug-non-line.
+  llvm::SmallVector<char, 1> ReducedDebugData;
+
----------------
MaskRay wrote:
> Is size 1 `ReducedDebugData` common? If not, a container other than `SmallVector<char, 1>` may be better.
I used the same container as for CompressedData bellow. I'll use std::vector. Anything is fine here as I only allocate memory once.


================
Comment at: lld/test/ELF/Inputs/strip-debug-non-line-multi-cu-bar.s:6
+	.file	"-"
+	.globl	bar                     # -- Begin function bar
+	.type	bar, at function
----------------
MaskRay wrote:
> The comment markers are misaligned. Do you mix tabs and spaces? That'll render the comments badly on Phabricator.
This file was created using clang++. I'll convert the tabs to spaces. I thought it was preferred to keep this as close to the original clang output as possible.


================
Comment at: lld/test/ELF/strip-debug-non-line.s:208
+
+	.ident	"clang version 9.0.0 (https://github.com/llvm/llvm-project 01a99c0aa5ae5be47ea62bd6c87ca6bb63f5a454)"
+	.section	".note.GNU-stack","", at progbits
----------------
MaskRay wrote:
> .ident, .note.GNU-stack are unnecessary
I got this from clang output.

I thought I saw similar in other tests. 

I didn't want to edit strings to avoid messing offsets but these are fair game to remove. Will do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46628