Page MenuHomePhabricator

kledzik (kledzik@apple.com)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 3 2013, 3:07 PM (349 w, 2 d)

Recent Activity

Fri, Sep 13

kledzik added a comment to D67568: [LTO][Legacy] Add new C inferface to query libcall functions.

How many symbols are there today? Do we want this overhead of a function call for each? Could you instead return a count and array pointer (like argc and argv) or have a "foreach" function that takes a block and calls it back for each symbol?

Fri, Sep 13, 2:43 PM · Restricted Project

May 6 2019

kledzik added a comment to D60285: Make calls into the pthread library use weak symbols..

Can this be conditionalized to not happen on macOS/iOS? I can see needing this on platforms where pthreads is optional, but it looks like this code will apply to macOS too, which introduces risk there.

May 6 2019, 3:44 PM · Restricted Project

Jan 25 2019

kledzik added inline comments to D57190: [MC] Teach the MachO object writer about N_FUNC_COLD.
Jan 25 2019, 5:03 PM

Apr 10 2018

kledzik added a comment to D45472: [MachO] Emit Weak ReadOnlyWithRel to ConstDataSection.

LGTM

Apr 10 2018, 12:22 PM

Oct 31 2016

kledzik committed rL285636: fix _dyld_find_unwind_sections() for pre-10.7. Patch by Jeremy Sequoia.
fix _dyld_find_unwind_sections() for pre-10.7. Patch by Jeremy Sequoia
Oct 31 2016, 2:13 PM

Sep 30 2016

kledzik added inline comments to D24479: [YAML] Add basic support for class hierarchies.
Sep 30 2016, 4:13 PM

Jul 19 2016

kledzik added a comment to D22543: [libunwind] Properly align _Unwind_Exception..

LGTM

Jul 19 2016, 4:47 PM

Jan 7 2016

kledzik added inline comments to D15943: Add support for headerpad_max_install_names cmdline option.
Jan 7 2016, 11:16 AM · lld

Dec 11 2015

kledzik added inline comments to D15439: Change reference kind for FDE references the compiler already emitted.
Dec 11 2015, 1:21 PM · lld

Dec 10 2015

kledzik added inline comments to D15439: Change reference kind for FDE references the compiler already emitted.
Dec 10 2015, 6:52 PM · lld

Dec 8 2015

kledzik accepted D15360: Don't bypass the GOT for delta32toGOT references.

LGTM

Dec 8 2015, 4:33 PM · lld

Jul 24 2015

kledzik committed rL243190: Add initial CODE_OWNERS.TXT file.
Add initial CODE_OWNERS.TXT file
Jul 24 2015, 5:45 PM

Jun 24 2015

kledzik added a comment to D10720: libunwind: Fix unw_getcontext() return value on AArch64..

LGTM

Jun 24 2015, 5:50 PM

May 20 2015

kledzik committed rL237848: [darwin] fix libcompiler_rt.dylib build.
[darwin] fix libcompiler_rt.dylib build
May 20 2015, 3:41 PM
kledzik committed rL237840: [doc] Update Lexicon with C++ unwinder acronyms.
[doc] Update Lexicon with C++ unwinder acronyms
May 20 2015, 3:07 PM

Mar 2 2015

kledzik added a comment to D7695: [Mach-O] Dtrace Support Part 1: User SDT provider handling..

Thanks for working on this!

Mar 2 2015, 6:34 PM · lld

Mar 1 2015

kledzik added inline comments to D7378: llvm-readobj: implement MachODumper::printNeededLibraries.
Mar 1 2015, 11:32 AM

Feb 13 2015

kledzik added a comment to D7612: Update ARM and x86 ArchHandler to match 64bits counterparts. NFC.

LGTM

Feb 13 2015, 1:05 PM · lld

Feb 5 2015

kledzik added a comment to D7311: MachO: Move LayoutPass to MachO directory..

LGTM

Feb 5 2015, 11:55 AM · lld

Jan 27 2015

kledzik added a comment to D7212: [MachO] Implement -order_file as a separate ordering pass..

All those comparisons in compareAtomSub() are part of the system linker on darwin. See:

http://www.opensource.apple.com/source/ld64/ld64-123.2.1/src/ld/passes/order_file.cpp
Jan 27 2015, 5:37 PM · lld
kledzik added a comment to D7212: [MachO] Implement -order_file as a separate ordering pass..

Leaving this pass as-is and adding a new simple pass that ELF (and maybe COFF) uses instead is fine with me.

Jan 27 2015, 5:24 PM · lld
kledzik added a comment to D7212: [MachO] Implement -order_file as a separate ordering pass..

Sure two passes is conceptually clean, but if it turns out that running sort() twice for mach-o makes linking slower for mach-o than it currently is, then that is not ideal.

Jan 27 2015, 4:21 PM · lld
kledzik added a comment to D7212: [MachO] Implement -order_file as a separate ordering pass..

There maybe lots of other parts of this LayoutPass that mach-o needs but ELF does not. Why not just prune away at this until you get pass that is fast enough for ELF. Then split this in to two passes. One for mach-o and one for ELF, and let the drivers choose the pass to use.

Jan 27 2015, 2:52 PM · lld

Jan 26 2015

kledzik added a comment to D7189: Remove kindInGroup reference.

From what I recall, ELF has the semantics that you cannot just remove an atom in the middle of a section, because there is not enough relocations in the section to fix up the remaining (e.g CALLs to static functions in the same section don't have relocations on them, so if the distance changed because some atom between the two was removed, the CALL now be to the new location). The group reference was a way to keep all atoms in a section together.

Jan 26 2015, 4:26 PM · lld

Jan 14 2015

kledzik added a comment to D6960: Always add -lc++abi when using vptr sanitizer on Darwin..

but they went away after adding -lc++abi.

Jan 14 2015, 2:40 PM

Jan 13 2015

kledzik added a comment to D6960: Always add -lc++abi when using vptr sanitizer on Darwin..

It will just work. libstdc++.dylib re-exports symbols from libc++abi.dylib:

Jan 13 2015, 6:39 PM
kledzik added inline comments to D6960: Always add -lc++abi when using vptr sanitizer on Darwin..
Jan 13 2015, 5:22 PM

Jan 12 2015

kledzik added a comment to D6874: Convert other drivers to use WrapperNode..

I see. Given my comment in the previous patch about parallelism, I was expecting to see a change in the latest patch about when parsing happens. But you changed mach-o parseFile() a while ago to instantiate a File object, but not parse it until doParse() was invoked. So all is good. Just the parseFile() name confusing me.

Jan 12 2015, 5:21 PM · lld
kledzik added a comment to D6874: Convert other drivers to use WrapperNode..

It still looks like mach-o files are parsed immediately (not in parallel). If that is intentional, because reworking the mach-o logic is complicated, then leave a FIXME comment there.

Jan 12 2015, 5:02 PM · lld

Jan 7 2015

kledzik added a comment to D6874: Convert other drivers to use WrapperNode..

This seems to have lost the parallel reading/parsing of input files. It looks like parseFile() called by the driver is triggering the actual parsing and parse() called by the Driver in parallel just returns the error code from the parse.

Jan 7 2015, 7:29 PM · lld

Jan 6 2015

kledzik added a comment to D6653: Convert CoreInputGraph..

Two comments:

  1. This refactoring has taken so many steps, I'm beginning to think one big patch where we can see the end design might have been better...
  2. The ErrorFile concept makes me wonder if that should be the approach for all parseFile() implementations. Rather than having a result parameter is that is a vector they append to and returning an error_code, they simply return a vector of Files and any errors are encoded as ErrorFiles in that vector.
Jan 6 2015, 12:53 PM · lld

Jan 5 2015

kledzik committed rL225226: Add 64-bit multiply functions to iOS arm64 compiler-rt dylib.
Add 64-bit multiply functions to iOS arm64 compiler-rt dylib
Jan 5 2015, 4:28 PM

Dec 23 2014

kledzik added a comment to D6574: Remove darwin_fat.mk..

The libcompiler_rt.dylib in the OS is built using darwin_bni.mk. I don't know anything that uses darwin_fat.mk.

Dec 23 2014, 4:15 PM

Dec 19 2014

kledzik committed rL224657: [libunwind] improve x86_64 comments in compact_unwind_encoding.h.
[libunwind] improve x86_64 comments in compact_unwind_encoding.h
Dec 19 2014, 5:24 PM
kledzik committed rL224656: [libunwind] fix comment in compact_unwind_encoding.h.
[libunwind] fix comment in compact_unwind_encoding.h
Dec 19 2014, 5:15 PM
kledzik added a comment to D6736: [macho] Minor install_name fixes.

LGTM

Dec 19 2014, 1:59 PM · lld

Dec 18 2014

kledzik added a comment to D6732: [Object] Don't crash on empty export lists..

LGTM

Dec 18 2014, 6:19 PM
kledzik added a comment to D6724: [macho] -rpath support.

The static linker does not need to interpret LC_RPATH in linked dylibs, so there is no need to write a test case that it can.

Dec 18 2014, 12:00 PM · lld
kledzik added a comment to D6719: Atoms generated by a pass should be owned by the generated file and not the pass itself..

I see what you are trying to do, but it leaves the pass-generated atoms in an odd state. All the other atoms (from .o files) still have their file() method returning their original file. But the pass-generated atoms now have their file() return the temp mergedFile. When inspecting atoms in the Writer, it is nice to be able to see where they came from. This patch looses that.

Dec 18 2014, 9:38 AM · lld

Dec 12 2014

kledzik added a comment to D6633: Separate file parsing from File's constructors..

The method Reader::parseFile() no longer has an appropriate name (since it no longer parses). It now just constructs a File object.

Dec 12 2014, 12:24 PM · lld

Dec 5 2014

kledzik accepted D6518: BumpPtr allocate SimpleReferences.

commited in r 223528

Dec 5 2014, 2:17 PM · lld

Dec 4 2014

kledzik added a comment to D6518: BumpPtr allocate SimpleReferences.

Colin's calculation is based on the bumpvector containing only the *pointer* to the Reference. The current implementation has a vector< SimpleReference>, so the wasted space is not just a pointer size (repeatedly doubled). A SimpleReference is 64 bytes in size, so that value keeps getting doubled (and wasted).

Dec 4 2014, 2:02 PM · lld
kledzik added a comment to D6518: BumpPtr allocate SimpleReferences.

Won't "RefList _reference" leak the nodes of the linked list

Dec 4 2014, 12:03 PM · lld
kledzik added a comment to D6518: BumpPtr allocate SimpleReferences.

But using the BumpVector makes the code cleaner too right ?

Only because you are adding new BumpVector code. We could also add a new BumpList class to ADT and that would make the lld code very clean too.

Dec 4 2014, 11:47 AM · lld
kledzik added a comment to D6518: BumpPtr allocate SimpleReferences.

A downside to BumpVector is that as the vector grows (by adding References), the capacity is doubled by bump allocating a new chunk. All the previous allocations (e.g. if current at 8 elements, the allocs for 1, 2, and 4) are wasted space in the bumpptr pool.

Dec 4 2014, 11:13 AM · lld

Dec 3 2014

kledzik added a comment to D6518: BumpPtr allocate SimpleReferences.

Even if you add a std::vector<std::unique_ptr<Reference>> to lld::File (so the SimpleReferences are deleted when the File is deleted), you still need a std::vector<SimpleReference*> in each SimpleDefineAtom to track which references are used by that atom. And the allocations for that vector would be leaked if the Atom is BumpPtr allocated.

Dec 3 2014, 7:08 PM · lld
kledzik retitled D6518: BumpPtr allocate SimpleReferences from to BumpPtr allocate SimpleReferences.
Dec 3 2014, 5:13 PM · lld
kledzik added a comment to D6335: Rewrite InputGraph's Group.

This still seems too complicated. But it is a step in the right direction, so LGTM.

Dec 3 2014, 4:56 PM

Dec 2 2014

kledzik added a comment to D6488: [compiler-rt] Don't instrument globals from __objc_classname.

Can you just generalize ShouldInstrumentGlobal() to return false for all cstring_literals sections? The linker is always going to coalesce sections of that type.

Dec 2 2014, 5:44 PM

Nov 21 2014

kledzik added a comment to D6335: Rewrite InputGraph's Group.

The big problem I still see is that this does not fix the issue of multiple different file kinds in one InputElement.

Nov 21 2014, 10:43 AM

Nov 12 2014

kledzik added a comment to D6242: Initial commit for the llvm-dsymutil tool..

Regarding testing, if you are ok limiting the test to run on darwin, you could have .ll files which you compile and use the system linker to produce the final executable which then has its debug map processed.

Nov 12 2014, 6:18 PM
kledzik added a comment to D6236: [ELF] Add CodeModel attribute to the DefinedAtom class.

The common code looks good to me.

Nov 12 2014, 2:30 PM · lld

Nov 6 2014

kledzik added a comment to D6161: Fix Mach-O unit tests breakage on Windows.

Funny. I just ran into this myself linking libc for arm64. Some of the .o files inside static archives are not 8-byte aligned, so the read64() is asserting. I think the read64() assert may be too aggressive. It may need to fallback to memcpy() for reading unaligned data, rather that asserting.

Nov 6 2014, 6:51 PM

Oct 29 2014

kledzik added a comment to D6018: [compiler-rt] Use @rpath as LC_ID_DYLIB for ASan dylib on OS X.

I'm not a clang driver guy, but it would probably be cleaner to leave AddLinkRuntimeLib() as is, and instead add the CmdArgs.push_back()s inside the if (Sanitize.needsAsanRt()).

Oct 29 2014, 1:10 PM

Oct 27 2014

kledzik added a comment to D5811: [lld] [mach-o] make lld respect alignment constraints more.

LGTM

Oct 27 2014, 2:05 PM

Oct 21 2014

kledzik closed D5876: Add DarwinInputGraph .

committed in r220330

Oct 21 2014, 2:38 PM

Oct 20 2014

kledzik retitled D5876: Add DarwinInputGraph from to Add DarwinInputGraph .
Oct 20 2014, 7:39 PM

Oct 16 2014

kledzik added a comment to D5811: [lld] [mach-o] make lld respect alignment constraints more.

The ArchHandlers look much better!

Oct 16 2014, 3:55 PM

Oct 15 2014

kledzik added a comment to D5811: [lld] [mach-o] make lld respect alignment constraints more.

How come in the ArchHandler_x86_64 you use:

write32(loc, value, false);

but inside ArchHandler_arm64 you use:

write32(loc, value, _isBig);
Oct 15 2014, 4:43 PM
kledzik added a comment to D5811: [lld] [mach-o] make lld respect alignment constraints more.

How come in the ArchHandler_x86_64 you use:

write32(loc, value, false);

but inside

Oct 15 2014, 4:39 PM

Oct 14 2014

kledzik added a comment to D5785: Use isa<> instead of checking return value of definition()..

LGTM

Oct 14 2014, 2:27 PM
kledzik added a comment to D5752: Add MachOObjectFile::getUuid().

LGTM

Oct 14 2014, 10:02 AM

Oct 13 2014

kledzik added a comment to D5754: [libcxxabi] Correctly export _Unwind_[GS]et(GR|IP) for EHABI..

LGTM

Oct 13 2014, 2:02 PM
kledzik added a comment to D5752: Add MachOObjectFile::getUuid().

The MachOObjectFile constructor is already walking the load commands. If there is a LC_UUID, you can have the constructor save it off in an ivar.

Oct 13 2014, 9:46 AM

Oct 7 2014

kledzik closed D5423: [Support] Add MemoryBuffer::getFileSlice().

committed in r219260

Oct 7 2014, 5:33 PM
kledzik updated the diff for D5423: [Support] Add MemoryBuffer::getFileSlice().

fixes from latest Rafael comments.

Oct 7 2014, 3:49 PM
kledzik added a comment to D5423: [Support] Add MemoryBuffer::getFileSlice().
Oct 7 2014, 3:47 PM
kledzik updated the diff for D5423: [Support] Add MemoryBuffer::getFileSlice().

Rework to use getFileAux()

Oct 7 2014, 2:21 PM

Oct 2 2014

kledzik closed D5570: Fix merge-by-content to not merge atoms with different custom sections.

committed in r218893

Oct 2 2014, 10:43 AM

Oct 1 2014

kledzik retitled D5570: Fix merge-by-content to not merge atoms with different custom sections from to Fix merge-by-content to not merge atoms with different custom sections.
Oct 1 2014, 6:39 PM

Sep 30 2014

kledzik closed D5549: demangle undefined symbol names in error message.

committed in r218718

Sep 30 2014, 4:26 PM · lld
kledzik retitled D5549: demangle undefined symbol names in error message from to demangle undefined symbol names in error message.
Sep 30 2014, 2:08 PM · lld

Sep 26 2014

kledzik added a comment to D5505: [lld] [ELF] Support for general dynamic TLS relocations on X86_64.
should we add a special case to the reader to convert them to weak undefines?

That is what I was thinking. But you should investigate how this works with the gnu linker. Does the linker really hard code "__tls_get_addr" to be special? Or does it always implicitly link with ld.so?

Sep 26 2014, 7:05 PM · lld
kledzik added a comment to D5505: [lld] [ELF] Support for general dynamic TLS relocations on X86_64.

to complete linking an executable, all symbols must be resolved and we cannot finish it with undefined references

Sep 26 2014, 5:25 PM · lld
kledzik added a comment to D5505: [lld] [ELF] Support for general dynamic TLS relocations on X86_64.

I guess I was not clear. I did not mean where does the *definition* of tls_get_addr come from. I was asking where does the *undefine* for tls_get_addr come from? If that undefine is "weak", that tells the linker that if no definition is found, to assume it was in some DSO. Seems like that is what you are special casing tls_get_addr to do.

Sep 26 2014, 1:50 PM · lld
kledzik added a comment to D5505: [lld] [ELF] Support for general dynamic TLS relocations on X86_64.

One area of linking not fleshed out in lld is the "canBeNullAtBuildtime" attribute for UndefinedSymbols. That was intended to model ELF weak undefines. Is that how tls_get_addr works with gnu ld? If so, then getting canBeNullAtBuildtime to work in general could solve this specific issue without needing to special case tls_get_addr.

Sep 26 2014, 1:16 PM · lld

Sep 25 2014

kledzik added a comment to D5423: [Support] Add MemoryBuffer::getFileSlice().

There are two issues:

Sep 25 2014, 4:25 PM
kledzik closed D5390: [Support] Proposed alternative to llvm::format().

committed in r218463

Sep 25 2014, 1:41 PM

Sep 19 2014

kledzik retitled D5423: [Support] Add MemoryBuffer::getFileSlice() from to [Support] Add MemoryBuffer::getFileSlice().
Sep 19 2014, 6:35 PM
kledzik updated the diff for D5390: [Support] Proposed alternative to llvm::format().

Add doxygen comments and unit tests.

Sep 19 2014, 1:37 PM
kledzik retitled D5390: [Support] Proposed alternative to llvm::format() from [PATCH] Proposed alternative to llvm::format() to [Support] Proposed alternative to llvm::format().
Sep 19 2014, 1:35 PM

Sep 18 2014

kledzik added a comment to D5405: [lld] Fixes wrong Twine uses in FileNode::errStr() and in LayoutPass.cpp .

Is there some C++ language feature we can use to mark the Twine class in a way that causes a warning/error on these incorrect usages?

Sep 18 2014, 4:30 PM · lld

Sep 17 2014

kledzik retitled D5390: [Support] Proposed alternative to llvm::format() from to [PATCH] Proposed alternative to llvm::format().
Sep 17 2014, 7:34 PM
kledzik added a comment to D5384: lld: handling of -flavor / -core command line switches.
  1. Don't most build system invoke the compiler to invoke the linker? How are you overriding the linker? Can you redirect to a script that invokes lld with -flavor and $@
  2. Yes, shifting argv is not right. We need the -flavor and its arg removed, so argv[0] is still the command name.
Sep 17 2014, 2:34 PM · lld
kledzik added a comment to D5384: lld: handling of -flavor / -core command line switches.

The original design was that -flavor had to be the first arg (so incrementing argv worked). But now the use of ParseArgs() means that -flavor is being searched for everywhere in the arg list. That is a mismatch.

Sep 17 2014, 12:44 PM · lld

Sep 12 2014

kledzik added a comment to D5340: Add preload method..

I assume that preload() return immediately, and that it is expected to spin off some thread to parse an archive member? If so, we have no overall throttle on how many threads will be started (a hundred undefines could spin up 100 threads). Also, how is the archive reader to coordinate if the Resolver gets to the point it really wants an object file to fulfill and undefine but some other thread is busy parsing that member?

Sep 12 2014, 4:22 PM
kledzik accepted D5323: Make anonymous namespace as small as possible..

Interesting. That is the way I used to write C++, but I thought the LLVM convention preferred anonymous namespaces. After reading the current LLVM convention doc, I see that static is better in this case. I'll retrain my fingers :-)

Sep 12 2014, 10:34 AM

Sep 11 2014

kledzik accepted D5261: [lld] [mach-o] implement minimal __unwind_info support.

LGTM

Sep 11 2014, 10:50 AM

Sep 10 2014

kledzik closed D5282: If lld binary is named 'ld' on darwin, use darwin driver mode.

committed as r217566

Sep 10 2014, 6:02 PM · lld
kledzik updated the diff for D5282: If lld binary is named 'ld' on darwin, use darwin driver mode.
Sep 10 2014, 5:02 PM · lld

Sep 9 2014

kledzik added a comment to D5282: If lld binary is named 'ld' on darwin, use darwin driver mode.

Adding support for "ld" on *nix meaning use-gnu-driver is reasonable, but it is an additional feature (which I have no way to test).

Sep 9 2014, 6:30 PM · lld
kledzik retitled D5282: If lld binary is named 'ld' on darwin, use darwin driver mode from to If lld binary is named 'ld' on darwin, use darwin driver mode.
Sep 9 2014, 5:36 PM · lld
kledzik added a comment to D5261: [lld] [mach-o] implement minimal __unwind_info support.

A few nits, but over all a great start.

Sep 9 2014, 1:34 PM

Sep 5 2014

kledzik accepted D5164: [ELF] Export strong defined symbol if it coalesces away a weak symbol defined in a shared library.

LGTM

Sep 5 2014, 6:33 PM · lld

Sep 3 2014

kledzik closed D5178: If lld binary is named "ld" on darwin system, use darwin driver.

committed in r217112

Sep 3 2014, 5:15 PM · lld
kledzik updated the diff for D5178: If lld binary is named "ld" on darwin system, use darwin driver.
Sep 3 2014, 3:53 PM · lld
kledzik retitled D5178: If lld binary is named "ld" on darwin system, use darwin driver from to Add CMake rules for lld-darwin.
Sep 3 2014, 3:07 PM · lld
kledzik added a comment to D5164: [ELF] Export strong defined symbol if it coalesces away a weak symbol defined in a shared library.

Just add a StringMap ivar to ELFLinkingContext. Have notifySymbolTableCoalesce() add the atom name to the map when overriding a weak atom. In buildDynamicSymbolTable() check for atom->dynamicExport() OR (atom->scope() == scopeGlobal and its name is in the StringMap).

Sep 3 2014, 12:46 PM · lld

Sep 2 2014

kledzik added a comment to D5154: Fix PR20800: correctly calculate the offset of the subq instruction when generating compact unwind info..

LGTM. Thanks for tracking this down.

Sep 2 2014, 11:34 AM

Aug 29 2014

kledzik closed D5129: [Object, llvm-objdump] allow dumping of mach-o exports trie.

committed in r216808

Aug 29 2014, 5:37 PM