From 4e94f06fe4d4435a33c774d04adb1d80205ea25c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 18 Jul 2019 01:08:12 +0200 Subject: [PATCH 01/44] WIP Mbed TLS makefile generator Create a build directory with subdirectories and symlinks. Generate a makefile. Collect dependencies by running `cpp -MM`. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 691 ++++++++++++++++++++++++++++++++ 1 file changed, 691 insertions(+) create mode 100755 tools/bin/mbedtls-prepare-build diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build new file mode 100755 index 0000000..52c50d1 --- /dev/null +++ b/tools/bin/mbedtls-prepare-build @@ -0,0 +1,691 @@ +#!/usr/bin/env python3 +"""Generate a makefile for Mbed Crypto or Mbed TLS. +""" + +import argparse +import glob +import itertools +import os +import re +import shutil +import subprocess +import sys +import tempfile + +def sjoin(*args): + return ' '.join(args) + +def append_to_value(d, key, *values): + lst = d.setdefault(key, []) + lst += values + +class MakefileMaker: + def __init__(self, options, source_path): + self.options = options + if self.options.indirect_extensions: + self.executable_extension = '$(EXEXT)' + self.library_extension = '$(LIBEXT)' + self.object_extension = '$(OBJEXT)' + self.shared_library_extension = '$(DLEXT)' + else: + self.executable_extension = self.options.executable_extension + self.library_extension = self.options.library_extension + self.object_extension = self.options.object_extension + self.shared_library_extension = self.options.shared_library_extension + if self.options.cpp_mg is None: + self.options.cpp_mg = self.options.cc + ' -MM' + self.source_path = source_path + self.out = None + self.libraries = None + self.help = {'help': 'Show this help.'} + self.clean = [] + # Unset fields that are only meaningful at certain later times. + # Setting them here makes Pylint happy, but having set them here + # makes it harder to diagnose if some method is buggy and attempts + # to use a field whose value isn't actually known. + del self.libraries # Set when generating the library targets + del self.out # Set only while writing the output file + + def list_source_files(self, directory, pattern): + sources = glob.glob(os.path.join(self.options.source, + directory, pattern)) + start = len(self.options.source) + 1 + return sorted([src[start:] for src in sources]) + + def line(self, text): + self.out.write(text + '\n') + + def words(self, *words): + self.line(' '.join(words)) + + def format(self, template, *args): + self.line(template.format(*args)) + + def comment(self, template, *args): + self.format('## ' + template, *args) + + def target(self, name, dependencies, commands, + help=None, phony=False, short=None): + for dep in dependencies: + self.format('{}: {}', name, dep) + if not dependencies: + self.format('{}:', name) + if short is not None: + self.format('\t@$(ECHO_IF_QUIET) " {}"', short) + for com in commands: + self.format('\t{}{}', + ('' if short is None else '$(Q)'), + com.replace('\n', ' \\\n\t')) + if help is not None: + self.help[name] = help + if phony: + self.format('.PHONY: {}', name) + + def settings_section(self): + self.comment('Path settings') + self.words('SOURCE_DIR =', self.source_path) + self.words('SOURCE_DIR_FROM_TESTS = ../$(SOURCE_DIR)') + self.line('') + self.comment('Tool settings') + self.words('AR =', self.options.ar) + self.words('ARFLAGS =', self.options.arflags) + self.words('CC =', self.options.cc) + self.words('CFLAGS =', self.options.cflags) + self.words('WARNING_CFLAGS =', self.options.warning_cflags) + self.words('LDFLAGS =', self.options.ldflags) + self.words('PERL =', self.options.perl) + self.words('PYTHON =', self.options.python) + self.words('RM =', self.options.rm) + self.line('') + self.comment('Configuration') + if self.options.indirect_extensions: + self.line('DLEXT = ' + self.options.shared_library_extension) + self.line('LIBEXT = ' + self.options.library_extension) + self.line('OBJEXT = ' + self.options.object_extension) + self.line('EXEXT =' + self.options.executable_extension) + self.line('') + self.comment('Internal variables') + self.line('AUX_ECHO_IF_QUIET_ = :') + self.line('AUX_Q_ =') + self.line('AUX_ECHO_IF_QUIET_$(V) = echo') + self.line('AUX_Q_$(V) = @') + self.line('ECHO_IF_QUIET = $(AUX_ECHO_IF_QUIET_)') + self.line('Q = $(AUX_Q_)') + + def ensure_file(self, filename, created_list): + path = os.path.join(self.options.dir, filename) + if os.path.exists(path): + return + created_list.append(path) + open(path, 'w').close() + + def c_dependencies(self, filenames, prefix, + ensure_dependencies=None, + postprocess=None): + cmd = re.split(r'\s+', self.options.cpp_mg.strip()) + cmd += ['-I', 'include/mbedtls'] # for "config.h" + cmd += ['-I', 'include'] # for "mbedtls/config.h" + cmd += ['-I', 'source/include'] + cmd += ['-I', os.path.dirname(filenames[0])] + cmd += ['-I', 'source/library'] + cmd += ['source/' + filename for filename in filenames] + files_to_remove = [] + try: + if ensure_dependencies is not None: + ensure_dependencies(prefix, files_to_remove) + dependencies_b = subprocess.check_output(cmd, cwd=self.options.dir) + finally: + for filename in files_to_remove: + if os.path.exists(filename): + os.remove(filename) + dependencies = re.sub(r'[\t ]*\\\n[\t ]*', r' ', + dependencies_b.decode('ascii')) + for deps in dependencies.rstrip('\n').split('\n'): + deps = re.sub(r'[^ ]*/\.\./source/', r'source/', deps) + self.out.write(prefix + deps.strip() + '\n') + + def tests_c_dependencies(self, function_files): + with tempfile.TemporaryDirectory(dir=self.options.dir) as tmpdir: + for function_file in sorted(function_files.keys()): + c_files = [os.path.basename(c_file) + for c_file in function_files[function_file]] + for c_file in c_files: + with open(os.path.join(tmpdir, c_file), 'w') as out: + for part in (['tests/suites/{}.function'.format(base) + for base in ['helpers', + 'host_test', + #'main_test', + ]] + + [function_file]): + out.write('#include "../source/{}"\n'.format(part)) + #TODO: need to postprocess to remove tmpdir and change .o to exe + self.c_dependencies(glob.glob(os.path.join(tmpdir, '*')), 'tests/') + + _potential_libraries = ['crypto', 'x509', 'tls'] + @staticmethod + def library_of(module): + module = os.path.basename(module) + if module.startswith('x509') or \ + module in ['certs', 'pkcs11']: + return 'x509' + elif module.startswith('ssl') or \ + module in ['debug', 'net', 'net_sockets']: + return 'tls' + else: + return 'crypto' + + def library_section(self): + self.comment('Library targets') + self.words('LIBRARY_CFLAGS =', + '-I include/mbedtls', # must come first, for "config.h" + '-I include', + '-I $(SOURCE_DIR)/library', + '-I $(SOURCE_DIR)/include') + # Enumerate modules and emit the rules to build them + source_files = self.list_source_files('library', '*.c') + modules = [os.path.splitext(source)[0] for source in source_files] + for module in modules: + o_file = module + self.object_extension + c_file = '$(SOURCE_DIR)/' + module + '.c' + self.target(o_file, [c_file], + [sjoin('$(CC)', + '$(WARNING_CFLAGS)', + '$(CFLAGS)', + '$(LIBRARY_CFLAGS)', + '-o $@', + '-c', c_file)], + short=('CC ' + c_file)) + self.c_dependencies(source_files, 'library/') + contents = {} + # Enumerate libraries and the rules to build them + for lib in self._potential_libraries: + contents[lib] = [] + for module in modules: + contents[self.library_of(module)].append(module) + libraries = [lib for lib in reversed(self._potential_libraries) + if contents[lib]] + for lib in libraries: + self.format('libmbed{}_modules = {}', lib, ' '.join(contents[lib])) + self.format('libmbed{}_objects = $(libmbed{}_modules:={})', + lib, lib, self.object_extension) + self.target('library/libmbed{}{}'.format(lib, self.library_extension), + ['$(libmbed{}_objects)'.format(lib)], + ['$(AR) $(ARFLAGS) $@ $(libmbed{}_objects)'.format(lib)], + short='AR $@') + self.libraries = ['library/libmbed{}{}'.format(lib, self.library_extension) + for lib in libraries] + self.target('lib', self.libraries, + [], + help='Build the static libraries.', + phony=True) + self.clean.append(sjoin(*['library/*' + ext + for ext in (self.library_extension, + self.object_extension, + self.shared_library_extension)])) + + @staticmethod + def dash_l_lib(lib): + base = os.path.splitext(os.path.basename(lib))[0] + if base.startswith('lib'): + base = base[3:] + if not base.startswith('mbed'): + base = 'mbed' + base + return '-l' + base + + _auxiliary_objects = { + 'programs/ssl/ssl_client2': ['programs/ssl/query_config'], + 'programs/ssl/ssl_server2': ['programs/ssl/query_config'], + 'programs/test/query_compile_time_config': ['programs/ssl/query_config'], + } + _auxiliary_sources = set([obj + for _main, objs in _auxiliary_objects.items() + for obj in objs]) + + @staticmethod + def program_uses_lib(app, lib): + basename = os.path.basename(app) + subdir = os.path.basename(os.path.dirname(app)) + if lib == 'crypto': + return True + elif lib == 'x509': + return (subdir in ['ssl', 'x509'] or + (subdir == 'test' and basename == 'selftest')) + elif lib == 'tls': + return (subdir == 'ssl' or + (subdir == 'x509' and basename == 'cert_app') or + basename.endswith('_client') or + basename.endswith('_server') or + basename.endswith('_proxy')) + + def ensure_programs_dependencies(self, prefix, files_to_remove): + if prefix == 'programs/psa/': + self.ensure_file('programs/psa/psa_constant_names_generated.c', + files_to_remove) + + def program_subsection(self, source_file, executables): + base = os.path.splitext(source_file)[0] + object_file = base + self.object_extension + source_path = '$(SOURCE_DIR)/' + source_file + self.target(object_file, [source_path], + [sjoin('$(CC)', + '$(WARNING_CFLAGS)', + '$(CFLAGS)', + '$(PROGRAMS_CFLAGS)', + '-c', source_path, + '-o $@')], + short='CC $@') + if base in self._auxiliary_sources: + return + exe_file = base + self.executable_extension + object_deps = [dep + self.object_extension + for dep in self._auxiliary_objects.get(base, [])] + libs = [lib for lib in reversed(self._potential_libraries) + if self.program_uses_lib(base, lib)] + lib_files = ['library/libmbed{}{}'.format(lib, self.library_extension) + for lib in libs] + dash_l_libs = [self.dash_l_lib(lib) for lib in libs] + self.target(exe_file, [object_file] + object_deps + lib_files, + [sjoin('$(CC)', + object_file, + sjoin(*object_deps), + '$(LDFLAGS)', + '$(PROGRAMS_LDFLAGS)', + sjoin(*dash_l_libs), + '-o $@')], + short='LD $@') + executables.append(exe_file) + + def programs_section(self): + self.comment('Sample programs') + self.words('PROGRAMS_CFLAGS =', + '-I include', + '-I $(SOURCE_DIR)/library', + '-I $(SOURCE_DIR)/include') + self.words('PROGRAMS_LDFLAGS =', + '-L library') + sources = self.list_source_files('', 'programs/*/*.c') + executables = [] + for source_file in sources: + self.program_subsection(source_file, executables) + def ensure_dependencies(*args): + return self.ensure_programs_dependencies(*args) + for subdir, files in itertools.groupby(sources, os.path.dirname): + self.target(subdir + '/seedfile', ['tests/seedfile'], + ['cp tests/seedfile $@']) + self.c_dependencies(list(files), subdir + '/', + ensure_dependencies=ensure_dependencies) + self.words('programs =', *executables) + self.target('programs', ['$(programs)'], + [], + help='Build the sample programs.', + phony=True) + self.clean.append('programs/*/*{} $(programs)' + .format(self.object_extension)) + + def test_subsection(self, data_file, executables, function_files): + base = os.path.splitext(os.path.basename(data_file))[0] + try: + function_base = base[:base.index('.')] + except ValueError: + function_base = base + function_file = os.path.join(os.path.dirname(data_file), + function_base + '.function') + c_file = os.path.join('tests', base + '.c') + datax_file = os.path.join('tests', base + '.datax') + exe_file = os.path.join('tests', base + self.executable_extension) + self.target(sjoin(c_file, datax_file), + ['$(SOURCE_DIR)/' + function_file, + '$(SOURCE_DIR)/' + data_file, + 'test_host_generate_deps'], + [sjoin('cd tests &&', + '$(PYTHON) $(SOURCE_DIR_FROM_TESTS)/tests/scripts/generate_test_code.py', + '-f $(SOURCE_DIR_FROM_TESTS)/' + function_file, + '-d $(SOURCE_DIR_FROM_TESTS)/' + data_file, + '-t $(SOURCE_DIR_FROM_TESTS)/tests/suites/main_test.function', + '-p $(SOURCE_DIR_FROM_TESTS)/tests/suites/host_test.function', + '--helpers-file $(SOURCE_DIR_FROM_TESTS)/tests/suites/helpers.function', + '-s $(SOURCE_DIR_FROM_TESTS)/tests/suites', + '-o .')], + short='Gen $@') + self.target(exe_file, [c_file, '$(lib)', 'test_host_build_deps'], + [sjoin('$(CC)', + '$(WARNING_CFLAGS)', + '$(CFLAGS)', + '$(TESTS_CFLAGS)', + c_file, + '$(LDFLAGS)', + '$(TESTS_LDFLAGS)', + '$(test_libs)', + '-o $@')], + short='CC $@') + executables.append(exe_file) + self.target(base + '.run', [exe_file, 'tests/seedfile'], + ['cd tests && ./' + os.path.basename(exe_file)], + phony=True) + append_to_value(function_files, function_file, c_file) + + def tests_section(self): + self.comment('Test targets') + self.words('TESTS_CFLAGS = -Wno-unused-function', + '-I include', + '-I $(SOURCE_DIR)/library', + '-I $(SOURCE_DIR)/include', + '-I $(SOURCE_DIR)/tests') + self.words('TESTS_LDFLAGS =', + '-L library') + self.words('test_libs =', + *[self.dash_l_lib(lib) for lib in self.libraries]) + self.target('test_common_generate_deps', + ['$(SOURCE_DIR)/tests/suites/helpers.function', + '$(SOURCE_DIR)/tests/suites/main_test.function'], + [], phony=True) + self.target('test_host_generate_deps', + ['test_common_generate_deps', + '$(SOURCE_DIR)/tests/suites/host_test.function'], + [], phony=True) + self.target('test_target_generate_deps', + ['test_common_generate_deps', + '$(SOURCE_DIR)/tests/suites/target_test.function'], + [], phony=True) + self.target('test_host_build_deps', [], #TODO + [], phony=True) + data_files = self.list_source_files('tests/suites', '*.data') + function_files = {} + executables = [] + for data_file in data_files: + self.test_subsection(data_file, executables, function_files) + self.tests_c_dependencies(function_files) + self.words('test_apps =', *executables) + self.target('tests', ['$(test_apps)'], + [], + help='Build the host tests.', + phony=True) + self.target('tests/seedfile', [], + ['dd bs=64 count=1 $@']) + self.target('check', ['$(test_apps)', 'tests/seedfile'], + ['cd tests && $(PERL) scripts/run-test-suites.pl --skip=$(SKIP_TEST_SUITES)'], + help='Run all the test suites.', + phony=True) + self.clean.append('tests/*.c tests/*.datax $(test_apps)') + + def help_lines(self): + return ['{} - {}'.format(name, self.help[name]) + for name in sorted(self.help.keys())] + + def output_all(self): + self.comment('Generated by {}', ' '.join(sys.argv)) + self.comment('Do not edit this file! All modifications will be lost.') + self.line('') + self.settings_section() + self.line('') + self.target('default', ['all'], [], phony=True) + self.line('') + self.target('all', ['lib', 'programs', 'tests'], [], phony=True) + self.line('') + self.target('pwd', [], ['pwd'], phony=True, short='PWD') # for testing + self.line('') + self.library_section() + self.line('') + self.programs_section() + self.line('') + self.tests_section() + self.line('') + self.target('clean', [], + ['$(RM) ' + patterns for patterns in self.clean], + help='Remove all generated files.', + short='RM {generated files}', + phony=True) + self.line('') + self.target('help', [], + ['@echo "{}"'.format(line) for line in self.help_lines()], + phony=True) + self.line('') + self.comment('End of generated file.') + + def generate(self): + destination = os.path.join(self.options.dir, 'Makefile') + temp_file = destination + '.new' + with open(temp_file, 'w') as out: + try: + self.out = out + self.output_all() + finally: + del self.out + os.replace(temp_file, destination) + +class ConfigMaker: + def __init__(self, options): + self.options = options + self.source_file = options.config_file + if self.source_file is None: + self.source_file = os.path.join(options.source, + 'include', 'mbedtls', 'config.h') + self.source_file_path = 'source/include/mbedtls/config.h' + else: + self.source_file_path = os.path.abspath(source_file) + self.target_file = os.path.join(options.dir, + 'include', 'mbedtls', 'config.h') + + def start(self): + raise NotImplementedError + + def set(self, name, value): + raise Exception("Configuration method {} does not support setting options" + .format(options.config_mode)) + + def unset(self, name): + raise Exception("Configuration method {} does not support unsetting options" + .format(options.config_mode)) + + def batch(self, name): + raise Exception("Configuration method {} does not support batch-setting options" + .format(options.config_mode)) + + def finish(self): + raise NotImplementedError + + def run(self): + self.start() + if self.options.config_name is not None: + self.batch(self.options.config_name) + for spec in self.options.config_unset: + for name in re.split(r'[\t ,]+'): + self.unset(name) + for spec in self.options.config_set: + m = re.match(r'(?P[0-9A-Z_a-z]+)' + + r'(?P\([\t ,0-9A-Z_a-z]*\))?' + + r'(?P[=,]?)', spec) + if m is None or \ + (m.group('args') is not None and m.group('sep') == ',') : + raise Exception("Invalid argument to --config-set") + if m.group('sep') == ',': + for name in spec.split(','): + self.set(name) + else: + name = spec[:m.start('sep')] + value = spec[m.end('sep'):] + self.set(name, value) + self.finish() +_config_classes = {} + +class ConfigCopy(ConfigMaker): + def start(self): + shutil.copyfile(self.source_file, self.target_file) + + def run_config_script(self, *args): + subprocess.check_call(['perl', 'scripts/config.pl'] + list(args), + cwd=self.options.dir) + + def set(self, name, value): + if value is None: + self.run_config_script('set', name) + else: + self.run_config_script('set', name, value) + + def unset(self, name): + self.run_config_script('unset', name) + + def batch(self, name): + self.run_config_script(name) + + def finish(self): + pass +_config_classes['copy'] = ConfigCopy + +class ConfigInclude(ConfigMaker): + def __init__(self, *args): + super().__init__(*args) + self.lines = [] + + def start(self): + source_path = self.source_file_path + if not os.path.isabs(source_path): + source_path = os.path.join(os.pardir, os.pardir, source_path) + self.lines.append('#ifndef MBEDTLS_CHECK_CONFIG_H') + self.lines.append('#include "{}"'.format(source_path)) + self.lines.append('') + + def set(self, name, value): + if value is None: + self.lines.append('#define ' + name) + else: + self.lines.append('#define ' + name + ' ' + value) + + def unset(self, name): + self.lines.append('#undef ' + name) + + def finish(self): + self.lines.append('') + self.lines.append('#undef MBEDTLS_CHECK_CONFIG_H') + self.lines.append('#include "mbedtls/check_config.h"') + self.lines.append('#endif') + with open(self.target_file, 'w') as out: + for line in self.lines: + out.write(line + '\n') +_config_classes['include'] = ConfigInclude + +class BuildTreeMaker: + def __init__(self, options): + self.options = options + self.source_path = os.path.abspath(options.source) + self.makefile = MakefileMaker(options, 'source') + if options.config_mode is None: + if options.config_name is None: + options.config_mode = 'include' + else: + options.config_mode = 'copy' + self.config = _config_classes[options.config_mode](options) + + def programs_subdirs(self): + top = self.options.source + return [os.path.basename(d) + for d in glob.glob(os.path.join(top, 'programs', '*')) + if os.path.isdir(d)] + + def run(self): + for subdir in ([['include', 'mbedtls'], ['library'], ['tests']] + + [['programs', d] for d in self.programs_subdirs()]): + path = os.path.join(self.options.dir, *subdir) + if not os.path.exists(path): + os.makedirs(path) + source_link = os.path.join(self.options.dir, 'source') + if not os.path.samefile(self.options.source, self.options.dir) and \ + not os.path.exists(source_link): + os.symlink(self.source_path, source_link) + for link in [['scripts'], + ['tests', 'compat.sh'], + ['tests', 'data_files'], + ['tests', 'scripts'], + ['tests', 'ssl-opt.sh']]: + if os.path.exists(os.path.join(self.options.source, *link)): + link_path = os.path.join(self.options.dir, *link) + if not os.path.exists(link_path): + os.symlink(os.path.join(*([os.pardir] * (len(link) - 1) + + ['source'] + link)), + link_path) + self.makefile.generate() + self.config.run() + +def arg_type_bool(arg): + if not isinstance(arg, str): + return arg + arg = arg.lower() + if arg in ['1', 't', 'true', 'y', 'yes']: + return True + elif arg in ['0', 'f', 'false', 'n', 'no']: + return False + else: + raise argparse.ArgumentTypeError('invalid boolean value: ' + repr(arg)) + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument('--ar', + default=os.getenv('AR', 'ar'), + help='Archive building tool (AR)') + parser.add_argument('--arflags', + default=os.getenv('ARFLAGS', '-src'), + help='Options to pass to ${AR} (e.g. "rcs") (ARFLAGS)') + parser.add_argument('--cc', + default=os.getenv('CC', 'cc'), + help='C compiler (CC)') + parser.add_argument('--cflags', + default=os.getenv('CFLAGS', '-Os'), + help='Options to always pass to ${CC} when compiling (CFLAGS)') + parser.add_argument('--config-file', + help='Base config.h to use') + parser.add_argument('--config-mode', + choices=_config_classes.keys(), + help='What to do with config.h') + parser.add_argument('--config-name', + help='Configuration to set with scripts/config.pl') + parser.add_argument('--config-set', + action='append', default=[], + help='Additional symbol to set in config.h') + parser.add_argument('--config-unset', + action='append', default=[], + help='Symbol to unset in config.h') + parser.add_argument('--cpp-mg', + help='C preprocessor command to generate dependencies (default: ${CC} -MM)') + parser.add_argument('--dir', '-d', + default=os.curdir, + help='Build directory to create') + parser.add_argument('--executable-extension', + default='', + help='File extension for executables') + parser.add_argument('--indirect-extensions', + type=arg_type_bool, default=False, + help='Whether to use makefile variable for file extensions') + parser.add_argument('--ldflags', + default=os.getenv('LDFLAGS', ''), + help='Options to always pass to ${CC} when linking (CFLAGS)') + parser.add_argument('--library-extension', + default='.a', + help='File extension for static libraries') + parser.add_argument('--object-extension', + help='File extension for object files', + default='.o') + parser.add_argument('--perl', + default=os.getenv('PERL', 'perl'), + help='Perl interpreter (PERL)') + parser.add_argument('--python', + default=os.getenv('PYTHON', 'python3'), + help='Python3 interpreter (PYTHON3)') + parser.add_argument('--rm', + default=os.getenv('RM', 'rm -f'), + help='Program to remove files (e.g. "rm -f") (RM)') + parser.add_argument('--shared-library-extension', + help='File extension for shared libraries', + default='.so') + parser.add_argument('--source', '-s', + help='Root directory of the source tree', + default=os.curdir) + parser.add_argument('--warning-cflags', + default=os.getenv('WARNING_CFLAGS', + '-Wall -Wextra -Werror'), + help='Options to always pass to ${CC} (WARNING_CFLAGS)') + options = parser.parse_args() + builder = BuildTreeMaker(options) + builder.run() + +if __name__ == '__main__': + main() From 08cb5dc2be61bb332d0a7c5fdb7ef114025b3ea9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 18 Jul 2019 01:02:06 +0200 Subject: [PATCH 02/44] WIP Mbed TLS makefile generator, with standalone dependency analysis Create a build directory with subdirectories and symlinks. Generate a makefile. Collect dependencies by looking for #include directives without consideration for the current configuration, so that the resulting makefile doesn't need to be regenerated when the configuration changes. Work in progress. Test dependencies are wrong for submodules. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 527 ++++++++++++++++++++------------ 1 file changed, 329 insertions(+), 198 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 52c50d1..75a667d 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -19,6 +19,84 @@ def append_to_value(d, key, *values): lst = d.setdefault(key, []) lst += values +class EnvironmentOption: + def __init__(self, var, default='', help=None, + option=None): + self.var = var + self.option = ('--' + var.lower().replace('_', '-') + if option is None else option) + self.default = default + self.help = help + +_environment_options = [ + EnvironmentOption('AR', 'ar', + 'Archive building tool'), + EnvironmentOption('ARFLAGS', '-src', + 'Options to pass to ${AR} (e.g. "rcs"),'), + EnvironmentOption('CC', 'cc', + 'C compiler'), + EnvironmentOption('CFLAGS', '-Os', + 'Options to always pass to ${CC} when compiling'), + EnvironmentOption('LDFLAGS', '', + 'Options to always pass to ${CC} when linking'), + EnvironmentOption('PERL', 'perl', + 'Perl interpreter'), + EnvironmentOption('PYTHON', 'python3', + 'Python3 interpreter'), + EnvironmentOption('RM', 'rm -f', + 'Program to remove files (e.g. "rm -f"),'), + EnvironmentOption('WARNING_CFLAGS', '-Wall -Wextra -Werror', + 'Options to always pass to ${CC}'), +] + +_submodule_names = ['crypto'] + +class ClassicTestGenerator: + def __init__(self, options): + self.options = options + + def target(self, c_file): + return c_file + + dependencies = [ + '$(SOURCE_DIR)/tests/scripts/generate_code.pl', + '$(SOURCE_DIR)/tests/suites/helpers.function', + '$(SOURCE_DIR)/tests/suites/main_test.function' + ] + + def command(self, function_file, data_file): + return sjoin('$(PERL)', + '$(SOURCE_DIR_FROM_TESTS)/tests/scripts/generate_code.pl', + '$(SOURCE_DIR_FROM_TESTS)', + os.path.splitext(function_file)[0], + os.path.splitext(data_file)[0]) + +class OnTargetTestGenerator: + def __init__(self, options): + self.options = options + + def target(self, c_file): + datax_file = os.path.splitext(c_file)[0] + '.datax' + return sjoin(c_file, datax_file) + + #FIXME: not right for crypto tests in submodule + dependencies = [ + '$(SOURCE_DIR)/tests/scripts/generate_test_code.py', + '$(SOURCE_DIR)/tests/suites/helpers.function', + '$(SOURCE_DIR)/tests/suites/main_test.function' + ] + + def command(self, function_file, data_file): + #FIXME: not right for crypto tests in submodule + return sjoin('$(PYTHON) $(SOURCE_DIR_FROM_TESTS)/tests/scripts/generate_test_code.py', + '-f $(SOURCE_DIR_FROM_TESTS)/' + function_file, + '-d $(SOURCE_DIR_FROM_TESTS)/' + data_file, + '-t $(SOURCE_DIR_FROM_TESTS)/tests/suites/main_test.function', + '-p $(SOURCE_DIR_FROM_TESTS)/tests/suites/host_test.function', + '--helpers-file $(SOURCE_DIR_FROM_TESTS)/tests/suites/helpers.function', + '-s $(SOURCE_DIR_FROM_TESTS)/tests/suites', + '-o .') + class MakefileMaker: def __init__(self, options, source_path): self.options = options @@ -32,13 +110,18 @@ class MakefileMaker: self.library_extension = self.options.library_extension self.object_extension = self.options.object_extension self.shared_library_extension = self.options.shared_library_extension - if self.options.cpp_mg is None: - self.options.cpp_mg = self.options.cc + ' -MM' self.source_path = source_path self.out = None self.libraries = None self.help = {'help': 'Show this help.'} self.clean = [] + self.dependency_cache = {} + self.submodules = [submodule for submodule in _submodule_names + if self.source_exists(submodule)] + if self.source_exists('tests/scripts/generate_test_code.py'): + self.test_generator = OnTargetTestGenerator(options) + else: + self.test_generator = ClassicTestGenerator(options) # Unset fields that are only meaningful at certain later times. # Setting them here makes Pylint happy, but having set them here # makes it harder to diagnose if some method is buggy and attempts @@ -46,11 +129,47 @@ class MakefileMaker: del self.libraries # Set when generating the library targets del self.out # Set only while writing the output file + def get_file_submodule(self, filename): + for submodule in self.submodules: + if filename.startswith(submodule + os.sep): + return submodule, filename[len(submodule) + 1:] + return None, filename + + def crypto_file_path(self, filename): + in_crypto = os.path.join('crypto', filename) + if os.path.exists(in_crypto): + filename = in_crypto + return '$(SOURCE_DIR)/' + filename + + def source_exists(self, filename): + return os.path.exists(os.path.join(self.options.source, filename)) + + def list_submodule_files(self, start, sub_start, pattern, all_sources): + sub_sources = [src[start:] for src in glob.glob(pattern)] + for sub_src in sub_sources: + # Remove submodule and extension + base = os.path.splitext(sub_src[sub_start:])[0] + # Skip generated files that may be present in the source tree. + if base.endswith('_generated'): + continue + # Skip files that were seen in an earlier submodule. + if base not in all_sources: + all_sources[base] = sub_src + def list_source_files(self, directory, pattern): - sources = glob.glob(os.path.join(self.options.source, - directory, pattern)) + # Files in earlier submodules shadow files in later ones. start = len(self.options.source) + 1 - return sorted([src[start:] for src in sources]) + all_sources = {} + for submodule in self.submodules: + sub_start = len(submodule) + 1 + sub_pattern = os.path.join(self.options.source, submodule, + directory, pattern) + self.list_submodule_files(start, sub_start, sub_pattern, + all_sources) + root_pattern = os.path.join(self.options.source, directory, pattern) + self.list_submodule_files(start, 0, root_pattern, + all_sources) + return all_sources def line(self, text): self.out.write(text + '\n') @@ -58,16 +177,28 @@ class MakefileMaker: def words(self, *words): self.line(' '.join(words)) + def assign(self, name, *value_words): + nonempty_words = [word for word in value_words if word] + self.line(' '.join([name, '='] + nonempty_words)) + def format(self, template, *args): self.line(template.format(*args)) def comment(self, template, *args): self.format('## ' + template, *args) - def target(self, name, dependencies, commands, - help=None, phony=False, short=None): + def add_dependencies(self, name, *dependencies): for dep in dependencies: self.format('{}: {}', name, dep) + + def add_existing_dependencies(self, name, *dependencies): + for dep in dependencies: + if os.path.exists(os.path.join(self.options.source, dep)): + self.format('{}: $(SOURCE_DIR)/{}', name, dep) + + def target(self, name, dependencies, commands, + help=None, phony=False, short=None): + self.add_dependencies(name, *dependencies) if not dependencies: self.format('{}:', name) if short is not None: @@ -81,21 +212,20 @@ class MakefileMaker: if phony: self.format('.PHONY: {}', name) + def environment_option_subsection(self): + self.comment('Tool settings') + for envopt in _environment_options: + if envopt.help is not None: + self.comment('{}', envopt.help) + self.assign(envopt.var, + getattr(self.options, envopt.var.lower())) + def settings_section(self): self.comment('Path settings') - self.words('SOURCE_DIR =', self.source_path) - self.words('SOURCE_DIR_FROM_TESTS = ../$(SOURCE_DIR)') + self.assign('SOURCE_DIR', self.source_path) + self.assign('SOURCE_DIR_FROM_TESTS', '../$(SOURCE_DIR)') self.line('') - self.comment('Tool settings') - self.words('AR =', self.options.ar) - self.words('ARFLAGS =', self.options.arflags) - self.words('CC =', self.options.cc) - self.words('CFLAGS =', self.options.cflags) - self.words('WARNING_CFLAGS =', self.options.warning_cflags) - self.words('LDFLAGS =', self.options.ldflags) - self.words('PERL =', self.options.perl) - self.words('PYTHON =', self.options.python) - self.words('RM =', self.options.rm) + self.environment_option_subsection() self.line('') self.comment('Configuration') if self.options.indirect_extensions: @@ -112,54 +242,64 @@ class MakefileMaker: self.line('ECHO_IF_QUIET = $(AUX_ECHO_IF_QUIET_)') self.line('Q = $(AUX_Q_)') - def ensure_file(self, filename, created_list): - path = os.path.join(self.options.dir, filename) - if os.path.exists(path): - return - created_list.append(path) - open(path, 'w').close() - - def c_dependencies(self, filenames, prefix, - ensure_dependencies=None, - postprocess=None): - cmd = re.split(r'\s+', self.options.cpp_mg.strip()) - cmd += ['-I', 'include/mbedtls'] # for "config.h" - cmd += ['-I', 'include'] # for "mbedtls/config.h" - cmd += ['-I', 'source/include'] - cmd += ['-I', os.path.dirname(filenames[0])] - cmd += ['-I', 'source/library'] - cmd += ['source/' + filename for filename in filenames] - files_to_remove = [] - try: - if ensure_dependencies is not None: - ensure_dependencies(prefix, files_to_remove) - dependencies_b = subprocess.check_output(cmd, cwd=self.options.dir) - finally: - for filename in files_to_remove: - if os.path.exists(filename): - os.remove(filename) - dependencies = re.sub(r'[\t ]*\\\n[\t ]*', r' ', - dependencies_b.decode('ascii')) - for deps in dependencies.rstrip('\n').split('\n'): - deps = re.sub(r'[^ ]*/\.\./source/', r'source/', deps) - self.out.write(prefix + deps.strip() + '\n') - - def tests_c_dependencies(self, function_files): - with tempfile.TemporaryDirectory(dir=self.options.dir) as tmpdir: - for function_file in sorted(function_files.keys()): - c_files = [os.path.basename(c_file) - for c_file in function_files[function_file]] - for c_file in c_files: - with open(os.path.join(tmpdir, c_file), 'w') as out: - for part in (['tests/suites/{}.function'.format(base) - for base in ['helpers', - 'host_test', - #'main_test', - ]] + - [function_file]): - out.write('#include "../source/{}"\n'.format(part)) - #TODO: need to postprocess to remove tmpdir and change .o to exe - self.c_dependencies(glob.glob(os.path.join(tmpdir, '*')), 'tests/') + def include_path(self, filename): + dirs = [] + submodule, base = self.get_file_submodule(filename) + subdirs = ['include', 'include/mbedtls', 'library'] + if base.startswith('tests'): + subdirs.append('tests') + for subdir in subdirs: + if submodule is None: + dirs += [os.path.join(submodule, subdir) + for submodule in self.submodules] + dirs.append(subdir) + if submodule is not None: + dirs.append(os.path.join(submodule, subdir)) + return dirs + + def include_path_options(self, filename): + return ' '.join(['-I $(SOURCE_DIR)/' + dir + for dir in self.include_path(filename)]) + + def collect_c_dependencies(self, c_file, stack=frozenset()): + if c_file in self.dependency_cache: + return self.dependency_cache[c_file] + if c_file in stack: + return set() + include_path = ([os.path.dirname(c_file)] + self.include_path(c_file)) + dependencies = set() + extra = set() + with open(os.path.join(self.options.source, c_file)) as stream: + for line in stream: + m = re.match(r'#include "(.*)"', line) + if m is None: + continue + filename = m.group(1) + for subdir in include_path: + if os.path.exists(os.path.join(self.options.source, + subdir, filename)): + dependencies.add('/'.join([subdir, filename])) + break + else: + if filename.endswith('.c'): + extra.add(os.path.dirname(c_file) + '/' + filename) + for dep in dependencies: + dependencies |= self.collect_c_dependencies(dep, stack | {dep}) + dependencies |= extra + self.dependency_cache[c_file] = dependencies + return dependencies + + def c_with_dependencies(self, c_file): + deps = self.collect_c_dependencies(c_file) + return [(self.get_file_submodule(filename)[1] + if '_generated.' in filename else + '$(SOURCE_DIR)/' + filename) + for filename in sorted(deps) + [c_file]] + + def c_dependencies_only(self, c_files): + deps = set.union(*[self.collect_c_dependencies(c_file) + for c_file in c_files]) + return ['$(SOURCE_DIR)/' + filename for filename in sorted(deps)] _potential_libraries = ['crypto', 'x509', 'tls'] @staticmethod @@ -176,18 +316,16 @@ class MakefileMaker: def library_section(self): self.comment('Library targets') - self.words('LIBRARY_CFLAGS =', - '-I include/mbedtls', # must come first, for "config.h" - '-I include', - '-I $(SOURCE_DIR)/library', - '-I $(SOURCE_DIR)/include') + self.assign('LIBRARY_CFLAGS', + '-I include/mbedtls', # must come first, for "config.h" + '-I include', + self.include_path_options('library/*')) # Enumerate modules and emit the rules to build them - source_files = self.list_source_files('library', '*.c') - modules = [os.path.splitext(source)[0] for source in source_files] - for module in modules: + modules = self.list_source_files('library', '*.c') + for module in sorted(modules.keys()): o_file = module + self.object_extension - c_file = '$(SOURCE_DIR)/' + module + '.c' - self.target(o_file, [c_file], + c_file = '$(SOURCE_DIR)/' + modules[module] + self.target(o_file, self.c_with_dependencies(modules[module]), [sjoin('$(CC)', '$(WARNING_CFLAGS)', '$(CFLAGS)', @@ -195,14 +333,13 @@ class MakefileMaker: '-o $@', '-c', c_file)], short=('CC ' + c_file)) - self.c_dependencies(source_files, 'library/') contents = {} # Enumerate libraries and the rules to build them for lib in self._potential_libraries: contents[lib] = [] - for module in modules: + for module in sorted(modules.keys()): contents[self.library_of(module)].append(module) - libraries = [lib for lib in reversed(self._potential_libraries) + libraries = [lib for lib in self._potential_libraries if contents[lib]] for lib in libraries: self.format('libmbed{}_modules = {}', lib, ' '.join(contents[lib])) @@ -235,21 +372,24 @@ class MakefileMaker: _auxiliary_objects = { 'programs/ssl/ssl_client2': ['programs/ssl/query_config'], 'programs/ssl/ssl_server2': ['programs/ssl/query_config'], - 'programs/test/query_compile_time_config': ['programs/ssl/query_config'], + 'programs/test/query_compile_time_config': [ + 'programs/ssl/query_config', # in Mbed TLS + 'programs/test/query_config', # in Mbed Crypto + ], } _auxiliary_sources = set([obj for _main, objs in _auxiliary_objects.items() for obj in objs]) - @staticmethod - def program_uses_lib(app, lib): + def program_uses_lib(self, app, lib): basename = os.path.basename(app) subdir = os.path.basename(os.path.dirname(app)) if lib == 'crypto': return True elif lib == 'x509': return (subdir in ['ssl', 'x509'] or - (subdir == 'test' and basename == 'selftest')) + (subdir == 'test' and basename == 'selftest' and + self.source_exists('library/x509.c'))) elif lib == 'tls': return (subdir == 'ssl' or (subdir == 'x509' and basename == 'cert_app') or @@ -257,20 +397,26 @@ class MakefileMaker: basename.endswith('_server') or basename.endswith('_proxy')) - def ensure_programs_dependencies(self, prefix, files_to_remove): - if prefix == 'programs/psa/': - self.ensure_file('programs/psa/psa_constant_names_generated.c', - files_to_remove) - - def program_subsection(self, source_file, executables): - base = os.path.splitext(source_file)[0] + def program_subsection(self, base, source_file, executables): object_file = base + self.object_extension source_path = '$(SOURCE_DIR)/' + source_file - self.target(object_file, [source_path], + if os.path.basename(base) == 'psa_constant_names': + script_path = self.crypto_file_path('scripts/generate_psa_constants.py') + self.target(base + '_generated.c', + ([script_path] + + [self.crypto_file_path( + os.path.join('include', 'psa', filename) + ) + for filename in ['crypto_extra.h', + 'crypto_values.h']]), + [script_path], + short='Gen $@') + self.target(object_file, self.c_with_dependencies(source_file), [sjoin('$(CC)', '$(WARNING_CFLAGS)', '$(CFLAGS)', '$(PROGRAMS_CFLAGS)', + '-I', os.path.dirname(base), # for generated files '-c', source_path, '-o $@')], short='CC $@') @@ -278,7 +424,8 @@ class MakefileMaker: return exe_file = base + self.executable_extension object_deps = [dep + self.object_extension - for dep in self._auxiliary_objects.get(base, [])] + for dep in self._auxiliary_objects.get(base, []) + if self.source_exists(dep + '.c')] libs = [lib for lib in reversed(self._potential_libraries) if self.program_uses_lib(base, lib)] lib_files = ['library/libmbed{}{}'.format(lib, self.library_extension) @@ -297,33 +444,38 @@ class MakefileMaker: def programs_section(self): self.comment('Sample programs') - self.words('PROGRAMS_CFLAGS =', - '-I include', - '-I $(SOURCE_DIR)/library', - '-I $(SOURCE_DIR)/include') - self.words('PROGRAMS_LDFLAGS =', - '-L library') - sources = self.list_source_files('', 'programs/*/*.c') + self.assign('PROGRAMS_CFLAGS', + '-I include', + self.include_path_options('programs/*/*')) + self.assign('PROGRAMS_LDFLAGS', + '-L library') + programs = self.list_source_files('', 'programs/*/*.c') executables = [] - for source_file in sources: - self.program_subsection(source_file, executables) - def ensure_dependencies(*args): - return self.ensure_programs_dependencies(*args) - for subdir, files in itertools.groupby(sources, os.path.dirname): + for base in sorted(programs.keys()): + self.program_subsection(base, programs[base], executables) + dirs = itertools.groupby(sorted(programs.values()), os.path.dirname) + for subdir, _ in dirs: self.target(subdir + '/seedfile', ['tests/seedfile'], ['cp tests/seedfile $@']) - self.c_dependencies(list(files), subdir + '/', - ensure_dependencies=ensure_dependencies) - self.words('programs =', *executables) + self.assign('programs', *executables) self.target('programs', ['$(programs)'], [], help='Build the sample programs.', phony=True) self.clean.append('programs/*/*{} $(programs)' .format(self.object_extension)) + # TODO: *_demo.sh - def test_subsection(self, data_file, executables, function_files): - base = os.path.splitext(os.path.basename(data_file))[0] + def test_generator_rule(self, function_file, data_file, c_file, exe_file): + cmd = self.test_generator.command(function_file, data_file) + self.target(self.test_generator.target(c_file), + ['$(SOURCE_DIR)/' + function_file, + '$(SOURCE_DIR)/' + data_file, + '$(test_host_generate_deps)'], + ['cd tests && ' + cmd], + short='Gen $@') + + def test_subsection(self, base, data_file, executables): try: function_base = base[:base.index('.')] except ValueError: @@ -331,23 +483,11 @@ class MakefileMaker: function_file = os.path.join(os.path.dirname(data_file), function_base + '.function') c_file = os.path.join('tests', base + '.c') - datax_file = os.path.join('tests', base + '.datax') exe_file = os.path.join('tests', base + self.executable_extension) - self.target(sjoin(c_file, datax_file), - ['$(SOURCE_DIR)/' + function_file, - '$(SOURCE_DIR)/' + data_file, - 'test_host_generate_deps'], - [sjoin('cd tests &&', - '$(PYTHON) $(SOURCE_DIR_FROM_TESTS)/tests/scripts/generate_test_code.py', - '-f $(SOURCE_DIR_FROM_TESTS)/' + function_file, - '-d $(SOURCE_DIR_FROM_TESTS)/' + data_file, - '-t $(SOURCE_DIR_FROM_TESTS)/tests/suites/main_test.function', - '-p $(SOURCE_DIR_FROM_TESTS)/tests/suites/host_test.function', - '--helpers-file $(SOURCE_DIR_FROM_TESTS)/tests/suites/helpers.function', - '-s $(SOURCE_DIR_FROM_TESTS)/tests/suites', - '-o .')], - short='Gen $@') - self.target(exe_file, [c_file, '$(lib)', 'test_host_build_deps'], + self.test_generator_rule(function_file, data_file, c_file, exe_file) + self.target(exe_file, + (self.c_dependencies_only([function_file]) + + ['$(lib)', '$(test_host_build_deps)', c_file]), [sjoin('$(CC)', '$(WARNING_CFLAGS)', '$(CFLAGS)', @@ -362,40 +502,39 @@ class MakefileMaker: self.target(base + '.run', [exe_file, 'tests/seedfile'], ['cd tests && ./' + os.path.basename(exe_file)], phony=True) - append_to_value(function_files, function_file, c_file) def tests_section(self): self.comment('Test targets') - self.words('TESTS_CFLAGS = -Wno-unused-function', - '-I include', - '-I $(SOURCE_DIR)/library', - '-I $(SOURCE_DIR)/include', - '-I $(SOURCE_DIR)/tests') - self.words('TESTS_LDFLAGS =', - '-L library') - self.words('test_libs =', - *[self.dash_l_lib(lib) for lib in self.libraries]) - self.target('test_common_generate_deps', - ['$(SOURCE_DIR)/tests/suites/helpers.function', - '$(SOURCE_DIR)/tests/suites/main_test.function'], - [], phony=True) - self.target('test_host_generate_deps', - ['test_common_generate_deps', - '$(SOURCE_DIR)/tests/suites/host_test.function'], - [], phony=True) - self.target('test_target_generate_deps', - ['test_common_generate_deps', - '$(SOURCE_DIR)/tests/suites/target_test.function'], - [], phony=True) - self.target('test_host_build_deps', [], #TODO - [], phony=True) + self.assign('TESTS_CFLAGS', + '-Wno-unused-function', + '-I include', + self.include_path_options('tests/*')) + self.assign('TESTS_LDFLAGS', + '-L library') + self.assign('test_libs', + *[self.dash_l_lib(lib) for lib in reversed(self.libraries)]) + self.assign('test_common_generate_deps', + *self.test_generator.dependencies) + # FIXME: grab the function files from the submodule + self.assign('test_host_generate_deps', + '$(test_common_generate_deps)', + '$(SOURCE_DIR)/tests/suites/host_test.function') + self.assign('test_target_generate_deps', + '$(test_common_generate_deps)', + '$(SOURCE_DIR)/tests/suites/target_test.function') + host_build_deps = self.c_dependencies_only( + [filename for filename in [ + 'tests/suites/helpers.function', + 'tests/suites/main_test.function' + ] if self.source_exists(filename)]) + self.assign('test_host_build_deps', *host_build_deps) data_files = self.list_source_files('tests/suites', '*.data') - function_files = {} executables = [] - for data_file in data_files: - self.test_subsection(data_file, executables, function_files) - self.tests_c_dependencies(function_files) - self.words('test_apps =', *executables) + for base in sorted(data_files.keys()): + self.test_subsection(os.path.basename(base), + data_files[base], + executables) + self.assign('test_apps', *executables) self.target('tests', ['$(test_apps)'], [], help='Build the host tests.', @@ -407,6 +546,7 @@ class MakefileMaker: help='Run all the test suites.', phony=True) self.clean.append('tests/*.c tests/*.datax $(test_apps)') + # TODO: test_psa_constant_names.py def help_lines(self): return ['{} - {}'.format(name, self.help[name]) @@ -489,7 +629,7 @@ class ConfigMaker: if self.options.config_name is not None: self.batch(self.options.config_name) for spec in self.options.config_unset: - for name in re.split(r'[\t ,]+'): + for name in re.split(r'[\t ,]+', spec): self.unset(name) for spec in self.options.config_set: m = re.match(r'(?P[0-9A-Z_a-z]+)' + @@ -577,32 +717,49 @@ class BuildTreeMaker: self.config = _config_classes[options.config_mode](options) def programs_subdirs(self): - top = self.options.source + tops = ([self.options.source] + + [os.path.join(self.options.source, submodule) + for submodule in _submodule_names]) return [os.path.basename(d) + for top in tops for d in glob.glob(os.path.join(top, 'programs', '*')) if os.path.isdir(d)] + def make_subdir(self, subdir): + path = os.path.join(self.options.dir, *subdir) + if not os.path.exists(path): + os.makedirs(path) + + def make_link(self, sub_link, link): + link_path = os.path.join(self.options.dir, *link) + if not os.path.exists(link_path): + os.symlink(os.path.join(*([os.pardir] * (len(link) - 1) + + ['source'] + sub_link)), + link_path) + + def make_link_maybe(self, link): + for submodule in [''] + _submodule_names: + sub_link = [submodule] + link + if os.path.exists(os.path.join(self.options.source, *sub_link)): + self.make_link(sub_link, link) + def run(self): - for subdir in ([['include', 'mbedtls'], ['library'], ['tests']] + + for subdir in ([['include', 'mbedtls'], + ['library'], + ['tests']] + [['programs', d] for d in self.programs_subdirs()]): - path = os.path.join(self.options.dir, *subdir) - if not os.path.exists(path): - os.makedirs(path) + self.make_subdir(subdir) source_link = os.path.join(self.options.dir, 'source') if not os.path.samefile(self.options.source, self.options.dir) and \ not os.path.exists(source_link): os.symlink(self.source_path, source_link) - for link in [['scripts'], + for link in [['include', 'psa'], # hack for psa_constant_names.py + ['scripts'], ['tests', 'compat.sh'], ['tests', 'data_files'], ['tests', 'scripts'], ['tests', 'ssl-opt.sh']]: - if os.path.exists(os.path.join(self.options.source, *link)): - link_path = os.path.join(self.options.dir, *link) - if not os.path.exists(link_path): - os.symlink(os.path.join(*([os.pardir] * (len(link) - 1) + - ['source'] + link)), - link_path) + self.make_link_maybe(link) self.makefile.generate() self.config.run() @@ -619,18 +776,10 @@ def arg_type_bool(arg): def main(): parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument('--ar', - default=os.getenv('AR', 'ar'), - help='Archive building tool (AR)') - parser.add_argument('--arflags', - default=os.getenv('ARFLAGS', '-src'), - help='Options to pass to ${AR} (e.g. "rcs") (ARFLAGS)') - parser.add_argument('--cc', - default=os.getenv('CC', 'cc'), - help='C compiler (CC)') - parser.add_argument('--cflags', - default=os.getenv('CFLAGS', '-Os'), - help='Options to always pass to ${CC} when compiling (CFLAGS)') + for envopt in _environment_options: + parser.add_argument(envopt.option, + default=envopt.default, + help='{} ({})'.format(envopt.help, envopt.var)) parser.add_argument('--config-file', help='Base config.h to use') parser.add_argument('--config-mode', @@ -644,8 +793,6 @@ def main(): parser.add_argument('--config-unset', action='append', default=[], help='Symbol to unset in config.h') - parser.add_argument('--cpp-mg', - help='C preprocessor command to generate dependencies (default: ${CC} -MM)') parser.add_argument('--dir', '-d', default=os.curdir, help='Build directory to create') @@ -655,34 +802,18 @@ def main(): parser.add_argument('--indirect-extensions', type=arg_type_bool, default=False, help='Whether to use makefile variable for file extensions') - parser.add_argument('--ldflags', - default=os.getenv('LDFLAGS', ''), - help='Options to always pass to ${CC} when linking (CFLAGS)') parser.add_argument('--library-extension', default='.a', help='File extension for static libraries') parser.add_argument('--object-extension', help='File extension for object files', default='.o') - parser.add_argument('--perl', - default=os.getenv('PERL', 'perl'), - help='Perl interpreter (PERL)') - parser.add_argument('--python', - default=os.getenv('PYTHON', 'python3'), - help='Python3 interpreter (PYTHON3)') - parser.add_argument('--rm', - default=os.getenv('RM', 'rm -f'), - help='Program to remove files (e.g. "rm -f") (RM)') parser.add_argument('--shared-library-extension', help='File extension for shared libraries', default='.so') parser.add_argument('--source', '-s', help='Root directory of the source tree', default=os.curdir) - parser.add_argument('--warning-cflags', - default=os.getenv('WARNING_CFLAGS', - '-Wall -Wextra -Werror'), - help='Options to always pass to ${CC} (WARNING_CFLAGS)') options = parser.parse_args() builder = BuildTreeMaker(options) builder.run() From 50039b66dfe06bdb4afa79fd0bd4a693324c7cd6 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 18 Jul 2019 01:03:21 +0200 Subject: [PATCH 03/44] WIP Mbed TLS makefile generator: Add SourceFile class Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 82 ++++++++++++++++++++++++++++----- 1 file changed, 70 insertions(+), 12 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 75a667d..407f0c4 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -51,6 +51,62 @@ _environment_options = [ _submodule_names = ['crypto'] +class SourceFile: + def __init__(self, root, submodule, inner_path): + self.root = root + self.submodule = submodule + self.inner_path = inner_path + + def sort_key(self): + # Compare by inner path first, then by submodule. + # The empty submodule comes last. + return (self.inner_path, + not self.submodule, self.submodule) + + def __lt__(self, other): + if self.root != other.root: + raise TypeError("Cannot compare source files under different rootss" + , self, other) + return self.sort_key() < other.sort_key() + + def relative_path(self): + return os.path.join(self.submodule, self.inner_path) + + def real_path(self): + return os.path.join(self.root, self.submodule, self.inner_path) + + def make_path(self): + if self.submodule: + return '/'.join(['$(SOURCE_DIR)', self.submodule, self.inner_path]) + else: + return '$(SOURCE_DIR)/' + self.inner_path + + def base(self): + return os.path.splitext(self.inner_path)[0] + + def target(self, extension): + return self.base() + extension + +def source_files(root, pattern): + all_sources = {} + for submodule in _submodule_names + ['']: + submodule_root = os.path.join(root, submodule) + start = len(submodule_root) + if submodule: + start += 1 + abs_pattern = os.path.join(submodule_root, pattern) + sources = [src[start:] for src in glob.glob(abs_pattern)] + for source_name in sources: + src = SourceFile(root, submodule, source_name) + base = src.base() + # Skip generated files that may be present in the source tree. + if base.endswith('_generated'): + continue + # Skip files that were seen in an earlier submodule. + if base not in all_sources: + all_sources[base] = src + return sorted(all_sources.values()) + class ClassicTestGenerator: def __init__(self, options): self.options = options @@ -321,11 +377,12 @@ class MakefileMaker: '-I include', self.include_path_options('library/*')) # Enumerate modules and emit the rules to build them - modules = self.list_source_files('library', '*.c') - for module in sorted(modules.keys()): - o_file = module + self.object_extension - c_file = '$(SOURCE_DIR)/' + modules[module] - self.target(o_file, self.c_with_dependencies(modules[module]), + modules = source_files(self.options.source, 'library/*.c') + for module in modules: + o_file = module.target(self.object_extension) + c_file = module.make_path() + self.target(o_file, + self.c_with_dependencies(module.relative_path()), [sjoin('$(CC)', '$(WARNING_CFLAGS)', '$(CFLAGS)', @@ -337,8 +394,8 @@ class MakefileMaker: # Enumerate libraries and the rules to build them for lib in self._potential_libraries: contents[lib] = [] - for module in sorted(modules.keys()): - contents[self.library_of(module)].append(module) + for module in modules: + contents[self.library_of(module.base())].append(module.base()) libraries = [lib for lib in self._potential_libraries if contents[lib]] for lib in libraries: @@ -449,12 +506,13 @@ class MakefileMaker: self.include_path_options('programs/*/*')) self.assign('PROGRAMS_LDFLAGS', '-L library') - programs = self.list_source_files('', 'programs/*/*.c') + programs = source_files(self.options.source, 'programs/*/*.c') executables = [] - for base in sorted(programs.keys()): - self.program_subsection(base, programs[base], executables) - dirs = itertools.groupby(sorted(programs.values()), os.path.dirname) - for subdir, _ in dirs: + for src in programs: + self.program_subsection(src.base(), src.relative_path(), + executables) + dirs = set(os.path.dirname(src.base()) for src in programs) + for subdir in sorted(dirs): self.target(subdir + '/seedfile', ['tests/seedfile'], ['cp tests/seedfile $@']) self.assign('programs', *executables) From c94e95f1253f1b214e852f0c85d51a5fe82a968d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 18 Jul 2019 01:04:42 +0200 Subject: [PATCH 04/44] Mbed TLS makefile generator Create a build directory with subdirectories and symlinks. Generate a makefile to build the library, sample programs and unit tests. Support both in-tree and out-of-tree builds. Collect dependencies by looking for #include directives without consideration for the current configuration, so that the resulting makefile doesn't need to be regenerated when the configuration changes. Support changing the configuration when creating the build directory. This can be done either by copying the configuration file and modifying it, or by including the original configuratiom and modifying it through additional #define and #undef. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 467 +++++++++++++++++++++++--------- 1 file changed, 341 insertions(+), 126 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 407f0c4..ee3a7ac 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -13,13 +13,36 @@ import sys import tempfile def sjoin(*args): + """Join the arguments (strings) with a single space between each.""" return ' '.join(args) def append_to_value(d, key, *values): + """Append to a value in a dictionary. + + Append values to d[key]. Create an empty list if d[key] does not exist. + """ lst = d.setdefault(key, []) lst += values +def are_same_existing_files(*files): + for file1 in files: + if not os.path.exists(file1): + return False + for file1 in files[1:]: + if not os.path.samefile(files[0], file1): + return False + return True + class EnvironmentOption: + """A description of options that set makefile variables. + + Such an option has the following fields: + * var: the variable name (e.g. 'FOO_BAR'). + * option: the command line option for this script (e.g. '--foo-bar'). + * help: help text for the option and the variable. + * default: a default value if the variable is not in the environment. + """ + def __init__(self, var, default='', help=None, option=None): self.var = var @@ -28,15 +51,21 @@ class EnvironmentOption: self.default = default self.help = help +"""A list of makefile variables that can be set through command line options. +""" _environment_options = [ EnvironmentOption('AR', 'ar', 'Archive building tool'), EnvironmentOption('ARFLAGS', '-src', - 'Options to pass to ${AR} (e.g. "rcs"),'), + 'Options to pass to ${AR} (e.g. "rcs")'), EnvironmentOption('CC', 'cc', 'C compiler'), + EnvironmentOption('CP', 'cp', + 'Program to copy files (e.g. "cp")'), EnvironmentOption('CFLAGS', '-Os', 'Options to always pass to ${CC} when compiling'), + EnvironmentOption('COMMON_FLAGS', '', + 'Options to always pass to ${CC} when compiling or linking'), EnvironmentOption('LDFLAGS', '', 'Options to always pass to ${CC} when linking'), EnvironmentOption('PERL', 'perl', @@ -44,14 +73,34 @@ _environment_options = [ EnvironmentOption('PYTHON', 'python3', 'Python3 interpreter'), EnvironmentOption('RM', 'rm -f', - 'Program to remove files (e.g. "rm -f"),'), + 'Program to remove files (e.g. "rm -f")'), + EnvironmentOption('VALGRIND', 'valgrind', + 'Path to valgrind'), + EnvironmentOption('VALGRIND_FLAGS', sjoin('-q', + '--tool=memcheck', + '--leak-check=yes', + '--show-reachable=yes', + '--num-callers=50'), + 'Options to pass to ${VALGRIND}'), EnvironmentOption('WARNING_CFLAGS', '-Wall -Wextra -Werror', 'Options to always pass to ${CC}'), ] +"""The list of potential submodules. + +A submodule is a subdirectory of the source tree which has the same +general structure as the source tree. +""" _submodule_names = ['crypto'] class SourceFile: + """A description of a file path in the source tree. + + Each file path is broken down into three parts: the root of the source + tree, the path to the submodule (the empty string for files that are + not in a submodule), and the path inside the submodule. + """ + def __init__(self, root, submodule, inner_path): self.root = root self.submodule = submodule @@ -70,24 +119,51 @@ class SourceFile: return self.sort_key() < other.sort_key() def relative_path(self): + """Path to the file from the root of the source tree.""" return os.path.join(self.submodule, self.inner_path) + def source_dir(self): + """Path to the directory containing the file, from the root of the + source tree.""" + return os.path.dirname(self.relative_path()) + def real_path(self): + """A path at which the file can be opened during makefile generation.""" return os.path.join(self.root, self.submodule, self.inner_path) def make_path(self): + """A path to the file that is valid in the makefile.""" if self.submodule: return '/'.join(['$(SOURCE_DIR)', self.submodule, self.inner_path]) else: return '$(SOURCE_DIR)/' + self.inner_path + def target_dir(self): + """The target directory for build products of this source file. + + This is the path to the directory containing the source file + inside the submodule. + """ + return os.path.dirname(self.inner_path) + def base(self): + """The path to the file inside the submodule, without the extension.""" return os.path.splitext(self.inner_path)[0] def target(self, extension): + """A build target for this source file, with the specified extension.""" return self.base() + extension -def source_files(root, pattern): +def list_source_files(root, pattern): + """List the source files matching the specified pattern. + + Look for the specified wildcard pattern under all submodules, including + the root tree. If a given file name is present in multiple submodules, + only the earliest matching submodule is kept, with the root tree being + looked up last. + + This function returns a sorted list of SourceFile objects. + """ all_sources = {} for submodule in _submodule_names + ['']: submodule_root = os.path.join(root, submodule) @@ -108,53 +184,97 @@ def source_files(root, pattern): return sorted(all_sources.values()) class ClassicTestGenerator: + """Test generator script description for the classic (<<2.13) test generator + (generate_code.pl).""" + def __init__(self, options): self.options = options - def target(self, c_file): + @staticmethod + def target(c_file): return c_file - dependencies = [ - '$(SOURCE_DIR)/tests/scripts/generate_code.pl', - '$(SOURCE_DIR)/tests/suites/helpers.function', - '$(SOURCE_DIR)/tests/suites/main_test.function' - ] + @staticmethod + def script(_source_dir): + return 'tests/scripts/generate_code.pl' - def command(self, function_file, data_file): + @staticmethod + def function_files(function_file): + return ['tests/suites/helpers.function', + 'tests/suites/main_test.function', + function_file] + + @staticmethod + def command(function_file, data_file): + source_dir = os.path.dirname(function_file) + if source_dir != os.path.dirname(data_file): + raise Exception('Function file and data file are not in the same directory', + function_file, data_file) + if not function_file.endswith('.function'): + raise Exception('Function file does not have the .function extension', + function_file) + if not data_file.endswith('.data'): + raise Exception('Data file does not have the .data extension', + data_file) return sjoin('$(PERL)', '$(SOURCE_DIR_FROM_TESTS)/tests/scripts/generate_code.pl', - '$(SOURCE_DIR_FROM_TESTS)', - os.path.splitext(function_file)[0], - os.path.splitext(data_file)[0]) + '$(SOURCE_DIR_FROM_TESTS)/tests/suites', + os.path.splitext(os.path.basename(function_file))[0], + os.path.splitext(os.path.basename(data_file))[0]) class OnTargetTestGenerator: + """Test generator script description for the >=2.13 test generator + with on-target testing support (generate_test_code.py).""" + def __init__(self, options): self.options = options - def target(self, c_file): + @staticmethod + def target(c_file): datax_file = os.path.splitext(c_file)[0] + '.datax' return sjoin(c_file, datax_file) - #FIXME: not right for crypto tests in submodule - dependencies = [ - '$(SOURCE_DIR)/tests/scripts/generate_test_code.py', - '$(SOURCE_DIR)/tests/suites/helpers.function', - '$(SOURCE_DIR)/tests/suites/main_test.function' - ] + @staticmethod + def script(source_dir): + return os.path.dirname(source_dir) + '/scripts/generate_test_code.py' - def command(self, function_file, data_file): - #FIXME: not right for crypto tests in submodule - return sjoin('$(PYTHON) $(SOURCE_DIR_FROM_TESTS)/tests/scripts/generate_test_code.py', + @staticmethod + def function_files(function_file, on_target=False): + source_dir = os.path.dirname(function_file) + return (['{}/{}.function'.format(source_dir, helper) + for helper in ['helpers', 'main_test', + 'target_test' if on_target else 'host_test']] + + [function_file]) + + @classmethod + def command(cls, function_file, data_file): + source_dir = os.path.dirname(function_file) + suite_path = '$(SOURCE_DIR_FROM_TESTS)/' + source_dir + return sjoin('$(PYTHON)', + '$(SOURCE_DIR_FROM_TESTS)/' + cls.script(source_dir), '-f $(SOURCE_DIR_FROM_TESTS)/' + function_file, '-d $(SOURCE_DIR_FROM_TESTS)/' + data_file, - '-t $(SOURCE_DIR_FROM_TESTS)/tests/suites/main_test.function', - '-p $(SOURCE_DIR_FROM_TESTS)/tests/suites/host_test.function', - '--helpers-file $(SOURCE_DIR_FROM_TESTS)/tests/suites/helpers.function', - '-s $(SOURCE_DIR_FROM_TESTS)/tests/suites', + '-t', suite_path + '/main_test.function', + '-p', suite_path + '/host_test.function', + '--helpers-file', suite_path + '/helpers.function', + '-s', suite_path, '-o .') class MakefileMaker: + """A class to generate a makefile for Mbed TLS or Mbed Crypto. + + Typical usage: + MakefileMaker(options, source_path).generate() + """ + def __init__(self, options, source_path): + """Initialize a makefile generator. + + options is the command line option object. + + source_path is a path to the root of the source directory, + relative to the root of the build directory. + """ self.options = options if self.options.indirect_extensions: self.executable_extension = '$(EXEXT)' @@ -186,77 +306,91 @@ class MakefileMaker: del self.out # Set only while writing the output file def get_file_submodule(self, filename): + """Break up a path into submodule and inner path. + + More precisely, given a path filename from the root of the source + tree, return a tuple (submodule, inner_path) where submodule + is the submodule containing the file and inner_path is the path + to the file inside the submodule. If the file is not in a submodule, + return None for the submodule. + """ + # This function and the ones that use it should be rewritten + # to work on SourceFile objects. for submodule in self.submodules: if filename.startswith(submodule + os.sep): return submodule, filename[len(submodule) + 1:] return None, filename def crypto_file_path(self, filename): + """Return the path to a crypto file. + + Look for the file at the given path in the crypto submodule, and if + it exists, return its path from the root of the source tree. + Otherwise return filename unchanged. + """ in_crypto = os.path.join('crypto', filename) if os.path.exists(in_crypto): filename = in_crypto return '$(SOURCE_DIR)/' + filename def source_exists(self, filename): - return os.path.exists(os.path.join(self.options.source, filename)) + """Test if the given path exists in the source tree. - def list_submodule_files(self, start, sub_start, pattern, all_sources): - sub_sources = [src[start:] for src in glob.glob(pattern)] - for sub_src in sub_sources: - # Remove submodule and extension - base = os.path.splitext(sub_src[sub_start:])[0] - # Skip generated files that may be present in the source tree. - if base.endswith('_generated'): - continue - # Skip files that were seen in an earlier submodule. - if base not in all_sources: - all_sources[base] = sub_src - - def list_source_files(self, directory, pattern): - # Files in earlier submodules shadow files in later ones. - start = len(self.options.source) + 1 - all_sources = {} - for submodule in self.submodules: - sub_start = len(submodule) + 1 - sub_pattern = os.path.join(self.options.source, submodule, - directory, pattern) - self.list_submodule_files(start, sub_start, sub_pattern, - all_sources) - root_pattern = os.path.join(self.options.source, directory, pattern) - self.list_submodule_files(start, 0, root_pattern, - all_sources) - return all_sources + This function does not try different submodules. If the file is + in a submodule, filename must include the submodule part. + """ + return os.path.exists(os.path.join(self.options.source, filename)) def line(self, text): + """Emit a makefile line.""" self.out.write(text + '\n') def words(self, *words): + """Emit a makefile line obtain by joining the words with spaces.""" self.line(' '.join(words)) def assign(self, name, *value_words): + """Emit a makefile line that contains an assignment. + + The assignment is to the variable called name, and its value + is value_words joined with spaces as the separator. + """ nonempty_words = [word for word in value_words if word] self.line(' '.join([name, '='] + nonempty_words)) def format(self, template, *args): + """Emit a makefile line containing the given formatted template.""" self.line(template.format(*args)) def comment(self, template, *args): + """Emit a makefile comment line containing the given formatted template.""" self.format('## ' + template, *args) def add_dependencies(self, name, *dependencies): - for dep in dependencies: - self.format('{}: {}', name, dep) - - def add_existing_dependencies(self, name, *dependencies): - for dep in dependencies: - if os.path.exists(os.path.join(self.options.source, dep)): - self.format('{}: $(SOURCE_DIR)/{}', name, dep) + """Generate dependencies for name.""" + parts = (name + ':',) + dependencies + simple = ' '.join(parts) + if len(simple) < 80: + self.line(simple) + else: + self.line(' \\\n\t\t'.join(parts)) def target(self, name, dependencies, commands, help=None, phony=False, short=None): + """Generate a makefile rule. + + * name: the target(s) of the rule. This is a string. If there are + multiple targets, separate them with spaces. + * dependencies: a list of dependencies. + * commands: a list of commands to run (the recipe). + * help: documentation to show for this target in "make help". + If this is omitted, the target is not listed in "make help". + * phony: if true, declare this target as phony. + * short: if this is specified, the command(s) in the recipe + will not be shown in the make transcript, and instead the + make transcript will display this string. + """ self.add_dependencies(name, *dependencies) - if not dependencies: - self.format('{}:', name) if short is not None: self.format('\t@$(ECHO_IF_QUIET) " {}"', short) for com in commands: @@ -269,6 +403,7 @@ class MakefileMaker: self.format('.PHONY: {}', name) def environment_option_subsection(self): + """Generate the assignments to customizable options.""" self.comment('Tool settings') for envopt in _environment_options: if envopt.help is not None: @@ -277,17 +412,32 @@ class MakefileMaker: getattr(self.options, envopt.var.lower())) def settings_section(self): + """Generate assignments to customizable and internal variables. + + Some additional section-specified variables are assigned in each + section. + """ + if self.options.var: + self.comment('Auxiliary variables') + for var in self.options.var: + if '=' in var: + self.line(re.sub(r'\s*([:?+]?=)\s*', ' \1 ', var)) + else: + value = os.getenv(var) + if value is None: + raise KeyError(var) + self.format('{} = {}', var, value) + self.line('') self.comment('Path settings') self.assign('SOURCE_DIR', self.source_path) - self.assign('SOURCE_DIR_FROM_TESTS', '../$(SOURCE_DIR)') self.line('') self.environment_option_subsection() self.line('') self.comment('Configuration') if self.options.indirect_extensions: - self.line('DLEXT = ' + self.options.shared_library_extension) - self.line('LIBEXT = ' + self.options.library_extension) self.line('OBJEXT = ' + self.options.object_extension) + self.line('LIBEXT = ' + self.options.library_extension) + self.line('DLEXT = ' + self.options.shared_library_extension) self.line('EXEXT =' + self.options.executable_extension) self.line('') self.comment('Internal variables') @@ -297,8 +447,18 @@ class MakefileMaker: self.line('AUX_Q_$(V) = @') self.line('ECHO_IF_QUIET = $(AUX_ECHO_IF_QUIET_)') self.line('Q = $(AUX_Q_)') + self.line('') + self.comment('Auxiliary paths') + self.assign('SOURCE_DIR_FROM_TESTS', '../$(SOURCE_DIR)') + self.assign('VALGRIND_LOG_DIR_FROM_TESTS', '.') def include_path(self, filename): + """Return the include path for filename. + + filename must be a path relative to the root of the source tree. + + Return a list of directories relative to the root of the source tree. + """ dirs = [] submodule, base = self.get_file_submodule(filename) subdirs = ['include', 'include/mbedtls', 'library'] @@ -314,10 +474,29 @@ class MakefileMaker: return dirs def include_path_options(self, filename): + """Return the include path options (-I ...) for filename.""" return ' '.join(['-I $(SOURCE_DIR)/' + dir for dir in self.include_path(filename)]) def collect_c_dependencies(self, c_file, stack=frozenset()): + """Find the build dependencies of the specified C source file. + + c_file must be an existing C file in the source tree. + Return a set of directory paths from the root of the source tree. + + The dependencies of a C source files are the files mentioned + in an #include directive that are present in the source tree, + as well as dependencies of dependencies recursively. + This function does not consider which preprocessor symbols + might be defined: it bases its analysis solely on the textual + presence of "#include". + + This function uses a cache internally, so repeated calls with + the same argument return almost instantly. + + The optional argument stack is only used for recursive calls + to prevent infinite loops. + """ if c_file in self.dependency_cache: return self.dependency_cache[c_file] if c_file in stack: @@ -346,6 +525,14 @@ class MakefileMaker: return dependencies def c_with_dependencies(self, c_file): + """A list of C dependencies in makefile syntax. + + Generate the depdendencies of c_file with collect_c_dependencies, + and make it into a list where each file name is given without + the submodule part if any. + + c_file itself is included in the resulting list. + """ deps = self.collect_c_dependencies(c_file) return [(self.get_file_submodule(filename)[1] if '_generated.' in filename else @@ -353,11 +540,21 @@ class MakefileMaker: for filename in sorted(deps) + [c_file]] def c_dependencies_only(self, c_files): + """A list of C dependencies in makefile syntax. + + Generate the depdendencies of each element of c_files with + collect_c_dependencies, and make it into a list where each file name + is given without the submodule part if any. + + The elements of c_files themselves are included not in the resulting + list unless they are a dependency of another element. + """ deps = set.union(*[self.collect_c_dependencies(c_file) for c_file in c_files]) return ['$(SOURCE_DIR)/' + filename for filename in sorted(deps)] _potential_libraries = ['crypto', 'x509', 'tls'] + @staticmethod def library_of(module): module = os.path.basename(module) @@ -377,14 +574,15 @@ class MakefileMaker: '-I include', self.include_path_options('library/*')) # Enumerate modules and emit the rules to build them - modules = source_files(self.options.source, 'library/*.c') + modules = list_source_files(self.options.source, 'library/*.c') for module in modules: o_file = module.target(self.object_extension) c_file = module.make_path() self.target(o_file, self.c_with_dependencies(module.relative_path()), [sjoin('$(CC)', - '$(WARNING_CFLAGS)', + '$(WARNING_CFLAGS)', + '$(COMMON_FLAGS)', '$(CFLAGS)', '$(LIBRARY_CFLAGS)', '-o $@', @@ -454,9 +652,10 @@ class MakefileMaker: basename.endswith('_server') or basename.endswith('_proxy')) - def program_subsection(self, base, source_file, executables): - object_file = base + self.object_extension - source_path = '$(SOURCE_DIR)/' + source_file + def program_subsection(self, src, executables): + base = src.base() + object_file = src.target(self.object_extension) + source_path = src.make_path() if os.path.basename(base) == 'psa_constant_names': script_path = self.crypto_file_path('scripts/generate_psa_constants.py') self.target(base + '_generated.c', @@ -468,18 +667,21 @@ class MakefileMaker: 'crypto_values.h']]), [script_path], short='Gen $@') - self.target(object_file, self.c_with_dependencies(source_file), + self.clean.append(base + '_generated.c') + self.target(object_file, + self.c_with_dependencies(src.relative_path()), [sjoin('$(CC)', '$(WARNING_CFLAGS)', + '$(COMMON_FLAGS)', '$(CFLAGS)', '$(PROGRAMS_CFLAGS)', - '-I', os.path.dirname(base), # for generated files + '-I', src.target_dir(), # for generated files '-c', source_path, '-o $@')], short='CC $@') if base in self._auxiliary_sources: return - exe_file = base + self.executable_extension + exe_file = src.target(self.executable_extension) object_deps = [dep + self.object_extension for dep in self._auxiliary_objects.get(base, []) if self.source_exists(dep + '.c')] @@ -492,6 +694,7 @@ class MakefileMaker: [sjoin('$(CC)', object_file, sjoin(*object_deps), + '$(COMMON_FLAGS)', '$(LDFLAGS)', '$(PROGRAMS_LDFLAGS)', sjoin(*dash_l_libs), @@ -506,15 +709,14 @@ class MakefileMaker: self.include_path_options('programs/*/*')) self.assign('PROGRAMS_LDFLAGS', '-L library') - programs = source_files(self.options.source, 'programs/*/*.c') + programs = list_source_files(self.options.source, 'programs/*/*.c') executables = [] for src in programs: - self.program_subsection(src.base(), src.relative_path(), - executables) - dirs = set(os.path.dirname(src.base()) for src in programs) + self.program_subsection(src, executables) + dirs = set(src.target_dir() for src in programs) for subdir in sorted(dirs): self.target(subdir + '/seedfile', ['tests/seedfile'], - ['cp tests/seedfile $@']) + ['$(CP) tests/seedfile $@']) self.assign('programs', *executables) self.target('programs', ['$(programs)'], [], @@ -524,30 +726,33 @@ class MakefileMaker: .format(self.object_extension)) # TODO: *_demo.sh - def test_generator_rule(self, function_file, data_file, c_file, exe_file): - cmd = self.test_generator.command(function_file, data_file) - self.target(self.test_generator.target(c_file), - ['$(SOURCE_DIR)/' + function_file, - '$(SOURCE_DIR)/' + data_file, - '$(test_host_generate_deps)'], - ['cd tests && ' + cmd], - short='Gen $@') - - def test_subsection(self, base, data_file, executables): + def test_subsection(self, src, executables): + base = os.path.basename(src.base()) + source_dir = src.source_dir() try: function_base = base[:base.index('.')] except ValueError: function_base = base - function_file = os.path.join(os.path.dirname(data_file), - function_base + '.function') + data_file = src.relative_path() + function_file = os.path.join(source_dir, function_base + '.function') + function_files = self.test_generator.function_files(function_file) c_file = os.path.join('tests', base + '.c') - exe_file = os.path.join('tests', base + self.executable_extension) - self.test_generator_rule(function_file, data_file, c_file, exe_file) + exe_basename = base + self.executable_extension + exe_file = os.path.join('tests', exe_basename) + generate_command = self.test_generator.command(function_file, data_file) + self.target(self.test_generator.target(c_file), + ['$(SOURCE_DIR)/' + base + for base in ([self.test_generator.script(source_dir)] + + function_files + + [data_file])], + ['cd tests && ' + generate_command], + short='Gen $@') self.target(exe_file, - (self.c_dependencies_only([function_file]) + - ['$(lib)', '$(test_host_build_deps)', c_file]), + (self.c_dependencies_only(function_files) + + ['$(lib)', '$(test_build_deps)', c_file]), [sjoin('$(CC)', '$(WARNING_CFLAGS)', + '$(COMMON_FLAGS)', '$(CFLAGS)', '$(TESTS_CFLAGS)', c_file, @@ -557,8 +762,24 @@ class MakefileMaker: '-o $@')], short='CC $@') executables.append(exe_file) - self.target(base + '.run', [exe_file, 'tests/seedfile'], - ['cd tests && ./' + os.path.basename(exe_file)], + # Strictly speaking, the .run target also depends on the .datax + # file, since running the test reads the .datax file. However, + # all the dependencies of the .datax file are also dependencies + # of the test executable, so if the executable is up to date, + # so is the .datax file. + self.target('tests/' + base + '.run', + [exe_file, 'tests/seedfile'], + ['cd tests && ./' + exe_basename], + short='RUN tests/' + exe_basename, + phony=True) + valgrind_log = 'MemoryChecker.{}.log'.format(base) + self.target('tests/' + base + '.valgrind', + [exe_file, 'tests/seedfile'], + [sjoin('cd tests &&', + '$(VALGRIND) $(VALGRIND_FLAGS)', + '--log-file=$(VALGRIND_LOG_DIR_FROM_TESTS)/' + valgrind_log, + './' + exe_basename)], + short='VALGRIND tests/' + exe_basename, phony=True) def tests_section(self): @@ -571,27 +792,11 @@ class MakefileMaker: '-L library') self.assign('test_libs', *[self.dash_l_lib(lib) for lib in reversed(self.libraries)]) - self.assign('test_common_generate_deps', - *self.test_generator.dependencies) - # FIXME: grab the function files from the submodule - self.assign('test_host_generate_deps', - '$(test_common_generate_deps)', - '$(SOURCE_DIR)/tests/suites/host_test.function') - self.assign('test_target_generate_deps', - '$(test_common_generate_deps)', - '$(SOURCE_DIR)/tests/suites/target_test.function') - host_build_deps = self.c_dependencies_only( - [filename for filename in [ - 'tests/suites/helpers.function', - 'tests/suites/main_test.function' - ] if self.source_exists(filename)]) - self.assign('test_host_build_deps', *host_build_deps) - data_files = self.list_source_files('tests/suites', '*.data') + self.assign('test_build_deps', *self.libraries) + data_files = list_source_files(self.options.source, 'tests/suites/*.data') executables = [] - for base in sorted(data_files.keys()): - self.test_subsection(os.path.basename(base), - data_files[base], - executables) + for src in data_files: + self.test_subsection(src, executables) self.assign('test_apps', *executables) self.target('tests', ['$(test_apps)'], [], @@ -658,7 +863,9 @@ class ConfigMaker: if self.source_file is None: self.source_file = os.path.join(options.source, 'include', 'mbedtls', 'config.h') - self.source_file_path = 'source/include/mbedtls/config.h' + self.source_file_path = 'include/mbedtls/config.h' + if not options.in_tree_build: + self.source_file_path = 'source/' + self.source_file_path else: self.source_file_path = os.path.abspath(source_file) self.target_file = os.path.join(options.dir, @@ -708,11 +915,14 @@ _config_classes = {} class ConfigCopy(ConfigMaker): def start(self): - shutil.copyfile(self.source_file, self.target_file) + if not are_same_existing_files(self.source_file, self.target_file): + shutil.copyfile(self.source_file, self.target_file) def run_config_script(self, *args): - subprocess.check_call(['perl', 'scripts/config.pl'] + list(args), - cwd=self.options.dir) + cmd = ['perl', 'scripts/config.pl', + '-f', os.path.abspath(self.target_file)] + list(args) + print(cmd) + subprocess.check_call(cmd, cwd=self.options.dir) def set(self, name, value): if value is None: @@ -766,9 +976,12 @@ class BuildTreeMaker: def __init__(self, options): self.options = options self.source_path = os.path.abspath(options.source) - self.makefile = MakefileMaker(options, 'source') + options.in_tree_build = are_same_existing_files(self.options.source, + self.options.dir) + self.makefile = MakefileMaker(options, + '.' if options.in_tree_build else 'source') if options.config_mode is None: - if options.config_name is None: + if options.config_name is None and not options.in_tree_build: options.config_mode = 'include' else: options.config_mode = 'copy' @@ -790,7 +1003,7 @@ class BuildTreeMaker: def make_link(self, sub_link, link): link_path = os.path.join(self.options.dir, *link) - if not os.path.exists(link_path): + if not os.path.lexists(link_path): os.symlink(os.path.join(*([os.pardir] * (len(link) - 1) + ['source'] + sub_link)), link_path) @@ -808,8 +1021,7 @@ class BuildTreeMaker: [['programs', d] for d in self.programs_subdirs()]): self.make_subdir(subdir) source_link = os.path.join(self.options.dir, 'source') - if not os.path.samefile(self.options.source, self.options.dir) and \ - not os.path.exists(source_link): + if not self.options.in_tree_build and not os.path.exists(source_link): os.symlink(self.source_path, source_link) for link in [['include', 'psa'], # hack for psa_constant_names.py ['scripts'], @@ -872,6 +1084,9 @@ def main(): parser.add_argument('--source', '-s', help='Root directory of the source tree', default=os.curdir) + parser.add_argument('--var', + action='append', default=[], + help='Extra variable to define in the makefile') options = parser.parse_args() builder = BuildTreeMaker(options) builder.run() From d7b34ff8b22ed9ad63a3b09bc0e25df562b4d7e2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 18 Jul 2019 17:34:13 +0200 Subject: [PATCH 05/44] Document more methods Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 88 ++++++++++++++++++++++++++++++++- 1 file changed, 87 insertions(+), 1 deletion(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index ee3a7ac..25d3bed 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -557,6 +557,11 @@ class MakefileMaker: @staticmethod def library_of(module): + """Identify which Mbed TLS library contains the specified module. + + This function bases the result on known module names, defaulting + to crypto. + """ module = os.path.basename(module) if module.startswith('x509') or \ module in ['certs', 'pkcs11']: @@ -568,6 +573,11 @@ class MakefileMaker: return 'crypto' def library_section(self): + """Generate the section of the makefile for the library directory. + + The targets are object files for library modules and + static and dynamic library files. + """ self.comment('Library targets') self.assign('LIBRARY_CFLAGS', '-I include/mbedtls', # must come first, for "config.h" @@ -617,6 +627,7 @@ class MakefileMaker: @staticmethod def dash_l_lib(lib): + """Return the -l option to link with the specified library.""" base = os.path.splitext(os.path.basename(lib))[0] if base.startswith('lib'): base = base[3:] @@ -624,6 +635,14 @@ class MakefileMaker: base = 'mbed' + base return '-l' + base + """Auxiliary files used by sample programs. + + This is a map from the base of the file containing the main() + function of the sample program to the list of bases of other + source files to link into the program. The base of a file + is the subdirectory path without the submodule part and the + basename of the file. Non-existing files are ignored. + """ _auxiliary_objects = { 'programs/ssl/ssl_client2': ['programs/ssl/query_config'], 'programs/ssl/ssl_server2': ['programs/ssl/query_config'], @@ -632,11 +651,21 @@ class MakefileMaker: 'programs/test/query_config', # in Mbed Crypto ], } + """List of bases of source files that are an auxiliary object for + some sample program. + """ _auxiliary_sources = set([obj for _main, objs in _auxiliary_objects.items() for obj in objs]) def program_uses_lib(self, app, lib): + """Test whether the sample program app uses lib. + + app is the base of the main file of a sample program (directory + without the submodule part and basename of the file). + lib is the core part of the library name (no extension or "libmbed" + prefix). + """ basename = os.path.basename(app) subdir = os.path.basename(os.path.dirname(app)) if lib == 'crypto': @@ -653,6 +682,15 @@ class MakefileMaker: basename.endswith('_proxy')) def program_subsection(self, src, executables): + """Emit the makefile rules for the given sample program. + + src is a SourceFile object refering to a source file under programs/. + This can either be a file containing a main function or an + auxiliary source file. + + This function appends the path to the program executable to + the list executables, unless src refers to an auxiliary file. + """ base = src.base() object_file = src.target(self.object_extension) source_path = src.make_path() @@ -703,6 +741,7 @@ class MakefileMaker: executables.append(exe_file) def programs_section(self): + """Emit the makefile rules to build the sample programs.""" self.comment('Sample programs') self.assign('PROGRAMS_CFLAGS', '-I include', @@ -727,6 +766,12 @@ class MakefileMaker: # TODO: *_demo.sh def test_subsection(self, src, executables): + """Emit the makefile rules to build one test suite. + + src is a SourceFile object for a .data file. + + This function appens the path to the test executable to the list + executables.)""" base = os.path.basename(src.base()) source_dir = src.source_dir() try: @@ -783,6 +828,7 @@ class MakefileMaker: phony=True) def tests_section(self): + """Emit makefile rules to build and run test suites.""" self.comment('Test targets') self.assign('TESTS_CFLAGS', '-Wno-unused-function', @@ -812,10 +858,12 @@ class MakefileMaker: # TODO: test_psa_constant_names.py def help_lines(self): + """Emit a makefile rule for the 'help' target.""" return ['{} - {}'.format(name, self.help[name]) for name in sorted(self.help.keys())] def output_all(self): + """Emit the whole makefile.""" self.comment('Generated by {}', ' '.join(sys.argv)) self.comment('Do not edit this file! All modifications will be lost.') self.line('') @@ -846,6 +894,7 @@ class MakefileMaker: self.comment('End of generated file.') def generate(self): + """Generate the makefile.""" destination = os.path.join(self.options.dir, 'Makefile') temp_file = destination + '.new' with open(temp_file, 'w') as out: @@ -857,7 +906,13 @@ class MakefileMaker: os.replace(temp_file, destination) class ConfigMaker: + """Parent class for config.h builders. + + Typical usage: ChildClass(options).run() + """ + def __init__(self, options): + """Initialize a config.h builder with the given command line options.""" self.options = options self.source_file = options.config_file if self.source_file is None: @@ -872,24 +927,30 @@ class ConfigMaker: 'include', 'mbedtls', 'config.h') def start(self): + """Builder-specific method which is called first.""" raise NotImplementedError def set(self, name, value): + """Builder-specific method to set name to value.""" raise Exception("Configuration method {} does not support setting options" .format(options.config_mode)) def unset(self, name): + """Builder-specific method to unset name.""" raise Exception("Configuration method {} does not support unsetting options" .format(options.config_mode)) def batch(self, name): + """Builder-specific method to set the configuration with the given name.""" raise Exception("Configuration method {} does not support batch-setting options" .format(options.config_mode)) def finish(self): + """Builder-specific method which is called last.""" raise NotImplementedError def run(self): + """Go ahead and generate config.h.""" self.start() if self.options.config_name is not None: self.batch(self.options.config_name) @@ -914,6 +975,8 @@ class ConfigMaker: _config_classes = {} class ConfigCopy(ConfigMaker): + """ConfigMaker implementation that copies config.h and runs config.pl.""" + def start(self): if not are_same_existing_files(self.source_file, self.target_file): shutil.copyfile(self.source_file, self.target_file) @@ -921,7 +984,6 @@ class ConfigCopy(ConfigMaker): def run_config_script(self, *args): cmd = ['perl', 'scripts/config.pl', '-f', os.path.abspath(self.target_file)] + list(args) - print(cmd) subprocess.check_call(cmd, cwd=self.options.dir) def set(self, name, value): @@ -941,6 +1003,8 @@ class ConfigCopy(ConfigMaker): _config_classes['copy'] = ConfigCopy class ConfigInclude(ConfigMaker): + """ConfigMaker implementation that makes a config.h that #includes the base one.""" + def __init__(self, *args): super().__init__(*args) self.lines = [] @@ -973,6 +1037,16 @@ class ConfigInclude(ConfigMaker): _config_classes['include'] = ConfigInclude class BuildTreeMaker: + """Prepare an Mbed TLS/Crypto build tree. + + * Create a directory structure. + * Create symbolic links to some files and directories from the source. + * Create a config.h. + * Create a Makefile. + + Typical usage: BuildTreeMaker(options).run() + """ + def __init__(self, options): self.options = options self.source_path = os.path.abspath(options.source) @@ -988,6 +1062,7 @@ class BuildTreeMaker: self.config = _config_classes[options.config_mode](options) def programs_subdirs(self): + """Create subdirectories for sample programs.""" tops = ([self.options.source] + [os.path.join(self.options.source, submodule) for submodule in _submodule_names]) @@ -997,11 +1072,13 @@ class BuildTreeMaker: if os.path.isdir(d)] def make_subdir(self, subdir): + """Create the given subdirectory of the build tree.""" path = os.path.join(self.options.dir, *subdir) if not os.path.exists(path): os.makedirs(path) def make_link(self, sub_link, link): + """Create a symbolic link in the build tree to sub_link under the source.""" link_path = os.path.join(self.options.dir, *link) if not os.path.lexists(link_path): os.symlink(os.path.join(*([os.pardir] * (len(link) - 1) + @@ -1009,12 +1086,19 @@ class BuildTreeMaker: link_path) def make_link_maybe(self, link): + """Create a symbolic link in the build tree to a target of the same + name in the source tree in any submodule. + + Check the root first, then the submodules in the order given by + _submodule_names. + """ for submodule in [''] + _submodule_names: sub_link = [submodule] + link if os.path.exists(os.path.join(self.options.source, *sub_link)): self.make_link(sub_link, link) def run(self): + """Go ahead and prepate the build tree.""" for subdir in ([['include', 'mbedtls'], ['library'], ['tests']] + @@ -1034,6 +1118,7 @@ class BuildTreeMaker: self.config.run() def arg_type_bool(arg): + """Boolean argument type for argparse.add_argument.""" if not isinstance(arg, str): return arg arg = arg.lower() @@ -1045,6 +1130,7 @@ def arg_type_bool(arg): raise argparse.ArgumentTypeError('invalid boolean value: ' + repr(arg)) def main(): + """Process the command line and prepare a build tree accordingly.""" parser = argparse.ArgumentParser(description=__doc__) for envopt in _environment_options: parser.add_argument(envopt.option, From 52819232584650dfcdb20faab3d7bc6489f93f23 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 22 Jul 2019 20:54:34 +0200 Subject: [PATCH 06/44] Add support for presets Add support for presets, which set default values for certain options. This allows creating a build directory for a commonly-used target by passing a single command line option. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 139 ++++++++++++++++++++++++++++---- 1 file changed, 125 insertions(+), 14 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 25d3bed..04eb9e8 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -39,6 +39,7 @@ class EnvironmentOption: Such an option has the following fields: * var: the variable name (e.g. 'FOO_BAR'). * option: the command line option for this script (e.g. '--foo-bar'). + * attr: the attribute name in the options object. * help: help text for the option and the variable. * default: a default value if the variable is not in the environment. """ @@ -46,6 +47,7 @@ class EnvironmentOption: def __init__(self, var, default='', help=None, option=None): self.var = var + self.attr = var self.option = ('--' + var.lower().replace('_', '-') if option is None else option) self.default = default @@ -409,7 +411,7 @@ class MakefileMaker: if envopt.help is not None: self.comment('{}', envopt.help) self.assign(envopt.var, - getattr(self.options, envopt.var.lower())) + getattr(self.options, envopt.attr)) def settings_section(self): """Generate assignments to customizable and internal variables. @@ -1117,6 +1119,115 @@ class BuildTreeMaker: self.makefile.generate() self.config.run() +"""Named presets. + +This is a dictionary mapping preset names to their descriptions. The +description of a preset is a namespace object that represents the options to +set for this preset. The field _help in a description has a special meaning: +it's the documentation of the preset. +""" +_preset_options = { + '': {}, # empty preset = use defaults + 'asan': argparse.Namespace( + _help='Clang with Asan, current configuration', + config_unset=['MBEDTLS_MEMORY_BUFFER_ALLOC_C'], + CC='clang', + CFLAGS='-O', + COMMON_FLAGS='-fsanitize=address -fno-common -g3', + ), + 'coverage': argparse.Namespace( + _help='Build with coverage instrumentation', + config_name='full', + config_unset=['MBEDTLS_MEMORY_BUFFER_ALLOC_C'], + CFLAGS='-O0', + COMMON_FLAGS='--coverage -g3', + ), + 'debug': argparse.Namespace( + _help='Debug build', + CFLAGS='-O0', + COMMON_FLAGS='-g3', + ), + 'full': argparse.Namespace( + _help='Full configuration', + config_name='full', + ), + 'm0plus': argparse.Namespace( + _help='Baremetal configuration for Cortex-M0+ target', + config_name='baremetal', + CC='arm-none-eabi-gcc', + CFLAGS='-Os', + COMMON_FLAGS='-mthumb -mcpu=cortex-m0plus', + ), + 'msan': argparse.Namespace( + _help='Clang Memory sanitizer (MSan), current configuration', + config_unset=['MBEDTLS_AESNI_C', + 'MBEDTLS_MEMORY_BUFFER_ALLOC_C'], + CC='clang', + CFLAGS='-O', + COMMON_FLAGS='-fsanitize=memory -g3', + ), + 'valgrind': argparse.Namespace( + _help='Build for Valgrind, current configuration', + config_unset=['MBEDTLS_AESNI_C'], + CFLAGS='-O3', + ), +} + +"""Default values for some options. + +The keys are the field names in the options object. +""" +_default_options = { + 'dir': os.curdir, + 'executable_extension': '', + 'indirect_extensions': False, + 'library_extension': '.a', + 'object_extension': '.o', + 'shared_library_extension': '.so', + 'source': os.curdir, +} + +def set_default_option(options, attr, value): + if getattr(options, attr) is None: + setattr(options, attr, value) + elif isinstance(value, list): + setattr(options, attr, value + getattr(options, attr)) + +def set_default_options(options): + """Apply the preset if any and set default for remaining options. + + We set defaults via this function rather than via the `default` + keyword argument to `parser.add_argument` in order to apply + settings in the correct precedence order: default, then preset, + then explicit. + + For options that can be used more than once and whose values + are accumulated in a list, the default is always empty, and a + preset puts things at the beginning of the list. A command line + option can only append to the preset, not remove preset elements. + This is implemented by prepending the preset to the explicit elements + """ + # Step 1: apply preset. + if options.preset: + for attr, value in _preset_options[options.preset]._get_kwargs(): + if attr.startswith('_'): + continue + set_default_option(options, attr, value) + set_default_option(options, 'dir', 'build-' + options.preset) + # Step 2: set remaining defaults. + for attr, value in _default_options.items(): + set_default_option(options, attr, value) + for envopt in _environment_options: + set_default_option(options, envopt.attr, envopt.default) + +def preset_help(): + """Return a documentation string for the presets.""" + return '\n'.join(['Presets:'] + + ['{}: {}'.format(name, _preset_options[name]._help) + for name in sorted(_preset_options.keys()) + if hasattr(_preset_options[name], '_help')] + + ['']) + def arg_type_bool(arg): """Boolean argument type for argparse.add_argument.""" if not isinstance(arg, str): @@ -1131,10 +1242,12 @@ def arg_type_bool(arg): def main(): """Process the command line and prepare a build tree accordingly.""" - parser = argparse.ArgumentParser(description=__doc__) + parser = argparse.ArgumentParser(formatter_class=argparse.RawDescriptionHelpFormatter, + description=__doc__, + epilog=preset_help()) for envopt in _environment_options: parser.add_argument(envopt.option, - default=envopt.default, + dest=envopt.attr, help='{} ({})'.format(envopt.help, envopt.var)) parser.add_argument('--config-file', help='Base config.h to use') @@ -1150,30 +1263,28 @@ def main(): action='append', default=[], help='Symbol to unset in config.h') parser.add_argument('--dir', '-d', - default=os.curdir, - help='Build directory to create') + help='Build directory to create (default: build-PRESET if given "-p PRESET", otherwise current directory)') parser.add_argument('--executable-extension', - default='', help='File extension for executables') parser.add_argument('--indirect-extensions', - type=arg_type_bool, default=False, + type=arg_type_bool, help='Whether to use makefile variable for file extensions') parser.add_argument('--library-extension', - default='.a', help='File extension for static libraries') parser.add_argument('--object-extension', - help='File extension for object files', - default='.o') + help='File extension for object files') + parser.add_argument('--preset', '-p', + choices = sorted(_preset_options.keys()), + help='Apply a preset configuration before processing other options') parser.add_argument('--shared-library-extension', - help='File extension for shared libraries', - default='.so') + help='File extension for shared libraries') parser.add_argument('--source', '-s', - help='Root directory of the source tree', - default=os.curdir) + help='Root directory of the source tree (default: current directory)') parser.add_argument('--var', action='append', default=[], help='Extra variable to define in the makefile') options = parser.parse_args() + set_default_options(options) builder = BuildTreeMaker(options) builder.run() From d82450b6780b8b269e6be63d3c1de121696b3c59 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 22 Jul 2019 21:10:21 +0200 Subject: [PATCH 07/44] New option --default-target Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 04eb9e8..6dac544 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -871,7 +871,7 @@ class MakefileMaker: self.line('') self.settings_section() self.line('') - self.target('default', ['all'], [], phony=True) + self.target('default', [self.options.default_target], [], phony=True) self.line('') self.target('all', ['lib', 'programs', 'tests'], [], phony=True) self.line('') @@ -1154,6 +1154,7 @@ _preset_options = { 'm0plus': argparse.Namespace( _help='Baremetal configuration for Cortex-M0+ target', config_name='baremetal', + default_target='lib', CC='arm-none-eabi-gcc', CFLAGS='-Os', COMMON_FLAGS='-mthumb -mcpu=cortex-m0plus', @@ -1178,6 +1179,7 @@ _preset_options = { The keys are the field names in the options object. """ _default_options = { + 'default_target': 'all', 'dir': os.curdir, 'executable_extension': '', 'indirect_extensions': False, @@ -1262,6 +1264,8 @@ def main(): parser.add_argument('--config-unset', action='append', default=[], help='Symbol to unset in config.h') + parser.add_argument('--default-target', + help='Default makefile target (default: all)') parser.add_argument('--dir', '-d', help='Build directory to create (default: build-PRESET if given "-p PRESET", otherwise current directory)') parser.add_argument('--executable-extension', From 73872171704cb5a080fbbfe475842a8e31d12964 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 22 Jul 2019 21:41:32 +0200 Subject: [PATCH 08/44] Add 'test' target, alias of 'check' Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 6dac544..be63649 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -856,6 +856,10 @@ class MakefileMaker: ['cd tests && $(PERL) scripts/run-test-suites.pl --skip=$(SKIP_TEST_SUITES)'], help='Run all the test suites.', phony=True) + self.target('test', ['check'], + [], + help='Run all the test suites.', + phony=True) self.clean.append('tests/*.c tests/*.datax $(test_apps)') # TODO: test_psa_constant_names.py From 91ee5845dfb66df1585d73346f709eda7584b07c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 22 Jul 2019 21:42:07 +0200 Subject: [PATCH 09/44] Makefile help and presentations improvements Document variables that can be set on the command line (make help-variables). Document targets tests/test_suite_%.run and tests/test_suite_%.valgrind. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 35 ++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index be63649..c0f0a62 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -291,7 +291,7 @@ class MakefileMaker: self.source_path = source_path self.out = None self.libraries = None - self.help = {'help': 'Show this help.'} + self.help = {} self.clean = [] self.dependency_cache = {} self.submodules = [submodule for submodule in _submodule_names @@ -393,7 +393,7 @@ class MakefileMaker: make transcript will display this string. """ self.add_dependencies(name, *dependencies) - if short is not None: + if short: self.format('\t@$(ECHO_IF_QUIET) " {}"', short) for com in commands: self.format('\t{}{}', @@ -855,19 +855,33 @@ class MakefileMaker: self.target('check', ['$(test_apps)', 'tests/seedfile'], ['cd tests && $(PERL) scripts/run-test-suites.pl --skip=$(SKIP_TEST_SUITES)'], help='Run all the test suites.', + short='', phony=True) self.target('test', ['check'], [], help='Run all the test suites.', + short='', phony=True) + self.help['tests/test_suite_%.run'] = 'Run one test suite.' + self.help['tests/test_suite_%.valgrind'] = 'Run one test suite with valgrind.' self.clean.append('tests/*.c tests/*.datax $(test_apps)') # TODO: test_psa_constant_names.py def help_lines(self): - """Emit a makefile rule for the 'help' target.""" - return ['{} - {}'.format(name, self.help[name]) + """Return the lines of text to show for the 'help' target.""" + return ['{:<14} : {}'.format(name, self.help[name]) for name in sorted(self.help.keys())] + def variables_help_lines(self): + """Return the lines of text to show for the 'help-variables' target.""" + env_opts = [(envopt.var, envopt.help) + for envopt in _environment_options] + ad_hoc = [ + ('V', 'Show commands in full if non-empty.') + ] + return ['{:<14} # {}'.format(name, text) + for (name, text) in sorted(env_opts + ad_hoc)] + def output_all(self): """Emit the whole makefile.""" self.comment('Generated by {}', ' '.join(sys.argv)) @@ -877,7 +891,10 @@ class MakefileMaker: self.line('') self.target('default', [self.options.default_target], [], phony=True) self.line('') - self.target('all', ['lib', 'programs', 'tests'], [], phony=True) + self.target('all', ['lib', 'programs', 'tests'], + [], + help='Build the library, the tests and the sample programs.', + phony=True) self.line('') self.target('pwd', [], ['pwd'], phony=True, short='PWD') # for testing self.line('') @@ -893,6 +910,14 @@ class MakefileMaker: short='RM {generated files}', phony=True) self.line('') + self.target('help-variables', [], + ['@echo "{}"'.format(line) for line in self.variables_help_lines()], + help='Show useful variables to pass on the make command line.', + phony=True) + # The help target must come last because it displays accumulated help + # text set by previous calls to self.target. Set its own help manually + # because self.target would set it too late for it to be printed. + self.help['help'] = 'Show this help listing the most commonly-used non-file targets.' self.target('help', [], ['@echo "{}"'.format(line) for line in self.help_lines()], phony=True) From 6c4f5b96a555e3d2d2b1063c6c332ce1dfa5497a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 26 Jul 2019 15:28:24 +0200 Subject: [PATCH 10/44] Avoid trailing spaces Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index c0f0a62..12f9613 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1049,10 +1049,10 @@ class ConfigInclude(ConfigMaker): self.lines.append('') def set(self, name, value): - if value is None: - self.lines.append('#define ' + name) - else: + if value: self.lines.append('#define ' + name + ' ' + value) + else: + self.lines.append('#define ' + name) def unset(self, name): self.lines.append('#undef ' + name) From fd01046eec9bcdf1e287c9656870d620333d022b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 26 Jul 2019 16:04:59 +0200 Subject: [PATCH 11/44] Hack up a link to the suites directory The generated makefile compiles the unit tests from the root directory. This has the advantage of not requiring different paths (-I, -L). For the unit tests, unfortunately, this results in incorrect debugging information for the common .function files that get pulled in via the test generator rather than an #include directive. For these files, the .c file to compile contains #line directives with a path that's relative to the tests directory, and therefore the debugging data lists them as e.g. `.../build/suites/main_test.function` rather than `.../source/tests/suites/main_test.function`. As a workaround, create a symbolic link .../build/suites that points to the correct directory. In fact, this is only the correct directory if the test doesn't come from a submodule, so this is only a partial workaround. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 39 ++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 12f9613..06b19d7 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1108,15 +1108,24 @@ class BuildTreeMaker: if not os.path.exists(path): os.makedirs(path) - def make_link(self, sub_link, link): - """Create a symbolic link in the build tree to sub_link under the source.""" - link_path = os.path.join(self.options.dir, *link) + def make_link(self, target, link): + """Create a symbolic link called link pointing to target. + + link is a path relative to the build directory. + + If the link already exists, it is not modified. + """ + link_path = os.path.join(self.options.dir, link) if not os.path.lexists(link_path): - os.symlink(os.path.join(*([os.pardir] * (len(link) - 1) + + os.symlink(target, link_path) + + def link_to_source(self, sub_link, link): + """Create a symbolic link in the build tree to sub_link under the source.""" + self.make_link(os.path.join(*([os.pardir] * (len(link) - 1) + ['source'] + sub_link)), - link_path) + os.path.join(*link)) - def make_link_maybe(self, link): + def link_to_source_maybe(self, link): """Create a symbolic link in the build tree to a target of the same name in the source tree in any submodule. @@ -1126,7 +1135,20 @@ class BuildTreeMaker: for submodule in [''] + _submodule_names: sub_link = [submodule] + link if os.path.exists(os.path.join(self.options.source, *sub_link)): - self.make_link(sub_link, link) + self.link_to_source(sub_link, link) + + def link_test_suites(self): + # This is a hack due to the weird paths produced by the test generator. + # The test generator generates a file with #file directives that + # point to "suites/xxx.function". Since we run the compiler from the + # top of the build tree rather than from the tests directory, + # the debug information in the binary contains a path of the form + # "build-directory/suites/xxx.function" rather than + # the correct source path. So arrange for this directory to exist. + # This is only a partial workaround, since it points to the main + # source tree, but the correct file for any given test may be + # in a submodule. + self.make_link('source/tests/suites', 'suites') def run(self): """Go ahead and prepate the build tree.""" @@ -1144,7 +1166,8 @@ class BuildTreeMaker: ['tests', 'data_files'], ['tests', 'scripts'], ['tests', 'ssl-opt.sh']]: - self.make_link_maybe(link) + self.link_to_source_maybe(link) + self.link_test_suites() self.makefile.generate() self.config.run() From 2dcb8c1637a3c34f4e41062998d3c7b61a568130 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 13 Sep 2019 18:39:56 +0200 Subject: [PATCH 12/44] Fix --config-set with no value Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 06b19d7..dbe6cf0 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -961,7 +961,7 @@ class ConfigMaker: """Builder-specific method which is called first.""" raise NotImplementedError - def set(self, name, value): + def set(self, name, value=None): """Builder-specific method to set name to value.""" raise Exception("Configuration method {} does not support setting options" .format(options.config_mode)) @@ -1017,7 +1017,7 @@ class ConfigCopy(ConfigMaker): '-f', os.path.abspath(self.target_file)] + list(args) subprocess.check_call(cmd, cwd=self.options.dir) - def set(self, name, value): + def set(self, name, value=None): if value is None: self.run_config_script('set', name) else: @@ -1048,7 +1048,7 @@ class ConfigInclude(ConfigMaker): self.lines.append('#include "{}"'.format(source_path)) self.lines.append('') - def set(self, name, value): + def set(self, name, value=None): if value: self.lines.append('#define ' + name + ' ' + value) else: From 7b23a832c396dafe9c6d775a6ee0c5d3c9f6f871 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 13 Sep 2019 18:40:38 +0200 Subject: [PATCH 13/44] Enable UBSan with ASan Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index dbe6cf0..f2127b9 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1181,11 +1181,11 @@ it's the documentation of the preset. _preset_options = { '': {}, # empty preset = use defaults 'asan': argparse.Namespace( - _help='Clang with Asan, current configuration', + _help='Clang with ASan+UBSan, current configuration', config_unset=['MBEDTLS_MEMORY_BUFFER_ALLOC_C'], CC='clang', CFLAGS='-O', - COMMON_FLAGS='-fsanitize=address -fno-common -g3', + COMMON_FLAGS='-fsanitize=address,undefined -fno-common -g3', ), 'coverage': argparse.Namespace( _help='Build with coverage instrumentation', From 416b89274e8a5222c7a7080fc9c587ae7721c37e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 13 Sep 2019 18:41:12 +0200 Subject: [PATCH 14/44] More presets: full-asan, full-debug, full-thumb, thumb Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index f2127b9..9f71d46 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1202,6 +1202,30 @@ _preset_options = { 'full': argparse.Namespace( _help='Full configuration', config_name='full', + config_unset=['MBEDTLS_MEMORY_BUFFER_ALLOC_C'], + ), + 'full-asan': argparse.Namespace( + _help='Full configuration with GCC+ASan+UBSan', + config_name='full', + config_unset=['MBEDTLS_MEMORY_BUFFER_ALLOC_C'], + CC='gcc', + CFLAGS='-O', + COMMON_FLAGS='-fsanitize=address,undefined -fno-common -g3', + ), + 'full-debug': argparse.Namespace( + _help='Full configuration, debug build', + config_name='full', + config_unset=['MBEDTLS_MEMORY_BUFFER_ALLOC_C'], + CFLAGS='-O0', + COMMON_FLAGS='-g3', + ), + 'full-thumb': argparse.Namespace( + _help='Full configuration for arm-linux-gnueabi', + config_name='full', + config_unset=['MBEDTLS_MEMORY_BUFFER_ALLOC_C'], + CC='arm-linux-gnueabi-gcc', + CFLAGS='-Os', + COMMON_FLAGS='-mthumb', ), 'm0plus': argparse.Namespace( _help='Baremetal configuration for Cortex-M0+ target', @@ -1219,6 +1243,12 @@ _preset_options = { CFLAGS='-O', COMMON_FLAGS='-fsanitize=memory -g3', ), + 'thumb': argparse.Namespace( + _help='Default configuration for arm-linux-gnueabi', + CC='arm-linux-gnueabi-gcc', + CFLAGS='-Os', + COMMON_FLAGS='-mthumb', + ), 'valgrind': argparse.Namespace( _help='Build for Valgrind, current configuration', config_unset=['MBEDTLS_AESNI_C'], From 0fcb0252f4e8a14d519b5e3a177dffb30f82eea0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 13 Sep 2019 18:42:05 +0200 Subject: [PATCH 15/44] Fix the detection of which programs use which libraries Simpler interface that lists the libraries for each program. Fixes at least cert_app. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 9f71d46..e5b4d8e 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -660,28 +660,26 @@ class MakefileMaker: for _main, objs in _auxiliary_objects.items() for obj in objs]) - def program_uses_lib(self, app, lib): - """Test whether the sample program app uses lib. + def program_libraries(self, app): + """Return the list of libraries that app uses. app is the base of the main file of a sample program (directory without the submodule part and basename of the file). - lib is the core part of the library name (no extension or "libmbed" - prefix). + + Return the list of library base names in their dependency order + (e.g. ["crypto", "x509", "tls"]). """ basename = os.path.basename(app) subdir = os.path.basename(os.path.dirname(app)) - if lib == 'crypto': - return True - elif lib == 'x509': - return (subdir in ['ssl', 'x509'] or - (subdir == 'test' and basename == 'selftest' and - self.source_exists('library/x509.c'))) - elif lib == 'tls': - return (subdir == 'ssl' or - (subdir == 'x509' and basename == 'cert_app') or - basename.endswith('_client') or - basename.endswith('_server') or - basename.endswith('_proxy')) + if (subdir == 'ssl' or basename.startswith('ssl') or + basename in {'cert_app', 'dh_client', 'dh_server', 'udp_proxy'} + ): + return ['crypto', 'x509', 'tls'] + if (subdir == 'x509' or + (basename == 'selftest' and self.source_exists('library/x509.c')) + ): + return ['crypto', 'x509'] + return ['crypto'] def program_subsection(self, src, executables): """Emit the makefile rules for the given sample program. @@ -725,8 +723,7 @@ class MakefileMaker: object_deps = [dep + self.object_extension for dep in self._auxiliary_objects.get(base, []) if self.source_exists(dep + '.c')] - libs = [lib for lib in reversed(self._potential_libraries) - if self.program_uses_lib(base, lib)] + libs = list(reversed(self.program_libraries(base))) lib_files = ['library/libmbed{}{}'.format(lib, self.library_extension) for lib in libs] dash_l_libs = [self.dash_l_lib(lib) for lib in libs] From d1dfce3842c848e9404e610ed233c12ae165f088 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 13 Sep 2019 18:57:36 +0200 Subject: [PATCH 16/44] Factor the compilation of a .c file to a common function Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 43 +++++++++++++++------------------ 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index e5b4d8e..b325d9a 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -555,6 +555,22 @@ class MakefileMaker: for c_file in c_files]) return ['$(SOURCE_DIR)/' + filename for filename in sorted(deps)] + def object_target(self, section, src, extra): + c_file = src.make_path() + o_file = src.target(self.object_extension) + dependencies = self.c_with_dependencies(src.relative_path()) + self.target(o_file, + dependencies, + [sjoin('$(CC)', + '$(WARNING_CFLAGS)', + '$(COMMON_FLAGS)', + '$(CFLAGS)', + ' '.join(['$({}_CFLAGS)'.format(section)] + + extra), + '-o $@', + '-c', c_file)], + short=('CC ' + c_file)) + _potential_libraries = ['crypto', 'x509', 'tls'] @staticmethod @@ -588,18 +604,7 @@ class MakefileMaker: # Enumerate modules and emit the rules to build them modules = list_source_files(self.options.source, 'library/*.c') for module in modules: - o_file = module.target(self.object_extension) - c_file = module.make_path() - self.target(o_file, - self.c_with_dependencies(module.relative_path()), - [sjoin('$(CC)', - '$(WARNING_CFLAGS)', - '$(COMMON_FLAGS)', - '$(CFLAGS)', - '$(LIBRARY_CFLAGS)', - '-o $@', - '-c', c_file)], - short=('CC ' + c_file)) + self.object_target('LIBRARY', module, []) contents = {} # Enumerate libraries and the rules to build them for lib in self._potential_libraries: @@ -706,17 +711,9 @@ class MakefileMaker: [script_path], short='Gen $@') self.clean.append(base + '_generated.c') - self.target(object_file, - self.c_with_dependencies(src.relative_path()), - [sjoin('$(CC)', - '$(WARNING_CFLAGS)', - '$(COMMON_FLAGS)', - '$(CFLAGS)', - '$(PROGRAMS_CFLAGS)', - '-I', src.target_dir(), # for generated files - '-c', source_path, - '-o $@')], - short='CC $@') + extra_includes = ['-I', src.target_dir()] # for generated files + object_file = src.target(self.object_extension) + self.object_target('PROGRAMS', src, extra_includes) if base in self._auxiliary_sources: return exe_file = src.target(self.executable_extension) From 75f9fc0088425ab4f0f1baffdb96ab039ef65cda Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 9 Mar 2020 20:34:16 +0100 Subject: [PATCH 17/44] Recent mbedtls has moved query_config to the same location as crypto Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index b325d9a..f6721ce 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -642,6 +642,10 @@ class MakefileMaker: base = 'mbed' + base return '-l' + base + _query_config = [ + 'programs/ssl/query_config', # in Mbed TLS up to 2.21 + 'programs/test/query_config', # in Mbed Crypto + ] """Auxiliary files used by sample programs. This is a map from the base of the file containing the main() @@ -651,12 +655,9 @@ class MakefileMaker: basename of the file. Non-existing files are ignored. """ _auxiliary_objects = { - 'programs/ssl/ssl_client2': ['programs/ssl/query_config'], - 'programs/ssl/ssl_server2': ['programs/ssl/query_config'], - 'programs/test/query_compile_time_config': [ - 'programs/ssl/query_config', # in Mbed TLS - 'programs/test/query_config', # in Mbed Crypto - ], + 'programs/ssl/ssl_client2': _query_config, + 'programs/ssl/ssl_server2': _query_config, + 'programs/test/query_compile_time_config': _query_config, } """List of bases of source files that are an auxiliary object for some sample program. From c1d36125f9f8fabd53ff5ec79355586bbc459a4c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 28 Apr 2020 22:32:30 +0200 Subject: [PATCH 18/44] Inject $(RUN) and $(RUNS) when running tests When running tests, pass $(RUN) before the executable and $(RUNS) afterwards. This allows executing tests via a runtime analysis tool and passing extra arguments to the tool. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index f6721ce..b93b2c9 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -76,6 +76,10 @@ _environment_options = [ 'Python3 interpreter'), EnvironmentOption('RM', 'rm -f', 'Program to remove files (e.g. "rm -f")'), + EnvironmentOption('RUN', '', + 'Command prefix before executable programs'), + EnvironmentOption('RUNS', '', + 'Command suffix after executable programs'), EnvironmentOption('VALGRIND', 'valgrind', 'Path to valgrind'), EnvironmentOption('VALGRIND_FLAGS', sjoin('-q', @@ -811,7 +815,7 @@ class MakefileMaker: # so is the .datax file. self.target('tests/' + base + '.run', [exe_file, 'tests/seedfile'], - ['cd tests && ./' + exe_basename], + ['cd tests && $(RUN) ./' + exe_basename + ' $(RUNS)'], short='RUN tests/' + exe_basename, phony=True) valgrind_log = 'MemoryChecker.{}.log'.format(base) From 5a3ba930869bc9d6128de1bbf37bfef26176f1f7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 28 Apr 2020 22:34:44 +0200 Subject: [PATCH 19/44] New target programs/test/selftest.run Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index b93b2c9..89f7d26 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -691,6 +691,14 @@ class MakefileMaker: return ['crypto', 'x509'] return ['crypto'] + def add_run_target(self, program, executable=None): + if executable is None: + executable = program + self.executable_extension + self.target(program + '.run', + [executable], + ['$(RUN) ' + executable + ' $(RUNS)'], + phony=True) + def program_subsection(self, src, executables): """Emit the makefile rules for the given sample program. @@ -764,6 +772,7 @@ class MakefileMaker: phony=True) self.clean.append('programs/*/*{} $(programs)' .format(self.object_extension)) + self.add_run_target('programs/test/selftest') # TODO: *_demo.sh def test_subsection(self, src, executables): From 0fa6d64b65ba252989b6c095709883174ea4b1ac Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 28 Apr 2020 22:36:23 +0200 Subject: [PATCH 20/44] Add targets to generate assembly with "$(CC) -S" Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 89f7d26..b3684f0 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -285,11 +285,13 @@ class MakefileMaker: if self.options.indirect_extensions: self.executable_extension = '$(EXEXT)' self.library_extension = '$(LIBEXT)' + self.assembly_extension = '$(ASMEXT)' self.object_extension = '$(OBJEXT)' self.shared_library_extension = '$(DLEXT)' else: self.executable_extension = self.options.executable_extension self.library_extension = self.options.library_extension + self.assembly_extension = self.options.assembly_extension self.object_extension = self.options.object_extension self.shared_library_extension = self.options.shared_library_extension self.source_path = source_path @@ -441,6 +443,7 @@ class MakefileMaker: self.line('') self.comment('Configuration') if self.options.indirect_extensions: + self.line('ASMEXT = ' + self.options.assembly_extension) self.line('OBJEXT = ' + self.options.object_extension) self.line('LIBEXT = ' + self.options.library_extension) self.line('DLEXT = ' + self.options.shared_library_extension) @@ -561,19 +564,22 @@ class MakefileMaker: def object_target(self, section, src, extra): c_file = src.make_path() - o_file = src.target(self.object_extension) dependencies = self.c_with_dependencies(src.relative_path()) - self.target(o_file, - dependencies, - [sjoin('$(CC)', - '$(WARNING_CFLAGS)', - '$(COMMON_FLAGS)', - '$(CFLAGS)', - ' '.join(['$({}_CFLAGS)'.format(section)] + - extra), - '-o $@', - '-c', c_file)], - short=('CC ' + c_file)) + for short, switch, extension in [ + ('CC -S ', '-S', self.assembly_extension), + ('CC ', '-c', self.object_extension), + ]: + self.target(src.target(extension), + dependencies, + [sjoin('$(CC)', + '$(WARNING_CFLAGS)', + '$(COMMON_FLAGS)', + '$(CFLAGS)', + ' '.join(['$({}_CFLAGS)'.format(section)] + + extra), + '-o $@', + switch, c_file)], + short=(short + c_file)) _potential_libraries = ['crypto', 'x509', 'tls'] @@ -1274,6 +1280,7 @@ _default_options = { 'executable_extension': '', 'indirect_extensions': False, 'library_extension': '.a', + 'assembly_extension': '.s', 'object_extension': '.o', 'shared_library_extension': '.so', 'source': os.curdir, @@ -1341,6 +1348,8 @@ def main(): parser.add_argument(envopt.option, dest=envopt.attr, help='{} ({})'.format(envopt.help, envopt.var)) + parser.add_argument('--assembly-extension', + help='File extension for assembly files') parser.add_argument('--config-file', help='Base config.h to use') parser.add_argument('--config-mode', From 17435619e4595ea1f62d81266602d99220bc2cef Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 28 Apr 2020 22:38:08 +0200 Subject: [PATCH 21/44] Support programs/fuzz These programs need to be linked with onefile.o. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index b3684f0..dc2c513 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -672,9 +672,18 @@ class MakefileMaker: """List of bases of source files that are an auxiliary object for some sample program. """ - _auxiliary_sources = set([obj - for _main, objs in _auxiliary_objects.items() - for obj in objs]) + _auxiliary_sources = (frozenset(obj + for objs in _auxiliary_objects.values() + for obj in objs) + .union({ + 'programs/fuzz/common', 'programs/fuzz/onefile', + })) + + def auxiliary_objects(self, base): + if base.startswith('programs/fuzz'): + return ['programs/fuzz/common', 'programs/fuzz/onefile'] + else: + return self._auxiliary_objects.get(base, []) def program_libraries(self, app): """Return the list of libraries that app uses. @@ -688,6 +697,7 @@ class MakefileMaker: basename = os.path.basename(app) subdir = os.path.basename(os.path.dirname(app)) if (subdir == 'ssl' or basename.startswith('ssl') or + subdir == 'fuzz' or basename in {'cert_app', 'dh_client', 'dh_server', 'udp_proxy'} ): return ['crypto', 'x509', 'tls'] @@ -737,7 +747,7 @@ class MakefileMaker: return exe_file = src.target(self.executable_extension) object_deps = [dep + self.object_extension - for dep in self._auxiliary_objects.get(base, []) + for dep in self.auxiliary_objects(base) if self.source_exists(dep + '.c')] libs = list(reversed(self.program_libraries(base))) lib_files = ['library/libmbed{}{}'.format(lib, self.library_extension) From afea624941727c21f9f4cdac1995409ba1c9706c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 28 Apr 2020 22:39:22 +0200 Subject: [PATCH 22/44] Support programs that require -lpthread Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index dc2c513..5a7ae7a 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -707,6 +707,20 @@ class MakefileMaker: return ['crypto', 'x509'] return ['crypto'] + def extra_link_flags(self, app): + """Return the list of extra link flags for app. + + app is the base of the main file of a sample program (directory + without the submodule part and basename of the file). + + Return a list of strings to insert on the command line when + linking app. + """ + flags = [] + if 'thread' in app: + flags.append('-lpthread') + return flags + def add_run_target(self, program, executable=None): if executable is None: executable = program + self.executable_extension @@ -760,7 +774,7 @@ class MakefileMaker: '$(COMMON_FLAGS)', '$(LDFLAGS)', '$(PROGRAMS_LDFLAGS)', - sjoin(*dash_l_libs), + sjoin(*(dash_l_libs + self.extra_link_flags(base))), '-o $@')], short='LD $@') executables.append(exe_file) From ce7d21bf94b19673dd5cc91fb8c57f012f7c7c94 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 28 Apr 2020 22:40:34 +0200 Subject: [PATCH 23/44] Asan: do fail on error Don't try to recover: this can mask errors. We're doing this correctly in CMakeLists.txt but I hadn't copied that option. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 5a7ae7a..8a7ac78 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1223,7 +1223,7 @@ _preset_options = { config_unset=['MBEDTLS_MEMORY_BUFFER_ALLOC_C'], CC='clang', CFLAGS='-O', - COMMON_FLAGS='-fsanitize=address,undefined -fno-common -g3', + COMMON_FLAGS='-fsanitize=address,undefined -fno-sanitize-recover=all -fno-common -g3', ), 'coverage': argparse.Namespace( _help='Build with coverage instrumentation', From 3c86c4f792e8ceb26ffbecbcec6045eee3971d26 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 28 Apr 2020 22:42:44 +0200 Subject: [PATCH 24/44] Fix gccism in config-mode=include Work around a double definition of a type due to the double includion of check_config.h in config-mode=incude. This allows the build to pass in strict c99 compliant mode (it worked before in gcc and clang). Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 8a7ac78..27da994 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1098,7 +1098,10 @@ class ConfigInclude(ConfigMaker): def finish(self): self.lines.append('') self.lines.append('#undef MBEDTLS_CHECK_CONFIG_H') + # Avoid a redefinition of this typedef + self.lines.append('#define mbedtls_iso_c_forbids_empty_translation_units mbedtls_iso_c_forbids_empty_translation_units2') self.lines.append('#include "mbedtls/check_config.h"') + self.lines.append('#undef mbedtls_iso_c_forbids_empty_translation_units') self.lines.append('#endif') with open(self.target_file, 'w') as out: for line in self.lines: From 45a2170479611e5066dc6df911ab5c05c98de023 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 28 Apr 2020 22:46:24 +0200 Subject: [PATCH 25/44] Document some known bugs Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 27da994..a729893 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -170,6 +170,7 @@ def list_source_files(root, pattern): This function returns a sorted list of SourceFile objects. """ + # FIXME: for error.c at least, we need the root, not the submodule. all_sources = {} for submodule in _submodule_names + ['']: submodule_root = os.path.join(root, submodule) @@ -542,6 +543,8 @@ class MakefileMaker: c_file itself is included in the resulting list. """ + # Bug: if xxx_generated.c is present in the source directory, + # it gets used instead of the one generated in the build directory. deps = self.collect_c_dependencies(c_file) return [(self.get_file_submodule(filename)[1] if '_generated.' in filename else @@ -1291,6 +1294,8 @@ _preset_options = { COMMON_FLAGS='-mthumb', ), 'valgrind': argparse.Namespace( + # This is misleading: it doesn't actually run programs through + # valgrind when you run e.g. `make check` _help='Build for Valgrind, current configuration', config_unset=['MBEDTLS_AESNI_C'], CFLAGS='-O3', From bbb292219b3910b12880dbc6b312fd950a6abbfd Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 28 Apr 2020 22:52:02 +0200 Subject: [PATCH 26/44] Fix dependency analysis which could sometimes produce loops Fix a bug in dependency analysis whereby if file A was first noticed as a dependency of B, it could end up being considered a dependency of itself. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index a729893..4a2e695 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -511,6 +511,7 @@ class MakefileMaker: return self.dependency_cache[c_file] if c_file in stack: return set() + stack |= {c_file} include_path = ([os.path.dirname(c_file)] + self.include_path(c_file)) dependencies = set() extra = set() @@ -528,8 +529,8 @@ class MakefileMaker: else: if filename.endswith('.c'): extra.add(os.path.dirname(c_file) + '/' + filename) - for dep in dependencies: - dependencies |= self.collect_c_dependencies(dep, stack | {dep}) + for dep in frozenset(dependencies): + dependencies |= self.collect_c_dependencies(dep, stack) dependencies |= extra self.dependency_cache[c_file] = dependencies return dependencies From 3e69d344d6ad37386c05c5fcf6777d4c47ecdf03 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 28 Apr 2020 22:55:35 +0200 Subject: [PATCH 27/44] Fix --config-file breakage in --config-mode=copy Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 4a2e695..15904d0 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -995,7 +995,7 @@ class ConfigMaker: if not options.in_tree_build: self.source_file_path = 'source/' + self.source_file_path else: - self.source_file_path = os.path.abspath(source_file) + self.source_file_path = os.path.abspath(self.source_file) self.target_file = os.path.join(options.dir, 'include', 'mbedtls', 'config.h') From 0579fbc5e2a3358f997dc0fb58f1523a6840e83f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 28 Apr 2020 22:58:08 +0200 Subject: [PATCH 28/44] Add support for building shared libraries This is work in progress. You need to manually pass -fPIC when compiling the object files, otherwise the ld step will fail. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 69 +++++++++++++++++++++++---------- 1 file changed, 49 insertions(+), 20 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 15904d0..3d461c3 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -68,6 +68,8 @@ _environment_options = [ 'Options to always pass to ${CC} when compiling'), EnvironmentOption('COMMON_FLAGS', '', 'Options to always pass to ${CC} when compiling or linking'), + EnvironmentOption('DLLFLAGS', '-shared', + 'Options to pass to ${CC} when building a shared library'), EnvironmentOption('LDFLAGS', '', 'Options to always pass to ${CC} when linking'), EnvironmentOption('PERL', 'perl', @@ -297,7 +299,7 @@ class MakefileMaker: self.shared_library_extension = self.options.shared_library_extension self.source_path = source_path self.out = None - self.libraries = None + self.static_libraries = None self.help = {} self.clean = [] self.dependency_cache = {} @@ -311,7 +313,7 @@ class MakefileMaker: # Setting them here makes Pylint happy, but having set them here # makes it harder to diagnose if some method is buggy and attempts # to use a field whose value isn't actually known. - del self.libraries # Set when generating the library targets + del self.static_libraries # Set when generating the library targets del self.out # Set only while writing the output file def get_file_submodule(self, filename): @@ -604,6 +606,16 @@ class MakefileMaker: else: return 'crypto' + @staticmethod + def dash_l_lib(lib): + """Return the -l option to link with the specified library.""" + base = os.path.splitext(os.path.basename(lib))[0] + if base.startswith('lib'): + base = base[3:] + if not base.startswith('mbed'): + base = 'mbed' + base + return '-l' + base + def library_section(self): """Generate the section of the makefile for the library directory. @@ -631,31 +643,48 @@ class MakefileMaker: self.format('libmbed{}_modules = {}', lib, ' '.join(contents[lib])) self.format('libmbed{}_objects = $(libmbed{}_modules:={})', lib, lib, self.object_extension) - self.target('library/libmbed{}{}'.format(lib, self.library_extension), - ['$(libmbed{}_objects)'.format(lib)], - ['$(AR) $(ARFLAGS) $@ $(libmbed{}_objects)'.format(lib)], + self.static_libraries = [] + shared_libraries = [] + for idx, lib in enumerate(libraries): + objects = '$(libmbed{}_objects)'.format(lib) + static = 'library/libmbed{}{}'.format(lib, self.library_extension) + shared = 'library/libmbed{}{}'.format(lib, self.shared_library_extension) + self.static_libraries.append(static) + shared_libraries.append(shared) + self.target(static, [objects], + ['$(AR) $(ARFLAGS) $@ ' + objects], short='AR $@') - self.libraries = ['library/libmbed{}{}'.format(lib, self.library_extension) - for lib in libraries] - self.target('lib', self.libraries, + dependent_libraries = libraries[:idx] + if dependent_libraries: + dash_l_dependent = ('-L . ' + + sjoin(*[self.dash_l_lib(lib) + for lib in dependent_libraries])) + else: + dash_l_dependent = '' + self.target(shared, [objects] + ['library/libmbed{}{}' + .format(lib, self.shared_library_extension) + for lib in dependent_libraries], + [sjoin('$(CC)', + '$(COMMON_FLAGS)', + '$(LDFLAGS)', + '$(DLLFLAGS)', + '-o $@', + dash_l_dependent, + objects)], + short='LD $@') + self.target('lib', self.static_libraries, [], help='Build the static libraries.', phony=True) + self.target('dll', shared_libraries, + [], + help='Build the shared libraries.', + phony=True) self.clean.append(sjoin(*['library/*' + ext for ext in (self.library_extension, self.object_extension, self.shared_library_extension)])) - @staticmethod - def dash_l_lib(lib): - """Return the -l option to link with the specified library.""" - base = os.path.splitext(os.path.basename(lib))[0] - if base.startswith('lib'): - base = base[3:] - if not base.startswith('mbed'): - base = 'mbed' + base - return '-l' + base - _query_config = [ 'programs/ssl/query_config', # in Mbed TLS up to 2.21 'programs/test/query_config', # in Mbed Crypto @@ -881,8 +910,8 @@ class MakefileMaker: self.assign('TESTS_LDFLAGS', '-L library') self.assign('test_libs', - *[self.dash_l_lib(lib) for lib in reversed(self.libraries)]) - self.assign('test_build_deps', *self.libraries) + *[self.dash_l_lib(lib) for lib in reversed(self.static_libraries)]) + self.assign('test_build_deps', *self.static_libraries) data_files = list_source_files(self.options.source, 'tests/suites/*.data') executables = [] for src in data_files: From 4d5f00b8666bfc9c61ba33fcd88a5e308b8ae7af Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 28 Apr 2020 23:00:09 +0200 Subject: [PATCH 29/44] Remove some dead code Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 2 -- 1 file changed, 2 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 3d461c3..21174a0 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -773,8 +773,6 @@ class MakefileMaker: the list executables, unless src refers to an auxiliary file. """ base = src.base() - object_file = src.target(self.object_extension) - source_path = src.make_path() if os.path.basename(base) == 'psa_constant_names': script_path = self.crypto_file_path('scripts/generate_psa_constants.py') self.target(base + '_generated.c', From c5826d309a1655df6a8ecb9043ca2df35cc02b8a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 12 May 2020 19:41:33 +0200 Subject: [PATCH 30/44] Move list_source_files to be a method in SourceFiles It's only used in this class, and it will soon need to have access to other methods in that class. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 68 +++++++++++++++++---------------- 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 21174a0..ba4425f 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -162,36 +162,6 @@ class SourceFile: """A build target for this source file, with the specified extension.""" return self.base() + extension -def list_source_files(root, pattern): - """List the source files matching the specified pattern. - - Look for the specified wildcard pattern under all submodules, including - the root tree. If a given file name is present in multiple submodules, - only the earliest matching submodule is kept, with the root tree being - looked up last. - - This function returns a sorted list of SourceFile objects. - """ - # FIXME: for error.c at least, we need the root, not the submodule. - all_sources = {} - for submodule in _submodule_names + ['']: - submodule_root = os.path.join(root, submodule) - start = len(submodule_root) - if submodule: - start += 1 - abs_pattern = os.path.join(submodule_root, pattern) - sources = [src[start:] for src in glob.glob(abs_pattern)] - for source_name in sources: - src = SourceFile(root, submodule, source_name) - base = src.base() - # Skip generated files that may be present in the source tree. - if base.endswith('_generated'): - continue - # Skip files that were seen in an earlier submodule. - if base not in all_sources: - all_sources[base] = src - return sorted(all_sources.values()) - class ClassicTestGenerator: """Test generator script description for the classic (<<2.13) test generator (generate_code.pl).""" @@ -616,6 +586,37 @@ class MakefileMaker: base = 'mbed' + base return '-l' + base + @staticmethod + def list_source_files(root, pattern): + """List the source files matching the specified pattern. + + Look for the specified wildcard pattern under all submodules, including + the root tree. If a given file name is present in multiple submodules, + only the earliest matching submodule is kept, with the root tree being + looked up last. + + This function returns a sorted list of SourceFile objects. + """ + # FIXME: for error.c at least, we need the root, not the submodule. + all_sources = {} + for submodule in _submodule_names + ['']: + submodule_root = os.path.join(root, submodule) + start = len(submodule_root) + if submodule: + start += 1 + abs_pattern = os.path.join(submodule_root, pattern) + sources = [src[start:] for src in glob.glob(abs_pattern)] + for source_name in sources: + src = SourceFile(root, submodule, source_name) + base = src.base() + # Skip generated files that may be present in the source tree. + if base.endswith('_generated'): + continue + # Skip files that were seen in an earlier submodule. + if base not in all_sources: + all_sources[base] = src + return sorted(all_sources.values()) + def library_section(self): """Generate the section of the makefile for the library directory. @@ -628,7 +629,7 @@ class MakefileMaker: '-I include', self.include_path_options('library/*')) # Enumerate modules and emit the rules to build them - modules = list_source_files(self.options.source, 'library/*.c') + modules = self.list_source_files(self.options.source, 'library/*.c') for module in modules: self.object_target('LIBRARY', module, []) contents = {} @@ -818,7 +819,7 @@ class MakefileMaker: self.include_path_options('programs/*/*')) self.assign('PROGRAMS_LDFLAGS', '-L library') - programs = list_source_files(self.options.source, 'programs/*/*.c') + programs = self.list_source_files(self.options.source, 'programs/*/*.c') executables = [] for src in programs: self.program_subsection(src, executables) @@ -910,7 +911,8 @@ class MakefileMaker: self.assign('test_libs', *[self.dash_l_lib(lib) for lib in reversed(self.static_libraries)]) self.assign('test_build_deps', *self.static_libraries) - data_files = list_source_files(self.options.source, 'tests/suites/*.data') + data_files = self.list_source_files(self.options.source, + 'tests/suites/*.data') executables = [] for src in data_files: self.test_subsection(src, executables) From 49dc68bc4efc6f32507d4a867a90355211740d07 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 12 May 2020 19:44:51 +0200 Subject: [PATCH 31/44] Common source of truth for whether a source file is generated Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index ba4425f..9ee8e3e 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -507,6 +507,15 @@ class MakefileMaker: self.dependency_cache[c_file] = dependencies return dependencies + def is_generated(self, filename): + """Whether the specified C file is generated. + + Implemented with heuristics based on the name. + """ + if '_generated.' in filename: + return True + return False + def c_with_dependencies(self, c_file): """A list of C dependencies in makefile syntax. @@ -520,7 +529,7 @@ class MakefileMaker: # it gets used instead of the one generated in the build directory. deps = self.collect_c_dependencies(c_file) return [(self.get_file_submodule(filename)[1] - if '_generated.' in filename else + if self.is_generated(filename) else '$(SOURCE_DIR)/' + filename) for filename in sorted(deps) + [c_file]] @@ -586,8 +595,7 @@ class MakefileMaker: base = 'mbed' + base return '-l' + base - @staticmethod - def list_source_files(root, pattern): + def list_source_files(self, root, pattern): """List the source files matching the specified pattern. Look for the specified wildcard pattern under all submodules, including @@ -610,7 +618,7 @@ class MakefileMaker: src = SourceFile(root, submodule, source_name) base = src.base() # Skip generated files that may be present in the source tree. - if base.endswith('_generated'): + if self.is_generated(src.relative_path()): continue # Skip files that were seen in an earlier submodule. if base not in all_sources: From 8a9b123c410e176c3e2910228853a9ef22367b66 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 15 Jul 2020 13:48:54 +0200 Subject: [PATCH 32/44] Support tests/include and tests/src Support building tests with include files in tests/include and common code in tests/src/*.c, as added in Mbed TLS 2.23. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 9ee8e3e..8453563 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -446,6 +446,8 @@ class MakefileMaker: subdirs = ['include', 'include/mbedtls', 'library'] if base.startswith('tests'): subdirs.append('tests') + if self.source_exists('tests/include'): + subdirs.append('tests/include') for subdir in subdirs: if submodule is None: dirs += [os.path.join(submodule, subdir) @@ -883,6 +885,7 @@ class MakefileMaker: c_file, '$(LDFLAGS)', '$(TESTS_LDFLAGS)', + '$(test_common_objects)', '$(test_libs)', '-o $@')], short='CC $@') @@ -916,9 +919,18 @@ class MakefileMaker: self.include_path_options('tests/*')) self.assign('TESTS_LDFLAGS', '-L library') + tests_common_sources = self.list_source_files(self.options.source, + 'tests/src/*.c') + tests_common_objects = [] + for src in tests_common_sources: + self.object_target('TESTS', src, []) + object_file = src.target(self.object_extension) + tests_common_objects.append(object_file) + self.assign('test_common_objects', *tests_common_objects) self.assign('test_libs', *[self.dash_l_lib(lib) for lib in reversed(self.static_libraries)]) - self.assign('test_build_deps', *self.static_libraries) + self.assign('test_build_deps', + '$(test_common_objects)', *self.static_libraries) data_files = self.list_source_files(self.options.source, 'tests/suites/*.data') executables = [] @@ -1236,7 +1248,7 @@ class BuildTreeMaker: """Go ahead and prepate the build tree.""" for subdir in ([['include', 'mbedtls'], ['library'], - ['tests']] + + ['tests', 'src']] + [['programs', d] for d in self.programs_subdirs()]): self.make_subdir(subdir) source_link = os.path.join(self.options.dir, 'source') From 45c2792f10f231bc7fd261bc6d32aa6b72afaccc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 15 Jul 2020 13:49:51 +0200 Subject: [PATCH 33/44] Minor documentation fixes Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 8453563..82bcdc2 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -122,7 +122,7 @@ class SourceFile: def __lt__(self, other): if self.root != other.root: - raise TypeError("Cannot compare source files under different rootss" + raise TypeError("Cannot compare source files under different roots" , self, other) return self.sort_key() < other.sort_key() @@ -1187,7 +1187,7 @@ class BuildTreeMaker: self.config = _config_classes[options.config_mode](options) def programs_subdirs(self): - """Create subdirectories for sample programs.""" + """Detect subdirectories for sample programs.""" tops = ([self.options.source] + [os.path.join(self.options.source, submodule) for submodule in _submodule_names]) From 0930a843b34f9e0703e76006eb1ee7a01dc5fa27 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 10 Mar 2021 20:51:38 +0100 Subject: [PATCH 34/44] Support linking tests and programs with modules in tests/src Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 34 +++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 82bcdc2..7118029 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -444,7 +444,7 @@ class MakefileMaker: dirs = [] submodule, base = self.get_file_submodule(filename) subdirs = ['include', 'include/mbedtls', 'library'] - if base.startswith('tests'): + if base.startswith('tests') or base.startswith('programs'): subdirs.append('tests') if self.source_exists('tests/include'): subdirs.append('tests/include') @@ -805,11 +805,13 @@ class MakefileMaker: object_deps = [dep + self.object_extension for dep in self.auxiliary_objects(base) if self.source_exists(dep + '.c')] + object_deps.append('$(test_common_objects)') libs = list(reversed(self.program_libraries(base))) lib_files = ['library/libmbed{}{}'.format(lib, self.library_extension) for lib in libs] dash_l_libs = [self.dash_l_lib(lib) for lib in libs] - self.target(exe_file, [object_file] + object_deps + lib_files, + self.target(exe_file, + [object_file] + object_deps + lib_files, [sjoin('$(CC)', object_file, sjoin(*object_deps), @@ -847,6 +849,22 @@ class MakefileMaker: self.add_run_target('programs/test/selftest') # TODO: *_demo.sh + def define_tests_common_objects(self): + """Emit the definition of tests_common_objects. + + These objects are needed for unit tests and for sample programs + (or at least for some programs in some configurations), so this + definition must come before both targets for programs and tests. + """ + tests_common_sources = self.list_source_files(self.options.source, + 'tests/src/*.c') + tests_common_objects = [] + for src in tests_common_sources: + self.object_target('TESTS', src, []) + object_file = src.target(self.object_extension) + tests_common_objects.append(object_file) + self.assign('test_common_objects', *tests_common_objects) + def test_subsection(self, src, executables): """Emit the makefile rules to build one test suite. @@ -882,6 +900,7 @@ class MakefileMaker: '$(COMMON_FLAGS)', '$(CFLAGS)', '$(TESTS_CFLAGS)', + '$(TESTS_EXTRA_OBJECTS)', c_file, '$(LDFLAGS)', '$(TESTS_LDFLAGS)', @@ -919,14 +938,7 @@ class MakefileMaker: self.include_path_options('tests/*')) self.assign('TESTS_LDFLAGS', '-L library') - tests_common_sources = self.list_source_files(self.options.source, - 'tests/src/*.c') - tests_common_objects = [] - for src in tests_common_sources: - self.object_target('TESTS', src, []) - object_file = src.target(self.object_extension) - tests_common_objects.append(object_file) - self.assign('test_common_objects', *tests_common_objects) + self.assign('TESTS_EXTRA_OBJECTS') self.assign('test_libs', *[self.dash_l_lib(lib) for lib in reversed(self.static_libraries)]) self.assign('test_build_deps', @@ -991,6 +1003,8 @@ class MakefileMaker: self.line('') self.library_section() self.line('') + self.define_tests_common_objects() + self.line('') self.programs_section() self.line('') self.tests_section() From f4a2521df768be9b456ef2bab041736835416d04 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 10 Mar 2021 20:53:24 +0100 Subject: [PATCH 35/44] Support programs/ssl/ssl_test_lib.c Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 7118029..8769c6a 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -700,6 +700,7 @@ class MakefileMaker: 'programs/ssl/query_config', # in Mbed TLS up to 2.21 'programs/test/query_config', # in Mbed Crypto ] + _ssl_test_lib = ['programs/ssl/ssl_test_lib'] """Auxiliary files used by sample programs. This is a map from the base of the file containing the main() @@ -709,8 +710,8 @@ class MakefileMaker: basename of the file. Non-existing files are ignored. """ _auxiliary_objects = { - 'programs/ssl/ssl_client2': _query_config, - 'programs/ssl/ssl_server2': _query_config, + 'programs/ssl/ssl_client2': _query_config + _ssl_test_lib, + 'programs/ssl/ssl_server2': _query_config + _ssl_test_lib, 'programs/test/query_compile_time_config': _query_config, } """List of bases of source files that are an auxiliary object for From b3ee4605af78a0fec87499c058d5b6ed40b20494 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 10 Mar 2021 20:56:15 +0100 Subject: [PATCH 36/44] Better support for generated files Fix out-of-tree builds when there's an incompatible version of the generated file in the source tree. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 34 ++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 8769c6a..e6eaff3 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -162,6 +162,19 @@ class SourceFile: """A build target for this source file, with the specified extension.""" return self.base() + extension +class GeneratedFile(SourceFile): + """A generated file which can be used as an intermediate source file. + + These objects behave like SourceFile objects, but they designate a file + inside the build tree rather than a file inside the source tree. + """ + + def __init__(self, path): + super().__init__('.', '', path) + + def make_path(self): + return self.inner_path + class ClassicTestGenerator: """Test generator script description for the classic (<<2.13) test generator (generate_code.pl).""" @@ -516,8 +529,22 @@ class MakefileMaker: """ if '_generated.' in filename: return True + if 'psa_crypto_driver_wrappers.c' in filename: + # TODO: not in the preliminary work, but upcoming + return False + #return True return False + def is_include_only(self, filename): + """Whether the specified C file is only meant for use in "#include" directive. + + Implemented with heuristics based on the name. + """ + return os.path.basename(filename) in { + 'psa_constant_names_generated.c', + 'ssl_test_common_source.c', + } + def c_with_dependencies(self, c_file): """A list of C dependencies in makefile syntax. @@ -619,8 +646,9 @@ class MakefileMaker: for source_name in sources: src = SourceFile(root, submodule, source_name) base = src.base() - # Skip generated files that may be present in the source tree. - if self.is_generated(src.relative_path()): + # Skip .c files that are only meant to be #include'd + # in other files, and can't be compiled on their own. + if self.is_include_only(src.relative_path()): continue # Skip files that were seen in an earlier submodule. if base not in all_sources: @@ -796,7 +824,7 @@ class MakefileMaker: 'crypto_values.h']]), [script_path], short='Gen $@') - self.clean.append(base + '_generated.c') + self.add_clean(base + '_generated.c') extra_includes = ['-I', src.target_dir()] # for generated files object_file = src.target(self.object_extension) self.object_target('PROGRAMS', src, extra_includes) From 03d77ca3f5f910478222a969b85e3fed9736ee66 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 10 Mar 2021 20:57:07 +0100 Subject: [PATCH 37/44] Dependencies: detect '#include <...>' as well as '#include "..."' Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index e6eaff3..e834831 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -504,7 +504,7 @@ class MakefileMaker: extra = set() with open(os.path.join(self.options.source, c_file)) as stream: for line in stream: - m = re.match(r'#include "(.*)"', line) + m = re.match(r'#include ["<](.*)[">]', line) if m is None: continue filename = m.group(1) From 111ff855e0cdc909308c04fceab07f6aeb955509 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 10 Mar 2021 20:59:35 +0100 Subject: [PATCH 38/44] Fix mixup of the directory part of the valgrind log location Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index e834831..0255d44 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -948,13 +948,15 @@ class MakefileMaker: ['cd tests && $(RUN) ./' + exe_basename + ' $(RUNS)'], short='RUN tests/' + exe_basename, phony=True) - valgrind_log = 'MemoryChecker.{}.log'.format(base) + valgrind_log_basename = 'MemoryChecker.{}.log'.format(base) + valgrind_log = '$(VALGRIND_LOG_DIR_FROM_TESTS)/' + valgrind_log_basename self.target('tests/' + base + '.valgrind', [exe_file, 'tests/seedfile'], [sjoin('cd tests &&', '$(VALGRIND) $(VALGRIND_FLAGS)', - '--log-file=$(VALGRIND_LOG_DIR_FROM_TESTS)/' + valgrind_log, - './' + exe_basename)], + '--log-file=' + valgrind_log, + './' + exe_basename), + sjoin('cd tests && ! grep . ' + valgrind_log)], short='VALGRIND tests/' + exe_basename, phony=True) @@ -994,6 +996,12 @@ class MakefileMaker: help='Run all the test suites.', short='', phony=True) + self.target('test.valgrind', + ['$(test_apps:$(EXEXT)=.valgrind)', + 'tests/seedfile'], + [], + help='Run all the test suites with Valgrind.', + phony=True) self.help['tests/test_suite_%.run'] = 'Run one test suite.' self.help['tests/test_suite_%.valgrind'] = 'Run one test suite with valgrind.' self.clean.append('tests/*.c tests/*.datax $(test_apps)') @@ -1391,7 +1399,7 @@ _preset_options = { # valgrind when you run e.g. `make check` _help='Build for Valgrind, current configuration', config_unset=['MBEDTLS_AESNI_C'], - CFLAGS='-O3', + CFLAGS='-g -O3', ), } From ae1eadff57fd6620e7e00ba551040112505b949a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 10 Mar 2021 21:00:29 +0100 Subject: [PATCH 39/44] Support extra CFLAGS and LDFLAGS under each toplevel directory Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 0255d44..2a6e7ca 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -70,10 +70,20 @@ _environment_options = [ 'Options to always pass to ${CC} when compiling or linking'), EnvironmentOption('DLLFLAGS', '-shared', 'Options to pass to ${CC} when building a shared library'), + # TODO: allow linking extra libraries. This requires passing -l options + # _after_ the objects to link. But LDFLAGS and xxx_EXTRA_LDFLAGS are + # currently passed before. Should they be moved after? Or should there be + # different variables? EnvironmentOption('LDFLAGS', '', 'Options to always pass to ${CC} when linking'), + EnvironmentOption('LIBRARY_EXTRA_CFLAGS', '', + 'Options to pass to ${CC} when compiling library sources'), EnvironmentOption('PERL', 'perl', 'Perl interpreter'), + EnvironmentOption('PROGRAMS_EXTRA_CFLAGS', '', + 'Options to pass to ${CC} when compiling sample programs'), + EnvironmentOption('PROGRAMS_EXTRA_LDFLAGS', '', + 'Options to pass to ${CC} when linking sample programs'), EnvironmentOption('PYTHON', 'python3', 'Python3 interpreter'), EnvironmentOption('RM', 'rm -f', @@ -82,6 +92,10 @@ _environment_options = [ 'Command prefix before executable programs'), EnvironmentOption('RUNS', '', 'Command suffix after executable programs'), + EnvironmentOption('TESTS_EXTRA_CFLAGS', '', + 'Options to pass to ${CC} when compiling unit tests'), + EnvironmentOption('TESTS_EXTRA_LDFLAGS', '', + 'Options to pass to ${CC} when linking unit tests'), EnvironmentOption('VALGRIND', 'valgrind', 'Path to valgrind'), EnvironmentOption('VALGRIND_FLAGS', sjoin('-q', @@ -665,7 +679,8 @@ class MakefileMaker: self.assign('LIBRARY_CFLAGS', '-I include/mbedtls', # must come first, for "config.h" '-I include', - self.include_path_options('library/*')) + self.include_path_options('library/*'), + '$(LIBRARY_EXTRA_CFLAGS)') # Enumerate modules and emit the rules to build them modules = self.list_source_files(self.options.source, 'library/*.c') for module in modules: @@ -857,9 +872,11 @@ class MakefileMaker: self.comment('Sample programs') self.assign('PROGRAMS_CFLAGS', '-I include', - self.include_path_options('programs/*/*')) + self.include_path_options('programs/*/*'), + '$(PROGRAMS_EXTRA_CFLAGS)') self.assign('PROGRAMS_LDFLAGS', - '-L library') + '-L library', + '$(PROGRAMS_EXTRA_LDFLAGS)') programs = self.list_source_files(self.options.source, 'programs/*/*.c') executables = [] for src in programs: @@ -966,9 +983,11 @@ class MakefileMaker: self.assign('TESTS_CFLAGS', '-Wno-unused-function', '-I include', - self.include_path_options('tests/*')) + self.include_path_options('tests/*'), + '$(TESTS_EXTRA_CFLAGS)') self.assign('TESTS_LDFLAGS', - '-L library') + '-L library', + '$(TESTS_EXTRA_LDFLAGS)') self.assign('TESTS_EXTRA_OBJECTS') self.assign('test_libs', *[self.dash_l_lib(lib) for lib in reversed(self.static_libraries)]) From 31ce724cfebc070627aadb08950f3203a73c9037 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 10 Mar 2021 21:00:57 +0100 Subject: [PATCH 40/44] Add .gmon targets for executables Run the program (assumed to be compiled with profiling support) and save its profiling data. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 2a6e7ca..2a516b0 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -816,6 +816,10 @@ class MakefileMaker: [executable], ['$(RUN) ' + executable + ' $(RUNS)'], phony=True) + self.target(program + '.gmon', + [executable], + ['$(RUN) ' + executable + ' $(RUNS)', + 'mv gmon.out $@']) def program_subsection(self, src, executables): """Emit the makefile rules for the given sample program. @@ -965,6 +969,11 @@ class MakefileMaker: ['cd tests && $(RUN) ./' + exe_basename + ' $(RUNS)'], short='RUN tests/' + exe_basename, phony=True) + self.target('tests/' + base + '.gmon', + [exe_file, 'tests/seedfile'], + ['cd tests && $(RUN) ./' + exe_basename + ' $(RUNS)', + 'mv tests/gmon.out $@'], + short='RUN tests/' + exe_basename) valgrind_log_basename = 'MemoryChecker.{}.log'.format(base) valgrind_log = '$(VALGRIND_LOG_DIR_FROM_TESTS)/' + valgrind_log_basename self.target('tests/' + base + '.valgrind', From 2714c0864697712be9da422e64e29e573981e8ae Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 10 Mar 2021 21:01:50 +0100 Subject: [PATCH 41/44] Support the tests/configs directory Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 2a516b0..511c178 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -1335,6 +1335,7 @@ class BuildTreeMaker: os.symlink(self.source_path, source_link) for link in [['include', 'psa'], # hack for psa_constant_names.py ['scripts'], + ['tests', 'configs'], ['tests', 'compat.sh'], ['tests', 'data_files'], ['tests', 'scripts'], From b180127df5caca291378375a0ef52799f26a7055 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 10 Mar 2021 21:03:35 +0100 Subject: [PATCH 42/44] Add many missing files to the 'clean' target Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 83 +++++++++++++++++++++++++++++---- 1 file changed, 74 insertions(+), 9 deletions(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 511c178..917925b 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -383,8 +383,46 @@ class MakefileMaker: else: self.line(' \\\n\t\t'.join(parts)) + _glob2re_re = re.compile(r'[.^$*+?{}\|()]') + @staticmethod + def _glob2re_repl(m): + if m.group(0) == '*': + return '[^/]*' + elif m.group(0) == '?': + return '[^/]' + else: + return '\\' + m.group(0) + @classmethod + def glob2re(cls, pattern): + # Simple glob pattern to regex translator. Does not support + # character sets properly, which is ok because we don't use them. + # We don't use fnmatch or PurePath.match because they let + # '*' and '?' match '/'. + return re.sub(cls._glob2re_re, cls._glob2re_repl, pattern) + + def is_already_cleaned(self, name): + if not self.clean: + return False + regex = ''.join(['\A(?:', + '|'.join([self.glob2re(pattern) + for patterns in self.clean + for pattern in patterns.split()]), + ')\Z']) + return re.match(regex, name) + + def add_clean(self, *elements): + """Add one or more element to the list of things to clean. + + These can be file paths or wildcard patterns. They can contain + macro expansions. + + Elements that are added in the same call to this function are grouped + together for cosmetic purposes. + """ + self.clean.append(' '.join(elements)) + def target(self, name, dependencies, commands, - help=None, phony=False, short=None): + help=None, phony=False, clean=None, short=None): """Generate a makefile rule. * name: the target(s) of the rule. This is a string. If there are @@ -394,6 +432,10 @@ class MakefileMaker: * help: documentation to show for this target in "make help". If this is omitted, the target is not listed in "make help". * phony: if true, declare this target as phony. + * clean: if true, add this target to the list of things to clean. + This parameter only has an effect on non-phony targets. + By default, add this target to the list of things to clean unless + it's already there, possibly via a wildcard pattern. * short: if this is specified, the command(s) in the recipe will not be shown in the make transcript, and instead the make transcript will display this string. @@ -409,6 +451,18 @@ class MakefileMaker: self.help[name] = help if phony: self.format('.PHONY: {}', name) + else: + if clean is None: + # TODO: is_already_cleaned is slow, and only works as intended + # if the pattern is added before the specific item, which + # isn't very natural. Maybe instead I should do an elimination + # pass at the end, to remove redundant entries. Or maybe + # just don't worry about it: the only purpose is to avoid + # having "rm foo.bar" if there's also "rm *.bar", and that's + # cosmetic. + clean = True #not self.is_already_cleaned(name) + if clean: + self.add_clean(name) def environment_option_subsection(self): """Generate the assignments to customizable options.""" @@ -681,6 +735,11 @@ class MakefileMaker: '-I include', self.include_path_options('library/*'), '$(LIBRARY_EXTRA_CFLAGS)') + self.add_clean(*['library/*' + ext + for ext in (self.assembly_extension, + self.library_extension, + self.object_extension, + self.shared_library_extension)]) # Enumerate modules and emit the rules to build them modules = self.list_source_files(self.options.source, 'library/*.c') for module in modules: @@ -734,10 +793,6 @@ class MakefileMaker: [], help='Build the shared libraries.', phony=True) - self.clean.append(sjoin(*['library/*' + ext - for ext in (self.library_extension, - self.object_extension, - self.shared_library_extension)])) _query_config = [ 'programs/ssl/query_config', # in Mbed TLS up to 2.21 @@ -868,6 +923,7 @@ class MakefileMaker: '$(PROGRAMS_LDFLAGS)', sjoin(*(dash_l_libs + self.extra_link_flags(base))), '-o $@')], + clean=False, short='LD $@') executables.append(exe_file) @@ -881,6 +937,9 @@ class MakefileMaker: self.assign('PROGRAMS_LDFLAGS', '-L library', '$(PROGRAMS_EXTRA_LDFLAGS)') + self.add_clean(*['programs/*/*' + ext + for ext in (self.assembly_extension, + self.object_extension)]) programs = self.list_source_files(self.options.source, 'programs/*/*.c') executables = [] for src in programs: @@ -888,14 +947,14 @@ class MakefileMaker: dirs = set(src.target_dir() for src in programs) for subdir in sorted(dirs): self.target(subdir + '/seedfile', ['tests/seedfile'], - ['$(CP) tests/seedfile $@']) + ['$(CP) tests/seedfile $@'], + clean=False) self.assign('programs', *executables) self.target('programs', ['$(programs)'], [], help='Build the sample programs.', phony=True) - self.clean.append('programs/*/*{} $(programs)' - .format(self.object_extension)) + self.add_clean('$(programs)') self.add_run_target('programs/test/selftest') # TODO: *_demo.sh @@ -957,6 +1016,7 @@ class MakefileMaker: '$(test_common_objects)', '$(test_libs)', '-o $@')], + clean=False, short='CC $@') executables.append(exe_file) # Strictly speaking, the .run target also depends on the .datax @@ -1002,6 +1062,11 @@ class MakefileMaker: *[self.dash_l_lib(lib) for lib in reversed(self.static_libraries)]) self.assign('test_build_deps', '$(test_common_objects)', *self.static_libraries) + self.add_clean(*['tests' + sub + '/*' + ext + for sub in ('', '/*') + for ext in (self.assembly_extension, + self.object_extension)]) + self.add_clean('tests/*.c', 'tests/*.datax') data_files = self.list_source_files(self.options.source, 'tests/suites/*.data') executables = [] @@ -1032,7 +1097,7 @@ class MakefileMaker: phony=True) self.help['tests/test_suite_%.run'] = 'Run one test suite.' self.help['tests/test_suite_%.valgrind'] = 'Run one test suite with valgrind.' - self.clean.append('tests/*.c tests/*.datax $(test_apps)') + self.add_clean('$(test_apps)') # TODO: test_psa_constant_names.py def help_lines(self): From 87f377b668157793320607cdc273bef59c16ed48 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 10 Mar 2021 21:03:49 +0100 Subject: [PATCH 43/44] Add programs/test/benchmark as a runnable program Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 917925b..2c45d2f 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -955,6 +955,7 @@ class MakefileMaker: help='Build the sample programs.', phony=True) self.add_clean('$(programs)') + self.add_run_target('programs/test/benchmark') self.add_run_target('programs/test/selftest') # TODO: *_demo.sh From e7baeab08a0a79d3bd0e78dbe44859bb06148e0a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 10 Mar 2021 21:04:07 +0100 Subject: [PATCH 44/44] Preliminary support for PSA driver wrapper generation This is very preliminary, like the work on driver wrapper generation. Signed-off-by: Gilles Peskine --- tools/bin/mbedtls-prepare-build | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/tools/bin/mbedtls-prepare-build b/tools/bin/mbedtls-prepare-build index 2c45d2f..5ebb15d 100755 --- a/tools/bin/mbedtls-prepare-build +++ b/tools/bin/mbedtls-prepare-build @@ -273,6 +273,15 @@ class MakefileMaker: MakefileMaker(options, source_path).generate() """ + PSA_CRYPTO_DRIVER_WRAPPERS_DEPENDENCIES = frozenset([ + 'include/psa/crypto_driver_common.h', + 'include/psa/crypto_platform.h', + 'include/psa/crypto_types.h', + 'include/psa/crypto_values.h', + 'library/psa_crypto_core.h', + 'library/psa_crypto_driver_wrappers.h', + ]) + def __init__(self, options, source_path): """Initialize a makefile generator. @@ -299,7 +308,12 @@ class MakefileMaker: self.static_libraries = None self.help = {} self.clean = [] - self.dependency_cache = {} + self.dependency_cache = { + # TODO: arrange to find dependencies of this generated file. + # They're hard-coded for now, but that won't work if the + # dependencies change over time. + 'library/psa_crypto_driver_wrappers.c': self.PSA_CRYPTO_DRIVER_WRAPPERS_DEPENDENCIES, + } self.submodules = [submodule for submodule in _submodule_names if self.source_exists(submodule)] if self.source_exists('tests/scripts/generate_test_code.py'): @@ -692,6 +706,17 @@ class MakefileMaker: base = 'mbed' + base return '-l' + base + def psa_crypto_driver_wrappers_subsection(self, contents): + generated = 'library/psa_crypto_driver_wrappers.c' + script_path = self.crypto_file_path('scripts/psa_crypto_driver_wrappers.py') + sources = [self.crypto_file_path(drv) + for drv in self.options.psa_driver] + contents['crypto'].append(os.path.splitext(generated)[0]) + self.target(generated, + [script_path] + sources, + [sjoin(script_path, '-o $@', *sources)]) + self.object_target('LIBRARY', GeneratedFile(generated), []) + def list_source_files(self, root, pattern): """List the source files matching the specified pattern. @@ -750,6 +775,8 @@ class MakefileMaker: contents[lib] = [] for module in modules: contents[self.library_of(module.base())].append(module.base()) + if self.options.psa_driver: + self.psa_crypto_driver_wrappers_subsection(contents) libraries = [lib for lib in self._potential_libraries if contents[lib]] for lib in libraries: @@ -1607,6 +1634,9 @@ def main(): parser.add_argument('--preset', '-p', choices = sorted(_preset_options.keys()), help='Apply a preset configuration before processing other options') + parser.add_argument('--psa-driver', + action='append', default=[], + help='PSA crypto driver description file') parser.add_argument('--shared-library-extension', help='File extension for shared libraries') parser.add_argument('--source', '-s',