Page MenuHomePhabricator

lldProject
ActivePublic

Watchers

  • This project does not have any watchers.

Details

Description

LLVM Linker

Recent Activity

Fri, Jun 14

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

@echristo Hi Eric, Could check this revision, please ? Is that patch Ok for you ?

Fri, Jun 14, 2:10 PM · lld, Restricted Project
peter.smith added a comment to D63191: [lld][ELF] Check length of subsection in .ARM.attributes.

would be to drop the size field to 0xFE, 0xFF, 0xFF, 0xFF.

This would cause problems on arm-be, no?

Fri, Jun 14, 6:23 AM · Restricted Project, lld
evgeny777 added a comment to D63191: [lld][ELF] Check length of subsection in .ARM.attributes.

would be to drop the size field to 0xFE, 0xFF, 0xFF, 0xFF.

Fri, Jun 14, 6:17 AM · Restricted Project, lld
peter.smith added a comment to D63191: [lld][ELF] Check length of subsection in .ARM.attributes.

Strange, I haven't got notification. Are buildbot notifications working?
Anyway I think making Offset uint64_t instead of size_t will fix the issue.

Fri, Jun 14, 6:14 AM · Restricted Project, lld
evgeny777 added a comment to D63191: [lld][ELF] Check length of subsection in .ARM.attributes.

Strange, I haven't got notification. Are Phab notifications working?
Anyway I think making Offset uint64_t instead of size_t will fix the issue.

Fri, Jun 14, 6:08 AM · Restricted Project, lld
peter.smith added a comment to D63191: [lld][ELF] Check length of subsection in .ARM.attributes.
I've not worked out why yet, it could be a 32/64-bit host issue.
Fri, Jun 14, 5:59 AM · Restricted Project, lld
peter.smith added a comment to D63191: [lld][ELF] Check length of subsection in .ARM.attributes.

Ironically bad-arm-attributes2.s test is failing on an Arm buildbot http://lab.llvm.org:8011/builders/clang-cmake-armv8-lld/builds/4/steps/ninja%20check%201/logs/FAIL%3A%20lld%3A%3Abad-arm-attributes2.s

Fri, Jun 14, 5:56 AM · Restricted Project, lld

Thu, Jun 13

evgeny777 closed D63191: [lld][ELF] Check length of subsection in .ARM.attributes.
Thu, Jun 13, 6:39 AM · Restricted Project, lld
MaskRay added inline comments to D63191: [lld][ELF] Check length of subsection in .ARM.attributes.
Thu, Jun 13, 2:27 AM · Restricted Project, lld
MaskRay added inline comments to D63191: [lld][ELF] Check length of subsection in .ARM.attributes.
Thu, Jun 13, 2:24 AM · Restricted Project, lld
grimar accepted D63191: [lld][ELF] Check length of subsection in .ARM.attributes.

LGTM with one nit and Rui's comments addressed.

Thu, Jun 13, 2:21 AM · Restricted Project, lld
ruiu accepted D63191: [lld][ELF] Check length of subsection in .ARM.attributes.

LGTM with the following changes.

Thu, Jun 13, 1:55 AM · Restricted Project, lld

Wed, Jun 12

evgeny777 updated the diff for D63191: [lld][ELF] Check length of subsection in .ARM.attributes.

Addressed

Wed, Jun 12, 5:33 AM · Restricted Project, lld
ruiu added inline comments to D63191: [lld][ELF] Check length of subsection in .ARM.attributes.
Wed, Jun 12, 2:55 AM · Restricted Project, lld
evgeny777 created D63191: [lld][ELF] Check length of subsection in .ARM.attributes.
Wed, Jun 12, 2:50 AM · Restricted Project, lld

Mon, Jun 10

ruiu closed D63042: [LLD][COFF] Fix missing MergeChunk::Instances cleanup in COFF::link().
Mon, Jun 10, 5:13 AM · Restricted Project, lld
ruiu added a comment to D63042: [LLD][COFF] Fix missing MergeChunk::Instances cleanup in COFF::link().

Got it. I'll commit this patch on behalf of you.

Mon, Jun 10, 3:21 AM · Restricted Project, lld
blackhole12 added a comment to D63042: [LLD][COFF] Fix missing MergeChunk::Instances cleanup in COFF::link().

I do not have access to anything. The only repo I currently have cloned is my fork of the official git repository.

Mon, Jun 10, 3:19 AM · Restricted Project, lld
ruiu added a comment to D63042: [LLD][COFF] Fix missing MergeChunk::Instances cleanup in COFF::link().

Do you have a commit access?

Mon, Jun 10, 3:05 AM · Restricted Project, lld
ruiu added a comment to D63042: [LLD][COFF] Fix missing MergeChunk::Instances cleanup in COFF::link().

I was not trying to force you to use lld in some particular way. I was just trying to help you by suggesting one solution that might work for some use cases, and I didn't know that that wouldn't work for you. If it doesn't work for you, you can continue using lld as a library as you are already doing. We support that as well, and a bug like this should be fixed.

Mon, Jun 10, 2:04 AM · Restricted Project, lld
blackhole12 added a comment to D63042: [LLD][COFF] Fix missing MergeChunk::Instances cleanup in COFF::link().

That is not an acceptable solution. This compiler is an embedded, statically linked library that can be used by other programs. It is not a standalone compiler. If the library is statically linked, there is literally no way to start a new process without forcing every single user of my library to support their own program being called with a special option for the sole purpose of invoking a linker. Since one of the intended uses, for example, is a game engine script library, which would compile a webassembly script for a map, this compilation process cannot require the game to support calling it's own executable, or link the script engine as a DLL for the sole purpose of having the script engine then load it's own DLL in a new process. These are not reasonable restrictions to have on what is supposed to be a self-contained compiler.

Mon, Jun 10, 1:36 AM · Restricted Project, lld
ruiu added a comment to D63042: [LLD][COFF] Fix missing MergeChunk::Instances cleanup in COFF::link().

LGTM

Yes, I suspect that there are other globals that are not reset between multiple executions of this linker.

As to how to invoke the linker as a separate process, I wonder if you can run your compiler as a new subprocess of the compiler. If you can do that, you can invoke the linker with a special option to call lld::link immediately on process startup, which effectively transforms your compiler to a linker. This way, you still have to statically link lld to your compiler, but you can run the linker as a separate child process, which is generally preferred because bad thing like this or unknown bugs in the linker doesn't harm your compiler.

Mon, Jun 10, 12:51 AM · Restricted Project, lld
ruiu accepted D63042: [LLD][COFF] Fix missing MergeChunk::Instances cleanup in COFF::link().

Yes, I suspect that there are other globals that are not reset between multiple executions of this linker.

Mon, Jun 10, 12:51 AM · Restricted Project, lld

Fri, Jun 7

blackhole12 created D63042: [LLD][COFF] Fix missing MergeChunk::Instances cleanup in COFF::link().
Fri, Jun 7, 8:34 PM · Restricted Project, lld

Wed, Jun 5

rnk added a comment to D62837: [LLD][COFF] Don't take into account the 'age' when looking for PDB type server.

Phabricator memes are the best thing about Phabricator :)

shipit has been around since Jun 14 2016 according to https://reviews.llvm.org/macro/, so I'm not sure why it would suddenly start appearing. Macros are only active if the name appears by itself on a line though, so perhaps you just never happened to do that before?

Wed, Jun 5, 9:45 AM · Restricted Project, lld

Tue, Jun 4

aganea closed D62837: [LLD][COFF] Don't take into account the 'age' when looking for PDB type server.
Tue, Jun 4, 7:01 PM · Restricted Project, lld
aganea added a comment to D62837: [LLD][COFF] Don't take into account the 'age' when looking for PDB type server.

Maybe Reid was talking about this icon, was it there before?

Tue, Jun 4, 6:44 PM · Restricted Project, lld
smeenai added a comment to D62837: [LLD][COFF] Don't take into account the 'age' when looking for PDB type server.

Phabricator memes are the best thing about Phabricator :)

Tue, Jun 4, 6:21 PM · Restricted Project, lld
thakis accepted D62837: [LLD][COFF] Don't take into account the 'age' when looking for PDB type server.
Tue, Jun 4, 5:05 PM · Restricted Project, lld
rnk accepted D62837: [LLD][COFF] Don't take into account the 'age' when looking for PDB type server.

Wait, did someone enable phab memes in the last update? This is too silly. =p

Tue, Jun 4, 4:54 PM · Restricted Project, lld
aganea updated the diff for D62837: [LLD][COFF] Don't take into account the 'age' when looking for PDB type server.

@thakis : Updated the existing test to fail without this patch.

Tue, Jun 4, 4:37 PM · Restricted Project, lld
kristina added reviewers for D62842: tvos_version_min and watchos_version_min flag: lhames, kledzik.

As trivial as it may seem, this is missing tests, even if it's for a few flags. Also please try to include context with your patches as it makes them much easier to review. If you have some sort of plan for actually bringing Mach-O LLD closer to ld64 feature-wise, I think llvm-dev may be a good place to actually outline your plan for this, which will give everyone a more clear picture of what future changes you have planned in mind.

Tue, Jun 4, 1:07 PM · Restricted Project, lld
thakis added a comment to D62837: [LLD][COFF] Don't take into account the 'age' when looking for PDB type server.

Test?

Tue, Jun 4, 12:43 PM · Restricted Project, lld
kristina added a reviewer for D62842: tvos_version_min and watchos_version_min flag: ruiu.
Tue, Jun 4, 12:37 PM · Restricted Project, lld
mstorsjo added a reviewer for D62844: lld/macho application_extension support : ruiu.

Can you add a testcase for this? Also it's appreciated if the uploaded diffs are made with a larger context (diff -U999 or similar) to allow expanding the context here in phabricator.

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

@echristo What is your opinion on this patch ? Is it worth to integrate it ?

Tue, Jun 4, 7:31 AM · lld, Restricted Project
aganea updated the summary of D62837: [LLD][COFF] Don't take into account the 'age' when looking for PDB type server.
Tue, Jun 4, 6:44 AM · Restricted Project, lld
aganea added a reviewer for D62837: [LLD][COFF] Don't take into account the 'age' when looking for PDB type server: thakis.
Tue, Jun 4, 6:41 AM · Restricted Project, lld
carlokok created D62844: lld/macho application_extension support .
Tue, Jun 4, 1:08 AM · Restricted Project, lld

Mon, Jun 3

carlokok created D62842: tvos_version_min and watchos_version_min flag.
Mon, Jun 3, 11:55 PM · Restricted Project, lld
aganea created D62837: [LLD][COFF] Don't take into account the 'age' when looking for PDB type server.
Mon, Jun 3, 7:40 PM · Restricted Project, lld
ruiu 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.

Mon, Jun 3, 12:01 AM · lld, Restricted Project

Sun, Jun 2

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

ping. @ruiu Do you think it would be useful to integrate this patch ?

Sun, Jun 2, 11:59 PM · lld, Restricted Project

Tue, May 28

carlokok updated the diff for D62568: lld/mach-o should set MH_NO_REEXPORTED_DYLIBS if there are no REEXPORT_DYLIB libraries.

Remove unused var

Tue, May 28, 10:33 PM · Restricted Project, lld
carlokok created D62568: lld/mach-o should set MH_NO_REEXPORTED_DYLIBS if there are no REEXPORT_DYLIB libraries.
Tue, May 28, 10:30 PM · Restricted Project, lld

Mon, May 27

avl updated the diff for D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used.

deleted redundant tests.

Mon, May 27, 4:08 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.

Mon, May 27, 4:08 AM · lld, Restricted Project

Tue, May 21

tcwang added a comment to D61711: [ELF][Driver] Fix precedence of symbol ordering file and CGProfile.

The commit description of rLLD361190 does not contain Differential Revision: https://reviews.llvm.org/D61711, so the revision was not closed when you committed that...

Also, the description should probably say the two options are now exclusive:

(1) When both --symbol-ordering-file=<file> and --call-graph-order-file=<file> are present, whichever flag comes later will take precedence.

Tue, May 21, 9:08 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'm personally happy with the implementation now, but obviously there needs to be agreement from the other reviewers.

Tue, May 21, 2:08 AM · lld, Restricted Project

Mon, May 20

MaskRay closed D61711: [ELF][Driver] Fix precedence of symbol ordering file and CGProfile.

The commit description of rLLD361190 does not contain Differential Revision: https://reviews.llvm.org/D61711, so the revision was not closed when you committed that...

Mon, May 20, 6:43 PM · Restricted Project, lld