marsupial (Frederich Munch)Email Not Verified
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 25 2016, 10:43 PM (47 w, 5 d)

Recent Activity

Mon, Jun 19

marsupial added a comment to D33658: Extend DynamicLibrary class to be usable without loading permanently..

Ping.

Mon, Jun 19, 8:21 AM
marsupial added a comment to D33381: Avoid constructing GlobalExtensions only to find out it is empty..

Ping.

Mon, Jun 19, 8:20 AM
marsupial added a comment to D30709: Handle IMAGE_REL_AMD64_ADDR32NB in RuntimeDyldCOFF.

Ping.

Mon, Jun 19, 8:19 AM
marsupial added a comment to D33529: Allow clients to specify search order of DynamicLibraries..

Ping.

Mon, Jun 19, 8:19 AM

Wed, Jun 14

marsupial updated the summary of D33515: Force RegisterStandardPasses to construct std::function in the IPO library..
Wed, Jun 14, 5:17 PM
marsupial added a comment to D33515: Force RegisterStandardPasses to construct std::function in the IPO library..

From the summary:

Fixes an issue using RegisterStandardPasses from a statically linked object before PassManagerBuilder::addGlobalExtension is called from a dynamic library.

Wed, Jun 14, 4:05 PM
marsupial updated the diff for D33515: Force RegisterStandardPasses to construct std::function in the IPO library..

Format.

Wed, Jun 14, 12:28 PM
marsupial committed rL305408: Hide dbgs() stream for when built with -fmodules..
Hide dbgs() stream for when built with -fmodules.
Wed, Jun 14, 12:17 PM
marsupial closed D34214: Hide dbgs() stream for when built with -fmodules. by committing rL305408: Hide dbgs() stream for when built with -fmodules..
Wed, Jun 14, 12:16 PM
marsupial added a comment to D33515: Force RegisterStandardPasses to construct std::function in the IPO library..

I'm missing the *why* using a "real function" (I guess you meant "function pointer") matter?

Wed, Jun 14, 12:16 PM
marsupial updated the diff for D34214: Hide dbgs() stream for when built with -fmodules..

Remove spurious newline.

Wed, Jun 14, 11:48 AM
marsupial updated the diff for D34214: Hide dbgs() stream for when built with -fmodules..

Formatting.

Wed, Jun 14, 11:45 AM
marsupial updated the diff for D34214: Hide dbgs() stream for when built with -fmodules..

This should be more 'obviously' correct.
Sorry, some CMake confusion here....

Wed, Jun 14, 11:34 AM
marsupial created D34214: Hide dbgs() stream for when built with -fmodules..
Wed, Jun 14, 11:09 AM
marsupial updated the diff for D33515: Force RegisterStandardPasses to construct std::function in the IPO library..

Formatting.

Wed, Jun 14, 11:07 AM
marsupial updated the diff for D33515: Force RegisterStandardPasses to construct std::function in the IPO library..

Relocate tests into Transforms/IPO.

Wed, Jun 14, 7:11 AM

Tue, Jun 13

marsupial updated the diff for D33515: Force RegisterStandardPasses to construct std::function in the IPO library..
Tue, Jun 13, 5:21 PM
marsupial updated the diff for D33515: Force RegisterStandardPasses to construct std::function in the IPO library..

Having trouble with the test part of this and CMake.

Tue, Jun 13, 5:08 PM
marsupial updated the diff for D33515: Force RegisterStandardPasses to construct std::function in the IPO library..

Add explicit dependency for CMake when built with LLVM_ENABLE_MODULES.

Tue, Jun 13, 2:20 PM
marsupial committed rL305318: Revert r305313 & r305303, self-hosting build-bot isn’t liking it..
Revert r305313 & r305303, self-hosting build-bot isn’t liking it.
Tue, Jun 13, 12:06 PM
marsupial committed rL305313: Fix self hosting build-bot failure from r305303 by adjusting….
Fix self hosting build-bot failure from r305303 by adjusting…
Tue, Jun 13, 11:12 AM
marsupial committed rL305303: Force RegisterStandardPasses to construct std::function in the IPO library..
Force RegisterStandardPasses to construct std::function in the IPO library.
Tue, Jun 13, 9:49 AM
marsupial closed D33515: Force RegisterStandardPasses to construct std::function in the IPO library. by committing rL305303: Force RegisterStandardPasses to construct std::function in the IPO library..
Tue, Jun 13, 9:49 AM
marsupial updated the diff for D33515: Force RegisterStandardPasses to construct std::function in the IPO library..

Conflict resolution & additional comment.

Tue, Jun 13, 9:40 AM

Fri, Jun 9

marsupial added a comment to D33789: Export the required symbol from DynamicLibraryTests.

LGTM.

Fri, Jun 9, 3:22 AM

Mon, Jun 5

marsupial added inline comments to D33789: Export the required symbol from DynamicLibraryTests.
Mon, Jun 5, 10:11 AM
marsupial added inline comments to D33789: Export the required symbol from DynamicLibraryTests.
Mon, Jun 5, 10:07 AM
marsupial added inline comments to D30709: Handle IMAGE_REL_AMD64_ADDR32NB in RuntimeDyldCOFF.
Mon, Jun 5, 9:53 AM
marsupial updated the diff for D30709: Handle IMAGE_REL_AMD64_ADDR32NB in RuntimeDyldCOFF.
Mon, Jun 5, 9:44 AM
marsupial committed rL304720: Close DynamicLibraries in reverse order they were opened..
Close DynamicLibraries in reverse order they were opened.
Mon, Jun 5, 9:27 AM
marsupial closed D33652: Close DynamicLibraries in reverse order they were opened. by committing rL304720: Close DynamicLibraries in reverse order they were opened..
Mon, Jun 5, 9:27 AM

Tue, May 30

marsupial added inline comments to D33651: Add note for location of other convenience functions..
Tue, May 30, 3:04 PM
marsupial updated the diff for D33652: Close DynamicLibraries in reverse order they were opened..

Add test case.

Tue, May 30, 2:24 PM
marsupial updated the diff for D33529: Allow clients to specify search order of DynamicLibraries..
Tue, May 30, 11:24 AM
marsupial updated the diff for D33658: Extend DynamicLibrary class to be usable without loading permanently..
Tue, May 30, 11:24 AM
marsupial added a dependency for D33658: Extend DynamicLibrary class to be usable without loading permanently.: D33529: Allow clients to specify search order of DynamicLibraries..
Tue, May 30, 11:23 AM
marsupial added a dependent revision for D33529: Allow clients to specify search order of DynamicLibraries.: D33658: Extend DynamicLibrary class to be usable without loading permanently..
Tue, May 30, 11:23 AM
marsupial updated the diff for D33658: Extend DynamicLibrary class to be usable without loading permanently..
Tue, May 30, 11:01 AM
marsupial updated the diff for D33657: Allow libraries to be loaded with RTLD_LOCAL on Unix..
Tue, May 30, 11:00 AM
marsupial updated the diff for D33529: Allow clients to specify search order of DynamicLibraries..

Allow forward and backward search of loaded modules.

Tue, May 30, 11:00 AM
marsupial removed a dependency for D33652: Close DynamicLibraries in reverse order they were opened.: D33651: Add note for location of other convenience functions..
Tue, May 30, 10:58 AM
marsupial removed a dependent revision for D33651: Add note for location of other convenience functions.: D33652: Close DynamicLibraries in reverse order they were opened..
Tue, May 30, 10:58 AM
marsupial updated the diff for D33652: Close DynamicLibraries in reverse order they were opened..
Tue, May 30, 10:58 AM
marsupial updated the diff for D33651: Add note for location of other convenience functions..
Tue, May 30, 10:57 AM
marsupial added inline comments to D33651: Add note for location of other convenience functions..
Tue, May 30, 8:53 AM

Mon, May 29

marsupial added a dependent revision for D33658: Extend DynamicLibrary class to be usable without loading permanently.: D33659: Extend DynamicLibrary class to be usable without loading permanently..
Mon, May 29, 2:08 PM
marsupial added a dependency for D33659: Extend DynamicLibrary class to be usable without loading permanently.: D33658: Extend DynamicLibrary class to be usable without loading permanently..
Mon, May 29, 2:08 PM
marsupial created D33659: Extend DynamicLibrary class to be usable without loading permanently..
Mon, May 29, 2:07 PM
marsupial added a dependency for D33658: Extend DynamicLibrary class to be usable without loading permanently.: D33657: Allow libraries to be loaded with RTLD_LOCAL on Unix..
Mon, May 29, 2:02 PM
marsupial added a dependent revision for D33657: Allow libraries to be loaded with RTLD_LOCAL on Unix.: D33658: Extend DynamicLibrary class to be usable without loading permanently..
Mon, May 29, 2:02 PM
marsupial created D33658: Extend DynamicLibrary class to be usable without loading permanently..
Mon, May 29, 2:01 PM
marsupial added a dependency for D33657: Allow libraries to be loaded with RTLD_LOCAL on Unix.: D33529: Allow clients to specify search order of DynamicLibraries..
Mon, May 29, 1:59 PM
marsupial added a dependent revision for D33529: Allow clients to specify search order of DynamicLibraries.: D33657: Allow libraries to be loaded with RTLD_LOCAL on Unix..
Mon, May 29, 1:59 PM
marsupial added inline comments to D33529: Allow clients to specify search order of DynamicLibraries..
Mon, May 29, 1:59 PM
marsupial created D33657: Allow libraries to be loaded with RTLD_LOCAL on Unix..
Mon, May 29, 1:54 PM
marsupial updated the diff for D33529: Allow clients to specify search order of DynamicLibraries..

Spelling.

Mon, May 29, 1:33 PM
marsupial updated the diff for D33529: Allow clients to specify search order of DynamicLibraries..

Restore default ordering after llvm_shutdown called.

Mon, May 29, 1:31 PM
marsupial added a dependent revision for D33651: Add note for location of other convenience functions.: D33652: Close DynamicLibraries in reverse order they were opened..
Mon, May 29, 11:14 AM
marsupial added a dependency for D33652: Close DynamicLibraries in reverse order they were opened.: D33651: Add note for location of other convenience functions..
Mon, May 29, 11:14 AM
marsupial created D33652: Close DynamicLibraries in reverse order they were opened..
Mon, May 29, 11:14 AM
marsupial created D33651: Add note for location of other convenience functions..
Mon, May 29, 10:57 AM
marsupial added a comment to D33529: Allow clients to specify search order of DynamicLibraries..

That looks very good. Can we add a test case with dlopening a library with RTLD_LOCAL.

Mon, May 29, 10:32 AM

Fri, May 26

marsupial added a comment to D30107: Refactor DynamicLibrary so searching for a symbol will have a defined order and libraries are properly unloaded when llvm_shutdown is called..

Exactly, so there is no way to override the symbol at run-time. This counter-intuitive (well wrong in my opinion) in an interactive-compiler.

I understand you folks feel it is wrong and I have been trying to work towards a solution.
But there are other use cases besides yours.

Fri, May 26, 7:21 PM
marsupial added a comment to D30709: Handle IMAGE_REL_AMD64_ADDR32NB in RuntimeDyldCOFF.

ping

Fri, May 26, 6:42 PM
marsupial updated the diff for D33529: Allow clients to specify search order of DynamicLibraries..

Add tests for symbol resolution order.

Fri, May 26, 6:38 PM
marsupial updated the diff for D33529: Allow clients to specify search order of DynamicLibraries..

Formatting.

Fri, May 26, 5:59 PM
marsupial updated the diff for D33529: Allow clients to specify search order of DynamicLibraries..
Fri, May 26, 5:35 PM

May 26 2017

marsupial added a comment to D30107: Refactor DynamicLibrary so searching for a symbol will have a defined order and libraries are properly unloaded when llvm_shutdown is called..

Indeed we are not. We are arguing the current set of patch (and proposed) patch imposed the order (1) then (maybe) (2) while we need (and the previous behavior was 'close' to) (2) then (1)

May 26 2017, 3:54 PM
marsupial added a comment to rL304020: DebugInfo: Do not emit empty CUs.

Seems this might have broken many bots.

May 26 2017, 1:17 PM
marsupial committed rL304027: Fix the ManagedStatic list ordering when using DynamicLibrary….
Fix the ManagedStatic list ordering when using DynamicLibrary…
May 26 2017, 12:43 PM
marsupial closed D33581: Fix the ManagedStatic list ordering when using DynamicLibrary::addPermanentLibrary. by committing rL304027: Fix the ManagedStatic list ordering when using DynamicLibrary….
May 26 2017, 12:43 PM
marsupial updated the diff for D33515: Force RegisterStandardPasses to construct std::function in the IPO library..

Formatting errors.

May 26 2017, 12:27 PM
marsupial added inline comments to D33515: Force RegisterStandardPasses to construct std::function in the IPO library..
May 26 2017, 12:23 PM
marsupial updated the diff for D33515: Force RegisterStandardPasses to construct std::function in the IPO library..
May 26 2017, 12:20 PM
marsupial updated the diff for D33515: Force RegisterStandardPasses to construct std::function in the IPO library..
May 26 2017, 12:13 PM
marsupial added inline comments to D33581: Fix the ManagedStatic list ordering when using DynamicLibrary::addPermanentLibrary..
May 26 2017, 11:59 AM
marsupial updated the diff for D33529: Allow clients to specify search order of DynamicLibraries..
May 26 2017, 11:52 AM
marsupial added a comment to D30107: Refactor DynamicLibrary so searching for a symbol will have a defined order and libraries are properly unloaded when llvm_shutdown is called..

https://reviews.llvm.org/D33529 is attempting to address this can we continue discussion there?
I'm totally understand the order being reversed, though I would like to keep an explicit bool to signify Don't resolve as the OS would
Otherwise from the example above someone who loads a lib with printf will pull that symbol from libA.so or libB.so and they might not actually want to.
They should explicitly request this behaviour.

May 26 2017, 10:18 AM
marsupial added a comment to D30107: Refactor DynamicLibrary so searching for a symbol will have a defined order and libraries are properly unloaded when llvm_shutdown is called..

Lets take a simple example of libRuntime.so, libA.so, libB.so.
libRuntime.so is linked to the binaries and contains stdlib functions, such as printf.
libA.so, and libB.so also contain a locally exported symbols printf, doSomething

May 26 2017, 10:00 AM
marsupial added a comment to D30107: Refactor DynamicLibrary so searching for a symbol will have a defined order and libraries are properly unloaded when llvm_shutdown is called..

It did neither (1) or (2) before, because the order the handles were stored was not defined.
This was particularly noticeable on Windows due to how it uses shim libs for runtime functions.
I assume you guys aren't arguing to going back to undefined behavior just because it worked better for you.

May 26 2017, 9:51 AM
marsupial added a comment to D30107: Refactor DynamicLibrary so searching for a symbol will have a defined order and libraries are properly unloaded when llvm_shutdown is called..

Why does https://reviews.llvm.org/D33529 not address your concerns?

May 26 2017, 7:56 AM

May 25 2017

marsupial added a reviewer for D33581: Fix the ManagedStatic list ordering when using DynamicLibrary::addPermanentLibrary.: efriedma.
May 25 2017, 6:36 PM
marsupial created D33581: Fix the ManagedStatic list ordering when using DynamicLibrary::addPermanentLibrary..
May 25 2017, 6:35 PM
marsupial added a comment to D33515: Force RegisterStandardPasses to construct std::function in the IPO library..

This works here and is way less of a hack.

May 25 2017, 6:33 PM
marsupial updated the diff for D33515: Force RegisterStandardPasses to construct std::function in the IPO library..
May 25 2017, 6:29 PM
marsupial updated the diff for D33515: Force RegisterStandardPasses to construct std::function in the IPO library..

This should fix the issue and adds a test case of what you're seeing.
Not sure if I want to be the one committing this as it's a bit ugly...

May 25 2017, 1:34 PM
marsupial added a comment to D33515: Force RegisterStandardPasses to construct std::function in the IPO library..

polly has historically worked as a plugin. In this context, polly isn't technically a "plugin" in the sense that it's not dynamically loaded, but it works like a plugin in every other sense (using the same extension points a plugin would use to integrate with LLVM).

May 25 2017, 12:57 PM
marsupial updated the diff for D33515: Force RegisterStandardPasses to construct std::function in the IPO library..

Perhaps we can try to close this one out before moving on to the larger problem?

May 25 2017, 12:34 PM
marsupial accepted D33490: Export the required symbol from DynamicLibraryTests.
May 25 2017, 9:20 AM
marsupial added a reviewer for D33381: Avoid constructing GlobalExtensions only to find out it is empty.: davide.
May 25 2017, 9:18 AM
marsupial updated the diff for D33529: Allow clients to specify search order of DynamicLibraries..
May 25 2017, 8:34 AM
marsupial added inline comments to D33529: Allow clients to specify search order of DynamicLibraries..
May 25 2017, 7:59 AM
marsupial added a comment to D33515: Force RegisterStandardPasses to construct std::function in the IPO library..

addGlobalExtension is documented to be used by plugins, where you are using it for a statically linked lib.
There doesn't seem to be any great solution and I going back to how it was (leaking) is one of the worse options.

May 25 2017, 7:44 AM
marsupial added inline comments to D33529: Allow clients to specify search order of DynamicLibraries..
May 25 2017, 7:28 AM

May 24 2017

marsupial added a comment to D30107: Refactor DynamicLibrary so searching for a symbol will have a defined order and libraries are properly unloaded when llvm_shutdown is called..

Can the discussion of this continue here https://reviews.llvm.org/D33529.

May 24 2017, 5:53 PM
marsupial updated the diff for D33529: Allow clients to specify search order of DynamicLibraries..

I'm not actually sure I'm in favor of this but wanted another opinion.
To me DynamicLibrary::addPermanentLibrary doesn't really seem all that useful and actually complicates things a bit.

May 24 2017, 5:52 PM
marsupial created D33529: Allow clients to specify search order of DynamicLibraries..
May 24 2017, 5:47 PM
marsupial added a comment to D33515: Force RegisterStandardPasses to construct std::function in the IPO library..

I'm sorry.
You're saying adding the changes to Hello.cpp,
make -j8 opt LLVMHello
bin/opt -load=lib/LLVMHello.so -S /dev/null
is still crashing?

May 24 2017, 2:42 PM
marsupial added a comment to D30107: Refactor DynamicLibrary so searching for a symbol will have a defined order and libraries are properly unloaded when llvm_shutdown is called..

You should test it with ROOT, there we do a dlopen(libcling, RTLD_LAZY|RTLD_LOCAL) and I think there the symbol resolution doesn't work as before.

RTLD_LOCAL means any symbols loaded will not be available for lookup.
If you are relying on symbol lookup via a process handle [ dlsym(dlopen(NULL, Flags), "Symbol") ] then you need to use RTLD_GLOBAL.

May 24 2017, 2:32 PM
marsupial added a comment to D30107: Refactor DynamicLibrary so searching for a symbol will have a defined order and libraries are properly unloaded when llvm_shutdown is called..

Symbol resolution is purposely different, as prior library search order was totally undefined which lead to serious problems (for cling in particular), see the Summary.

Right, all I am saying is it doesn't work and asking to revert because it makes the issue much worse on cling/ROOT end. I think you are mostly having in mind Windows which cling doesn't officially support, however, this patch breaks Unix.

Do you have more information or a test demonstrating the failure?

I think you should be able to reproduce it if you apply this patch and start up ROOT...

I'd really like to make sure that these issues aren't easily fixed or relying on prior behavior which was poorly defined before reversion.

I'd support that. What about reverting it now while you address the underlying out issues without pressure. If this patch was meant to address a cling issue, I'd have been happier to discuss and test it, before landing it.

May 24 2017, 1:59 PM