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.

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

Create proper debug configuration #6193

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 8 additions & 5 deletions cross/ffmpeg6/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,16 @@ COMMENT = FFmpeg is a complete, cross-platform solution to record, convert and
LICENSE = GPLv2

CONFIGURE_ARGS = --target-os=linux --cross-prefix=$(TC_PATH)$(TC_PREFIX) --prefix=$(INSTALL_PREFIX)
CONFIGURE_ARGS += --extra-cflags="-I$(WORK_DIR)/install$(INSTALL_PREFIX)/include"
CONFIGURE_ARGS += --extra-ldflags="-L$(WORK_DIR)/install$(INSTALL_PREFIX)/lib"
CONFIGURE_ARGS += --extra-libs="-lxml2 -ldl -lm" --pkg-config=/usr/bin/pkg-config --ranlib=$(RANLIB)
Comment on lines -13 to -15
Copy link
Contributor

Choose a reason for hiding this comment

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

This code dates way-back, before I started maintaining ffmpeg. Why is only pkg-config related code kept?

# no own pkg-config in toolchain:
CONFIGURE_ARGS += --pkg-config=/usr/bin/pkg-config
CONFIGURE_ARGS += --enable-cross-compile --enable-rpath --enable-pic
CONFIGURE_ARGS += --enable-shared
CONFIGURE_ARGS += --enable-gpl --enable-version3
CONFIGURE_ARGS += --disable-debug
ifeq ($(strip $(GCC_DEBUG_INFO)),1)
CONFIGURE_ARGS += --enable-debug=3 --disable-stripping --disable-optimizations --assert-level=2 --enable-ftrapv
else
CONFIGURE_ARGS += --disable-debug --enable-stripping
endif
CONFIGURE_ARGS += --disable-static
CONFIGURE_ARGS += --disable-doc

Expand Down Expand Up @@ -214,7 +217,7 @@ endif

ifeq ($(findstring $(ARCH),$(ARMv7_ARCHS) $(ARMv7L_ARCHS)),$(ARCH))
CONFIGURE_ARGS += --arch=arm --enable-neon --enable-thumb --disable-armv6 --disable-armv6t2 --disable-vfp --disable-armv5te
ifneq ($(findstring $(ARCH),alpine),$(ARCH))
ifeq ($(findstring alpine,$(ARCH)),alpine)
CONFIGURE_ARGS += --extra-cflags=-DSYNO_ALPINE_NEON
endif
endif
Expand Down
2 changes: 0 additions & 2 deletions cross/glibc-2.28/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ CONFIGURE_ARGS += --enable-stack-protector=strong
CONFIGURE_ARGS += --with-headers=$(TC_SYSROOT)/usr/include
CONFIGURE_ARGS += ac_cv_prog_MAKEINFO=false

ADDITIONAL_CFLAGS = -O2

include ../../mk/spksrc.cross-cc.mk

.PHONY: glibc_configure_target
Expand Down
5 changes: 4 additions & 1 deletion cross/libtheora/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ GNU_CONFIGURE = 1
CONFIGURE_ARGS = --disable-static
CONFIGURE_ARGS += --disable-examples
CONFIGURE_ARGS += --disable-spec
ADDITIONAL_CFLAGS = -O

ifeq ($(strip $(GCC_DEBUG_INFO)),1)
COMPILE_MAKE_OPTIONS = debug
endif

include ../../mk/spksrc.cross-cc.mk
5 changes: 4 additions & 1 deletion cross/libvorbis/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ LICENSE = BSD

GNU_CONFIGURE = 1
CONFIGURE_ARGS = --disable-static
ADDITIONAL_CFLAGS = -Os

ifeq ($(strip $(GCC_DEBUG_INFO)),1)
COMPILE_MAKE_OPTIONS = debug
endif

include ../../mk/spksrc.cross-cc.mk
2 changes: 0 additions & 2 deletions cross/nettle/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,4 @@ ifeq ($(call version_lt,${TCVERSION},6.0)$(call version_gt,${TCVERSION},2.0),11)
CONFIGURE_ARGS += --disable-assembler
endif

ADDITIONAL_CFLAGS = -O2

include ../../mk/spksrc.cross-cc.mk
1 change: 0 additions & 1 deletion cross/x264/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ GNU_CONFIGURE = 1
CONFIGURE_ARGS = --enable-shared
CONFIGURE_ARGS += --enable-pic
CONFIGURE_ARGS += --disable-opencl
CONFIGURE_ARGS += --prefix=$(INSTALL_PREFIX)
Copy link
Contributor

Choose a reason for hiding this comment

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

why is the prefix being removed? Presumably because the spksrc.configure.mk adds it anyway?

CONFIGURE_ARGS += --cross-prefix=$(TC_PATH)$(TC_PREFIX)

include ../../mk/spksrc.cross-cc.mk
Expand Down
17 changes: 11 additions & 6 deletions mk/spksrc.configure.mk
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,17 @@ endif

REAL_CONFIGURE_ARGS =
ifneq ($(strip $(GNU_CONFIGURE)),)
REAL_CONFIGURE_ARGS += $(TC_CONFIGURE_ARGS)
REAL_CONFIGURE_ARGS += --prefix=$(INSTALL_PREFIX)
# DSM7 appdir
ifeq ($(call version_ge, ${TCVERSION}, 7.0),1)
REAL_CONFIGURE_ARGS += --localstatedir=$(INSTALL_PREFIX_VAR)
endif
ifeq ($(strip $(GCC_DEBUG_INFO)),1)
REAL_CONFIGURE_ARGS += --enable-debug --disable-strip
else
REAL_CONFIGURE_ARGS += --enable-strip
endif
REAL_CONFIGURE_ARGS += $(TC_CONFIGURE_ARGS)
REAL_CONFIGURE_ARGS += --prefix=$(INSTALL_PREFIX)
# DSM7 appdir
ifeq ($(call version_ge, ${TCVERSION}, 7.0),1)
REAL_CONFIGURE_ARGS += --localstatedir=$(INSTALL_PREFIX_VAR)
endif
endif
REAL_CONFIGURE_ARGS += $(CONFIGURE_ARGS)

Expand Down
7 changes: 5 additions & 2 deletions mk/spksrc.cross-cmake-env.mk
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@ endif

# We normally build regular Release
ifeq ($(strip $(CMAKE_BUILD_TYPE)),)
CMAKE_ARGS += -DCMAKE_BUILD_TYPE=Release
ifeq ($(strip $(GCC_DEBUG_INFO)),1)
CMAKE_ARGS += -DCMAKE_BUILD_TYPE=Debug
endif
CMAKE_ARGS += -DCMAKE_BUILD_TYPE=Release
Comment on lines +10 to +12
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be ifeq / else / endif such as:

ifeq ($(strip $(CMAKE_BUILD_TYPE)),)
  ifeq ($(strip $(GCC_DEBUG_INFO)),1)
    CMAKE_ARGS += -DCMAKE_BUILD_TYPE=Debug
  else
    CMAKE_ARGS += -DCMAKE_BUILD_TYPE=Release
  endif
else
CMAKE_ARGS += -DCMAKE_BUILD_TYPE=$(CMAKE_BUILD_TYPE)
endif

else
CMAKE_ARGS += -DCMAKE_BUILD_TYPE=$(CMAKE_BUILD_TYPE)
CMAKE_ARGS += -DCMAKE_BUILD_TYPE=$(CMAKE_BUILD_TYPE)
endif

# Set the default install prefix
Expand Down
15 changes: 11 additions & 4 deletions mk/spksrc.cross-env.mk
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,19 @@ TOOLKIT_ROOT = $(WORK_DIR)/../../../toolkit/syno-$(ARCH)-$(TCVERSION)/work
ENV += TOOLKIT_ROOT=$(TOOLKIT_ROOT)
endif

ADDITIONAL_CFLAGS := $(patsubst -O%,,$(ADDITIONAL_CFLAGS))
ADDITIONAL_CPPFLAGS := $(patsubst -O%,,$(ADDITIONAL_CPPFLAGS))
ADDITIONAL_CXXFLAGS := $(patsubst -O%,,$(ADDITIONAL_CXXFLAGS))
ifeq ($(strip $(GCC_DEBUG_INFO)),1)
GCC_DEBUG_FLAGS = -O0 -g3
ADDITIONAL_CFLAGS := $(patsubst -O%,,$(ADDITIONAL_CFLAGS)) $(GCC_DEBUG_FLAGS)
ADDITIONAL_CPPFLAGS := $(patsubst -O%,,$(ADDITIONAL_CPPFLAGS)) $(GCC_DEBUG_FLAGS)
ADDITIONAL_CXXFLAGS := $(patsubst -O%,,$(ADDITIONAL_CXXFLAGS)) $(GCC_DEBUG_FLAGS)
# other options to consider:
#GCC_DEBUG_FLAGS += -fsanitize=address -fsanitize=undefined -fstack-protector-all
GCC_DEBUG_FLAGS = -ggdb3 -g3
else
GCC_DEBUG_FLAGS = -O3 -fomit-frame-pointer
endif
ADDITIONAL_CFLAGS := $(ADDITIONAL_CFLAGS) $(GCC_DEBUG_FLAGS)
ADDITIONAL_CPPFLAGS := $(ADDITIONAL_CPPFLAGS) $(GCC_DEBUG_FLAGS)
ADDITIONAL_CXXFLAGS := $(ADDITIONAL_CXXFLAGS) $(GCC_DEBUG_FLAGS)
Comment on lines +24 to +36
Copy link
Contributor

Choose a reason for hiding this comment

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

This has a much broader effect than only affecting debug builds. This is turn will enforce using -O3 -fomit-frame-pointer on all buids, which will not work due to some arch specific caveats that requires special flags to be used. GCC_DEBUG_FLAGS should only be used for GCC_DEBUG_INFO builds and not be affecting other regular builds.

Also I believe this sort of explains the reasons for removing -O? in some other Makefiles?


ifneq ($(strip $(TC)),)
TC_VARS_MK = $(WORK_DIR)/tc_vars.mk
Expand Down
8 changes: 6 additions & 2 deletions mk/spksrc.cross-meson-env.mk
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,13 @@ endif
# Set other build options
# We normally build regular Release
ifeq ($(strip $(MESON_BUILD_TYPE)),)
CONFIGURE_ARGS += -Dbuildtype=release
ifeq ($(strip $(GCC_DEBUG_INFO)),1)
CONFIGURE_ARGS += -Dbuildtype=debug
else
CONFIGURE_ARGS += -Dbuildtype=release
endif
else
CONFIGURE_ARGS += -Dbuildtype=$(MESON_BUILD_TYPE)
CONFIGURE_ARGS += -Dbuildtype=$(MESON_BUILD_TYPE)
endif

# Configuration for meson build
Expand Down
12 changes: 6 additions & 6 deletions mk/spksrc.tc.mk
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,9 @@ tc_vars: flag
gcc_version=$$(eval $$(echo $(WORK_DIR)/$(TC_TARGET)/bin/$(TC_PREFIX)$${source} -dumpversion) 2>/dev/null || true) ; \
fi ; \
done ; \
echo TC_ENV += CFLAGS=\"$(CFLAGS) $(GCC_DEBUG) $$\(ADDITIONAL_CFLAGS\)\" ; \
echo TC_ENV += CPPFLAGS=\"$(CPPFLAGS) $(GCC_DEBUG) $$\(ADDITIONAL_CPPFLAGS\)\" ; \
echo TC_ENV += CXXFLAGS=\"$(CXXFLAGS) $(GCC_DEBUG) $$\(ADDITIONAL_CXXFLAGS\)\" ; \
echo TC_ENV += CFLAGS=\"$(CFLAGS) $$\(ADDITIONAL_CFLAGS\)\" ; \
echo TC_ENV += CPPFLAGS=\"$(CPPFLAGS) $$\(ADDITIONAL_CPPFLAGS\)\" ; \
echo TC_ENV += CXXFLAGS=\"$(CXXFLAGS) $$\(ADDITIONAL_CXXFLAGS\)\" ; \
echo TC_ENV += LDFLAGS=\"$(LDFLAGS) $$\(ADDITIONAL_LDFLAGS\)\" ; \
echo TC_ENV += CARGO_HOME=\"$(realpath $(CARGO_HOME))\" ; \
echo TC_ENV += RUSTUP_HOME=\"$(realpath $(RUSTUP_HOME))\" ; \
Comment on lines -156 to 161
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't thig be GCC_DEBUG_FLAGS?

Expand All @@ -170,9 +170,9 @@ tc_vars: flag
echo TC_TARGET := $(TC_TARGET) ; \
echo TC_PREFIX := $(TC_PREFIX) ; \
echo TC_PATH := $(WORK_DIR)/$(TC_TARGET)/bin/ ; \
echo CFLAGS := $(CFLAGS) $(GCC_DEBUG) $$\(ADDITIONAL_CFLAGS\) ; \
echo CPPFLAGS := $(CPPFLAGS) $(GCC_DEBUG) $$\(ADDITIONAL_CPPFLAGS\) ; \
echo CXXFLAGS := $(CXXFLAGS) $(GCC_DEBUG) $$\(ADDITIONAL_CXXFLAGS\) ; \
echo CFLAGS := $(CFLAGS) $$\(ADDITIONAL_CFLAGS\) ; \
echo CPPFLAGS := $(CPPFLAGS) $$\(ADDITIONAL_CPPFLAGS\) ; \
echo CXXFLAGS := $(CXXFLAGS) $$\(ADDITIONAL_CXXFLAGS\) ; \
echo LDFLAGS := $(LDFLAGS) $$\(ADDITIONAL_LDFLAGS\) ; \
echo TC_INCLUDE := $(TC_INCLUDE) ; \
echo TC_LIBRARY := $(TC_LIBRARY) ; \
Comment on lines 170 to 178
Copy link
Contributor

Choose a reason for hiding this comment

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

likewise to previous comment.

Expand Down