Hi all

Sorry for jumping in since I'm not a ReactOS developer (though I've been following the progress regularly and looking for any ReactOS news for some years already).

From what I think, making some special allowance for the behaviour of
applications checking APIs whether they exist is certainly a good idea. But I don't think that shimming each and every DLL that may have differences is the way to go toward this end.

There's an old saying in Linux circles: "Make the common case fast..." - but from the maintenance point of view, particularly for a smaller project, I'd like to extend it to a slightly different interpretation: Make the common case less error-prone!

There are 2 main observations that are the basis for my reasoning. They are hardly "scientific", so correct me if you feel that they are wrong.

1. Most APIs of any one DLL, even if different vintage, can peacefully coexist in that same DLL at the same time - exceptions, if any, are rare.
2. The most common user-mode check for an API is through GetProcAddress()

The reasoning for 1. is that newer versions usualls only add APIs, but very rarely remove them. Deprecated APIs are mostly retained. If the DLL is made aware of what the process expects, it could still fine-tune special cases where they apply on an as-needed basis. As for 2. this is the documented way (rather than calling NTDLL APIs directly, which very few applications do, or parsing the export sections of system DLLs, which should be extremely rare for a "normal" program.

There is another observation (at least I guess that's the case, my knowledge of the Win32 API basically ends with XP):

3. There seems to be no well-known defined way (yet) to assign to a process the information, which API version it expects to see and use.

From my point of view, Microsoft made a huge mistake here, one that ReactOS
should rather avoid than copy. Having no standardized ability for the modules to know (to "publish" the information), on a per-process basis, what behaviour is expected by the application, leads to a lot of DLL duplication which is a maintenance nightmare. As evidenced by the tremendous size increase of the WinSxS directory in each Windows version in recent years, even Microsoft seems to have a struggle with it - despite the size and resources of that company.

Therefore I would like to propose a ReactOS specific addition in order to keep this potential nightmare under control:

Introduce the concept of a process specific ReactOS API version numbering.

In a central place (preferably inside NTDLL, I'll explain later why), in each process, there should be located a (new) structure that stores the process API version. It should be read-only to the process, accessible only through a special (new) API.

Maybe one like this example:

typedef struct {
   DWORD              cbSize;  /* of this structure */
   /* all the data that GetVersionEx may need  */
   OSVERSIONINFOEX    OSVersionInfo;
   /* possibly some space for future version-related things */
   BYTE               bReserved[64];
} NT_API_VERSION_PROCESS;

void NTAPI GetNtApiVersionProcess(NT_API_VERSION_PROCESS *lpVersion);

Although this is just an example, feel free to use something more fitting.

It should be populated early at the process creation, normally with the default ReactOS API version (now NT 5.2), but in special cases either a compatibility SDB shim or the process loader may write something else here (an application-specific dataset).

Particularly, if the loader finds (from the PE header where possible) that the application was linked for a higher API version, it should initialize the structure to the nearest compatible OS version that ReactOS is (in the future hopefully) able to support.

Because NTDLL is (hopefully, my knowledge in this regard may be outdated) the first DLL in a process to be loaded, this would make the version info easily accessible early during process startup, particularly during the DLL_PROCESS_ATTACH phase of all the other DLLs that this process will load during its lifetime.

Most DLLs won't need this info, but those that will (Kernel32, User32, GDI32, AdvApi32 most likely) can initialize whatever special behaviour they need to initialize based on this data. The data is read-only and static for the process lifetime, so early initialization should be OK.

A special case: GetProcAddress should respect the per-process API version info in order to present the caller with a consistent view of APIs for the core DLLs (it should know certain core DLLs and filter APIs and return NULL for those that are not supposed to exist form the point of view of the current process). The dynamic linker / process loader should also do accordingly (preferably by sharing the code, but treating them as unresolved externals). This way the early-load core DLLs (Kernel32, User32, GDI32, AdvAPI32, plus NTDLL itself) can be kept in a single version and without shims. Also GetVersionEx and all related functions should respect and return the per-process version info too.

Due to their early-load behaviour, shimming these "special" DLLs is tricky and may be very error-prone. Particularly NTDLL, making this one shimmable would likely require much code rewriting in the loader and crazy special case handling, but Kernel32 would also be a tough nut to crack.

(Side note: I've tried to shim a system dll in Windows 2000 some time ago in order to add a newer API, and even though I understood how it worked reasonably well, and already had some experience of writing trampolines in assembly, as soon as I added something delay-load-related, the side effects turned the whole idea into a pit full of snakes that took lots of debugging - and it was "only" a network-related DLL, WS2_32, not even a core one).

Therefore, in my opinion, if it was possible to keep the early-load DLLs in one piece (without trampoline loaders and such) and rather do some limited version-specific handling inside their code (plus making GetProcAddress and the dynamic linker version-aware for those specific DLLs), this should avoid the horrors of shimming early-load DLLs.

Plus, having an easily accessible central place, where any other (possibly future) version-aware DLLs can query the API version that the calling process expects, should make it possible to handle other aspects of compatibility in a more organized way in the future, avoiding unneeded code duplication, a mess of DLLs, and many shims that do nothing else but load "special" DLLs.

Of course, there will still be many other DLLs where WinSxS makes more sense and where WinSxS is clearly the correct approach to be used, but for the core and early-load DLLs that are difficult and error-prone to shim correctly, I would recommend for making them version-aware and against shimming them (even MS seems to keep them in one piece while they put all the various MSVCRTs into WinSxS).

Best Regards
Dimitrij


----- Original Message ----- From: "Alex Ionescu" <ion...@videotron.ca>
To: "ReactOS Development List" <ros-dev@reactos.org>
Sent: Wednesday, May 18, 2016 9:33 PM
Subject: Re: [ros-dev] Pale Moon drops ReactOS support


I don't believe I said 'let's add random exports to our DLLs'. In
fact, in your old thread, I was totally FOR your idea, I even wrote
that the only sane way of doing it is with the app compat work.

On Wed, May 18, 2016 at 9:27 PM, Timo Kreuzer <timo.kreu...@web.de> wrote:
We have been discussing this before and I wonder that Alex is not heavily
opposing the idea of randomly adding new exports to our user mode DLLs. It
is a well known fact that applications check for existance of exports to
decide how to behave, so ... not going to go over this again.

To addess this issue I suggested to implement a compatibility layer. And
there was a detailed discussion about that on this mailing list.
https://www.reactos.org/pipermail/ros-dev/2015-March/017216.html

Please take your time to read the whole thread again so we can avoid wasting
time, talking about the same things again.

Timo

Best regards,
Alex Ionescu


_______________________________________________
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

Reply via email to