Treat linker warnings as errors on all POSIX platforms

Treat linker warnings as errors on all POSIX platforms

BUG=352985
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=257419

[jochen (gone - plz use gerrit), 2014-03-16 15:48:41]

Nico, wdyt?

[Nico, 2014-03-16 16:40:44]

Do you have any examples what this catches? If that's always useful and
actionable I suppose it's fine, but AFAIK the linker doesn't support the
granular warning control that the compiler supports
(-Wno-i-before-e-ecxept-after-cxx etc). Without this, fatal warnings might
be annoying or dangerous.
On Mar 16, 2014 3:48 PM, <jochen@chromium.org> wrote:

> Reviewers: Nico (on GMT time Mar 15 - 24),
>
> Message:
> Nico, wdyt?
>
>
>
> Description:
> Treat linker warnings as errors on all POSIX platforms
>
> BUG=352985
>
> Please review this at https://codereview.chromium.org/196943020/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files (+1, -2 lines):
>   M build/common.gypi
>
>
> Index: build/common.gypi
> diff --git a/build/common.gypi b/build/common.gypi
> index 72eb96d94c07528ee98e90e0bdf56958f8bef7b5..
> bfc62e514006fd9c55b6668bbed8ef8474bb597e 100644
> --- a/build/common.gypi
> +++ b/build/common.gypi
> @@ -2972,6 +2972,7 @@
>      ['os_posix==1', {
>        'target_defaults': {
>          'ldflags': [
> +          '-Wl,--fatal-warnings',
>            '-Wl,-z,now',
>            '-Wl,-z,relro',
>          ],
> @@ -3049,7 +3050,6 @@
>              'conditions' : [
>                ['OS=="android"', {
>                  'ldflags': [
> -                  '-Wl,--fatal-warnings',
>                    # Only link with needed input sections. This is to avoid
>                    # getting undefined reference to __cxa_bad_typeid in
> the CDU
>                    # library.
> @@ -3131,7 +3131,6 @@
>                    '-fomit-frame-pointer',
>                  ],
>                  'ldflags': [
> -                  '-Wl,--fatal-warnings',
>                    # Warn in case of text relocations.
>                    '-Wl,--warn-shared-textrel',
>                  ],
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jochen (gone - plz use gerrit), 2014-03-16 16:45:08]

e.g. currently it catches this:
/mnt/scratch0/b_used/build/slave/linux_clang/build/src/third_party/gold/gold64:
warning: hidden symbol 'UnixDomainSocket::RecvMsg(int, void*, unsigned long,
std::__debug::vector<int, std::allocator<int> >*)' in
obj/base/posix/nacl_helper.unix_domain_socket_linux.o is referenced by DSO
lib/libcontent.so

which is the result of an incorrect dependency introduced here:
https://codereview.chromium.org/189713002

This doesn't really happen that often, because the flag is already on by default
on Android which covers large parts of our dependency mess in gyp

[Nico, 2014-03-16 16:56:14]

lgtm, we'll see how it goes I suppose. Making that warning an error certainly
makes sense.

https://codereview.chromium.org/196943020/diff/1/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/196943020/diff/1/build/common.gypi#newcode2975
build/common.gypi:2975: '-Wl,--fatal-warnings',
Should this only be done if werror% isn't empty?

[jochen (gone - plz use gerrit), 2014-03-16 17:01:43]

On 2014/03/16 16:56:14, Nico (on GMT time Mar 15 - 24) wrote:
> lgtm, we'll see how it goes I suppose. Making that warning an error certainly
> makes sense.
> 
> https://codereview.chromium.org/196943020/diff/1/build/common.gypi
> File build/common.gypi (right):
> 
> https://codereview.chromium.org/196943020/diff/1/build/common.gypi#newcode2975
> build/common.gypi:2975: '-Wl,--fatal-warnings',
> Should this only be done if werror% isn't empty?

The builders that don't want this (ASAN) already explicitly disable the flag. I
think that's preferable over mangling a compiler with a linker setting

[Nico, 2014-03-16 17:17:10]

ok


On Sun, Mar 16, 2014 at 5:01 PM, <jochen@chromium.org> wrote:

> On 2014/03/16 16:56:14, Nico (on GMT time Mar 15 - 24) wrote:
>
>> lgtm, we'll see how it goes I suppose. Making that warning an error
>> certainly
>> makes sense.
>>
>
>  https://codereview.chromium.org/196943020/diff/1/build/common.gypi
>> File build/common.gypi (right):
>>
>
>  https://codereview.chromium.org/196943020/diff/1/build/
>> common.gypi#newcode2975
>> build/common.gypi:2975: '-Wl,--fatal-warnings',
>> Should this only be done if werror% isn't empty?
>>
>
> The builders that don't want this (ASAN) already explicitly disable the
> flag. I
> think that's preferable over mangling a compiler with a linker setting
>
> https://codereview.chromium.org/196943020/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jochen (gone - plz use gerrit), 2014-03-17 12:14:49]

The CQ bit was checked by jochen@chromium.org

[commit-bot: I haz the power, 2014-03-17 12:14:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jochen@chromium.org/196943020/1

[commit-bot: I haz the power, 2014-03-17 12:15:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-17 12:15:54]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_clang_dbg

[jochen (gone - plz use gerrit), 2014-03-17 12:17:15]

The CQ bit was checked by jochen@chromium.org

[commit-bot: I haz the power, 2014-03-17 12:17:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jochen@chromium.org/196943020/1

[commit-bot: I haz the power, 2014-03-17 13:42:15]

Change committed as 257419