[Debugify] Expose original debug info preservation check as CC1 option

[djtodoro]

Summary

In order to test the preservation of the original Debug Info metadata in your projects, a front end option could be very useful, since users usually report that a concrete entity (e.g. variable x, or function fn2()) is missing debug info. The [0] is an example of running the utility on GDB Project.

Depends on: D82546 and D82545.

[0] https://djolertrk.github.io/di-checker-html-report-example/

[aprantl]

clang/test/DebugInfo/debugify-each-original.c

57

We probably shouldn't check the effects of LLVM passes in the Clang test suite. When someone breaks one of those LLVM passes and that shouldn't cause an error in the Clang *frontend* test suite. It's sufficient to check that the pass is passed on to LLVM here. The operation of it should be checked either in LLVM or in an end-to-end test, e.g., inside debug-info-tests.

[aprantl]

clang/test/DebugInfo/debugify-each-original.c

57

I probably should emphasize that an end-to-end test will probably just cause us trouble: Someone will make an unrelated change to a pass, that pushes a subsequent pass into a behavior that doesn't pass the debugify verifier, and now the author of the unrelated patch is on the hook to fix that other pass. I think that will make everyone unhappy. Maybe it should be treated more like the debug info statistics?

[djtodoro]

-Update the test

[djtodoro]

clang/test/DebugInfo/debugify-each-original.c

57

@aprantl Thanks for your comments!

I agree with this... I refactored the test a bit, please take look.

Maybe it should be treated more like the debug info statistics?

The final goal of this is to be used either from the python script that should process the JSON objects (shared within previous patch), or the JSON objects could be used from http://green.lab.llvm.org/green/view/LLDB/job/clang-3.4-debuginfo-statistics/ as well with some additional (groovy) scripts.

[aprantl]

clang/test/DebugInfo/debugify-each-original.c

17

I think this is still implicitly hardcoding the pass pipeline just through the order of CHECK lines. If there were a way to dump the flags Clang is passing to LLVM and check that, or get the pass manager to dump its configuration, that would be better. I'm not sure if there is such an affordance.

[djtodoro]

clang/test/DebugInfo/debugify-each-original.c

17

I see... It is hard to test it that way, but as far as I can see, it is enough to test only the Driver in situations like this, so I am adding a new test case and getting rid of this one.

[djtodoro]

• Add the Driver test
• Remove the old high level test in order to avoid troubles when someone updates an LLVM pass with the impact on Debugify output (e.g. a new debug info bugs)
• clang-formatted

[aprantl]

clang/test/Driver/debugify-each-original.c

9 ↗

(On Diff #274433)

I think this is redundant?

[djtodoro]

clang/test/Driver/debugify-each-original.c

9 ↗

(On Diff #274433)

It is, thanks!

[djtodoro]

• Remove redundant line from the test

[vsk]

clang/lib/CodeGen/BackendUtil.cpp

855

Please factor out OptCustomPassManager from opt and generalize it so it can be used by both opt and clang. That should help ensure that extensions and bug fixes are only made to one custom 'debugify' pass manager.

893

I don't think the discussion from 'RFC: Introduce LLVM DI Checker utility' is complete, and I'd ask that you split off changes for 'original mode' from this patch until there's some consensus about what that mode should look like.

There are open questions about to what extent a new mode is needed (e.g., it may be that the interesting questions compiler developers need to answer about debug info loss are simpler to determine some other way (which is not to say that that's true -- just that we haven't explored the space much yet)). Or what its output should look like.

[djtodoro]

clang/lib/CodeGen/BackendUtil.cpp

855

I'll try that with the latest code. I remember I've tried it once, but I ended up moving it into the IR library (since we need to link it within legacy pass manager).

893

OK, I'll split off this and notify you/resend a message on the RFC.

[djtodoro]

clang/lib/CodeGen/BackendUtil.cpp

855

Hi @vsk, I've posted the patch as D83391. Thanks!

[djtodoro]

• Rebase on top of D83391

[djtodoro]

• Rebase

[djtodoro]

• Rebasing

[djtodoro]

-Rebasing

[djtodoro]

• Rebase on top of trunk

[djtodoro]

Ping :)

[djtodoro]

• rebase on top of trunk
• refactor the code

ping :)

[djtodoro]

Ping.

[StephenTozer]

Apart from the one comment, LGTM.

clang/lib/Frontend/CompilerInvocation.cpp

794–800

Any particular behaviour if the user specifies a file for -fverify-debuginfo-preserve-export but doesn't set -fverify-debuginfo-preserve? It seems like it would be worth emitting a warning in this case, though I'm not sure if that's an established precedent.

[jansvoboda11]

clang/include/clang/Driver/Options.td

4872 ↗

(On Diff #329972)

Please, update the new options to use the marshalling infrastructure. You can then remove the code from CompilerInvocation.

https://clang.llvm.org/docs/InternalsManual.html#adding-new-command-line-option

[djtodoro]

clang/include/clang/Driver/Options.td

4872 ↗

(On Diff #329972)

Sure. Thanks!

clang/lib/Frontend/CompilerInvocation.cpp

794–800

it makes sense

[djtodoro]

• addressing comments

[jansvoboda11]

Thanks for the update. Changes touching the command line LGTM, but I'll let others confirm the changes to CodeGen are fine too.

[djtodoro]

Thanks.

I feel like this is ready to go. Any additional comment?

[jmorse]

FTR, the patch LGTM. I haven't really been following the development of debugify in detail so I have no feeling for whether this is good from a high level; if you're confident the mailing list discussion finished with this being the right direction, I'd say this is ready to land.

[djtodoro]

Thanks!

clang/include/clang/Basic/CodeGenOptions.h

public:
   /// if non-empty.
   std::string RecordCommandLine;
 
   std::map<std::string, std::string> DebugPrefixMap;
 
   /// The ABI to use for passing floating point arguments.
   std::string FloatABI;
 
+  /// The file to use for dumping bug report by `debugify` in `original` mode.
+  std::string DIOriginalBugsReportFilePath;
+
   /// The floating-point denormal mode to use.
   llvm::DenormalMode FPDenormalMode = llvm::DenormalMode::getIEEE();
 
   /// The floating-point denormal mode to use, for float.
   llvm::DenormalMode FP32DenormalMode = llvm::DenormalMode::getIEEE();
 
   /// The float precision limit to use, if non-empty.
   std::string LimitFloatPrecision;

clang/include/clang/Basic/CodeGenOptions.def

 CODEGENOPT(DisableO0ImplyOptNone , 1, 0) ///< Don't annonate function with optnone at O0
 CODEGENOPT(ExperimentalNewPassManager, 1, 0) ///< Enables the new, experimental
                                              ///< pass manager.
 CODEGENOPT(DebugPassManager, 1, 0) ///< Prints debug information for the new
                                    ///< pass manager.
 CODEGENOPT(DisableRedZone    , 1, 0) ///< Set when -mno-red-zone is enabled.
 CODEGENOPT(EmitCallSiteInfo, 1, 0) ///< Emit call site info only in the case of
                                    ///< '-g' + 'O>0' level.
+CODEGENOPT(EnableDebugifyEachOriginal, 1, 0) ///< Enable debugify each in
+                                             ///< original mode (it means check
+                                             ///< the original debug info
+                                             ///< metadata preservation).
 CODEGENOPT(IndirectTlsSegRefs, 1, 0) ///< Set when -mno-tls-direct-seg-refs
                                      ///< is specified.
 CODEGENOPT(DisableTailCalls  , 1, 0) ///< Do not emit tail calls.
 CODEGENOPT(NoEscapingBlockTailCalls, 1, 0) ///< Do not emit tail calls from
                                            ///< escaping blocks.
 CODEGENOPT(EmitDeclMetadata  , 1, 0) ///< Emit special metadata indicating what
                                      ///< Decl* various IR entities came from.
                                      ///< Only useful when running CodeGen as a

clang/include/clang/Driver/CC1Options.td

def flto_visibility_public_std:
     HelpText<"Use public LTO visibility for classes in std and stdext namespaces">;
 def flto_unit: Flag<["-"], "flto-unit">,
     HelpText<"Emit IR to support LTO unit features (CFI, whole program vtable opt)">;
 def fno_lto_unit: Flag<["-"], "fno-lto-unit">;
 def fdebug_pass_manager : Flag<["-"], "fdebug-pass-manager">,
     HelpText<"Prints debug information for the new pass manager">;
 def fno_debug_pass_manager : Flag<["-"], "fno-debug-pass-manager">,
     HelpText<"Disables debug printing for the new pass manager">;
+def fenable_debugify_each_original
+    : Flag<["-"], "fenable-debugify-each-original">,
+      HelpText<"Enable Debug Info Metadata preservation testing in "
+               "optimizations.">;
+def fenable_debugify_original_export
+    : Joined<["-"], "fenable-debugify-original-export=">,
+      MetaVarName<"<file>">,
+      HelpText<"Export debugify (by testing original Debug Info) failures into "
+               "specified (JSON) file (should be abs path as we use "
+               "append mode to insert new JSON objects).">;
 // The driver option takes the key as a parameter to the -msign-return-address=
 // and -mbranch-protection= options, but CC1 has a separate option so we
 // don't have to parse the parameter twice.
 def msign_return_address_key_EQ : Joined<["-"], "msign-return-address-key=">,
     Values<"a_key,b_key">;
 def mbranch_target_enforce : Flag<["-"], "mbranch-target-enforce">;
 def fno_dllexport_inlines : Flag<["-"], "fno-dllexport-inlines">;
 def cfguard_no_checks : Flag<["-"], "cfguard-no-checks">,

clang/lib/CodeGen/BackendUtil.cpp

 #include "llvm/Transforms/Instrumentation/MemorySanitizer.h"
 #include "llvm/Transforms/Instrumentation/SanitizerCoverage.h"
 #include "llvm/Transforms/Instrumentation/ThreadSanitizer.h"
 #include "llvm/Transforms/ObjCARC.h"
 #include "llvm/Transforms/Scalar.h"
 #include "llvm/Transforms/Scalar/GVN.h"
 #include "llvm/Transforms/Utils.h"
 #include "llvm/Transforms/Utils/CanonicalizeAliases.h"
+#include "llvm/Transforms/Utils/Debugify.h"
 #include "llvm/Transforms/Utils/EntryExitInstrumenter.h"
 #include "llvm/Transforms/Utils/NameAnonGlobals.h"
 #include "llvm/Transforms/Utils/SymbolRewriter.h"
 #include "llvm/Transforms/Utils/UniqueInternalLinkageNames.h"
 #include <memory>
 using namespace clang;
 using namespace llvm;
 
if (TM->addPassesToEmitFile(CodeGenPasses, OS, DwoOS, CGFT,
                               /*DisableVerify=*/!CodeGenOpts.VerifyModule)) {
     Diags.Report(diag::err_fe_unable_to_interface_with_target);
     return false;
   }
 
   return true;
 }
 
+class ClangCustomPassManager : public legacy::PassManager {
+public:
+  void add(Pass *P) override {
+    bool WrapWithDebugifyOriginal = DebugifyEachOriginalEnabled &&
+                                    !P->getAsImmutablePass() &&
+                                    !isIRPrintingPass(P);
+    if (!WrapWithDebugifyOriginal) {
+      PassManager::add(P);
+      return;
+    }
+
+    PassKind Kind = P->getPassKind();
+    StringRef Name = P->getPassName();
+
+    // TODO: Implement Debugify for LoopPass.
+    switch (Kind) {
+    case PT_Function:
+      assert(Mode == DebugifyMode::OriginalDebugInfo &&
+             "Must be original mode");
+      PassManager::add(createDebugifyFunctionPass(DebugifyMode::OriginalDebugInfo,
+                                            Name, &DIPreservationMap));
+      PassManager::add(P);
+      PassManager::add(createCheckDebugifyFunctionPass(
+          false, Name, nullptr, DebugifyMode::OriginalDebugInfo,
+          &DIPreservationMap, getDebugifyEachOriginalPath()));
+      break;
+    case PT_Module:
+      assert(Mode == DebugifyMode::OriginalDebugInfo &&
+             "Must be original mode");
+      PassManager::add(createDebugifyModulePass(DebugifyMode::OriginalDebugInfo,
+                                          Name, &DIPreservationMap));
+      PassManager::add(P);
+      PassManager::add(createCheckDebugifyModulePass(
+          false, Name, nullptr, DebugifyMode::OriginalDebugInfo,
+          &DIPreservationMap, getDebugifyEachOriginalPath()));
+      break;
+    default:
+      PassManager::add(P);
+      break;
+    }
+  }
+
+  void enableDebugifyEachOriginal() {
+    DebugifyEachOriginalEnabled = true;
+  }
+
+  void setDIOriginalBugsReportFilePath(
+      StringRef DIOriginalBugsReportFilePath_) {
+    DIOriginalBugsReportFilePath = DIOriginalBugsReportFilePath_;
+  }
+
+  llvm::StringRef getDebugifyEachOriginalPath() const {
+    return DIOriginalBugsReportFilePath;
+  }
+
+private:
+  DebugInfoPerPassMap DIPreservationMap;
+  llvm::StringRef DIOriginalBugsReportFilePath = "";
+  bool DebugifyEachOriginalEnabled = false;
+};
+
 void EmitAssemblyHelper::EmitAssembly(BackendAction Action,
                                       std::unique_ptr<raw_pwrite_stream> OS) {
   TimeRegion Region(FrontendTimesIsEnabled ? &CodeGenerationTime : nullptr);
 
   setCommandLineOpts(CodeGenOpts);
 
   bool UsesCodeGen = (Action != Backend_EmitNothing &&
                       Action != Backend_EmitBC &&
                       Action != Backend_EmitLL);
   CreateTargetMachine(UsesCodeGen);
 
   if (UsesCodeGen && !TM)
     return;
   if (TM)
     TheModule->setDataLayout(TM->createDataLayout());
 
-  legacy::PassManager PerModulePasses;
+  ClangCustomPassManager PerModulePasses;
+  if (CodeGenOpts.EnableDebugifyEachOriginal) {
+    PerModulePasses.enableDebugifyEachOriginal();
+
+    if (!CodeGenOpts.DIOriginalBugsReportFilePath.empty())
+      PerModulePasses.setDIOriginalBugsReportFilePath(
+          CodeGenOpts.DIOriginalBugsReportFilePath);
+  }
   PerModulePasses.add(
       createTargetTransformInfoWrapperPass(getTargetIRAnalysis()));
 
   legacy::FunctionPassManager PerFunctionPasses(TheModule);
   PerFunctionPasses.add(
       createTargetTransformInfoWrapperPass(getTargetIRAnalysis()));
 
   CreatePasses(PerModulePasses, PerFunctionPasses);

clang/lib/Frontend/CompilerInvocation.cpp

const llvm::Triple::ArchType DebugEntryValueArchs[] = {
       llvm::Triple::arm, llvm::Triple::armeb, llvm::Triple::mips,
       llvm::Triple::mipsel, llvm::Triple::mips64, llvm::Triple::mips64el};
 
   llvm::Triple T(TargetOpts.Triple);
   if (Opts.OptimizationLevel > 0 && Opts.hasReducedDebugInfo() &&
       llvm::is_contained(DebugEntryValueArchs, T.getArch()))
     Opts.EmitCallSiteInfo = true;
 
+  Opts.EnableDebugifyEachOriginal =
+      Args.hasArg(OPT_fenable_debugify_each_original);
+  // Ignore the option if the -fenable-debugify-each-original wasn't enabled.
+  if (Opts.EnableDebugifyEachOriginal &&
+      Args.hasArg(OPT_fenable_debugify_original_export)) {
+      Opts.DIOriginalBugsReportFilePath =
+          std::string(
+              Args.getLastArgValue(OPT_fenable_debugify_original_export));
+  }
+
   Opts.DisableO0ImplyOptNone = Args.hasArg(OPT_disable_O0_optnone);
   Opts.DisableRedZone = Args.hasArg(OPT_disable_red_zone);
   Opts.IndirectTlsSegRefs = Args.hasArg(OPT_mno_tls_direct_seg_refs);
   Opts.ForbidGuardVariables = Args.hasArg(OPT_fforbid_guard_variables);
   Opts.UseRegisterSizedBitfieldAccess = Args.hasArg(
     OPT_fuse_register_sized_bitfield_access);
   Opts.RelaxedAliasing = Args.hasArg(OPT_relaxed_aliasing);
   Opts.StructPathTBAA = !Args.hasArg(OPT_no_struct_path_tbaa);

clang/test/DebugInfo/debugify-each-original.c

+// RUN: %clang -g -Xclang -fenable-debugify-each-original -emit-llvm -S \
+// RUN:     -O1 -o - %s 2>&1 | FileCheck %s
+
+int main()
+{
+  int x = 1;
+  int y = 2;
+  return x + y;
+}
+
+// CHECK: Force set function attributes: PASS
+// CHECK-NEXT: Infer set function attributes: PASS
+// CHECK-NEXT: Interprocedural Sparse Conditional Constant Propagation: PASS
+// CHECK-NEXT: Called Value Propagation: PASS
+// CHECK-NEXT: Global Variable Optimizer: PASS
+// CHECK-NEXT: Promote Memory to Register: PASS
+// CHECK-NEXT: Dead Argument Elimination: PASS
+// CHECK-NEXT: Combine redundant instructions: PASS
+// CHECK-NEXT: Simplify the CFG: PASS
+// CHECK-NEXT: Globals Alias Analysis: PASS
+// CHECK-NEXT: SROA: PASS
+// CHECK-NEXT: Early CSE w/ MemorySSA: PASS
+// CHECK-NEXT: Simplify the CFG: PASS
+// CHECK-NEXT: Combine redundant instructions: PASS
+// CHECK-NEXT: Conditionally eliminate dead library calls: PASS
+// CHECK-NEXT: PGOMemOPSize: PASS
+// CHECK-NEXT: Simplify the CFG: PASS
+// CHECK-NEXT: Reassociate expressions: PASS
+// CHECK-NEXT: Simplify the CFG: PASS
+// CHECK-NEXT: Combine redundant instructions: PASS
+// CHECK-NEXT: MemCpy Optimization: PASS
+// CHECK-NEXT: Sparse Conditional Constant Propagation: PASS
+// CHECK-NEXT: Bit-Tracking Dead Code Elimination: PASS
+// CHECK-NEXT: Combine redundant instructions: PASS
+// CHECK-NEXT: Aggressive Dead Code Elimination: PASS
+// CHECK-NEXT: Simplify the CFG: PASS
+// CHECK-NEXT: Combine redundant instructions: PASS
+// CHECK-NEXT: A No-Op Barrier Pass: PASS
+// CHECK-NEXT: Deduce function attributes in RPO: PASS
+// CHECK-NEXT: Global Variable Optimizer: PASS
+// CHECK-NEXT: Dead Global Elimination: PASS
+// CHECK-NEXT: Globals Alias Analysis: PASS
+// CHECK-NEXT: Float to int: PASS
+// CHECK-NEXT: Lower constant intrinsics: PASS
+// CHECK-NEXT: Loop Distribution: PASS
+// CHECK-NEXT: Loop Vectorization: PASS
+// CHECK-NEXT: Loop Load Elimination: PASS
+// CHECK-NEXT: Combine redundant instructions: PASS
+// CHECK-NEXT: Simplify the CFG: PASS
+// CHECK-NEXT: Optimize scalar/vector ops: PASS
+// CHECK-NEXT: Combine redundant instructions: PASS
+// CHECK-NEXT: Warn about non-applied transformations: PASS
+// CHECK-NEXT: Alignment from assumptions: PASS
+// CHECK-NEXT: Strip Unused Function Prototypes: PASS
+// CHECK-NEXT: Remove redundant instructions: PASS
+// CHECK-NEXT: Hoist/decompose integer division and remainder: PASS
+// CHECK-NEXT: Simplify the CFG: PASS

llvm/docs/HowToUpdateDebugInfo.rst

 
 and then use the ``llvm/utils/llvm-debugify-original.py`` script to generate
 an HTML page with the issues reported in a more human readable form as follows:
 
 .. code-block:: bash
 
   $ llvm-debugify-original.py sample.json sample.html
 
+The `original` mode can be invoked from front-end level as follows:
+
+.. code-block:: bash
+
+  # Test each pass.
+  $ clang -Xclang -fenable-debugify-each-original -g -O2 sample.c
+
+  # Test each pass and export the issues report into the JSON file.
+  $ clang -Xclang -fenable-debugify-each-original -Xclang -fenable-debugify-original-export=sample.json -g -O2 sample.c
+
 Using ``debugify``
 ^^^^^^^^^^^^^^^^^^
 
 A simple way to use ``debugify`` is as follows:
 
 .. code-block:: bash
 
   $ opt -debugify -pass-to-test -check-debugify sample.ll