Bug 174490 – [CMake] Properly test if compiler supports compiler flags

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Michael Catanzaro]

Do not use -Wno-expansion-to-defined unless GCC actually supports it. It should only be used for GCC 7 and newer.

[Michael Catanzaro]

Created attachment 315397 [details]
Patch

[Adrian Perez]

Comment on attachment 315397 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=315397&action=review

Informal review: r-, please check the comments.

> Source/cmake/OptionsCommon.cmake:74
> +        NOT (COMPILER_IS_GNUCXX AND ${CMAKE_CXX_COMPILER_VERSION} VERSION_LESS 7.0))

Are you sure that only GCC 7+ supports this flag? Do you have any reference
(for example to GCC release notes) which justify using 7.0 for comparison?

I have just tried the following with a GCC 6:

 % ./output/host/bin/arm-buildroot-linux-gnueabihf-gcc -dumpversion
 6.4.0

 % ./output/host/bin/arm-buildroot-linux-gnueabihf-gcc \
     -Wno-expansion-to-defined -E -x c++ /dev/null > /dev/null

There command does not print anything to stderr, which means it is happily
accepting “-Wno-expansion-to-defined”.

[Konstantin Tokarev]

It seems like it's safe to pass unknown -Wno-* options to GCC:

    "When an unrecognized warning option is requested (e.g., -Wunknown-warning), GCC emits a diagnostic stating that the option is not recognized. However, if the -Wno- form is used, the behavior is slightly different: no diagnostic is produced for -Wno-unknown-warning unless other diagnostics are being produced.  This allows the use of new -Wno- options with old compilers, but if something goes wrong, the compiler warns that an unrecognized option is present."

[Michael Catanzaro]

(In reply to Konstantin Tokarev from comment #3)
> It seems like it's safe to pass unknown -Wno-* options to GCC:

Yeah, it doesn't hurt anything, I'm just trying to shut up that warning from the build logs on our bots since it's annoying. E.g. https://bugs.webkit.org/show_bug.cgi?id=174463#c11.

(In reply to Adrian Perez from comment #2)
> There command does not print anything to stderr, which means it is happily
> accepting “-Wno-expansion-to-defined”.

You're sure it doesn't print anything to stderr? It does on our bots, otherwise I would never have noticed this.

It's indeed new in GCC 7: https://gcc.gnu.org/gcc-7/changes.html

[Konstantin Tokarev]

>I'm just trying to shut up that warning from the build logs on our bots since it's annoying.

It prints to logs only when there are other warnings in the same file. Did you read the quote above?

[Michael Catanzaro]

(In reply to Konstantin Tokarev from comment #5)
> >I'm just trying to shut up that warning from the build logs on our bots since it's annoying.
> 
> It prints to logs only when there are other warnings in the same file. Did
> you read the quote above?

Ah, no. I did not. Reading is good. :)

Anyway, I still don't see why we should not ensure the flag is valid before using it. I'm going to look into using proper compiler flag checks, but I think we should still do this in the meantime.

[Konstantin Tokarev]

>I'm going to look into using proper compiler flag checks, but I think we should still do this in the meantime.

It's not that hard actually, and I would like it more than this monstrous condition that checks versions of two compilers. Example: https://bugs.webkit.org/attachment.cgi?id=314562&action=prettypatch

[Adrian Perez]

(In reply to Konstantin Tokarev from comment #7)
> >I'm going to look into using proper compiler flag checks, but I think we should still do this in the meantime.
> 
> It's not that hard actually, and I would like it more than this monstrous
> condition that checks versions of two compilers. Example:
> https://bugs.webkit.org/attachment.cgi?id=314562&action=prettypatch

This looks indeed much better! Provided that GCC will silently ignore
the “-Wno-foo” flags, the correct thing to do when we want to use one
of those would be to check that “-Wfoo” is supported, and only in that
case we would add “-Wno-foo” to the compiler flags.

[Michael Catanzaro]

(In reply to Konstantin Tokarev from comment #7)
> >I'm going to look into using proper compiler flag checks, but I think we should still do this in the meantime.
> 
> It's not that hard actually, and I would like it more than this monstrous
> condition that checks versions of two compilers. Example:
> https://bugs.webkit.org/attachment.cgi?id=314562&action=prettypatch

OK. I'll create a new bug for this.

[Michael Catanzaro]

Let's reuse this one.

This turned out to be a massive pain. I didn't want to merely check options before using them: I also wanted to organize the code to avoid setting similar flags in different places. Right now we set a bunch of global flags in OptionsCommon.cmake, and a bunch more flags in WEBKIT_SET_EXTRA_COMPILER_FLAGS on a per-target basis.

Setting flags per-target seems better in general, e.g. because it makes it very easy to disable warnings for particular ThirdParty targets. But it turns out that all the flags set on a per-target basis get passed to both the C compiler and the C++ compiler, so it's impossible to pass C++-only flags there. That's terrible. It's possible to make the flags language-conditional using generator expressions, but that doesn't work for the Visual Studio backend, so we would have to drop support for that (not going to happen). The CMake documentation suggests that C and C++ files ought to be built in separate targets to avoid this. It's a mess, basically.

So I've wound up gutting WEBKIT_SET_EXTRA_COMPILER_FLAGS and adding those flags to CMAKE_C_FLAGS and CMAKE_CXX_FLAGS instead. Really the only disadvantage of this is we now have to suppress individual warnings when building ANGLESupport in WebCore. That's not the end of the world.

[Michael Catanzaro]

Next step is to remove WEBKIT_SET_EXTRA_COMPILER_FLAGS.

[Michael Catanzaro]

Created attachment 315641 [details]
Patch

[Michael Catanzaro]

This was pretty hard. This is the best approach I could come up with. Thoughts?

I think we can now replace WEBKIT_SET_EXTRA_COMPILER_FLAGS by just turning on position independent code project-wide (I assume all ports probably want that?), but I've saved this for a follow-up.

[Michael Catanzaro]

(This will probably add a ton of warnings for all ports except GTK. They will just need to be suppressed.)

[Build Bot]

Comment on attachment 315641 [details]
Patch

Attachment 315641 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4134213

New failing tests:
http/tests/css/border-image-loading.html

[Build Bot]

Created attachment 315647 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Michael Catanzaro]

Konstantin, Alex, are you OK with this approach?

[Konstantin Tokarev]

Comment on attachment 315641 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=315641&action=review

> Source/PlatformGTK.cmake:27
> +        COMMAND CC=${CMAKE_C_COMPILER} CFLAGS=${CMAKE_C_FLAGS}\\ -Wno-unused-parameter ${CMAKE_SOURCE_DIR}/Tools/gtk/generate-gtkdoc ${_extra_args}

"${CMAKE_C_FLAGS} -Wno-unused-parameter"

Escaped scpace is highly confusing, IMO

> Source/cmake/WebKitCommon.cmake:41
> +    include(WebKitCompilerFlags)

Alphabetic order broken

> Source/cmake/WebKitCompilerFlags.cmake:118
> +            add_definitions(-D{CMAKE_CXX_FLAGS} -mno-ms-bitfields -Wno-unknown-pragmas)

This line needs to be removed, it accidentally crept in r218900

[Konstantin Tokarev]

Comment on attachment 315641 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=315641&action=review

> Source/cmake/OptionsCommon.cmake:39
> +    WEBKIT_PREPEND_COMPILER_FLAG_IF_SUPPORTED(-mfix-cortex-a53-835769)

WEBKIT_PREPEND_GLOBAL_COMPILER_FLAG?

[Michael Catanzaro]

(In reply to Konstantin Tokarev from comment #18)
> "${CMAKE_C_FLAGS} -Wno-unused-parameter"
> 
> Escaped scpace is highly confusing, IMO

I'll see if that works. I'd like to think I tried that first.

> > Source/cmake/WebKitCommon.cmake:41
> > +    include(WebKitCompilerFlags)
> 
> Alphabetic order broken

OK, I see I can reorder them now. At one point the order was significant because WebKitCompilerFlags needed to use macros from WebKitMacros, but I replaced use of WEBKIT_ADD_TARGET_PROPERTIES with target_compile_options, so that's not needed anymore.

> > Source/cmake/WebKitCompilerFlags.cmake:118
> > +            add_definitions(-D{CMAKE_CXX_FLAGS} -mno-ms-bitfields -Wno-unknown-pragmas)
> 
> This line needs to be removed, it accidentally crept in r218900

Yay!

(In reply to Konstantin Tokarev from comment #19)
> > Source/cmake/OptionsCommon.cmake:39
> > +    WEBKIT_PREPEND_COMPILER_FLAG_IF_SUPPORTED(-mfix-cortex-a53-835769)
> 
> WEBKIT_PREPEND_GLOBAL_COMPILER_FLAG?

Good catch.

[Konstantin Tokarev]

Comment on attachment 315641 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=315641&action=review

> Source/PlatformGTK.cmake:29
>          WORKING_DIRECTORY "${CMAKE_BINARY_DIR}"

Oh, VERBATIM is missing. That's why you had problem with space.

[Konstantin Tokarev]

Comment on attachment 315641 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=315641&action=review

> Source/cmake/WebKitCompilerFlags.cmake:56
> +endmacro()

macro(WEBKIT_APPEPEND_GLOBAL_COMPILER_FLAGS)
  foreach (_flag ${ARGN})
     ...

> Source/cmake/WebKitCompilerFlags.cmake:131
> +    WEBKIT_PREPEND_GLOBAL_COMPILER_FLAG(-Wwrite-strings)

It may be a good idea to change WEBKIT_PREPEND_GLOBAL_COMPILER_FLAG to accept list, or make separate macro, so that it shouldn't be repeated again for every flag

[Michael Catanzaro]

(In reply to Konstantin Tokarev from comment #22)
> > Source/cmake/WebKitCompilerFlags.cmake:56
> > +endmacro()
> 
> macro(WEBKIT_APPEPEND_GLOBAL_COMPILER_FLAGS)
>   foreach (_flag ${ARGN})
>      ...

Are you saying I should move the code that sets the compiler flags into a macro, and just call it exactly once? I'm not sure there would be much value to doing that?

> > Source/cmake/WebKitCompilerFlags.cmake:131
> > +    WEBKIT_PREPEND_GLOBAL_COMPILER_FLAG(-Wwrite-strings)
> 
> It may be a good idea to change WEBKIT_PREPEND_GLOBAL_COMPILER_FLAG to
> accept list, or make separate macro, so that it shouldn't be repeated again
> for every flag

I considered doing that, and I'll try that if you tell me to. But I decided I kind of like putting each flag on its own line, since it's easier to read a long list of flags (no horizontal scrolling).

[Konstantin Tokarev]

>it's easier to read a long list of flags (no horizontal scrolling).

Well, this works too and looks less cluttered:

 WEBKIT_APPEND_GLOBAL_COMPILER_FLAGS(
   -Wall
   -Wextra
   -Wcast-align
...

[Konstantin Tokarev]

Comment on attachment 315641 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=315641&action=review

>> Source/cmake/WebKitCompilerFlags.cmake:56
>> +endmacro()
> 
> macro(WEBKIT_APPEPEND_GLOBAL_COMPILER_FLAGS)
>   foreach (_flag ${ARGN})
>      ...

It was just illustration how WEBKIT_APPEND_GLOBAL_COMPILER_FLAG could be turned into WEBKIT_APPEND_GLOBAL_COMPILER_FLAGS where ... is old body

[Michael Catanzaro]

(In reply to Konstantin Tokarev from comment #24)
> >it's easier to read a long list of flags (no horizontal scrolling).
> 
> Well, this works too and looks less cluttered:
> 
>  WEBKIT_APPEND_GLOBAL_COMPILER_FLAGS(
>    -Wall
>    -Wextra
>    -Wcast-align
> ...

OK, I agree.

[Michael Catanzaro]

OK Konstantin, sorry for the delay. Please review take three. I added a macro that takes a list of arguments (not a CMake list ;) for the global flags helpers, but not for the target helpers since it's bad to pass a long list of target-specific flags.

[Michael Catanzaro]

Sorry, I'm too trigger-happy. Let me finish a test build before I upload.

[Michael Catanzaro]

(In reply to Konstantin Tokarev from comment #21)
> Comment on attachment 315641 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=315641&action=review
> 
> > Source/PlatformGTK.cmake:29
> >          WORKING_DIRECTORY "${CMAKE_BINARY_DIR}"
> 
> Oh, VERBATIM is missing. That's why you had problem with space.

The double backslash is still the best I could come up with. CMAKE_C_FLAGS is surrounded with double quotes, and the only way to avoid that is to make a new list and append -Wno-unused-parameter to it. Of course that's possible, but it's several extra lines of code relative to just adding a couple of backslashes.

[Michael Catanzaro]

Created attachment 316851 [details]
Patch

[Michael Catanzaro]

No cq? because I am thinking I should add an argument list version of the target compiler flags commands as well. Perhaps.

[Michael Catanzaro]

Comment on attachment 316851 [details]
Patch

Seems I accidentally deleted the new file.

[Darin Adler]

Comment on attachment 316851 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=316851&action=review

> Source/JavaScriptCore/b3/testb3.cpp:8484
> +    CHECK(compileAndRun<int>(proc) == static_cast<int>((things.size() * (things.size() - 1)) / 2));

I think the cast should go the other way. Cast the left side to size_t rather than casting the right side to int. Narrowing could hide a difference, while widening will make sure we correctly compare.

> Source/PlatformGTK.cmake:27
> +        COMMAND CC=${CMAKE_C_COMPILER} CFLAGS=${CMAKE_C_FLAGS}\\ -Wno-unused-parameter ${CMAKE_SOURCE_DIR}/Tools/gtk/generate-gtkdoc ${_extra_args}

Why are the two backslashes needed?

[Michael Catanzaro]

Sorry, still need to recover WebKitCompilerFlags.cmake. Pretty critical missing file there.

(In reply to Darin Adler from comment #33)
> I think the cast should go the other way. Cast the left side to size_t
> rather than casting the right side to int. Narrowing could hide a
> difference, while widening will make sure we correctly compare.

OK!

> > Source/PlatformGTK.cmake:27
> > +        COMMAND CC=${CMAKE_C_COMPILER} CFLAGS=${CMAKE_C_FLAGS}\\ -Wno-unused-parameter ${CMAKE_SOURCE_DIR}/Tools/gtk/generate-gtkdoc ${_extra_args}
> 
> Why are the two backslashes needed?

The first backslash escapes the second, so only one actually gets emitted in the shell command. And that one backslash is needed for the following space. It's not nice, but I had trouble coming up with something better.

[Konstantin Tokarev]

Comment on attachment 316851 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=316851&action=review

>> Source/PlatformGTK.cmake:27
>> +        COMMAND CC=${CMAKE_C_COMPILER} CFLAGS=${CMAKE_C_FLAGS}\\ -Wno-unused-parameter ${CMAKE_SOURCE_DIR}/Tools/gtk/generate-gtkdoc ${_extra_args}
> 
> Why are the two backslashes needed?

Because add_custom_command is used without VERBATIM (it shouldn't be)

[Michael Catanzaro]

I could not find any way to make it work when I added VERBATIM. Perhaps I was missing something obvious.

[Konstantin Tokarev]

(In reply to Michael Catanzaro from comment #36)
> I could not find any way to make it work when I added VERBATIM. Perhaps I
> was missing something obvious.

Try this with VERBATIM

COMMAND ${CMAKE_COMMAND} -E env "CC=${CMAKE_C_COMPILER} CFLAGS=${CMAKE_C_FLAGS}" ${CMAKE_SOURCE_DIR}/Tools/gtk/generate-gtkdoc ${_extra_args}

I'm strongly opposed to use of any custom commands without VERBATIM as it is potentialy non-portable between different CMake generators

[Konstantin Tokarev]

(In reply to Konstantin Tokarev from comment #37)
> (In reply to Michael Catanzaro from comment #36)
> > I could not find any way to make it work when I added VERBATIM. Perhaps I
> > was missing something obvious.
> 
> Try this with VERBATIM
> 
> COMMAND ${CMAKE_COMMAND} -E env "CC=${CMAKE_C_COMPILER}
> CFLAGS=${CMAKE_C_FLAGS}" ${CMAKE_SOURCE_DIR}/Tools/gtk/generate-gtkdoc
> ${_extra_args}
> 
> I'm strongly opposed to use of any custom commands without VERBATIM as it is
> potentialy non-portable between different CMake generators

Sorry, I've meant

COMMAND ${CMAKE_COMMAND} -E env "CC=${CMAKE_C_COMPILER}" "CFLAGS=${CMAKE_C_FLAGS} -Wno-unused-parameter" ${CMAKE_SOURCE_DIR}/Tools/gtk/generate-gtkdoc ${_extra_args}

[Darin Adler]

Comment on attachment 316851 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=316851&action=review

>>> Source/PlatformGTK.cmake:27
>>> +        COMMAND CC=${CMAKE_C_COMPILER} CFLAGS=${CMAKE_C_FLAGS}\\ -Wno-unused-parameter ${CMAKE_SOURCE_DIR}/Tools/gtk/generate-gtkdoc ${_extra_args}
>> 
>> Why are the two backslashes needed?
> 
> Because add_custom_command is used without VERBATIM (it shouldn't be)

I understand now. Originally I didn’t get that -Wno-unused-parameter is supposed to be part of CFLAGS, which now seem obvious.

[Michael Catanzaro]

(In reply to Konstantin Tokarev from comment #38)
> Sorry, I've meant
> 
> COMMAND ${CMAKE_COMMAND} -E env "CC=${CMAKE_C_COMPILER}"
> "CFLAGS=${CMAKE_C_FLAGS} -Wno-unused-parameter"
> ${CMAKE_SOURCE_DIR}/Tools/gtk/generate-gtkdoc ${_extra_args}

Sorry for the delay. That works.

[Michael Catanzaro]

Created attachment 317430 [details]
Patch

[Michael Catanzaro]

Created attachment 317431 [details]
Patch

[Michael Catanzaro]

Created attachment 317433 [details]
Patch

[Michael Catanzaro]

I missed a couple places that need to use the new macros. Need to fix these:

Source/WebCore/CMakeLists.txt:    WEBKIT_ADD_TARGET_PROPERTIES(WebCore COMPILE_FLAGS "-fno-tree-sra")
Source/WebCore/CMakeLists.txt:        WEBKIT_ADD_TARGET_PROPERTIES(ANGLESupport COMPILE_FLAGS "-Wno-null-conversion")
Tools/DumpRenderTree/TestNetscapePlugIn/CMakeLists.txt:WEBKIT_ADD_TARGET_PROPERTIES(TestNetscapePlugIn COMPILE_FLAGS "-Wno-unused-parameter")

[Michael Catanzaro]

Also need to fix these:

[5395/5654] Building CXX object Source...bDriver.dir/glib/SessionHostGlib.cpp.o
../../Source/WebDriver/glib/SessionHostGlib.cpp: In lambda function:
../../Source/WebDriver/glib/SessionHostGlib.cpp:65:25: warning: unused parameter ‘connection’ [-Wunused-parameter]
     [](GDBusConnection* connection, const gchar* sender, const gchar* objectPath, const gchar* interfaceName, const gchar* methodName, GVariant* parameters, GDBusMethodInvocation* invocation, gpointer userData) {
                         ^~~~~~~~~~
../../Source/WebDriver/glib/SessionHostGlib.cpp:65:50: warning: unused parameter ‘sender’ [-Wunused-parameter]
     [](GDBusConnection* connection, const gchar* sender, const gchar* objectPath, const gchar* interfaceName, const gchar* methodName, GVariant* parameters, GDBusMethodInvocation* invocation, gpointer userData) {
                                                  ^~~~~~
../../Source/WebDriver/glib/SessionHostGlib.cpp:65:71: warning: unused parameter ‘objectPath’ [-Wunused-parameter]
     [](GDBusConnection* connection, const gchar* sender, const gchar* objectPath, const gchar* interfaceName, const gchar* methodName, GVariant* parameters, GDBusMethodInvocation* invocation, gpointer userData) {
                                                                       ^~~~~~~~~~
../../Source/WebDriver/glib/SessionHostGlib.cpp:65:96: warning: unused parameter ‘interfaceName’ [-Wunused-parameter]
     [](GDBusConnection* connection, const gchar* sender, const gchar* objectPath, const gchar* interfaceName, const gchar* methodName, GVariant* parameters, GDBusMethodInvocation* invocation, gpointer userData) {
                                                                                                ^~~~~~~~~~~~~
../../Source/WebDriver/glib/SessionHostGlib.cpp: At global scope:
../../Source/WebDriver/glib/SessionHostGlib.cpp:97:1: warning: missing initializer for member ‘_GDBusInterfaceVTable::padding’ [-Wmissing-field-initializers]
 };
 ^

[Michael Catanzaro]

*** Bug 175020 has been marked as a duplicate of this bug. ***

[Michael Catanzaro]

Created attachment 317448 [details]
Patch

[Michael Catanzaro]

Created attachment 317449 [details]
Patch

[Michael Catanzaro]

Konstantin, want to give this r+ again? The only significant change is that I've remove WEBKIT_SET_EXTRA_COMPILER_FLAGS. It's no longer needed if we just set the CMAKE_POSITION_INDEPENDENT_CODE variable.

[Michael Catanzaro]

Comment on attachment 317449 [details]
Patch

And the iOS EWS failure is unrelated.

[Konstantin Tokarev]

I wish it was possible to see diff between patch versions, like in Gerrit

[Michael Catanzaro]

(In reply to Konstantin Tokarev from comment #51)
> I wish it was possible to see diff between patch versions, like in Gerrit

The only changes are:

 * Fix warnings in SessionHostGlib.cpp
 * Remove WEBKIT_SET_EXTRA_COMPILER_FLAGS and every call to it
 * Set CMAKE_POSITION_INDEPENDENT_CODE in toplevel CMakeLists.txt, except on Windows

[Konstantin Tokarev]

Comment on attachment 317449 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=317449&action=review

> CMakeLists.txt:129
> +if (NOT WIN32)

No need to exclude Windows, cmake will handle it correctly

> CMakeLists.txt:130
> +    set(CMAKE_POSITION_INDEPENDENT_CODE True)

This also makes all exectuables to be compiled with -fPIE

[Konstantin Tokarev]

Comment on attachment 317449 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=317449&action=review

> ChangeLog:33
> +        build target exclusively contains targets built with just one of those compilers. Yuck.

If this is the case, we shouldn't really have separate WEBKIT_ADD_TARGET_C_FLAGS and WEBKIT_ADD_TARGET_CXX_FLAGS macros, but replace them tih WEBKIT_ADD_TARGET_FLAGS that checks option with both compilers

[Michael Catanzaro]

(In reply to Konstantin Tokarev from comment #53)
> > CMakeLists.txt:130
> > +    set(CMAKE_POSITION_INDEPENDENT_CODE True)
> 
> This also makes all exectuables to be compiled with -fPIE

That wasn't intended, but it seems desirable anyway as it's a better default, right?

[Michael Catanzaro]

(In reply to Konstantin Tokarev from comment #54)
> If this is the case, we shouldn't really have separate
> WEBKIT_ADD_TARGET_C_FLAGS and WEBKIT_ADD_TARGET_CXX_FLAGS macros, but
> replace them tih WEBKIT_ADD_TARGET_FLAGS that checks option with both
> compilers

I wish, but it's not possible unfortunately. Well, for CFLAGS it would be fine, but for CXXFLAGS we often need to pass flags that are not going to pass the C compiler check. Currently in all such cases, the target is composed exclusively of C++ source files, so there is no problem.

[Konstantin Tokarev]

(In reply to Michael Catanzaro from comment #55)
> (In reply to Konstantin Tokarev from comment #53)
> > > CMakeLists.txt:130
> > > +    set(CMAKE_POSITION_INDEPENDENT_CODE True)
> > 
> > This also makes all exectuables to be compiled with -fPIE
> 
> That wasn't intended, but it seems desirable anyway as it's a better
> default, right?

Right, for WK2 processes this may count as hardening measure if ASLR is working.

For other kinds of executables (like unit tests) it is not really needed, but is harmless

[Michael Catanzaro]

Committed r220403: <http://trac.webkit.org/changeset/220403>

[Carlos Alberto Lopez Perez]

(In reply to Michael Catanzaro from comment #58)
> Committed r220403: <http://trac.webkit.org/changeset/220403>

This broke the build with GCC 5.4 (Ubuntu 16.04)

https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20Ubuntu%20LTS%20%28Build%29/builds/4351/steps/compile-webkit/logs/stdio

[Michael Catanzaro]

What a useful bot! I will try to fix it.

[Michael Catanzaro]

Committed r220425: <http://trac.webkit.org/changeset/220425>