Is it possible to encapsulate the target-specific logic into some derived classes from {{MCStreamer}} so that {{MCStreamer}} itself and DWARF emitter classes work through a common interface and have no target-specific adjustments?

I suppose that you describe how .dwsect pseudo-op works. That is quite interesting, why they designed the feature to work that way. Is it recommended to reference debug sections through the label minus the length field size (4 or 12) or they provide some means to simplify the calculation? How an assembler output of their own compiler looks like?

Trying to force CI to run.

• Updated as suggested. Thanks, @ldionne!

The current code violates the C++ standard, which says (in different sections, depending on the version):

It looks like lld/test/COFF/lto-new-pass-manager.ll.obj was added to the patch by accident and should be removed.

Well, this is a bit different from my original idea but is an overall good heuristic for many of the debug sections. It works for .debug_info, which is one of the biggest sections; it does not work for .debug_line, though, which is not that big as .debug_info, but potentially might become problematic in the (distant) future; it also does not work for .debug_abbrev, .debug_addr, .debug_ranges, and some others, which are usually not that big. However, the patch should be extended to support .debug_str, which can be even larger than .debug_info.

In D87011#2378347, @jhenderson wrote:

At least in LLD, it's not quite as simple as being added after the user's code: if a library appears on the link line it will be included in the output order as soon as it is determined it is needed. Thus if you have have three modules 1.o, 2.o, and 3.o, with 3.o in an archive 3.a and 1.o requiring 3.o, you end up with an output order of 1.o 3.o 2.o if the input order was 1.o 3.a 2.o or 3.a 1.o 2.o or an output order of 1.o 2.o 3.o if the input order was 1.o 2.o 3.a. In fact, with use of the --undefined linker switch, you can even force 3.o to appear first.

I accept using --undefined or rearranging the command-line order is less than ideal, but I'm really not convinced LLD should have any place in parsing the DWARF to determine output order. Furthermore, it's not even a reliable solution - if the objects built with DWARF32 (potentially all of which might have come from libraries) are large enough, no amount of reordering will fix the behaviour. I think users who need DWARF64 in their libraries are just going to have to request DWARF64 versions of the libraries, if the --undefined and reordering command line options are insufficient.

In D87011#2378224, @jhenderson wrote:

I wouldn't expect LLD to need to do anything specific for DWARF64 support. It should generally speaking be treating the sections as opaque, in my opinion, and treat them no differently to other sections. If a user is mixing DWARF32 and DWARF64, then I'd say it's on their heads if relocations can't reach (just the same as it is if they're using DWARF32 but really need DWARF64). Any interactions LLD does have with the contents of the DWARF sections should be controlled via the DebugInfo library, and therefore if that library works for DWARF64, LLD doesn't need any special handling.

1. The patch needs tests to check the added functionality.
2. DWARF64 can be generated only for a limited number of targets. There should be diagnostics for invalid switch combinations to prevent misuse. @MaskRay mentioned that in the patch for llc, D87011#2254749, but that makes a lot more sense for user-level tools.

I suppose that it would be helpful to arrange debugging information sections so that DWARF64 comes after DWARF32, otherwise, some 32-bit relocations in the 32-bit info could not be resolved. But that idea might be a bit controversial because usually debugging information is expected to have the same order as the sections it refers to.

To generate 64-bit debugging info, there should be enough to pass the switch through CLANG, right. Apart from that, we will probably need some compatibility checks so that using the switch in unsupported cases prints out diagnostics. There are also some improvements on the LLD side which are better to be done to support extremely large debugging information. Not all our tools fully support DWARF64 yet, etc.

The switch is implemented only internally in LLVM. There is still some work to be done to enable producing 64-bit debugging info in clang, but I strayed a bit for another task. Hope to come back later this year.

In D87011#2357977, @ayermolo wrote:

@ikudrin To clarify this will emit R_X86_64_64 bit relocations for .debug_info on 64 bit platform, correct?

In D88877#2350507, @MaskRay wrote:

I am still concerned that we go down this route (workaround in the linker rather than teach the user to add --no-whole-archive just because of convenience adding logic in the linker). Among --push-state pushed states, --as-needed and -Bstatic may be states users want to keep for the dependent libraries.

• Updated the test as suggested. Thanks, @bd1976llvm!

In D88877#2342920, @bd1976llvm wrote:

I think I agree that build systems might not easily be able to arrange for appending options to the end of the linker command line. Therefore, for the "dependent libraries" feature (I changed the name in https://reviews.llvm.org/D60274 because autolinking was too general) it would be reasonable to reset any temporal options that the build system would otherwise have to handle by appending an argument to the very end of the command line (e.g. --whole-archives). Where appropriate we can create new order-independent options to apply the options that are reset to the dependent libraries. This seems like an improvement over what we have now.

Hmm, that starts looking like a discussion for the mailing list, but I guess that all interested people are already here and the conversation will be better preserved for further references if being linked to the review.

In D88877#2331416, @bd1976llvm wrote:

Firstly, is this a real problem?

Gently ping. Is there anything I can improve in the fix so that it is accepted?

I've made some tests and speculatively estimate the saving up to several seconds for a relatively large library (about 100000 symbols) which is referenced several hundred times. For me, that is noticeable.

In D88944#2316011, @MaskRay wrote:

I am skeptical about the change.

• The duplicated libraries are usually indicator of user errors or build system brittleness.

• Simplified the comment.

• Fixed the comment.

D87008 added creating a test AsmPrinter to the CodeGen unit tests, so it is now possible to fix the DIEHashTest.MemberBlock so that it provides an instance to the tested class.

In D87021#2267059, @dblaikie wrote:

The test sources were based on some simple code, but then they were cleared as much as I could.

You mean the IR was modified after compilation? Or that the original C source was simplified as much as possible?

In D87009#2266648, @dblaikie wrote:

Let's take, for example, DIEExpr. As far as I can find, with DW_FORM_sec_offset it is used only in the DebugInfo/DWARF unit tests, in dwarfgen::DIE::addStrOffsetsBaseAttribute(). If we took the approach with end-to-end functional testing, this class would not be updated. And the path is not simple to track, by the way.

API only intended for unit tests would be tested/updated with unit tests.

Ping (for this and other not yet accepted revisions in the stack).

• Relaxed checking in the test. The line is a bit longer than I suggested because the test has to validate emitting the abbr_offset field.

• Updated the test as suggested. Thanks!

In D87009#2255174, @dblaikie wrote:

In D87009#2253973, @ikudrin wrote:

That would require to update a particular method several times and potentially would leave it in a partially updated state because some code paths are hard to reach.

Could you explain this in more detail?

• Added reporting a fatal error if the generated debug info exceeds the limits supported by the format.