Re: [RFC] Fix libltdl preloader issues with MSVC

[Peter Rosin]

Hi Gary,

On 2012-10-20 07:26, Gary V. Vaughan wrote:
> Hi Peter,
> 
> On Oct 20, 2012, at 4:06 AM, Peter Rosin <address@hidden> wrote:
>> With the latest dumpbin work in place, libtool starts to add
>> symbols from import libraries to the generated fooS.c files
>> in func_generate_dlsyms, as needed by the preloader. And then
>> the next problem naturally rears its head. This time it is
>> the age-old data exports problem responsible for code such
>> as this (in demo.at):
>>
>>         else if (STREQ ("nothing", name))
>> #ifndef _WIN32
>>      /* In an ideal world we could do this... */
>>      pnothing = (int*)s->address;
>> #else /* !_WIN32 */
>>      /* In an ideal world a shared lib would be able to export data */
>>      pnothing = (int*)&nothing;
>> #endif
>>
>> I.e., demo.at isn't even trying to use the data symbol from the
>> preloader, it just picks data up directly using the import
>> library on Windows. Which is of course cheating and circumvents
>> the API etc etc. However, MinGW has long supported pseudo-relocs
>> and anto-import and the "Ideal world" case should work just fine
>> with gcc even on _WIN32. But not with MSVC, which fails to link
>> since data symbols from import libraries are not declared
>> __declspec(dllimport).
> 
> That's because demo stems from the original test code that Gord
> wrote over 15 years ago alongside the embryonic preloading
> implementation, all before the current API was available! :)
> 
> Having said that, it's kinda nice to have this test of the low-level
> structure of preloaded library data, although calling it demo might
> seem like we're still encouraging folk to use preloaded libraries
> this way :(  If you feel inclined to move the existing low-level tests
> aside and rename them to something less inviting, and then rewrite
> the demo tests to actually use the API that would be great for our
> test coverage too!

Yes, I was kind of surprised that evidently the most involved preloader
test was "demo". I thought that libltdl basically worked with MSVC,
as long as you took care to apply the correct #ifdef madness. But that
proved very wrong when you moved demo/ to demo.at and hardened it
ever so slightly...

>> Example from demo.at:
>>
>> helldl.exeS.obj : error LNK2001: unresolved external symbol _nothing
>>
>> Fixing that, and making data symbols __declspec(dllimport) is
>> however not the entire solution, as that yields:
>>
>> helldl.exeS.c(47) : error C2099: initializer is not a constant
>>
>> during compilation instead.
>>
>> The underlying problem is that MSVC does not have pseudo-relocs.
> 
> More hoops to jump through at the Microsoft circus.  Hurray!

Ain't it fun? :-/

>> So, this patch adds an @INIT@ entry to the preloader symbol
>> table, with the address of a function that is intended to do the
>> relocations of data items from user-provided code instead of
>> relying on the tools. This function is then filled with
>> relocation code for MSVC, but left empty everywhere else
>> where it is not needed.
>>
>> This is clearly not a finalized patch (it's actually pretty
>> rough), it's just proof of concept. I have e.g. not optimized
>> it, but it is clearly possibly to set the @INIT@ address to
>> NULL and don't call @INIT@ at all in non-MSVC cases. It should
>> also not be necessary to relocate data symbols from static
>> libraries from user code, and it should be possible to add a
>> "done" variable to the relocation function, so that it only
>> does the work once.
>>
>> What I'm looking for is feedback. Is it acceptable to add a
>> new virtual @INIT@ entry like this?  I.e. is this approach
>> workable or should I just drop it?
> 
> I have no objections, except perhaps that it would be good to check
> that the ABI doesn't stop pre-patch libltdl from preloading libraries
> built with post-patch libtool.  But that's likely overly pedantic of me,
> since I don't think anyone is likely to preload libraries built from a
> different package (potentially using different libtool versions)... but
> if it's easy to be compatible, we might as well do that.

Building the actual library has not changed one bit, only the
preloading glue, so as long as the new glue is used I don't expect
any trouble. I didn't even stop to think that it could be a
problem...

> I can't think of any other use for @INIT@ than beating MSVC into
> submission, but as long as you can do the final implementation
> cleanly enough that it doesn't force all the well behaved platforms
> to carry around dead weight for MSVC (eg not adding @INIT@ to
> the symbol list when it's not going to be used), and that we can keep
> everything nicely separated so that people who don't need to
> understand it to contribute in other areas can easily skip past... then,
> sure, go right ahead :)

Good, I'll post an updated patch for review when I have applied
some polish. But not this week, as I'm traveling...

Cheers,
Peter