Page MenuHomePhabricator

lldProject
ActivePublic

Watchers

  • This project does not have any watchers.

Details

Description

LLVM Linker

Recent Activity

Yesterday

ruiu added a comment to D54747: Discard debuginfo for object files empty after GC.

I committed https://reviews.llvm.org/D59800 which I believe makes your change easier to follow once rebased. Could you rebase it and upload a patch? Thanks!

Mon, Mar 25, 4:30 PM · Restricted Project, lld
ruiu added inline comments to D54747: Discard debuginfo for object files empty after GC.
Mon, Mar 25, 1:20 PM · Restricted Project, lld
rocallahan added inline comments to D54747: Discard debuginfo for object files empty after GC.
Mon, Mar 25, 1:07 PM · Restricted Project, lld
ruiu added inline comments to D54747: Discard debuginfo for object files empty after GC.
Mon, Mar 25, 11:36 AM · Restricted Project, lld
ruiu added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

Please see https://reviews.llvm.org/D59780.

Mon, Mar 25, 10:22 AM · Restricted Project, lld
ruiu added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

As I said I was working on refining this patch. Actually it turned out that it was more like a complete rewrite than a refinement, as this patch was not aligned very well with the existing lld codebase and also have several performance issues. I'll post a patch shortly.

Mon, Mar 25, 9:57 AM · Restricted Project, lld
xiangzhangllvm updated the diff for D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

Update test to check splt's context

Mon, Mar 25, 1:29 AM · Restricted Project, lld

Sun, Mar 24

rocallahan added a comment to D54747: Discard debuginfo for object files empty after GC.

Another clarification:

DWARF type information currently shared between functions in the same CU would have to be duplicated.

Sun, Mar 24, 6:37 PM · Restricted Project, lld
rocallahan added a comment to D54747: Discard debuginfo for object files empty after GC.

To be clear, I think the best long-term solution is for LLD to rewrite the DWARF, but from my (admittedly limited) perspective that seems to be at best a distant prospect.

Sun, Mar 24, 6:30 PM · Restricted Project, lld
rocallahan added a comment to D54747: Discard debuginfo for object files empty after GC.

Do you have numbers with ld.bfd and gold?

Sun, Mar 24, 6:26 PM · Restricted Project, lld
rocallahan added a comment to D54747: Discard debuginfo for object files empty after GC.

@rocallahan I find that people are discussing a generic approach in D59553

Sun, Mar 24, 6:25 PM · Restricted Project, lld

Fri, Mar 22

grimar added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

I have no more comments, this looks fine to me now.
A minor nit is inline.

Fri, Mar 22, 11:00 AM · lld, Restricted Project
avl updated the diff for D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

updated tests as requested.

Fri, Mar 22, 10:43 AM · lld, Restricted Project
MaskRay added a comment to D54747: Discard debuginfo for object files empty after GC.

@rocallahan I find that people are discussing a generic approach in D59553

Fri, Mar 22, 8:24 AM · Restricted Project, lld
avl added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

@bd1976llvm

What does GDB make of -1/-2???

Fri, Mar 22, 6:59 AM · lld, Restricted Project
grimar added inline comments to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.
Fri, Mar 22, 4:10 AM · lld, Restricted Project
grimar added inline comments to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.
Fri, Mar 22, 3:57 AM · lld, Restricted Project
bd1976llvm added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

<Trying again via phabricator rather than replying on the list.>

Fri, Mar 22, 3:57 AM · lld, Restricted Project
probinson added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

@jhenderson and I did a prototype of DWARF-unit-per-function last year, and I was not favorably impressed by the numbers. But maybe he still has the actual data kicking around somewhere. We did not solve all the issues to our satisfaction before we ran out of time.
I've also had chats with @bd1976llvm about fragmenting .debug_info per-function without wrapping everything in units; you get into requiring some section-order things but it saves the unit-per-function overhead.

This is pretty much what I was talking about. You definitely don't need to do it as units if you're just including the concrete function. FWIW this is what I was talking about in the dwarf meeting where we talked about bringing units back into debug_info :)

Fri, Mar 22, 3:57 AM · lld, Restricted Project
jhenderson added inline comments to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.
Fri, Mar 22, 3:49 AM · lld, Restricted Project
grimar added inline comments to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.
Fri, Mar 22, 3:40 AM · lld, Restricted Project
avl updated the diff for D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

updated tests and addressed comments.

Fri, Mar 22, 3:31 AM · lld, Restricted Project

Thu, Mar 21

xiangzhangllvm updated the diff for D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

rebase

Thu, Mar 21, 8:37 PM · Restricted Project, lld
xiangzhangllvm added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

The update:

Thu, Mar 21, 7:56 PM · Restricted Project, lld
xiangzhangllvm updated the diff for D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

Hi peter, this update will remove redundant .splt in your upper ifunc test.
The reason of redundant .splt is that we scan the IFUNC symbol after we collected the CET feature. So the splt was created first and without being removed after we found IFUNC.
Anyway this patch is just evade ifunc, I planed to hand ifunc+cet after this patch.
Thank your test! thank you too

Thu, Mar 21, 7:46 PM · Restricted Project, lld
ruiu added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

Looks like the test files are huge. I order to test this functionality alone, you don't need a small file containing one relocation pointing to a to-be-discarded section. You don't really have to test it against actual data.

Thu, Mar 21, 2:28 PM · lld, Restricted Project
ruiu added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

Looks like the test files are huge. I order to test this functionality alone, you don't need a small file containing one relocation pointing to a to-be-discarded section. You don't really have to test it against actual data.

Thu, Mar 21, 2:25 PM · lld, Restricted Project
echristo added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

Oh, and as far as dwarf aware linking I'd arguably prefer a separate tool ala dsymutil or dwz rather than incorporating it into the linker. I do agree that such a tool is incredibly useful and should be part of our ongoing development plan around debug info.

Thu, Mar 21, 2:20 PM · lld, Restricted Project
echristo added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

@jhenderson and I did a prototype of DWARF-unit-per-function last year, and I was not favorably impressed by the numbers. But maybe he still has the actual data kicking around somewhere. We did not solve all the issues to our satisfaction before we ran out of time.
I've also had chats with @bd1976llvm about fragmenting .debug_info per-function without wrapping everything in units; you get into requiring some section-order things but it saves the unit-per-function overhead.

Thu, Mar 21, 2:17 PM · lld, Restricted Project
avl updated the diff for D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

addressed comments. added tests for 32 bit.

Thu, Mar 21, 7:39 AM · lld, Restricted Project
jrtc27 added a comment to D39324: [lld] Support TLS in RISC-V.

@PkmX Ping, any chance you could address the remaining few points, please?

Thu, Mar 21, 6:06 AM · lld
jrtc27 added a comment to D39323: [lld] Support dynamic linking in RISC-V.

@PkmX Ping, any chance you could address the remaining few points, please?

Thu, Mar 21, 6:06 AM · lld
probinson added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

This is straying far enough from the original review that maybe a dev thread is a better place to discuss "how to strip per-function debug info along with the functions."

Thu, Mar 21, 4:30 AM · lld, Restricted Project
probinson added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

@jhenderson and I did a prototype of DWARF-unit-per-function last year, and I was not favorably impressed by the numbers. But maybe he still has the actual data kicking around somewhere. We did not solve all the issues to our satisfaction before we ran out of time.
I've also had chats with @bd1976llvm about fragmenting .debug_info per-function without wrapping everything in units; you get into requiring some section-order things but it saves the unit-per-function overhead.

Thu, Mar 21, 4:29 AM · lld, Restricted Project
xiangzhangllvm updated the diff for D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

Bug fix for 32-bit, the Desc size is not same between 32-bit and 64-bit

Thu, Mar 21, 3:53 AM · Restricted Project, lld
bd1976llvm added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

Does this do the right thing for a 32-bit target? 64-bit is common, but not universal.

Re. the underlying DWARF issues:
I can't remember whether the DWARF committee has ever considered specifying a special value for "address does not exist" but I doubt it, because the DWARF spec states "If an entity has no associated machine code, none of these [address-related] attributes are specified." (DWARF v5 section 2.17, p.51.) So, the current position of the DWARF spec is that the linker is supposed to rewrite the DWARF, I guess.

I don't think that's the intent of the DWARF standard/committee - it's been pretty careful about designing features that work with existing linker behavior without requiring the linker to be DWARF aware in any way.

Likely the best way to solve this would be to put the debug information for the rest of it in the same section group as the function. In particular the subprogram die for the concrete function should be in a section group along with the function. This was part of the considerations with bringing other things back into the debug_info section in dwarf5 as well.

I'd worry about the overhead of making hunks of standalone DIEs for every function. (though, that said - we do actually use 4 bytes for both a CU-relative offset, and for a sec_offset, I think - so maybe it doesn't matter /that/ much (adding a unit header would be the main cost - and maybe that's significant enough to be problematic size-wise too)).

Currently linkers don't eliminate debug info for garbage-collected sections, so the debug info for exectuables/shared objects are larger than necessary, so you could save bytes of the final output by using more bytes for intermediate object files. That doesn't seem like a bad deal to me.

Thu, Mar 21, 3:26 AM · lld, Restricted Project
jhenderson added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

Looking at all the cases where addresses to sections that aren't allocated (& this happens not just in gc-sections, but also with inline functions (the DWARF for one copy of an inline function can't point to another copy, so its addresses become zero currently, etc) which is a more broadly applicable problem (though I suppose without gc-sections a CU's address range can't be empty, because it can't consist of only inline functions - but it does apply to the high/low pc of the subprogram DIE at least))

Worth noting that a CU could consist of just COMDAT functions though, so we still need to be aware of it. Also COMDATs mean that whatever we do here is beneficial even without --gc-sections.

Thu, Mar 21, 3:06 AM · lld, Restricted Project
grimar added inline comments to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.
Thu, Mar 21, 2:50 AM · lld, Restricted Project
avl added inline comments to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.
Thu, Mar 21, 2:43 AM · lld, Restricted Project
peter.smith added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

I've checked that it does work, although at the moment it does leave a redundant .splt section in the binary. I think that this can be removed by accounting for this in the SPltSection::empty(), .........

It shouldn't have redundant .splt section, there should not have any .splt section if IBT is disable or non-dynamic link. Could you show me your test please?

Thu, Mar 21, 2:43 AM · Restricted Project, lld
avl added inline comments to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.
Thu, Mar 21, 2:34 AM · lld, Restricted Project
grimar added inline comments to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.
Thu, Mar 21, 2:14 AM · lld, Restricted Project
avl added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

Does this do the right thing for a 32-bit target? 64-bit is common, but not universal.

Thu, Mar 21, 2:09 AM · lld, Restricted Project
MaskRay added a comment to D54747: Discard debuginfo for object files empty after GC.

Here are some results for the rusoto test in https://github.com/rust-lang/rust/issues/56068#issue-382175735:

LLDBinary size in bytes
LLD 6.0.143,791,192
LLD 8.0.0 1cfcf404f6b23943405bc3c5bb5940b10b91462443,861,056
LLD 8.0.0 1cfcf404f6b23943405bc3c5bb5940b10b914624 --start-lib43,844,760
LLD 8.0.0 1cfcf404f6b23943405bc3c5bb5940b10b914624 + this patch6,281,488

For --start-lib I wrapped --start-lib/--end-lib around all object files.

As I expected --start-lib didn't make much difference. The dependencies containing debuginfo that I want to be GCed away are already packaged into static libraries before being passed to the final link.

Thu, Mar 21, 1:07 AM · Restricted Project, lld
MaskRay added inline comments to D54747: Discard debuginfo for object files empty after GC.
Thu, Mar 21, 1:01 AM · Restricted Project, lld

Wed, Mar 20

Herald added a project to D54747: Discard debuginfo for object files empty after GC: Restricted Project.

Ping?

Wed, Mar 20, 10:31 PM · Restricted Project, lld
xiangzhangllvm added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

I've checked that it does work, although at the moment it does leave a redundant .splt section in the binary. I think that this can be removed by accounting for this in the SPltSection::empty(), .........

Wed, Mar 20, 6:45 PM · Restricted Project, lld
xiangzhangllvm added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

I'm having problems getting this to compile on at least GCC 5.4.0 on Linux:

lld/ELF/SyntheticSections.cpp:3104:28: warning: ISO C++ forbids zero-size array ‘Desc’ [-Wpedantic]
   GnuPropertyDescType Desc[]

lld/ELF/SyntheticSections.cpp:3110:55: error: invalid conversion from ‘llvm::cast_retty<GnuPropertyType, const unsigned int*>::ret_type {aka const GnuPropertyType*}’ to ‘GnuPropertyType*’ [-fpermissive]
   GnuPropertyType *GnuProperty = cast<GnuPropertyType>(Data.data());

The cast<> is an LLVM cast http://llvm.org/docs/ProgrammersManual.html#the-isa-cast-and-dyn-cast-templates and isn't something you can do on an arbitrary struct.

Assuming (x86) that your input data is always little endian then you'll need to use something like ulittle32_t instead of uint32_t or your tests will fail when run on a big-endian machine like PPC or some Mips machines.

Wed, Mar 20, 5:40 PM · Restricted Project, lld
ruiu added a comment to D58102: Support X86 Control-flow Enforcement Technology (CET) in LLD.

Overall this patch still look very different from the existing lld code base and does many things by hand without using LLVM libObject. You really need to read the existing code first and then try to mimic what the existing code is doing. What this patch is trying to do is not that new -- your patch should and could follow the existing patterns that I believe not too hard to find by reading existing code.

It is perhaps faster to actually show how it should look like than pointing out all the details in the review thread, so I'll apply your patch locally, modify that, and then upload it so that you can read it.

Hi, ruiu, I'll try to read them and let the code style like the existing lld code, I think the most un-beautiful code you sought is in the findGNUPropertyX86Feature1AND() function, here is too much details about the ABI, and I get the features in my way.

But I am not quite understanding your following review:

    In.GnuProperty = make<GnuPropertySection>();
    Add(In.GnuProperty);
ruiu:  I don't think you need this variable.

Did you mean I should not use the GnuPropertySection (SyntheticSection Class) to generate the output .note.gnu.property section? It seems a little different with peter's suggestion.

Wed, Mar 20, 3:49 PM · Restricted Project, lld
ruiu added inline comments to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.
Wed, Mar 20, 3:17 PM · lld, Restricted Project