Page MenuHomePhabricator

lldProject
ActivePublic

Watchers

  • This project does not have any watchers.

Details

Description

LLVM Linker

Recent Activity

Thu, Jul 11

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

Thank you, I apologize for impatience.

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

I am following, I've just been out for 3 weeks. I'm catching up now and this is in my queue.

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

I'm fine with this patch. We've discussed a lot about what is a desirable value for a "null" for debug info, but looks like as long as it does not appear as a valid address, it should be fine.

Is Eric fine with this?

Thu, Jul 11, 2:17 PM · lld, Restricted Project

Mon, Jul 8

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

Fri, Jul 5

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

Issues I noticed, some of which are possibly bugs, but it's hard to tell:

  • llvm-symbolizer reports the line number corresponding to the removed foo for the fourth case.
  • GNU addr2line line numbers are wrong in the first three cases, and foo is reported in the fourth.
  • GNU addr2line thinks foo is the right symbol for the fourth case, even though it has been removed. llvm-symbolizer just prints '??' (that's probably correct).

    I haven't checked to see how many of these issues are fixed by D60470.
Fri, Jul 5, 6:50 AM · lld, Restricted Project
avl updated the diff for D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

removed whitespaces, added handling x86 ILP32 case.

Fri, Jul 5, 5:37 AM · lld, Restricted Project
thakis added inline comments to D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.
Fri, Jul 5, 4:28 AM · Restricted Project, lld
jhenderson added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

I disagree. For PIE output, 0 (__ehdr_start) points to the ELF header. It cannot be a valid code address. Note, the discussion is really about code addresses (DW_AT_low_pc, DW_AT_ranges, DW_AT_entry_pc, etc), not an unconstrained relocated file offset.

Not if the user is using linker scripts:

/* test.script */
PHDRS
{
    ph_load PT_LOAD FLAGS(0x1|0x2|0x4);
}
Fri, Jul 5, 2:52 AM · lld, Restricted Project
MaskRay added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.
In D59553#1571050, @avl wrote:

error: Invalid address range [0xfffffffffffffffe, 0x0000000000000004)

DW_AT_low_pc DW_AT_high_pc pairs like these are less ideal. Before, tools have learned 0 address is special. They may or may not need a special case to handle this. Now, -2 is introduced (whatever value we choose here, GNU linkers still either relocate these to 0 or use their CB_PRETEND logic). They need to learn one more rule. I still believe, teaching the symbolizers (addr2line, llvm-symbolizer D60470) is the way forward.

the point is in that tools should NOT learn one more rule. There should not be done any additional logic for UINT64_MAX-1 in tools. the fact that llvm-dwarfdump --verify displays error message is probably normal. Because debug info references not existed address range. llvm-dwarfdump --verify dislays errors for current solution also. The proper way to fix llvm-dwarfdump --verify is to remove broken debug info.

I agree. As things stand, some tools have special case behaviour for 0, but many do not, and consequently get it wrong. Furthermore, there are some cases where it is impossible to determine whether 0 is special or not. For example for PIE output, 0 can be a valid address (not all linker scripts place the headers in memory), meaning the tools can't distinguish between a function at address 0, or a dead piece of debug information.

Fri, Jul 5, 2:23 AM · lld, Restricted Project
jhenderson added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.
In D59553#1571050, @avl wrote:

error: Invalid address range [0xfffffffffffffffe, 0x0000000000000004)

DW_AT_low_pc DW_AT_high_pc pairs like these are less ideal. Before, tools have learned 0 address is special. They may or may not need a special case to handle this. Now, -2 is introduced (whatever value we choose here, GNU linkers still either relocate these to 0 or use their CB_PRETEND logic). They need to learn one more rule. I still believe, teaching the symbolizers (addr2line, llvm-symbolizer D60470) is the way forward.

the point is in that tools should NOT learn one more rule. There should not be done any additional logic for UINT64_MAX-1 in tools. the fact that llvm-dwarfdump --verify displays error message is probably normal. Because debug info references not existed address range. llvm-dwarfdump --verify dislays errors for current solution also. The proper way to fix llvm-dwarfdump --verify is to remove broken debug info.

Fri, Jul 5, 2:03 AM · lld, Restricted Project

Thu, Jul 4

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

error: Invalid address range [0xfffffffffffffffe, 0x0000000000000004)

DW_AT_low_pc DW_AT_high_pc pairs like these are less ideal. Before, tools have learned 0 address is special. They may or may not need a special case to handle this. Now, -2 is introduced (whatever value we choose here, GNU linkers still either relocate these to 0 or use their CB_PRETEND logic). They need to learn one more rule. I still believe, teaching the symbolizers (addr2line, llvm-symbolizer D60470) is the way forward.

Thu, Jul 4, 11:44 PM · lld, Restricted Project
MaskRay added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

I thought CB_PRETEND could fix a "error on relocations to discarded SHF_ALLOC sections" issue but I fixed it in another way. Let's ignore my comment about CB_PRETEND.

Thu, Jul 4, 8:01 PM · lld, Restricted Project
rocallahan added a comment to D54747: Discard debuginfo for object files empty after GC.

Sorry for not responding earlier. Phabricator insists on sending me emails about all issues in the system, and I never figured out how to get GMail to show me only emails about Phabricator issues I'm CCed on.

Thu, Jul 4, 2:16 PM · Restricted Project, lld
thakis added inline comments to D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.
Thu, Jul 4, 2:14 PM · Restricted Project, lld
Herald added a project to D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}: Restricted Project.
Thu, Jul 4, 2:04 PM · Restricted Project, lld
avl added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

@jhenderson Thank you for pointing this out! Actually --verify complains about errors in both cases(with the fix and without the fix) for the testcase from this review:

Thu, Jul 4, 10:43 AM · lld, Restricted Project

Wed, Jul 3

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

@avl, something you might want to watch out for is the behaviour of llvm-dwarfdump --verify where this patching has happened. I've seen it spit out complaints about "invalid address ranges" for this situation.

Wed, Jul 3, 8:21 AM · lld, Restricted Project

Fri, Jun 28

ikudrin closed D63869: [ELF] Do not produce DT_JMPREL and DT_PLTGOT if .rela.plt is empty..
Fri, Jun 28, 3:16 AM · Restricted Project, lld
grimar accepted D63869: [ELF] Do not produce DT_JMPREL and DT_PLTGOT if .rela.plt is empty..

LGTM

Fri, Jun 28, 2:02 AM · Restricted Project, lld
ikudrin updated the diff for D63869: [ELF] Do not produce DT_JMPREL and DT_PLTGOT if .rela.plt is empty..
  • Extended the test and moved it into the linkerscript folder.
Fri, Jun 28, 12:54 AM · Restricted Project, lld

Thu, Jun 27

MaskRay added a reviewer for D63869: [ELF] Do not produce DT_JMPREL and DT_PLTGOT if .rela.plt is empty.: MaskRay.
Thu, Jun 27, 9:12 PM · Restricted Project, lld
MaskRay added inline comments to D63869: [ELF] Do not produce DT_JMPREL and DT_PLTGOT if .rela.plt is empty..
Thu, Jun 27, 7:18 AM · Restricted Project, lld
MaskRay added inline comments to D63869: [ELF] Do not produce DT_JMPREL and DT_PLTGOT if .rela.plt is empty..
Thu, Jun 27, 6:55 AM · Restricted Project, lld
grimar added a comment to D63869: [ELF] Do not produce DT_JMPREL and DT_PLTGOT if .rela.plt is empty..
  • Reworked the test. I am not sure about moving it into linkerscript, though, because it does not test the support for linker scripts per se.
Thu, Jun 27, 5:08 AM · Restricted Project, lld
ikudrin added inline comments to D63869: [ELF] Do not produce DT_JMPREL and DT_PLTGOT if .rela.plt is empty..
Thu, Jun 27, 4:55 AM · Restricted Project, lld
ikudrin updated the diff for D63869: [ELF] Do not produce DT_JMPREL and DT_PLTGOT if .rela.plt is empty..
  • Reworked the test.

I am not sure about moving it into linkerscript, though, because it does not test the support for linker scripts per se. Moreover, there are lots of tests in the main test folder which also use linker scripts.

Thu, Jun 27, 4:50 AM · Restricted Project, lld
MaskRay added inline comments to D63869: [ELF] Do not produce DT_JMPREL and DT_PLTGOT if .rela.plt is empty..
Thu, Jun 27, 4:41 AM · Restricted Project, lld
MaskRay added inline comments to D63869: [ELF] Do not produce DT_JMPREL and DT_PLTGOT if .rela.plt is empty..
Thu, Jun 27, 4:33 AM · Restricted Project, lld
grimar added a comment to D63869: [ELF] Do not produce DT_JMPREL and DT_PLTGOT if .rela.plt is empty..

I think this test case should live in test\ELF\linkerscript folder. Also it should be .test, not .s,
like our other tests that has no or little amount of asm code.
(See unused-synthetic2.test for example)

Thu, Jun 27, 4:24 AM · Restricted Project, lld
ruiu accepted D63869: [ELF] Do not produce DT_JMPREL and DT_PLTGOT if .rela.plt is empty..

LGTM

Thu, Jun 27, 4:20 AM · Restricted Project, lld
ikudrin added a comment to D63869: [ELF] Do not produce DT_JMPREL and DT_PLTGOT if .rela.plt is empty..

On Linux, the issue can be reproduced in the following way:

Thu, Jun 27, 4:20 AM · Restricted Project, lld
ikudrin created D63869: [ELF] Do not produce DT_JMPREL and DT_PLTGOT if .rela.plt is empty..
Thu, Jun 27, 4:11 AM · Restricted Project, lld

Wed, Jun 26

aganea closed D63775: [LLD][COFF] Case insensitive compares for /nodefaultlib.
Wed, Jun 26, 8:40 AM · Restricted Project, lld
ruiu accepted D63775: [LLD][COFF] Case insensitive compares for /nodefaultlib.

LGTM

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

@MaskRay Hi MaskRay, PRETEND is a connected but separate issue. I agree it would be good to implement some kind of the PRETEND/CB_PRETEND logic in lld. But it does not make obsolete this patch. implementing of PRETEND logic answers the question "how to select proper debug info for COMDAT section". That patch answers the question "How to avoid overlapping address ranges in case debug info referenced to deleted code presented". This patch makes sense not only for PRETEND. Thus, I do not agree with "If we have PRETEND, we will not need the -2 hack in relocateNonAlloc". Let`s not mixup issues.

Wed, Jun 26, 3:16 AM · lld, Restricted Project

Tue, Jun 25

rnk accepted D63775: [LLD][COFF] Case insensitive compares for /nodefaultlib.

lgtm

Tue, Jun 25, 3:47 PM · Restricted Project, lld
aganea updated the diff for D63775: [LLD][COFF] Case insensitive compares for /nodefaultlib.

Case-insensitive on Linux as requested.

Tue, Jun 25, 3:09 PM · Restricted Project, lld
rnk added a comment to D63775: [LLD][COFF] Case insensitive compares for /nodefaultlib.

I guess /nodefaultlib just isn't that common compared to /defaultlib, so this hadn't been reported yet. Fix makes sense to me.

Tue, Jun 25, 11:25 AM · Restricted Project, lld
aganea updated the diff for D63775: [LLD][COFF] Case insensitive compares for /nodefaultlib.

Updated test.

Tue, Jun 25, 9:09 AM · Restricted Project, lld
aganea created D63775: [LLD][COFF] Case insensitive compares for /nodefaultlib.
Tue, Jun 25, 9:05 AM · Restricted Project, lld
MaskRay added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.
In D59553#1557151, @avl wrote:
Tue, Jun 25, 8:51 AM · lld, Restricted Project
avl added a comment to D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

ping @echristo

Tue, Jun 25, 2:03 AM · lld, Restricted Project

Jun 21 2019

lebedev.ri abandoned D41159: [asan] LIT: Add lld testing config..
Jun 21 2019, 8:50 AM · Restricted Project, lld, Restricted Project

Jun 20 2019

MaskRay added a comment to D63564: Add undefined symbols from linker script to output file.

Is there any possibility that this is a GNU ld's bug? Their behavior is different from their man page, and the thing that defines what is the right behavior is usually a manual than an actual behavior.

Jun 20 2019, 7:47 AM · Restricted Project, lld
ruiu added a comment to D63564: Add undefined symbols from linker script to output file.

Is there any possibility that this is a GNU ld's bug? Their behavior is different from their man page, and the thing that defines what is the right behavior is usually a manual than an actual behavior.

Jun 20 2019, 6:16 AM · Restricted Project, lld
ihalip added a comment to D63564: Add undefined symbols from linker script to output file.

It and the behavior I observed from ld.bfd do appear to suggest -u and EXTERN() should be handled the same way. However, this patch adds an undefined symbol for EXTERN() but does not do the same for -u. I think if we want to fix the issue, we should make the two consistent.

You're right, this change only handles the EXTERN() case. It should be reworked to also handle -u/--undefined.

Jun 20 2019, 3:29 AM · Restricted Project, lld

Jun 19 2019

MaskRay added a comment to D63564: Add undefined symbols from linker script to output file.

If I read correctly, the Linux kernel use case boils down to:

Jun 19 2019, 9:31 PM · Restricted Project, lld
nickdesaulniers added a comment to D63564: Add undefined symbols from linker script to output file.

https://github.com/ClangBuiltLinux/linux/issues/515

Jun 19 2019, 11:21 AM · Restricted Project, lld
nickdesaulniers added reviewers for D63564: Add undefined symbols from linker script to output file: ruiu, grimar, peter.smith.
Jun 19 2019, 11:21 AM · Restricted Project, lld
nickdesaulniers added a reviewer for D63564: Add undefined symbols from linker script to output file: MaskRay.
Jun 19 2019, 11:21 AM · Restricted Project, lld