Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Test Unix x86 with asan/ubsan in CI #49

Merged
merged 1 commit into from
Sep 26, 2023
Merged

Test Unix x86 with asan/ubsan in CI #49

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/push.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ jobs:
runs-on: ubuntu-22.04
container:
image: ubuntu:22.04
env:
CFLAGS: '-fsanitize=address,undefined'
steps:
- uses: actions/checkout@v3
- name: apt
Expand All @@ -33,6 +35,8 @@ jobs:
runs-on: ubuntu-22.04
container:
image: ubuntu:22.04
env:
CFLAGS: '-fsanitize=address,undefined'
steps:
- uses: actions/checkout@v3
- name: apt
Expand Down Expand Up @@ -86,6 +90,8 @@ jobs:

MACOS_x86_64:
runs-on: macos-11
env:
CFLAGS: '-fsanitize=address,undefined'
steps:
- uses: actions/checkout@v3
- name: brew
Expand Down
12 changes: 6 additions & 6 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,32 +10,32 @@ EXAMPLES_BUILD_DIR = $(BUILD_DIR)/examples
CC = gcc
CXX = g++
BUILD_CC = gcc
CFLAGS = -Wall -Wextra -Wno-unused-parameter
override CFLAGS += -Wall -Wextra -Wno-unused-parameter
LDFLAGS = -lm -ldl
PHP = php
LLK = llk
#LLK = $(PHP) $(HOME)/php/llk/llk.php

ifeq (debug, $(BUILD))
CFLAGS += -O0 -g -DIR_DEBUG=1
override CFLAGS += -O0 -g -DIR_DEBUG=1
endif
Comment on lines 19 to 21
Copy link
Owner

Choose a reason for hiding this comment

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

I've never known about the "override" before.
I think, it may be better to use a different BUILD type. e.g. "asan" in addition to "release" and "debug".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://www.gnu.org/software/make/manual/html_node/Override-Directive.html

If a variable has been set with a command argument (see Overriding Variables), then ordinary assignments in the makefile are ignored. If you want to set the variable in the makefile even though it was set with a command argument, you can use an override directive, which is a line that looks like this:

override variable = value

I think override is a good flag either way, because otherwise users can still set CFLAGS via make CFLAGS=... but local CFLAGS in the Makefile are completely ignored. This can lead to unexpected behavior. I.e. in this case it will cancel the -Wall -Wextra -Wno-unused-parameter, and -O2 -g with the release build, etc. This is not immediately obvious when the removal of the flags doesn't break the build.

Copy link
Owner

Choose a reason for hiding this comment

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

OK

ifeq (release, $(BUILD))
CFLAGS += -O2 -g
override CFLAGS += -O2 -g
endif

ifeq (x86_64, $(TARGET))
CFLAGS += -DIR_TARGET_X64
override CFLAGS += -DIR_TARGET_X64
DASM_ARCH = x86
DASM_FLAGS = -M -D X64=1
endif
ifeq (x86, $(TARGET))
CFLAGS += -m32 -DIR_TARGET_X86
override CFLAGS += -m32 -DIR_TARGET_X86
DASM_ARCH = x86
DASM_FLAGS = -M
endif
ifeq (aarch64, $(TARGET))
CC= aarch64-linux-gnu-gcc --sysroot=$(HOME)/php/ARM64
CFLAGS += -DIR_TARGET_AARCH64
override CFLAGS += -DIR_TARGET_AARCH64
DASM_ARCH = aarch64
DASM_FLAGS = -M
endif
Expand Down