2
\$\begingroup\$

So I've started writing an OS to know how they work, and I came up with the following makefile to compile it.

The reason I used a makefile is because I wanted to see everything it was doing and have full control over it, to be as explicit as possible. Hence, please don't tell me about CMake (I don't care enough to switch to it, and I find the syntax quite... not ugly, but definitely too compacted at times), or autotools (except if you think it does exactly what I want. I want it to be as explicit as possible as well), or other Makefile-like generators.

Furthermore, please don't tell me about implicit rules or their brother .c.o. I know. I don't want them. They're implicit (and also they generate the object next to the source, which I find annoying and unwanted).


With that out of the way, here's what I want it to be able to do:

I only use GNU Make and a Linux distribution (though it's supposed to be a standalone build, not much should be impacted by the source OS...). I'm currently on an x86_64 computer, and I don't think I'll ever change, but if there are easy fixes I could make, please do tell. Because I like to be up-to-date, I don't care about features only present in recent makes, I can use it if it's useful.

My current tree (after a make distclean or a git clone) is:

.
├── Makefile
└── src
    ├── common
    │   └── include
    │       └── string.h
    ├── i686
    │   └── linker.ld
    └── kernel
        ├── include
        │   ├── arch
        │   │   └── i686
        │   │       └── [some headers]
        │   └── [some headers]
        └── source
            ├── arch
            │   └── i686
            │       └── [some sources]
            └── [some sources]

(If you need the exact tree, I can put it.)

As you can see, I want to be able to support multiple destination architectures, as well as a potential libc (and maybe libk when I understand what it's used for; I only have C and memory management setup hopefully correctly right now in it, so it's far from being complete).

The Makefile generates a parallel tree of src in makedeps for any .mk source dependencies and another in obj/[debug or release] for objects.

Once all objects are compiled, a new directory $(SYSROOT) is created and populated with the root directory of the image file (as I understand how grub-mkrescue works), including all required headers and/or object/binary/... files (of which I have none for now). It also generates a GRUB configure file and then packages everything in a single iso.

One feature my Makefile takes advantage of that I have not seen on this site is that I use evals and calls. I have made some "functions" to add a source file for the kernel (no wildcard, as explicit as possible), which could easily be adapted to add source files to a libc.

This makefile is thus structured as such:

  1. all default target
  2. Variables definitions
  3. "Functions"/macros definitions
  4. Files declarations

What I'd like to know is:

  1. What would you do differently (apart from "use $(YOUR_PREFERRED_MAKE_GENERATOR) instead")?
  2. Is there anything that seems wrong/hardly maintainable?
  3. Is this Makefile easily modifiable to fit for generic C/C++ compiling?
  4. Is this Makefile organized in a logical and readable way?
  5. Are there GNU Make features (or compiler options) that I am missing? (For instance, would VPATH be useful in this situation? As I understand it, it's useful for compilation out of the source tree, which seems... redundant.)
  6. What other things could I improve?
  7. More specific question: I use ANSI escape codes for colors. Should I assume such a computer also has mkdir -p support (which would remove some dependencies)?

Also, if I need to add some comments, please tell me.


all:
.PHONY: all

NAME?=kernel
SYSROOT?=sysroot
TARGET_MACHINE?=i686
OPTIM?=2
DEBUG?=1
# FORCE_COLOR: set to non-empty, non 0 to force colorized output
# ECHO: set to non-empty, non 0 to echo commands out

usage:
    @echo 'Targets:'
    @echo ' - usage (current target)'
    @echo ' - all (default target)'
    @echo ' - kernel.iso'
    @echo ' - obj/{target}/*.o'
    @echo ' - clean'
    @echo ' - distclean'
    @echo ''
    @echo 'Options:'
    @echo ' - NAME: rename the kernel in the GRUB menu'
    @echo ' - SYSROOT: system root directory'
    @echo ' - TARGET_MACHINE: target architecture'
    @echo ' - OPTIM: GCC optimization level (-O is prepended)'
    @echo ' - DEBUG: set to 0 for release build, set to non-0 for debug build (default)'
    @echo ' - FORCE_COLOR: set to non-0 to force colorized output'
    @echo ' - ECHO: set to non-0 to echo out commands executed'
.PHONY: usage

ifeq ($(ECHO:0=),)
SILENCER:=@
else
SILENCER:=
endif

ifeq ($(SYSROOT),)
$(error SYSROOT cannot be empty!)
endif

ifeq ($(strip $(TARGET_MACHINE)),i686)
AS:=i686-elf-as
CC:=i686-elf-gcc
CXX:=i686-elf-g++
else
$(error Unknown target machine $(strip $(TARGET_MACHINE)))
endif

ifneq ($(strip $(DEBUG)),0)
CFLAGS+= -g -DDEBUG
CXXFLAGS+= -g -DDEBUG
OBJDIR?=debug
else
CFLAGS+= -DRELEASE
CXXFLAGS+= -DRELEASE
OBJDIR?=release
endif

COMMON_WARNINGS:=-Wall -Wextra -Wfloat-equal -Wundef -Werror=shadow -Werror=implicit-function-declaration
COMMON_WARNINGS+= -Werror=return-type -Werror=pointer-arith -Werror=strict-overflow
COMMON_WARNINGS+= -Wwrite-strings -Waggregate-return -Wcast-qual -Werror=switch-enum -Wconversion -Wunreachable-code
COMMON_WARNINGS+= -Werror=format=2 -Werror=format-overflow=2 -Werror=format-signedness -Wformat-truncation=2
COMMON_WARNINGS+= -Wnull-dereference -Wimplicit-fallthrough=3 -Wfatal-errors
COMMON_WARNINGS+= -fanalyzer -Wmissing-include-dirs -Wshift-overflow=2 -Wunknown-pragmas -Wstringop-overflow=4
COMMON_WARNINGS+= -Wsuggest-attribute=pure -Wsuggest-attribute=const -Wsuggest-attribute=noreturn
COMMON_WARNINGS+= -Wsuggest-attribute=malloc -Wsuggest-attribute=format -Wsuggest-attribute=cold -Wmissing-noreturn
COMMON_WARNINGS+= -Wmissing-format-attribute -Walloc-zero -Werror=attribute-alias=2 -Wduplicated-branches
COMMON_WARNINGS+= -Werror=duplicated-cond -Wsystem-headers -Wtrampolines -Wstack-usage=1024 -Wunsafe-loop-optimizations
COMMON_WARNINGS+= -Wunused-macros -Wcast-align=strict -Wdate-time -Wlogical-op -Wredundant-decls -Winline
COMMON_WARNINGS+= -Wdisabled-optimization
CFLAGS_WARNINGS:= -Werror=jump-misses-init -Werror=strict-prototypes
CXXFLAGS_WARNINGS:= -Werror=overloaded-virtual

override ASFLAGS+=
override CFLAGS+= -Isrc/common/include -std=gnu17 -ffreestanding -O$(OPTIM) $(COMMON_WARNINGS) $(CFLAGS_WARNINGS)
override CXXFLAGS+= -Isrc/common/include -std=gnu++20 -ffreestanding -fdiagnostics-show-template-tree -O$(OPTIM) $(COMMON_WARNINGS)
override CXXFLAGS+= $(CXXFLAGS_WARNINGS)
# Machine specific
ifeq ($(strip $(TARGET_MACHINE)),i686)
# SSE3 is required, AVX/AVX512 is enabled if available
override CFLAGS+= -mmmx -msse -msse2 -msse3
override CXXFLAGS+= -mmmx -msse -msse2 -msse3
override CFLAGS+= -DIS_I686
override CXXFLAGS+= -DIS_I686

#CFLAGS/CXXFLAGS+= -mcmodel=kernel -mno-red-zone in x86-64
endif

# For the entry kernel file, if compiled in C++
# CXXFLAGS_ENTRYKER=-fno-exception -fno-rtti
override LDFLAGS+= -ffreestanding -O$(OPTIM) -nostdlib
override LDLIBS+= -lgcc

ASKERFLAGS+=
CKERFLAGS+= -D__kernel__ -Isrc/kernel/include
CXXKERFLAGS+= -D__kernel__ -Isrc/kernel/include

CRTBEGIN_OBJ:=$(shell $(CC) $(CFLAGS) -print-file-name=crtbegin.o)
CRTEND_OBJ:=$(shell $(CC) $(CFLAGS) -print-file-name=crtend.o)

OBJLIST=$(OBJLIST_KERNEL)
OBJLIST_KERNEL:=

SPECIAL_OBJS:=%/crti.o $(CRTBEGIN_OBJ) $(CRTEND_OBJ) %/crtn.o

INSTALL_HEADERS:=

# Until bug #101648 is fixed
obj/$(OBJDIR)/kernel/arch/i686/mm.o: private CFLAGS+= -Wno-analyzer-malloc-leak

.SUFFIXES:
.SECONDEXPANSION:

ifneq ($(MAKECMDGOALS),clean)
.: ;
# $(eval $(call reproduce_tree,<base>))
define reproduce_tree =
$(1): ; $(SILENCER)mkdir $$@
$(1)/kernel: | $(1) ; $(SILENCER)mkdir $$@
$(1)/kernel/arch: | $(1)/kernel ; $(SILENCER)mkdir $$@
$(1)/kernel/arch/$(TARGET_MACHINE): | $(1)/kernel/arch ; $(SILENCER)mkdir $$@
endef
obj: ; $(SILENCER)mkdir $@
obj/$(OBJDIR): | obj
$(eval $(call reproduce_tree,obj/$(OBJDIR)))
$(eval $(call reproduce_tree,makedir))
$(SYSROOT): ; $(SILENCER)mkdir $@
$(SYSROOT)/boot: | $(SYSROOT) ; $(SILENCER)mkdir $@
$(SYSROOT)/boot/grub: | $(SYSROOT)/boot ; $(SILENCER)mkdir $@
$(SYSROOT)/usr: | $(SYSROOT) ; $(SILENCER)mkdir $@
$(SYSROOT)/usr/include: | $(SYSROOT)/usr ; $(SILENCER)mkdir $@
endif

# Colors:
# -------
#   +----------+-----------+
#   |    3     |     9     |
# +-+----------+-----------+
# |0|          |           | Black
# |1|          |    RM     | Red
# |2|          |   [MSG]   | Green
# |3| Creating |    ISO    | Yellow
# |4|Installing|    CP     | Blue
# |5|   ---    |LD/Checking| Purple
# |6| AS/C/C++ |           | Cyan
# |7|          |           | White/gray
# +-+----------+-----------+

# $(call colorize,<br_color>,<br_text>,<text_color>,<text>)
ifdef $(if $(FORCE_COLOR:0=),FORCE_COLOR,MAKE_TERMOUT)
colorize=@echo "\033[$(1)m[$(2)]\033[m \033[$(3)m$(4)\033[m"
else
colorize=@echo "[$(2)] $(4)"
endif

define newline :=


endef

# $(call remove,<list of file_names to remove>)
define remove =
$(call colorize,1;91,RM ,91,Removing $(1))
$(SILENCER)$(RM) -r $(1)
endef

# $(eval $(call install_header,<install_dir>,<source_dir>,<file_name>))
define install_header =
INSTALL_HEADERS+=$(SYSROOT)/usr/include/$(1)$(3)
$(SYSROOT)/usr/include/$(1)$(3): src/$(2)$(3) | $$$$(@D)
    $(call colorize,94,CP ,34,Installing $(3))
    $(SILENCER)cp $$^ $$@
endef

# $(eval $(call add_deptree,<output_filename_noext>,<input_filename_withoutsrc>))
ifeq ($(MAKECMDGOALS),clean)
add_deptree=
else
define add_deptree =
makedir/$(1).mk: | $$$$(@D)
    $(call colorize,95,DEP,33,Creating $(2) dependancies)
    $(SILENCER)set -e; $$(CC) $$(CFLAGS) $$(CKERFLAGS) -MM src/$(2) \
| sed 's,\($$(notdir $$(basename $(2)))\)\.o[ :]*,src/$$(dir $(2))\1.o $$@: ,g' >$$@
include makedir/$(1).mk
endef
endif

# $(call kernel_o,<base_dir>,<source_filename>,<output_filename>)
kernel_o=obj/$(OBJDIR)/kernel/$(1)$(3).o
# $(eval $(call compile_kernel_s,<base_dir>,<source_filename>,<output_filename>))
define compile_kernel_s =
OBJLIST_KERNEL+=$(call kernel_o,$(1),$(2),$(3))
$(call kernel_o,$(1),$(2),$(3)): src/kernel/source/$(1)$(2).s | $$$$(@D)
    $(call colorize,36,AS ,92,Compiling $$@)
$(SILENCER)$$(AS) $$(ASFLAGS) $$(ASKERFLAGS) -c src/kernel/source/$(1)$(2).s -o $$@
endef
# $(eval $(call compile_kernel_c,<base_dir>,<source_filename>,<output_filename>))
define compile_kernel_c =
$$(eval $$(call add_deptree,kernel/$(1)$(3),kernel/source/$(1)$(2).c))
OBJLIST_KERNEL+=$(call kernel_o,$(1),$(2),$(3))
$(call kernel_o,$(1),$(2),$(3)): src/kernel/source/$(1)$(2).c | $$$$(@D)
$(call colorize,36,C  ,92,Compiling $$@)
    $(SILENCER)$$(CC) $$(CFLAGS) $$(CKERFLAGS) -c src/kernel/source/$(1)$(2).c -o $$@
endef
# $(eval $(call compile_kernel_cxx,<base_dir>,<source_filename>,<output_filename>))
define compile_kernel_cxx =
$$(eval $$(call add_deptree,kernel/$(1)$(3),kernel/source/$(1)$(2).cpp))
OBJLIST_KERNEL+=$(call kernel_o,$(1),$(2),$(3))
$(call kernel_o,$(1),$(2),$(3)): src/kernel/source/$(1)$(2).cpp | $$$$(@D)
    $(call colorize,36,C++,92,Compiling $$@)
$(SILENCER)$$(CXX) $$(CXXFLAGS) $$(CXXKERFLAGS) -c src/kernel/source/$(1)$(2).cpp -o $$@
endef
# $(eval $(call compile_arch_dependant,<arch list>,<group>,<lang>,<args...>)); arch will be added automatically
compile_arch_dependant = $(foreach arch,$(1),$\
$(if $(TARGET_MACHINE:$(arch)),$\
OBJLIST+=$(call $(2)_o,$(4),$(5),$(6))$(newline),$\
$(call compile_$(2)_$(3),arch/$(arch)/$(4),$(5),$(6))$(newline)))

kernel.iso: $(SYSROOT)/boot/kernel.kern | $(SYSROOT)/boot/grub
    $(call colorize,93,ISO,33,Creating iso)
    $(SILENCER)echo "menuentry \"$(NAME)\" {\n\tmultiboot /boot/kernel.kern\n}" >$(SYSROOT)/boot/grub/grub.cfg
    $(SILENCER)grub-mkrescue -o kernel.iso $(SYSROOT) -quiet

$(SYSROOT)/boot/kernel.kern: $$(OBJLIST_KERNEL) obj/$(OBJDIR)/kernel/arch/$(TARGET_MACHINE)/crti.o \
  obj/$(OBJDIR)/kernel/arch/$(TARGET_MACHINE)/crtn.o src/$(TARGET_MACHINE)/linker.ld | $$(@D)
    $(call colorize,95,LD ,92,Linking $@)
    $(SILENCER)$(CC) -T src/$(TARGET_MACHINE)/linker.ld -o $@ \
        obj/$(OBJDIR)/kernel/arch/$(TARGET_MACHINE)/crti.o $(CRTBEGIN_OBJ) $(OBJLIST_KERNEL) $(LDFLAGS) $(LDLIBS) \
        $(CRTEND_OBJ) obj/$(OBJDIR)/kernel/arch/$(TARGET_MACHINE)/crtn.o
    $(call colorize,35,---,95,Checking output kernel)
    $(SILENCER)grub-file --is-x86-multiboot $@

$(eval $(call compile_arch_dependant,i686,kernel,s,,crti,crti))
$(eval $(call compile_arch_dependant,i686,kernel,s,,boot,boot))
$(eval $(call compile_arch_dependant,i686,kernel,s,,tables,tables))
$(eval $(call compile_arch_dependant,i686,kernel,s,,interrupts,interrupts))
$(eval $(call compile_arch_dependant,i686,kernel,s,,apic,apic))
$(eval $(call compile_arch_dependant,i686,kernel,s,,atomic,atomic))
$(eval $(call compile_kernel_c,,entry,entry))
$(eval $(call compile_arch_dependant,i686,kernel,c,,kernel,kernel))
$(eval $(call compile_arch_dependant,i686,kernel,s,,tty,tty.s))
$(eval $(call compile_arch_dependant,i686,kernel,c,,tty,tty.c))
$(eval $(call compile_arch_dependant,i686,kernel,c,,mm,mm))
$(eval $(call compile_kernel_c,,multiboot,multiboot))
$(eval $(call compile_arch_dependant,i686,kernel,s,,crtn,crtn))
OBJLIST_KERNEL:=$(filter-out $(SPECIAL_OBJS),$(OBJLIST_KERNEL))

all: kernel.iso

clean:
    $(call remove,kernel.iso)
    $(call remove,isodir)
    $(call remove,$(OBJLIST))
    $(call remove,obj/$(OBJDIR)/kernel/arch/$(TARGET_MACHINE)/crti.o obj/$(OBJDIR)/kernel/arch/$(TARGET_MACHINE)/crtn.o)
    $(call remove,$(INSTALL_HEADERS))
.PHONY: clean

distclean: clean
    $(call remove,makedir)
.PHONY: distclean

.DELETE_ON_ERROR:
\$\endgroup\$
2
  • \$\begingroup\$ By the way, this was detected as spam... Any idea why? (I saw here that the length is the problem, but I don't really know why. Also, removing the makefile fixed the issue... Maybe someone should report this to meta? (Me maybe?)) \$\endgroup\$
    – rajdakin
    Aug 3 at 21:56
  • 1
    \$\begingroup\$ Implicit rules "generate the object next to the source" only if you're building in the source directory. You probably want an out-of-tree build, so you can have multiple builds with different configurations, and keep the sources read-only. Implicit rules work well for this, in conjunction with VPATH. \$\endgroup\$ Aug 4 at 7:54
2
\$\begingroup\$

Here are some things that may help you improve your code.

Understand your tools

There are a huge number of redundant compiler warnings that serve little purpose except to clutter up the Makefile. I would recommend trimming that to the smallest possible non-redundant equivalent. The reason is that it will be very painful if you decide that you need to, for example, ignore a particular cast warning, but can't easily turn it off because -Wall enables it anyway.

Also, alphabetizing the remaining flags will help in maintenance. The documentation for gcc, for example, shows the -Wall and -Wextra flags in alphabetical order, which makes it easier to scan the list.

Don't override all user settings

If I have set ASFLAGS, CFLAGS, and CXXFLAGS on my system, it seems rather presumptuous to override every single one of those in the Makefile. Better would be to either allow for using either user environment strings or at the very least letting the user know you're ignoring all of them.

Put user-adjustable variables at the top

I have the equivalent of i686-elf-gcc on my machine, but that is not the name of the executable on my machine. User variables such as these should be at the top of the Makefile, if they're used at all (see previous note).

Support out-of-tree build

Right now, everything is rigidly nailed in place with no flexibility about where or how the project is built. The problem with that is that it means that, for example, trying two different versions to see how they perform, is made more difficult because there is not any obvious way to specify the destination directory tree. That is something that autotools supports that is extremely useful.

Rethink your use of macros

The Makefile current contains this:

define compile_kernel_c =
$$(eval $$(call add_deptree,kernel/$(1)$(3),kernel/source/$(1)$(2).c))
OBJLIST_KERNEL+=$(call kernel_o,$(1),$(2),$(3))
$(call kernel_o,$(1),$(2),$(3)): src/kernel/source/$(1)$(2).c | $$$$(@D)
        $(call colorize,36,C  ,92,Compiling $$@)
        $(SILENCER)$$(CC) $$(CFLAGS) $$(CKERFLAGS) -c src/kernel/source/$(1)$(2).c -o $$@
endef

This is obtuse in the extreme. If the purpose is to provide separate flags for certain subsets of source code, the way to do that is to create variables such as your existing OBJLIST_KERNEL for each of the different types and then create either an explicit or implicit rule for each. You can have 100% control with implicit rules if you think carefully about what you're doing.

If you run gnu make with --trace --always-build you will see that the build is not reliable with this Makefile. If, for example, the obj directory already exists, it will fail.

Consider supporting a "help" target

Most Makefiles that I use or maintain support make help rather than make usage. At the least, I'd suggest supporting help as an alias for usage.

Fix the bug(s)

Your colorize macro does not seem to work on my 64-bit Linux machine. Instead of color, I get things like this:

make: i686-elf-gcc: No such file or directory
make: i686-elf-gcc: No such file or directory
\033[95m[DEP]\033[m \033[33mCreating kernel/source/multiboot.c dependancies\033[m
/bin/sh: i686-elf-gcc: command not found

The reason for this is that you are using \033 to represent ESC, but that requires the use of echo -e.

\$\endgroup\$
7
  • \$\begingroup\$ So, in order: Tools: Well, actually I don't think any of these flags are redundant. I've read the entire GCC documentation about common warnings to make sure only disabled flags are enabled. Yup, -Wall -Wextra doesn't do all... (There are still some left disabled.) One thing to consider too is that I specfically enabled all of those since I work with only my files which are warning-free. Obviously, I wouldn't want all of these in someone else's build :p Though, thanks for the feedback about disabling some warnings. I'll change the CFLAGS+= into a CFLAGS:=[warnings] $(CFLAGS) ...[1/x] \$\endgroup\$
    – rajdakin
    Aug 4 at 9:27
  • \$\begingroup\$ USER-adjustable variables: Yeah, I could just check where the variable come from before overriding it. (The problem is that their default value doesn't work for me, or rather shouldn't work for anyone...) Out-of-tree build: Now, it's not like I hate it. It's just, is it really useful here? And doesn't make -f somewhere else already do that? Macros: Well, I don't think that's really the way to go here, because of multiple destination targets support. I can see another way to do it though (foreach after adding all of the files), but the rule would still be huge. [2/3] \$\endgroup\$
    – rajdakin
    Aug 4 at 9:34
  • \$\begingroup\$ help: Yep, I'll fix that (the rest of the makefile I was reading the documentation in parallel, for this one I just said "Oh yeah I need something like that"). Bug: Huh, weird. For me, it works even when setting MAKE_SHELL to /bin/sh... After a quick search I found out I should rather use printf. Well, thanks for the review anyways! I'll mark it as the answer. [3/3] \$\endgroup\$
    – rajdakin
    Aug 4 at 9:40
  • \$\begingroup\$ If you read the gcc docs closely, you'll see that -Wimplicit-fallthrough=3 is enabled by -Wextra and is also separately listed in your makefile. That meets my definition of "redundant." \$\endgroup\$
    – Edward
    Aug 4 at 12:34
  • \$\begingroup\$ Whoops. Well, 1 doesn't meet my definition of "huge number"... :p (Still, I'll remove it. It'd be nicer if GCC could have a -Wyes-I-want-all though...) I've found another redundant warning, but 2 isn't "a lot"... (I agree though, it's still a huge block.) \$\endgroup\$
    – rajdakin
    Aug 4 at 13:40
0
\$\begingroup\$
colorize=@echo "\033[$(1)m[$(2)]\033[m \033[$(3)m$(4)\033[m"

How do you know what kind of terminal is being used? Or even that output is going to a terminal? This is the kind of crap that makes build logs hard to read.

If you really must do this, use tput to determine the codes you want (when supported by the output device, or a nice empty string when not).

\$\endgroup\$
1
  • \$\begingroup\$ Well, that's what MAKE_TERMOUT is for. I tested, and when redirecting to less colors are removed. If you don't like it, I could add support for a FORCE_NO_COLOR, but... is that really useful? \$\endgroup\$
    – rajdakin
    Aug 4 at 9:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.