LLVM C DIBuilder Creation APIs

[harlanhaskins]

Summary

This is an initial set of LLVM DIBuilder APIs that can create builders, compile units, files, and scopes, and introspect the debug info inside modules.

[harlanhaskins]

arc...picked some wrong branches here. I'll fix this diff 😅

[harlanhaskins]

Fix merge head

[harlanhaskins]

Fix 80-column limit for LLVMDWARFSourceLanguage

[harlanhaskins]

Add back remanining files (sorry for the email spam!)

[deadalnix]

Hi, you can put "HEAD^" in the file ".git/arc/default-relative-commit" , this ensure that only the comit you are currently on is sent to phabricator.

[whitequark]

lib/IR/DebugInfo.cpp

694 ↗

(On Diff #96208)

I believe our policy for all new LLVM-C code is to use pointer/length pairs for all array arguments, including strings.

[harlanhaskins]

lib/IR/DebugInfo.cpp

694 ↗

(On Diff #96208)

Ah! Yes, I had forgotten. Thanks!

[harlanhaskins]

• Fix 80-column limit for LLVMDWARFSourceLanguage
• Pass string parameters as pointer/length pairs

[deadalnix]

Ideally we'd like to have some test with this. The way it is usually done is to use the echo test: it reads a module using the C API and then recreate another identical module using the same C API. This ensure end to end testing. However this doesn't provide any reading facility so that me be more work than you are willing to put into this.

include/llvm-c/DebugInfo.h

63

Maybe having 2 function rather than a bool in the API that end up being a uint8_t ?

65

The doc should specify if it finalize or not when disposing.

90

DWOId yields notthing relevant on Google. Maybe this needs to be explained a bit more.

106

It hasn't been enforced consistently int he past, so that may not be immediately obvious, but we usually pass a pointer and a length for string in the newer API. This avoid useless calls to strlen and works even if the string isn't zero terminated which is a plus. Keep in mind that the C API is often used to bind to other languages, so while this may not be the most idiomatic thing to do in C, this ends up fitting our use case better.

lib/IR/DebugInfo.cpp

668 ↗

(On Diff #96208)

There is a facility to unwrap subclasses. You can see LLVMValueRef for how it is done. You shouldn't need to write this.

[harlanhaskins]

include/llvm-c/DebugInfo.h

65

This begs the question: Should it just call finalize itself?

106

This has been fixed in a later revision 👍

lib/IR/DebugInfo.cpp

668 ↗

(On Diff #96208)

Thanks! I was going off of the Rust implementation initially.

[harlanhaskins]

include/llvm-c/DebugInfo.h

90

I'll be completely honest here, this has stumped me for a while. I don't think there's even a facility within the wrappers to create a "split skeleton" compile unit...

[aprantl]

include/llvm-c/DebugInfo.h

10

Can you doxygenify this comment, too?

52

We have autobrief enabled in the doxygen configuration. Please don't use \brief any more.

[aprantl]

Could you also add some unit tests for the new interface?

[Wallbraker]

Hey cool that you are doing this work, finally!

See inline comments (also click the done tag on the comments when those are fixed, please).

Regarding the tests, only since this API is only about creating debug info I'm okay with just a creation test, this API already have a special status.

include/llvm-c/DebugInfo.h

106

Please use size_t for the the string sizes, no need to go to 64bit on 32bit systems.

[harlanhaskins]

• Fix 80-column limit for LLVMDWARFSourceLanguage
• Address comments

[harlanhaskins]

• uint64_t -> size_t for string lengths

[harlanhaskins]

• Add simple debuginfo tests

[harlanhaskins]

• Fix unused variable warning

[deadalnix]

This is looking good and I like where this is going. I have a few nits in comments.

Where I'm more doubtful is for the test. The way we've tested the C API in the past is by reading a module and then re-emiting a clone of it on the output. This patch doesn't have the ability to read debug infos, so that's a bit compromised. One way forward is obviously to add reading support for the elements we add emission support from. I'm not sure how much work that would be, so I'm not sure how practical this is within the scope of that diff. If that's too hard then an alternative should be chosen, like emitting a given module and checking its output. This is where this patch is going, however, it doesn't looks like this patch actually checks the output so that's a problem.

include/llvm-c/DebugInfo.h

30

These id needs to persist across versions, and this is not ensuring this. You can look at other enums in Core.h to see how it is done. The basic idea is to list all the enum values here, and then use the macro option to generate the mapping between exposed values and internal llvm values.

57

Please use LLVMBool

108

LLVMBool for flags.

[whitequark]

include/llvm-c/DebugInfo.h

103

size_t ProducerLen

104

LLVMBool IsOptimized

107

Are these last two also booleans? LLVMBool too then

128

What's the value for "line/column not available"?

lib/IR/DebugInfo.cpp

708 ↗

(On Diff #99612)

Ditto

709 ↗

(On Diff #99612)

LLVMBool IsOptimized

[aprantl]

(Having unit tests will avoid

include/llvm-c/DebugInfo.h

128

Compiler-generated locations (which are different from empty locations) are using line 0.

[harlanhaskins]

• uint8_t -> LLVMBool for flags
• ProducerLen should be size_t

[harlanhaskins]

include/llvm-c/DebugInfo.h

128

Should this be documented?

[harlanhaskins]

• Fix 80-column limit for LLVMDWARFSourceLanguage
• Address comments
• uint64_t -> size_t for string lengths
• Add simple debuginfo tests
• Fix unused variable warning
• uint8_t -> LLVMBool for flags
• ProducerLen should be size_t
• Add test and update CMakeLists to build debuginfo.c

[whitequark]

include/llvm-c/DebugInfo.h

50

Why is LLVMDebugMetadataVersion returning uint32_t and LLVMGetModuleDebugMetadataVersion returning unsigned?

[harlanhaskins]

include/llvm-c/DebugInfo.h

50

Just because of the types of the things they call into. Should they both be unsigned?

[deadalnix]

include/llvm-c/DebugInfo.h

50

I think it is better to use well defined types in the C API, so other language are not exposed to the C madness where nobody knows what an integral really is. It wasn't done consistently in the API in the past, but I think this is a good habit we should take.

[whitequark]

include/llvm-c/DebugInfo.h

50

That works

50

I disagree. Anyone building a binding already has to know what unsigned is, and using uint32_t here just makes it easier to accidentally break ABI.

[deadalnix]

include/llvm-c/DebugInfo.h

50

Can you elaborate ?

[whitequark]

include/llvm-c/DebugInfo.h

50

The C API uses unsigned practically everywhere, and that's not changing, so anything that binds to it will have to know what ABI width unsigned has. And having some functions use unsigned and some uint32_t for parameters that have the same underlying semantics is just a recipe for a footgun when going between 32 and 64-bit builds...

[deadalnix]

include/llvm-c/DebugInfo.h

50

This 32 bits on windows, linux and OSX both in 32bits build and 64bits builds. It is 64bits on solaris and some other obscures UNIXes. I think it's very much worth it moving everything to uint32_t (unless we have a lot of solaris 64bits users of the C API).

[whitequark]

include/llvm-c/DebugInfo.h

50

1. This should be done in a separate patch.
2. I am not as convinced as you that it is completely safe and I think this should be discussed on ML. This has the potential to screw binding authors quite badly if it turns out that they *do* indeed have to support those systems.

[deadalnix]

include/llvm-c/DebugInfo.h

50

I won't block the diff for this. unsigned it is then.

[harlanhaskins]

• Fix 80-column limit for LLVMDWARFSourceLanguage
• Address comments
• uint64_t -> size_t for string lengths
• Add simple debuginfo tests
• Fix unused variable warning
• uint8_t -> LLVMBool for flags
• ProducerLen should be size_t
• Add test and update CMakeLists to build debuginfo.c
• Fix test
• Use unsigned instead of explicitly sized unsigned integer types

[harlanhaskins]

include/llvm-c/DebugInfo.h

128

Really, the question is what _is_ the canonical value? Should we explicitly state 0 for line/column in the documentation or let implementers decide their sentinels?

[deadalnix]

I have one more comment. It's looking good.

include/llvm-c/DebugInfo.h

30

These 2 needs to have stable values. You can look in Core.h how it is done for instruction types for instance.

[harlanhaskins]

include/llvm-c/DebugInfo.h

30

I'm still unsure about this. Should they mirror the values defined in DINode::DIFlags? What happens if that gains or loses a case? What happens if their raw bit mask values change?

[echristo]

include/llvm-c/DebugInfo.h

128

From the dwarf standard:
"The compiler may emit the value 0 in cases where
an instruction cannot be attributed to any source
line."
So yes, that's the canonical "I have no idea where this is" value.

[harlanhaskins]

include/llvm-c/DebugInfo.h

128

Thank you! Alright, I'll add that to the comment..

[harlanhaskins]

• Add comment explaining 0 for debug locations

[deadalnix]

include/llvm-c/DebugInfo.h

30

The way it is done in the C API is that you make the enum in C without using macro codegen. Then you write a function that internally map the C enum value to the C++ enum value. To do so, it uses a switch which cases are generated with the macro codegen technique. The the API implementation uses this function internally.

See https://reviews.llvm.org/diffusion/L/browse/llvm/trunk/lib/IR/Core.cpp;306068$1085-1103

[CodaFi]

include/llvm-c/DebugInfo.h

30

Are you sure this is a good idea? For the other enums, this makes sense because they have values that aren't subject to change and have a manageable set of cases (less than 20). DWARF is going to continue to evolve over the years and LLVM's coverage of the API includes 96 tags and 200+ attributes, each of which needs to be handled by the requested mapping functions. This doesn't seem like a reasonable thing to try to maintain by hand.

[harlanhaskins]

• Added explicit enum definitions for C API DWARF values (Change made by @CodaFi)

[whitequark]

LGTM. @deadalnix?

[aprantl]

include/llvm-c/DebugInfo.h

30

I get that we don't want macros in the installed header files to make it easier for FFIs and other interface generators, but why can't we run the preprocessor on the file as a custom build step?

[harlanhaskins]

@whitequark Think this is good to merge in its current state? I have another patch for more functions Moreno or less ready to go once this is in.

include/llvm-c/DebugInfo.h

30

So I've updated this revision to explicitly lay out these enums and provide a mapping. Thoughts, @deadalnix?

[whitequark]

Sure, go ahead and commit.

[harlanhaskins]

I don't have commit access -- would you mind merging it, @whitequark?

[whitequark]

Please rebase the patch, it doesn't apply as-is.

[harlanhaskins]

wq - Fix 80-column limit for LLVMDWARFSourceLanguage

• Address comments
• uint64_t -> size_t for string lengths
• Add simple debuginfo tests
• Fix unused variable warning
• uint8_t -> LLVMBool for flags
• ProducerLen should be size_t
• Add test and update CMakeLists to build debuginfo.c
• Fix test
• Use unsigned instead of explicitly sized unsigned integer types
• Add comment explaining 0 for debug locations
• Expand enum definitions and translate to DI (#1)

[harlanhaskins]

Changes rebased on master.

include/llvm-c/DebugInfo.h

 //===------------ DebugInfo.h - LLVM C API Debug Info API -----------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file declares the C API endpoints for generating DWARF Debug Info
 //
 // Note: This interface is experimental. It is *NOT* stable, and may be
 //       changed without warning.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm-c/Core.h"
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
 /// Debug info flags.
 typedef enum {
 #define HANDLE_DI_FLAG(ID, NAME) LLVMDIFlag##NAME = ID,
     #include "llvm/IR/DebugInfoFlags.def"
     LLVMDIFlagAccessibility = LLVMDIFlagPrivate
                             | LLVMDIFlagProtected
                             | LLVMDIFlagPublic
 } LLVMDIFlags;
 
 /// Source languages known by DWARF.
 typedef enum {
-#define HANDLE_DW_LANG(ID, NAME, VERSION, VENDOR) LLVMDWARFSourceLanguage##NAME = ID,
+#define HANDLE_DW_LANG(ID, NAME, VERSION, VENDOR) \
+    LLVMDWARFSourceLanguage##NAME = ID,
     #include "llvm/Support/Dwarf.def"
 } LLVMDWARFSourceLanguage;
 
 /// The amount of debug information to emit.
 typedef enum {
     LLVMDWARFEmissionNone = 0,
     LLVMDWARFEmissionFull,
     LLVMDWARFEmissionLineTablesOnly
 } LLVMDWARFEmissionKind;
 
 /// The current debug metadata version number.
 uint32_t LLVMDebugMetadataVersion();
 
 /// The version of debug metadata that's present in the provided \c Module.
 unsigned LLVMGetModuleDebugMetadataVersion(LLVMModuleRef Module);
 
 /// \brief Strip debug info in the module if it exists.
 ///
 /// To do this, we remove all calls to the debugger intrinsics and any named
 /// metadata for debugging. We also remove debug locations for instructions.
 /// Return true if module is modified.
 uint8_t LLVMStripModuleDebugInfo(LLVMModuleRef Module);
 
 /// Construct a builder for a module.
 ///
 /// If \c AllowUnresolved, collect unresolved nodes attached to the module
 /// in order to resolve cycles during a call to \c LLVMDIBuilderFinalize.
 LLVMDIBuilderRef LLVMDIBuilderCreate(LLVMModuleRef M, uint8_t AllowUnresolved);
 
 /// Deallocates the DIBuilder and everything it owns.
 void LLVMDIBuilderDispose(LLVMDIBuilderRef Builder);
 
 /// Construct any deferred debug info descriptors.
 void LLVMDIBuilderFinalize(LLVMDIBuilderRef Builder);
 
 /// A CompileUnit provides an anchor for all debugging
 /// information generated during this instance of compilation.
 /// \param Lang          Source programming language, eg.
 ///                      \c LLVMDWARFSourceLanguageC99
 /// \param File          File info.
 /// \param Producer      Identify the producer of debugging information
 ///                      and code.  Usually this is a compiler
 ///                      version string.
 /// \param isOptimized   A boolean flag which indicates whether optimization
 ///                      is enabled or not.
 /// \param Flags         This string lists command line options. This
 ///                      string is directly embedded in debug info
 ///                      output which may be used by a tool
 ///                      analyzing generated debugging information.
 /// \param RuntimeVer    This indicates runtime version for languages like
 ///                      Objective-C.
 /// \param SplitName     The name of the file that we'll split debug info
 ///                      out into.
 /// \param Kind          The kind of debug information to generate.
 /// \param DWOId         The DWOId if this is a split skeleton compile unit.
 /// \param SplitDebugInlining    Whether to emit inline debug info.
 /// \param DebugInfoForProfiling Whether to emit extra debug info for
 ///                              profile collection.
 LLVMMetadataRef LLVMDIBuilderCreateCompileUnit(
     LLVMDIBuilderRef Builder, LLVMDWARFSourceLanguage Lang,
     LLVMMetadataRef FileRef, const char *Producer, uint8_t isOptimized,
     const char *Flags, unsigned RuntimeVer, const char *SplitName,
     LLVMDWARFEmissionKind Kind, uint64_t DWOId, uint8_t SplitDebugInlining,
     uint8_t DebugInfoForProfiling);
 
 /// Create a file descriptor to hold debugging information for a file.
 /// \param Builder   The DIBuilder.
 /// \param Filename  File name.
 /// \param Directory Directory.
 LLVMMetadataRef
 LLVMDIBuilderCreateFile(LLVMDIBuilderRef Builder, const char *Filename,
                         const char *Directory);
 
 /// Creates a new DebugLocation that describes a source location.
 /// \param Line The line in the source file.
 /// \param Column The column in the source file.
 /// \param Scope The scope in which the location resides.
 /// \param InlinedAt The scope where this location was inlined, if at all.
 ///                  (optional).
 LLVMMetadataRef
 LLVMDIBuilderCreateDebugLocation(LLVMContextRef Ctx, unsigned Line,
                                  unsigned Column, LLVMMetadataRef Scope,
                                  LLVMMetadataRef InlinedAt);
 
 #ifdef __cplusplus
 } // end extern "C"
 #endif