The new wil library for C++ code in drivers

For example, no object for KSPIN_LOCK and thus no guaranteed initialization

This is on its way. Feel free to file an issue on github or complain here if you’d like to see any other types.

What is preventing getting these updated to the latest file set?

Finding the time to do it. We seem to have finite hours in the day, and infinite tasks to work on… just like, I’m sure, everyone else here.

IMO, anything that allocates/deallocates behind the scenes would be a bad bad bad, very bad idea in the kernel.

A big part of C++'s offering is that it can hide things. E.g., it can hide a call to Dereference, Unlock, Free, Close, LowerIrql, or CompleteRequest. Whether you want this in your codebase is a matter of taste, and I can’t ever see Microsoft ever forcing this decision on a driver writer. If you can’t grep for all the Free calls because they’re invisible, how can you possibly debug a memory leak? But the counterargument is that if the Free code is automatic, there aren’t memory leaks. Whether you & your codebase buys into that is your choice.

potential for confusion “down stream”

This is very real, and something we’ve been (and still are) working through internally. At first, a lot of devs were startled to see bits of wil spackled throughout code reviews, and there were more than a few “WTF is this?” comments. Over time, though, the thing has caught on in many parts of Windows. So just as every win32 developer knows about the GetLastError() dance, we’re getting to the point where most win32 developers (internally) know about RETURN_IF_WIN32_BOOL_FALSE. (That macro is the equivalent of the GetLastError() dance, with some logging thrown in.)

Nobody (at this point, at least) is really pushing wil outside of Microsoft, but there’s some hazy dream of a future where every win32 developer just knows these macros and types, and it would look weird to manually spell out the error-checking or handle-closing.

I think you misunderstood me.
There are three different issues here:

  • list/map/array doing allocations/deallocations/reallocations behind
    the scenes, quite often. In case of a file system filter, this can
    easily cause memory fragmenttion issues. Without a lot of control of
    granularity of allocations (reallocates on 128 item boundary for
    example, rather than some random/internal number of items…), or a way
    to use lookasides in some way.
  • A class that internally allocates a variable sized pool, and is used
    extremely often. CLogInformation, as a DbgPrint alternate comes to
    mind here. We cannot address something like this, without changing the
    code to use lookasides (what we do in our code whenever strings are
    used)
  • General carelessness for allocations/deallocations. This is not a
    matter of tracking or anything us oldies may need to worry about. But,
    and I am thinking out loud here, rather than with some concrete data
    like above (IANAP - I am not a psychologist :)). I think this will
    cause newcomers to rely heavily on such structures, and often forget
    that the rest (most?) of the code needs manual cleanup.

So, for the first two, I have concrete suggestions: design the
structures or add helper APIs that would mitigate memory
fragmentation.
For the third one - I have no concrete idea :slight_smile:

The reason this is on my mind right now is that it just so happens
that my current project would feel issues #1/#2 in a few hours :slight_smile:

> IMO, anything that allocates/deallocates behind the scenes would be a bad
> bad bad, very bad idea in the kernel.

A big part of C++'s offering is that it can hide things. E.g., it can hide
a call to Dereference, Unlock, Free, Close, LowerIrql, or CompleteRequest.
Whether you want this in your codebase is a matter of taste, and I can’t
ever see Microsoft ever forcing this decision on a driver writer. If you
can’t grep for all the Free calls because they’re invisible, how can you
possibly debug a memory leak? But the counterargument is that if the Free
code is automatic, there aren’t memory leaks. Whether you & your codebase
buys into that is your choice.

On May 11, 2019, at 1:36 AM, Rourke wrote:
>
> The WDK is using standard library files that are older than some people here. What is preventing getting these updated to the latest file set? The language has marched a long way since the 90’s. No one is asking for exceptions and i/o. But certainly std::byte, std::array, std::unique_ptr, and others would be terrific to have in the kernel arsenal.

Developing, testing, and maintaining a C++ standard library is an enormous job. ENORMOUS. The special cases and performance guarantees are explicit, onerous, and painful, and there are relatively large changes every 3 years… I don’t believe any kernel developer is qualified to do that. It has to be someone from the compiler team, and they are busy with more important things.

Tim Roberts, timr@probo.com
Providenza & Boekelheide, Inc.

list/map/array doing allocations/deallocations/reallocations behind the scenes […] A class that internally allocates a variable sized pool

Gotcha. Currently, wil doesn’t include any data structures (list, array, tree, etc). Currently, wil is more about wrapping existing functionality. Since wil is cobbled together from the contributions of dozens of engineers who were all scratching some itch, it’s hard to say that it’ll never have this or that feature. But it’s more likely to get a wrapper for RTL_GENERIC_TABLE than to get its own tree written from scratch. So wil tends to not directly allocate things; it tends to wrap some C-language API that does the allocation.

NDIS does have its own generic data structure, which we’ve published here. But this will probably not graduate into wil. (Unlike, say, the spinlock and Ex-rundown wrappers in the same directory on that github repo – those are good candidates for wil.)

On the topic of error checking & logging, I did a lot of very similar stuff using WPP macros in my previous workplace. Is it the direction you’re locking for? Every place I’ve worked in defined “logging” differently.

I also recall writing something of an std::string / std::wstring wrapper for safe string, and UNICODE_STRING types, though we made some assumptions regarding dispatch level safety and unicode tables :slight_smile:

For example, I find RETURN_IF_NTSTATUS_FAILED(ZwCreateFile(. . . ));

heinous enough that I’d require it to be changed. But it becomes religion and reasonable folks will differ.

Well, probably some folks , indeed, just love to do things the way they are done in the sample below

https://github.com/microsoft/Terminal/blob/master/src/server/Entrypoints.cpp

If you ask my personal opinion, I would rather do everything in a “plain old C”, with all the cleanup done in the end of the function in the reverse order of creation/allocation/initialisation/etc, and a “goto xyz;” jump to the corresponding label in the “cleanup section” in the end of the function upon failure. Call me a retrograde if you wish, but it seems (at least to me) so much easier to read and understand the code written this way.

Certainly, it maybe not always be possible to do things in so straightforward way, so that in some cases some “ugly” code may have to be written anyway. For example, consider the case when you may have to re-acquire the lock that you have already acquired and released in the same function In this case you would have to release the lock in the “main” code before jumping to the “cleanup section” in the end of the function. Certainly, one may say that the code that does things like that is broken anyway and has to be re-factored, with re-acquisition block being placed in a separate function, but this is already more of a “philosophical issue” that has to be judged on case-by-case basis.

Anton Bassov

I think the RETURN_IF_NTSTATUS_FAIL creates too much noise and emphasis on the error path. I’d rather see the main function not buried in an argument, and a go to.

It’s also too easy for these RETURN_IF_xxx things to grow unanticipated appendages, in special cases, where they return if !NT_SUCCESS(status) … OR if the status == STATUS_MY_SPECIAL_THING.

Religion… whatever suits you and those for whom you work,

Peter

@Dejan_Maksimovic said:
std:array/list/map and even unique_ptr sound exactly like the kind of
stuff that would be the problem, without the user realizing it,

std::array does NOT allocate anything behind the scenes, ever. Neither does std::unique_ptr. Neither does anything in the original post about the wil library. No one is talking about lists and maps and behind the scenes allocations in the kernel except you babbling to yourself in error. So maybe before you post you can try to actually understand something about the new library or c++ and then try to make an intelligent comment. And by the way since you’ve been under a rock note that std::array is now approaching 10 years old and runs circles around c arrays and is a perfect fit for kernel usage. That’s why they made it.

The wil:: library has improved techniques for easy and fail safe freeing of resources. This is one of the big headaches of writing drivers is all the messy and error prone cleanup code. I like these new constructs much better than spaghetti of goto statements and I think if one posted a side by side comparison of the rat’s nest ndis code it would grow on people. Kind of like comparing the look of the mass of hand wiring on the back of very ancient mainframes compared to the neat and tidy look of land patterns on a brand new motherboard. And if you balk at all these auto free constructs then I better never see an automatic variable in your c code, anywhere, any kind. Because guess what? Automatic variables are freed automatically behind the scenes for you when they go out of scope–oh the horror!

On the topic of error checking & logging, I did a lot of very similar stuff using WPP macros in my previous workplace. Is it the direction you’re locking for? Every place I’ve worked in defined “logging” differently.

wil’s logging is configurable. The full story is documented here. The TL;DR is that you set some #defines to control whether errors get printed to DbgPrint, TraceLogging, or nowhere. You’d presumably have the noisy mode enabled for debug builds, and lower the verbosity for retail.

I would have liked to see WPP in there – I’m a fan of WPP. But WPP is not particuarly friendly to being included in someone else’s macro; it wants to own the top-level macro. So I’m doubtful that it’s technically possible to squeeze WPP into the wil error-handling framework. Well, at least without changing it so much it becomes some sort of WPP 2.0… :wink:

Keep in mind, though, that we haven’t done the work to hook up wil’s error-handling macros for kernel code yet, so this discussion is only immediately relevent for us in the context of UMDF. The big technical challenge for us is that, while wil’s error-handling accepts a variety of types of error codes, it internally converts everything to HRESULT, and its guts use HRESULT exclusively. Obviously that’s a non-starter for kernel code; we would need to find a way to plumb NDIS_STATUS, er, I mean NTSTATUS through there. Anything’s posisble with enough #defines, but the implementation of wil’s error macros is already teetering on the brink of over-abstracted unreadability; adding another dimension of abstraction might cause the moon to fall out of the sky.

since you’ve been under a rock

Ad hominem attacks & reddit-style hyperbole have no place in a community of professional engineers. We discuss the merits of ideas, not people. In this case, Dejan has expressed a very valid point, and considering that wil has basically no public documentation right now, it’s perfectly reasonable to ask if wil runs afoul of these concerns about invisible memory allocation.

This is one of the big headaches of writing drivers is all the messy and error prone cleanup code

Well, you don’t necessarily have to use “the tricks of cleanup masters” from MSFT SDK/WDK samples, do you…

…these new constructs much better than spaghetti of goto statements

Actually, “goto” statements don’t necessarily have to result in spaghetti code as long as you use them properly (if you need some practical examples you can check Linux and Illumos sources ). OTOH, if you decide to make your code jump all over the place like Mr.Ballmer in a "Monkey Boy " video shown on another thread, “goto” may, indeed, be quite useful for the purpose…

The wil:: library has improved techniques for easy and fail safe freeing of resources.

I don’t know about the other people, but I wold rather prefer to see the cleanup code in the same function where the target resources have been allocated. This is not just the question of writing the code, but mainly of maintenance. Certainly, this is just the question of taste.

Anton Bassov

>> std:array/list/map and even unique_ptr sound exactly like the kind of

>
> stuff that would be the problem, without the user realizing it,

std::array does NOT allocate anything behind the scenes, ever. Neither does
std::unique_ptr. Neither does anything in the original post about the wil
library. No one is talking about lists and maps and behind the scenes
allocations in the kernel except you babbling to yourself in error. So maybe
before you post you can try to actually understand something about the new
library or c++ and then try to make an intelligent comment. And by the way
since you’ve been under a rock note that std::array is now approaching 10
years old and runs circles around c arrays and is a perfect fit for kernel
usage. That’s why they made it.

Easy… I did not say they made it, I said if it were to be made, it
should take a lot of caution. And array is just one of the 3 I
mentioned.

Ad hominem attacks & reddit-style hyperbole have no place in a community of professional engineers

Too right.

Mr. Rourke… I’m now officially cautioning you. I don’t like your attitude, in general. And now, you have embarrassed me by your behavior in my house, to the point where another one of my valued guests has had to remind you of your manners.

Come correct, now. Lose the attitude, and decrease the stridency of your replies, immediately. Or go play elsewhere. Please. Thank you.

Peter

@“Jeffrey_Tippet_[MSFT]” said:

On the topic of error checking & logging, I did a lot of very similar stuff using WPP macros in my previous workplace. Is it the direction you’re locking for? Every place I’ve worked in defined “logging” differently.

wil’s logging is configurable. The full story is documented here. The TL;DR is that you set some #defines to control whether errors get printed to DbgPrint, TraceLogging, or nowhere. You’d presumably have the noisy mode enabled for debug builds, and lower the verbosity for retail.

I would have liked to see WPP in there – I’m a fan of WPP. But WPP is not particuarly friendly to being included in someone else’s macro; it wants to own the top-level macro. So I’m doubtful that it’s technically possible to squeeze WPP into the wil error-handling framework. Well, at least without changing it so much it becomes some sort of WPP 2.0… :wink:

Oh wow, I’ve only dived in to the code some hours after replying, and only then saw the scope. Indeed, it could take more than a single weekend to add WPP support :wink:

I imagined WPP would be the standard for kernel due to performance and security (erm, obfuscation) considerations, so it could be on by default when deployed. If I’m missing some obfuscation ability in TraceLogging I’ll be delighted to get a push in the right direction.

In any case, this is great work. I’m excited to see modern C++ idioms in Kernel space.

Nobody (at this point, at least) is really pushing wil outside of Microsoft, but there’s some hazy dream of a future where every win32 developer just knows these macros and types, and it would look weird to manually spell out the error-checking or handle-closing.

For me it looks like a nightmare. Marcos which influence code flow (contain return or goto) are IMO plain evil. They make code shorter but less readable and in turn less robust/reliable. Call me dinosaur but I prefer when the code is readable as a book and code flow is explicitly expressed. Here you’re trying to express it in the macro name which is an interesting idea but it leads to longer macro names and makes this problem worse:

I think the RETURN_IF_NTSTATUS_FAIL creates too much noise and emphasis on the error path. I’d rather see the main function not buried in an argument, and a go to.

I’ve seen too much code plagued by REQUIRE-like macros and it is far less readable than longer version which handles errors explicitly. For this reason. You see just a chain of REQUIREs and what is code really doing is hard to decipher.

You’re one of my favorite dinosaurs :slight_smile:

mm

Creating fossils museum? :wink:

Ad hominem attacks & reddit-style hyperbole have no place in a community of professional engineers.

Totally agree and I would never do that. A bigger problem I see is people who carelessly post false or misleading information in a forum dedicated to professionals helping each other. Faulty content has no place here and should be called into question for the benefit of the poster than made it and even more for others that read it. There is nothing personal about setting the record straight.

RETURN_IF_NTSTATUS_FAILED(ZwCreateFile(. . . ));

My first reaction when i saw this is it’s extremely ugly and breaks a lot of rules and no way I would ever want to see that in code. But thinking about it some more this is really just a poor man’s exception. Almost a throw statement if you will. If you look at it in that light it then becomes a thinking point on how it might be useful.