Skip to content

Conversation

@ben-craig
Copy link

WIP: I'm well aware that this isn't acceptable as is. This is initial work so that I can try to get concrete feedback earlier in the process.

This adds the capability to test the STL in the Windows kernel. The expectation is that the Windows kernel will support most of the freestanding facilities, with the likely exception of those involving RTTI and exceptions.

On the cmake side of things, I add a new executable that will do the necessary Windows things to load a kernel driver and run the unit test. I also create a static lib with shared per-test code. This includes a custom assert implementation that won't crash the system, but will instead continue executing and report the results up though the executable. This is basically downgrading the "assert" to an "expect". In practice, this hasn't caused me any problems so far.

On the lit side of things, I have code scattered all over the place (this is one of the things that I'm hoping to improve and get guidance on). I have a fair number of linker and compiler flags stuck in the lit code (as opposed to the .lst files). I currently have driver signing shoe-horned into the lit format class, so that I only sign tests that actually need to run. I also abuse a force include to inject the "name" of the driver into each .cpp file (this doesn't work for multi TU tests yet). This force include injection makes it so that I don't need to invoke the compiler an extra time per test.

The certificate used for driver signing is generated at test time, and is self signed. The certificate has a password that is generated during test time, and then forgotten when the test is done running. The certificate also has a very quick expiration date (2 days). All of these together should make the certificate both difficult to attack, and not worth attacking, as it will (at most) give you access to one CI VM for 2 days.

@ghost
Copy link

ghost commented Oct 20, 2020

CLA assistant check
All CLA requirements met.

@StephanTLavavej StephanTLavavej added the test Related to test code label Oct 20, 2020
@StephanTLavavej
Copy link
Member

@cbezault is planning to clean up the underlying test machinery, which should make your life easier (but will require changes to this PR).

Comment on lines 3 to +10

RUNALL_INCLUDE ..\universal_prefix.lst
RUNALL_CROSSLIST
PM_CL="/EHsc /MTd /std:c++latest /permissive- /FImsvc_stdlib_force_include.h /wd4643"
# PM_CL="/EHsc /MTd /std:c++latest /permissive- /FImsvc_stdlib_force_include.h /wd4643"
PM_CL="/kernel /std:c++latest /permissive- /FImsvc_stdlib_force_include.h /wd4643"
RUNALL_CROSSLIST
PM_CL="/analyze /Zc:preprocessor"
PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing"
# PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file shouldn't be merged, as it is just a prototype hack. We will need to figure out what to do with the /EHsc, /MTd, and /kernel flags at some point.

Comment on lines +4 to +6
// This header is intended to be force included into the .cpp under test.
// KERNEL_TEST_NAME will then be provided as a /D switch and preprocessor
// concatenated into one big string literal.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach may need to change, depending on whether we can figure out another way to do multi-TU tests or not.

Comment on lines +7 to +8
extern const wchar_t *STL_KERNEL_NT_DEVICE_NAME;
extern const wchar_t *STL_KERNEL_DOS_DEVICE_NAME;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed a const on the * here (and in the definition)

Comment on lines +22 to +26
std::string err_text = "Error: ";
err_text += routine;
err_text += " failed. GetLastError = " + std::to_string(GetLastError());
err_text += "\n";
err_text += error_buffer;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking forward to std::format!

if self.is_kernel and mode in (self.CM_PreProcess, self.CM_Compile, self.CM_Default):
name = str(out).replace('\\','.').replace(':','.')
cmd.extend(['/DKERNEL_TEST_NAME=L"' + name + '"'])
# TODO need to sign the linked binary
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO is TODONE, just in a different file

Comment on lines +410 to +418
if self.target_arch.casefold() == 'AMD64'.casefold():
arch_name = 'x64'
else:
arch_name = self.target_arch.lower()

self.default_compiler.link_flags.extend([
'/LIBPATH:' + self.wdk_lib + '/km/' + arch_name,
'/IGNORE:4210',
'/machine:'+arch_name,
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I haven't tested 32-bit kernels at all.

Comment on lines -79 to 81

def run(self, exe_path, cmd=None, work_dir='.', file_deps=None, env=None):
cmd = cmd or [exe_path]
return self.chain.run(exe_path, self.commandPrefix + cmd, work_dir,
def run(self, cmd, work_dir='.', file_deps=None, env=None):
return self.chain.run([self.commandPrefix] + cmd, work_dir,
file_deps, env=env)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was just broken. PostFix looks broken too, but I haven't had a need to use it.

Comment on lines +40 to +59
@dataclass
class SigningRunStep(TestStep):
is_kernel: bool = field(default=False)
wdk_bin: str = field(default=None)

cert_path: os.PathLike = field(default=Path(''))
cert_pass: str = field(default="")
def run(self):
# TODO clean up this hacky mess
if self.is_kernel:
# sign the binary before running it.
cmd = [self.wdk_bin + '/x64/signtool.exe', 'sign',
'/f', self.cert_path,
'/p', self.cert_pass,
self.cmd[0]]
out, err, rc = stl.util.executeCommand(cmd, self.work_dir)
if rc != 0:
self.should_fail = False
return (cmd, out, err, rc)
return super().run()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the comment suggests, I don't think too highly of this approach. A more correct approach may be to add a build step enum that covers signing, or to make the link step also do signing.

@StephanTLavavej
Copy link
Member

@cbezault says that #1421 is expected to supersede this PR; he's going to give you push access to his fork.

@cbezault
Copy link
Contributor

Let's move everything to #1421. (Unless you object)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Related to test code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants