[llvm-symbolizer] Add -no-demangle as alias for -demangle=false

[Quolyk]

Summary

Provides -no-demangle as alias for -demangle=false. Motivation: https://bugs.llvm.org/show_bug.cgi?id=40075

[Quolyk]

I'm not sure how do I handle situation when -demange and -no-demangle appear in one command. I mean what precedence to use.

[jhenderson]

I had a look at an option to disable demangling, as I worked on PR40054, but didn't come up with a clean way of handling both --demangle and --no-demangle, using the cl interface. There, I decided to abandon it, as demangling will be off by default, so we don't need to explicitly disable it. Here is different, because demangling is on by default (and the --demangle switch needs to exist for GNU compatibility). Using tablegen would make it possible to have both sensibly (see examples in e.g. LLD), but I don't think we should do that at this point.

Usually speaking in command-line tools, if there are both positive and negative versions of the option (e.g. --demangle and --no-demangle), the last one present on the command-line takes precedence, so I'd see if there's an API for that. If not, or it's not easy to use, you could probably just emit an error if both are specified.

test/DebugInfo/llvm-symbolizer.test

221

Nit: use double dash to match other options in this file.

tools/llvm-symbolizer/llvm-symbolizer.cpp

66

You don't need the cl::NotHidden here, as it is not a true alias option.

This should probably have an explicit cl::init(false), to "match" the ClDemangle option.

[ruiu]

-demangle=false is a weird option. Has it been ever documented? If not, probably we should remove it and document only -no-demangle.

[jhenderson]

In D56773#1359908, @ruiu wrote:

-demangle=false is a weird option. Has it been ever documented? If not, probably we should remove it and document only -no-demangle.

I agree with you, hence why I filed the bug in the first place. The "=false" syntax is documented on the CommandLine library page (see https://llvm.org/docs/CommandLine.html#boolean-arguments), but I don't think it is documented explicit in the llvm-symbolizer page.

docs/CommandGuide/llvm-symbolizer.rst

92

As noted by @ruiu, let's not document "demangle=false" explicitly, so remove the second sentence here.

[Quolyk]

Update tests. Add logic. Honestly, I'm not good at writing llvm-symbolizer test, so I picked test that would fail if --demangle=true to test --demangle, --no-demangle order.
That doesn't mean I want to merge this patch as is. If more specific tests are needed I would be happy to dive into.

[jhenderson]

test/DebugInfo/llvm-symbolizer.test

42–46

I think it would make more sense to extend the case below that you have already added --no-demangle to.

There you can test it something like the following (add the "DEMANGLED_FUNCTION_NAME" alongside the "SHORT_FUNCTION_NAME" check pattern):

RUN: llvm-symbolizer --functions=short --demangle --no-demangle < %t.input7 \
RUN:    | FileCheck %s --check-prefix=SHORT_FUNCTION_NAME
RUN: llvm-symbolizer --functions=short --no-demangle --demangle < %t.input7 \
RUN:    | FileCheck %s --check-prefix=DEMANGLED_FUNCTION_NAME

DEMANGLED_FUNCTION_NAME: c()

tools/llvm-symbolizer/llvm-symbolizer.cpp

211–212

Nice! I didn't know about the getPosition function.

[Quolyk]

Update tests.

[jhenderson]

test/DebugInfo/llvm-symbolizer.test

225

Sorry, I realised that I misread the test. The existing --demangle=false tests are testing that the "short" name is NOT matched (that's what the -NOT suffix is for on the line above, so this isn't testing what you think it is testing.

Instead, you'll want some new tests below this and the corresponding checks. The --functions=short means that it is printing the short version of the function name, taken from the debug information, I think. In the new tests you write, you don't want the --functions switch at all, I believe.

Try experimenting with running llvm-symbolizer on %t.input7 and look at the output. You should see a function name c() appear somewhere, if I'm not mistaken. Assuming you do, double-check that --no-demangle turns it into _Z1cv, and then add something like the following tests at the end of the file:

; Check that the last of --demangle and --no-demangle wins.
RUN: llvm-symbolizer --demangle < %t.input7 \
RUN:    | FileCheck %s --check-prefix=DEMANGLED_FUNCTION_NAME
RUN: llvm-symbolizer --no-demangle < %t.input7 \
RUN:    | FileCheck %s --check-prefix=MANGLED_FUNCTION_NAME
RUN: llvm-symbolizer --demangle --no-demangle < %t.input7 \
RUN:    | FileCheck %s --check-prefix=MANGLED_FUNCTION_NAME
RUN: llvm-symbolizer --no-demangle --demangle < %t.input7 \
RUN:    | FileCheck %s --check-prefix=DEMANGLED_FUNCTION_NAME

MANGLED_FUNCTION_NAME: _Z1cv
DEMANGLED_FUNCTION_NAME: c()

tools/llvm-symbolizer/llvm-symbolizer.cpp

209

Did you mean to remove the comment you had here before?

[Quolyk]

Update tests.

[Quolyk]

test/DebugInfo/llvm-symbolizer.test

225

Thanks for detailed explanation, now it becomes clear to me.

tools/llvm-symbolizer/llvm-symbolizer.cpp

209

It's back. Wasn't sure it's necessary.

[jhenderson]

tools/llvm-symbolizer/llvm-symbolizer.cpp

210

Nit: remove the "as demangle option".

Also, use clang-format to choose when to split the column onto multiple lines, rather than doing it by hand. I think most (maybe all) of this line will fit on the same line as the first line.

212

What does getPosition do if the option is not on the command-line?

214–216

Why has this changed?

[Quolyk]

Used clang-format -style='{ReflowComments: true}'.

[Quolyk]

tools/llvm-symbolizer/llvm-symbolizer.cpp

212

If option is not specified, cl::opt::getPosition() == 0.

[jhenderson]

LGTM.

[Quolyk]

@jhenderson thanks a lot for review and remarks. That is very valuable.

docs/CommandGuide/llvm-symbolizer.rst

 
  Prefer function names stored in symbol table to function names
  in debug info sections. Defaults to true.
 
 .. option:: -demangle, -C
 
  Print demangled function names. Defaults to true.
 
+.. option:: -no-demangle
+
+ Don't print demangled function names.
+
 .. option:: -inlining 
 
  If a source code location is in an inlined function, prints all the
  inlnied frames. Defaults to true.
 
 .. option:: -default-arch
 
  If a binary contains object files for multiple architectures (e.g. it is a

test/DebugInfo/llvm-symbolizer.test

 RUN: cp %p/Inputs/split-dwarf-dwp.o %t
 RUN: cp %p/Inputs/split-dwarf-dwp.o.dwp %t
 RUN: echo "%t/split-dwarf-dwp.o 0x4" >> %t.input
 
 RUN: cd %t
 RUN: llvm-symbolizer --functions=linkage --inlining --demangle=false \
 RUN:    --default-arch=i386 < %t.input | FileCheck --check-prefix=CHECK --check-prefix=SPLIT --check-prefix=DWO %s
 
+Ensure that --no-demangle flag exceeds --demangle
+
+RUN: cd %t
+RUN: llvm-symbolizer --functions=linkage --inlining --demangle --no-demangle \
+RUN:    --default-arch=i386 < %t.input | FileCheck --check-prefix=CHECK --check-prefix=SPLIT --check-prefix=DWO %s
+
 Ensure we get the same results in the absence of gmlt-like data in the executable but the presence of a .dwo file
 
 RUN: echo "%p/Inputs/split-dwarf-test-nogmlt 0x400504" >> %t.input
 RUN: echo "%p/Inputs/split-dwarf-test-nogmlt 0x4004f4" >> %t.input
 RUN: llvm-symbolizer --functions=linkage --inlining --demangle=false \
 RUN:    --default-arch=i386 < %t.input | FileCheck --check-prefix=SPLIT --check-prefix=DWO %s
 
 Ensure we get gmlt like results in the absence of a .dwo file but the presence of gmlt-like data in the executable

 
 STRIPPED:  global_func
 
 RUN: echo "%p/Inputs/dwarfdump-test4.elf-x86-64 0x62c" > %t.input7
 RUN: llvm-symbolizer --functions=short --demangle=false < %t.input7 \
 RUN:    | FileCheck %s --check-prefix=SHORT_FUNCTION_NAME
 RUN: llvm-symbolizer --functions=short -C=false < %t.input7 \
 RUN:    | FileCheck %s --check-prefix=SHORT_FUNCTION_NAME
+RUN: llvm-symbolizer --functions=short --no-demangle < %t.input7 \
+RUN:    | FileCheck %s --check-prefix=SHORT_FUNCTION_NAME
 
 SHORT_FUNCTION_NAME-NOT: _Z1cv

tools/llvm-symbolizer/llvm-symbolizer.cpp

static cl::opt<bool>
     ClUseRelativeAddress("relative-address", cl::init(false),
                          cl::desc("Interpret addresses as relative addresses"),
                          cl::ReallyHidden);
 
 static cl::opt<bool>
     ClPrintInlining("inlining", cl::init(true),
                     cl::desc("Print all inlined frames for a given address"));
 
-// -demangle, -C
+// -demangle, -C, -no-demangle
 static cl::opt<bool>
 ClDemangle("demangle", cl::init(true), cl::desc("Demangle function names"));
 static cl::alias
 ClDemangleShort("C", cl::desc("Alias for -demangle"),
                 cl::NotHidden, cl::aliasopt(ClDemangle));
+static cl::opt<bool>
+ClNoDemangle("no-demangle", cl::init(false),
+             cl::desc("Don't demangle function names"));
 
 static cl::opt<std::string> ClDefaultArch("default-arch", cl::init(""),
                                           cl::desc("Default architecture "
                                                    "(for multi-arch objects)"));
 
 // -obj, -exe, -e
 static cl::opt<std::string>
 ClBinaryName("obj", cl::init(""),
static void symbolizeInput(StringRef InputString, LLVMSymbolizer &Symbolizer,
   outs() << "\n";
   outs().flush();
 }
 
 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
 
   llvm::sys::InitializeCOMRAII COM(llvm::sys::COMThreadingMode::MultiThreaded);
-
   cl::ParseCommandLineOptions(argc, argv, "llvm-symbolizer\n");
-  LLVMSymbolizer::Options Opts(ClPrintFunctions, ClUseSymbolTable, ClDemangle,
-                               ClUseRelativeAddress, ClDefaultArch);
+  
+  // If both --demangle and --no-demangle are specified then pick
+  // the last one as demangle option.
+  if (ClNoDemangle.getPosition() > ClDemangle.getPosition())
+    ClDemangle = !ClNoDemangle;
+
+  LLVMSymbolizer::Options Opts(ClPrintFunctions, ClUseSymbolTable,
+                               ClDemangle, ClUseRelativeAddress,
+                               ClDefaultArch);
 
   for (const auto &hint : ClDsymHint) {
     if (sys::path::extension(hint) == ".dSYM") {
       Opts.DsymHints.push_back(hint);
     } else {
       errs() << "Warning: invalid dSYM hint: \"" << hint <<
                 "\" (must have the '.dSYM' extension).\n";
     }