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

Add template support for "--enable-debug" flag #1364

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

yosifkit
Copy link
Member

@yosifkit yosifkit commented Jan 26, 2023

DOCKER_PHP_ENABLE_DEBUG is not used or supported by official-images; this is for users who want to build their own php image with debug enabled

This will allow power users to generate their own PHP Dockerfiles with debug enabled. This is explicitly written to not change the regular versions of the Dockerfiles (i.e. running ./apply-templates.sh will result in no change). I have build tested one or two versions and they seem to work:

$ docker build 8.0/alpine3.16/cli/
...
Successfully built 7feb88eade75
$ docker run -it --rm 7feb88eade75 php -i
...
Debug Build => yes
...

To regenerate all the Dockerfiles with debug enabled (bash, jq, gawk, and wget are required to run it):

$ DOCKER_PHP_ENABLE_DEBUG=1 ./apply-templates
processing 8.0/bullseye/cli ...
processing 8.0/bullseye/apache ...
[...]
processing 8.2-rc/alpine3.16/fpm ...
processing 8.2-rc/alpine3.16/zts ...

Alternative to #1287 and #1280.

Closes #1287
Closes #1280

@enumag
Copy link

enumag commented Jan 26, 2023

I believe that #1280 is still relevant even with this, right?

@tianon
Copy link
Member

tianon commented Jan 26, 2023

Nope, the generated Dockerfiles from this will not include the strip block at all.

@tianon
Copy link
Member

tianon commented Jan 26, 2023

An example full generated diff, to illustrate:

diff --git a/8.1/bullseye/cli/Dockerfile b/8.1/bullseye/cli/Dockerfile
index 3609c4f2..954ceae5 100644
--- a/8.1/bullseye/cli/Dockerfile
+++ b/8.1/bullseye/cli/Dockerfile
@@ -174,18 +174,11 @@ RUN set -eux; \
 		\
 # https://github.com/docker-library/php/pull/939#issuecomment-730501748
 		--enable-embed \
+		--enable-debug \
 	; \
 	make -j "$(nproc)"; \
 	find -type f -name '*.a' -delete; \
 	make install; \
-	find \
-		/usr/local \
-		-type f \
-		-perm '/0111' \
-		-exec sh -euxc ' \
-			strip --strip-all "$@" || : \
-		' -- '{}' + \
-	; \
 	make clean; \
 	\
 # https://github.com/docker-library/php/issues/692 (copy default example "php.ini" files somewhere easily discoverable)

@enumag
Copy link

enumag commented Feb 7, 2023

@yosifkit @tianon Can we get this merged? I'd like to use it.

@enumag

This comment was marked as spam.

@joehoyle
Copy link

joehoyle commented Sep 14, 2023

This looks great. We'd like to be able to build a variant of the image without stripping symbols. This is especially useful when trying to debug core dumps. With the stripped binary it's very difficult to debug a core dump meaningfully. However, it seems in this PR this is tied to the --enable-debug build of PHP. As far as I understand, one can not take a core dump generated by a non-debug build (which you'd use in production) and later use gdb with the php build that has been built with --enable-debug.

If that's the use-case this PR is trying to address, I wanted to highlight it as a concern.

@enumag
Copy link

enumag commented Sep 14, 2023

Yeah... I wasn't aware that --enable-debug isn't required to get backtrace from core dumps until yesterday. Indeed it would be better if there was a separate flag to disable symbol stripping while keeping debug disabled.

@joehoyle
Copy link

joehoyle commented Sep 14, 2023

On that topic, for better backtraces adding -g2 to the CFLAGS will remove some optimizations, which will gives more frames etc in the backtrace. In my testing, it's safe to use -g2 on a "debug" (not --enable-debug) build of PHP for reading production code dumps.

Copy link
Member

@tianon tianon left a comment

Choose a reason for hiding this comment

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

I'm going to merge this -- I'm still really concerned about it, because we will not be testing it, especially over time, and we cannot guarantee it will continue to work properly in the future, but I hope the comments in the template make that clear enough.

@tianon tianon merged commit 02212e3 into docker-library:master Dec 20, 2023
@tianon tianon deleted the debug branch December 20, 2023 20:23
@tianon tianon mentioned this pull request Dec 20, 2023
docker-library-bot added a commit to docker-library-bot/official-images that referenced this pull request Dec 22, 2023
Changes:

- docker-library/php@22ecd27e: Update 8.3-rc
- docker-library/php@9f3c89c9: Update 8.2-rc
- docker-library/php@23b4a3f1: Update 8.1-rc
- docker-library/php@5fa6bfe5: Update 8.1 to 8.1.27
- docker-library/php@02212e33: Merge pull request docker-library/php#1364 from infosiftr/debug
docker-library-bot added a commit to docker-library-bot/official-images that referenced this pull request Dec 22, 2023
Changes:

- docker-library/php@aacb4822: Update 8.3 to 8.3.1
- docker-library/php@ce5371b4: Update 8.2 to 8.2.14
- docker-library/php@22ecd27e: Update 8.3-rc
- docker-library/php@9f3c89c9: Update 8.2-rc
- docker-library/php@23b4a3f1: Update 8.1-rc
- docker-library/php@5fa6bfe5: Update 8.1 to 8.1.27
- docker-library/php@02212e33: Merge pull request docker-library/php#1364 from infosiftr/debug
docker-library-bot added a commit to docker-library-bot/official-images that referenced this pull request Dec 22, 2023
Changes:

- docker-library/php@aacb4822: Update 8.3 to 8.3.1
- docker-library/php@ce5371b4: Update 8.2 to 8.2.14
- docker-library/php@22ecd27e: Update 8.3-rc
- docker-library/php@9f3c89c9: Update 8.2-rc
- docker-library/php@23b4a3f1: Update 8.1-rc
- docker-library/php@5fa6bfe5: Update 8.1 to 8.1.27
- docker-library/php@02212e33: Merge pull request docker-library/php#1364 from infosiftr/debug
martin-g pushed a commit to martin-g/docker-official-images that referenced this pull request Apr 3, 2024
Changes:

- docker-library/php@aacb4822: Update 8.3 to 8.3.1
- docker-library/php@ce5371b4: Update 8.2 to 8.2.14
- docker-library/php@22ecd27e: Update 8.3-rc
- docker-library/php@9f3c89c9: Update 8.2-rc
- docker-library/php@23b4a3f1: Update 8.1-rc
- docker-library/php@5fa6bfe5: Update 8.1 to 8.1.27
- docker-library/php@02212e33: Merge pull request docker-library/php#1364 from infosiftr/debug
@WoZ
Copy link

WoZ commented May 11, 2024

It is worth mentioning, that the image built with DOCKER_PHP_ENABLE_DEBUG will not have sources required for listing of the code.

Reading symbols from php-fpm...
(gdb) start
Temporary breakpoint 1 at 0x897d14: file /usr/src/php/sapi/fpm/fpm/fpm_main.c, line 1532.
Starting program: /usr/local/sbin/php-fpm -F
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/aarch64-linux-gnu/libthread_db.so.1".

Temporary breakpoint 1, main (argc=2, argv=0xfffffffffa38) at /usr/src/php/sapi/fpm/fpm/fpm_main.c:1532
1532	/usr/src/php/sapi/fpm/fpm/fpm_main.c: No such file or directory.

To fix this run

docker-php-source extract

Inside the container (or make it the part of the building process).

After extraction

Reading symbols from php-fpm...
(gdb) start
Temporary breakpoint 1 at 0x897d14: file /usr/src/php/sapi/fpm/fpm/fpm_main.c, line 1532.
Starting program: /usr/local/sbin/php-fpm -F
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/aarch64-linux-gnu/libthread_db.so.1".

Temporary breakpoint 1, main (argc=2, argv=0xfffffffffa38) at /usr/src/php/sapi/fpm/fpm/fpm_main.c:1532
1532	{
(gdb) list
1527		STANDARD_MODULE_PROPERTIES
1528	};
1529
1530	/* {{{ main */
1531	int main(int argc, char *argv[])
1532	{
1533		int exit_status = FPM_EXIT_OK;
1534		int cgi = 0, c, use_extended_info = 0;
1535		zend_file_handle file_handle;
1536
(gdb)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants