Remove "-rdynamic" flag for static builds #300

ericpruitt · 2019-05-25T19:33:13Z

I ran into problems trying to build the statically linked cmark executable using musl libc that was caused by the "-rdynamic" flag being implicitly added to the build command. The resulting executable would had references to a musl libc shared object instead of being hermetic as expected. I'm not sure if this has any unintended consequences, but I at least have no problems building a dynamically linked executable with this patch applied on my Linux and macOS machines.

ericpruitt · 2019-05-25T19:38:44Z

The CI pipeline shows this fails to build with MSVC due to the number of arguments used with the REGEX command. I have no experience with C development on Windows, and I'm a CMake novice, so I'm not sure how I should address the build failure.

jgm · 2019-05-27T17:34:59Z

I'm not a cmake expert either, but I'm sure this isn't the right approach to fix this problem. (If there is a problem: it would be good to have a fuller account of the problem, so we can confirm it.) Probably if something needs to be changed, it's in src/CMakeLists.txt, where the parameters for static and dynamic builds are being defined.

ericpruitt · 2019-05-27T18:03:04Z

Probably if something needs to be changed, it's in src/CMakeLists.txt, where the parameters for static and dynamic builds are being defined.

I don't know if this makes a difference, but from what I saw when using CMake with verbosity/debugging, "-rdynamic" is added in based on rules in my systems global CMake configuration files, not something in the cmark build rules.

(If there is a problem: it would be good to have a fuller account of the problem, so we can confirm it.)

In the text below, I've gone through the process of generating the faulty build by generating the musl compiler wrapper (musl-gcc) and using the wrapper to compile cmark:

Extract musl and generate musl-gcc

$ tar xf musl-1.1.20.tar.gz
$ cd musl-1.1.20/
musl-1.1.20$ ./configure --disable-shared --prefix=$PWD
checking for C compiler... gcc
checking whether C compiler works... yes
...
checking preprocessor condition __ILP32__... false
checking whether compiler's long double definition matches float.h... yes
checking preprocessor condition __FAST_MATH__... false
creating config.mak... done
musl-1.1.20$ make install
musl-1.1.20$ make install
mkdir -p lib
mkdir -p obj
...
nistd/unlinkat.lo obj/src/unistd/usleep.lo obj/src/unistd/write.lo obj/src/unistd/writev.lo
ranlib lib/libc.a
./tools/install.sh -D -m 644 lib/libc.a ...musl-1.1.20/lib/libc.a
musl-1.1.20$ ls bin/
musl-gcc

Prepare the cmark build

musl-1.1.20$ cd ../cmark-src/
cmark-src$ mkdir build && cd build
build$ CFLAGS='-static -fpie -fPIC' CFLAGS='-O3 -static -fpie -fPIC' CC=.../musl-1.1.20/bin/musl-gcc cmake -G "Unix Makefiles" -DCMARK_TESTS=OFF -DCMARK_SHARED=OFF ..
-- The C compiler identification is GNU 6.3.0
-- The CXX compiler identification is GNU 6.3.0
-- Check for working C compiler: .../musl-1.1.20/bin/musl-gcc
-- Check for working C compiler: .../musl-1.1.20/bin/musl-gcc -- works
...
-- Generating done
-- Build files have been written to: .../cmark/build

Create the executable

build$ make
Scanning dependencies of target libcmark_static
[  4%] Building C object src/CMakeFiles/libcmark_static.dir/node.c.o
[  9%] Building C object src/CMakeFiles/libcmark_static.dir/buffer.c.o
...
[ 95%] Building C object src/CMakeFiles/cmark.dir/main.c.o
[100%] Linking C executable cmark
[100%] Built target cmark

Test the executable

build$ ./src/cmark --help
bash: ./src/cmark: No such file or directory
(127)

This shows the embedded shared object path which shouldn't be there.

build$ strings src/cmark | head -n5
/lib/ld-musl-x86_64.so.1
__stdout_used
stdout
__lctrans_cur
__strtoull_internal

jgm · 2019-05-27T18:04:26Z

@nwellnhof do you have any insight on this issue?

nwellnhof · 2019-05-27T18:58:37Z

Seems like a CMake issue. The option probably comes from here: https://github.com/Kitware/CMake/blob/07cfb18f9d29cfc0588ede928846a03ec5599c48/Modules/Platform/Linux-GNU.cmake

Maybe setting -DCMAKE_SYSTEM_NAME=Generic helps.

ericpruitt · 2019-05-27T19:09:41Z

Maybe setting -DCMAKE_SYSTEM_NAME=Generic helps.

That doesn't make a difference for me:

build$ CFLAGS='-static -fpie -fPIC' CFLAGS='-O3 -static -fpie -fPIC' CC=.../musl-1.1.20/bin/musl-gcc cmake -G "Unix Makefiles" -DCMARK_TESTS=OFF -DCMARK_SHARED=OFF -DCMAKE_SYSTEM_NAME=Generic ..
-- The C compiler identification is GNU 6.3.0
-- The CXX compiler identification is Clang 3.8.1
-- Check for working C compiler: /home/ericpruitt/emus/core/musl-1.1.20/bin/musl-gcc
-- Check for working C compiler: /home/ericpruitt/emus/core/musl-1.1.20/bin/musl-gcc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /usr/bin/clang++
-- Check for working CXX compiler: /usr/bin/clang++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Performing Test HAVE_FLAG_ADDRESS_SANITIZER
-- Performing Test HAVE_FLAG_ADDRESS_SANITIZER - Failed
-- Performing Test HAVE_FLAG_SANITIZE_ADDRESS
-- Performing Test HAVE_FLAG_SANITIZE_ADDRESS - Failed
-- Performing Test COMPILER_HAS_HIDDEN_VISIBILITY
-- Performing Test COMPILER_HAS_HIDDEN_VISIBILITY - Success
-- Performing Test COMPILER_HAS_HIDDEN_INLINE_VISIBILITY
-- Performing Test COMPILER_HAS_HIDDEN_INLINE_VISIBILITY - Success
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR - Success
-- Looking for stdbool.h
-- Looking for stdbool.h - found
-- Performing Test HAVE___BUILTIN_EXPECT
-- Performing Test HAVE___BUILTIN_EXPECT - Success
-- Performing Test HAVE___ATTRIBUTE__
-- Performing Test HAVE___ATTRIBUTE__ - Success
-- Configuring done
-- Generating done
-- Build files have been written to: /home/ericpruitt/emus/core/cmark/build
build$ ./src/cmark --help
bash: ./src/cmark: No such file or directory
(127)

At the time of this writing, it seems that CMake only uses "-rdynamic" on Linux: ``` $ git clone https://github.com/Kitware/CMake.git Cloning into 'CMake'... remote: Enumerating objects: 335, done. remote: Counting objects: 100% (335/335), done. remote: Compressing objects: 100% (254/254), done. remote: Total 272793 (delta 86), reused 125 (delta 79), pack-reused 272458 Receiving objects: 100% (272793/272793), 88.11 MiB | 29.21 MiB/s, done. Resolving deltas: 100% (207174/207174), done. $ cd CMake/Modules/ Modules$ fgrep -r -w rdynamic . ./Platform/Linux-TinyCC-C.cmake:set(CMAKE_EXE_EXPORTS_C_FLAG "-rdynamic ") ./Platform/Linux-NAG-Fortran.cmake:set(CMAKE_SHARED_LIBRARY_LINK_Fortran_FLAGS "-Wl,-rdynamic") ./Platform/Linux-Intel.cmake: set(CMAKE_SHARED_LIBRARY_LINK_${lang}_FLAGS "-rdynamic") ./Platform/Linux-GNU.cmake: set(CMAKE_SHARED_LIBRARY_LINK_${lang}_FLAGS "-rdynamic") ``` So the conditional has been modified accordingly.

Fix typo by changing "STATIC" to "CMARK_STATIC".

ericpruitt · 2019-05-27T21:56:09Z

The only place I see mention of "rdynamic" in the CMake source is in the modules for Linux, so I've updated the conditional in my patch accordingly, and the build now succeeds on MSVC.

LINUX is not a CMake platform variable, so the expression has been updated accordingly and the build functionality verified.

- At the time of this writing, "-rdynamic" is only added when using CMake on Linux, so the conditional expression used for patching cmark's build system has been updated to specifically check for Linux. Thanks for Nick Wellnhofer (nwellnhof) for pointing this out in commonmark/cmark#300.

- At the time of this writing, "-rdynamic" is only added when using CMake on Linux, so the conditional expression used for patching cmark's build system has been updated to specifically check for Linux. Thanks to Nick Wellnhofer (nwellnhof) for pointing this out in commonmark/cmark#300.

nwellnhof · 2019-05-28T11:02:40Z

I think you have to put the final ${CMAKE_SHARED_LIBRARY_LINK_C_FLAGS} in double quotes to avoid an error if the variable isn't defined.

jgm · 2019-05-28T15:23:06Z

I found this post which recommends simply

SET(CMAKE_SHARED_LIBRARY_LINK_C_FLAGS)

That should be safe for a static build, right? And then we don't need to worry about regexes.

ericpruitt · 2019-05-28T20:04:08Z

SET(CMAKE_SHARED_LIBRARY_LINK_C_FLAGS)

That works. I've updated pull request.

jgm · 2019-05-28T21:53:46Z

Should we also now remove the check for Linux? If I recall, it was only necessary because of the regex stuff.

ericpruitt · 2019-05-28T23:40:18Z

I would argue that it should be scoped to Linux to minimize the chances of collateral damage because that's the only platform where this problem is known to exist since, when I grepped for "rdynamic" in the CMake repositories, the only matches were in the Linux modules directory. However, I'll do whatever's requested to get this merged as long as it fixes the problem for me :P, so I've updated the pull request accordingly.

jgm · 2019-05-29T04:03:21Z

Your argument is reasonable. I don't feel strongly about it either way. If you want to revert the last change, that's fine, I'll merge it either way!

ericpruitt · 2019-05-29T04:16:01Z

If you want to revert the last change, that's fine, I'll merge it either way!

Thanks, done.

jgm · 2019-05-29T16:28:15Z

Thanks!

- Update cmark to pull in commonmark/cmark#300 which includes a fix to ensure "-rdynamic" is not used for statically linked builds.

Remove "-rdynamic" flag for static builds

35b7e29

ericpruitt added 2 commits May 27, 2019 14:51

Correct typo

1c7e0a6

Fix typo by changing "STATIC" to "CMARK_STATIC".

Fix condition; "LINUX" is not a valid variable

ac6eef0

LINUX is not a CMake platform variable, so the expression has been updated accordingly and the build functionality verified.

Use SET(CMAKE_SHARED_LIBRARY_LINK_C_FLAGS)

c8675a4

Remove check for Linux

c6f5457

Restore Linux check and add explanation

b64eefd

jgm merged commit f4895a6 into commonmark:master May 29, 2019

ericpruitt added a commit to ericpruitt/emus that referenced this pull request May 29, 2019

Sync with latest upstream changes to cmark(1)

916d226

- Update cmark to pull in commonmark/cmark#300 which includes a fix to ensure "-rdynamic" is not used for statically linked builds.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Remove "-rdynamic" flag for static builds #300

Remove "-rdynamic" flag for static builds #300

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Remove "-rdynamic" flag for static builds #300

Remove "-rdynamic" flag for static builds #300

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants