Adjust CMAKE_MODULE_LINKER_FLAGS for asan

[Michael Catanzaro]

As suggested by Konstantin, it's good form to set CMAKE_MODULE_LINKER_FLAGS for when enabling asan.

[Michael Catanzaro]

Created attachment 347362 [details]
Patch

[Michael Catanzaro]

Comment on attachment 347362 [details]
Patch

Hm, maybe should also set CMAKE_STATIC_LINKER_FLAGS

[Konstantin Tokarev]

Comment on attachment 347362 [details]
Patch

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

> Source/cmake/WebKitCompilerFlags.cmake:177
> +        set(CMAKE_MODULE_LINKER_FLAGS "-lpthread ${CMAKE_MODULE_LINKER_FLAGS} -fsanitize=address")

It would be better to avoid duplication of flags, e.g.

  set(CMAKE_EXE_LINKER_FLAGS "-lpthread ${CMAKE_EXE_LINKER_FLAGS} -fsanitize=address")
  set(CMAKE_SHARED_LINKER_FLAGS ${CMAKE_EXE_LINKER_FLAGS})
  set(CMAKE_MODULE_LINKER_FLAGS ${CMAKE_EXE_LINKER_FLAGS})

[Konstantin Tokarev]

(In reply to Michael Catanzaro from comment #2)
> Hm, maybe should also set CMAKE_STATIC_LINKER_FLAGS

No, it's not needed

[Michael Catanzaro]

(In reply to Konstantin Tokarev from comment #3)
> It would be better to avoid duplication of flags, e.g.
> 
>   set(CMAKE_EXE_LINKER_FLAGS "-lpthread ${CMAKE_EXE_LINKER_FLAGS}
> -fsanitize=address")
>   set(CMAKE_SHARED_LINKER_FLAGS ${CMAKE_EXE_LINKER_FLAGS})
>   set(CMAKE_MODULE_LINKER_FLAGS ${CMAKE_EXE_LINKER_FLAGS})

Well that's no good, what if the user wants to set different flags?

I checked what ECMEnableSanitizers is doing and it boils down to link_libraries("asan")

[Konstantin Tokarev]

(In reply to Michael Catanzaro from comment #5)
> (In reply to Konstantin Tokarev from comment #3)
> > It would be better to avoid duplication of flags, e.g.
> > 
> >   set(CMAKE_EXE_LINKER_FLAGS "-lpthread ${CMAKE_EXE_LINKER_FLAGS}
> > -fsanitize=address")
> >   set(CMAKE_SHARED_LINKER_FLAGS ${CMAKE_EXE_LINKER_FLAGS})
> >   set(CMAKE_MODULE_LINKER_FLAGS ${CMAKE_EXE_LINKER_FLAGS})
> 
> Well that's no good, what if the user wants to set different flags?

Oops. Just wanted to get away without additional variable, but it was surely wrong

[Konstantin Tokarev]

(In reply to Michael Catanzaro from comment #5)
> I checked what ECMEnableSanitizers is doing and it boils down to
> link_libraries("asan")

Yep, it should be fine, at least ECMEnableSanitizers works fine with QtWebKit

[Michael Catanzaro]

Comment on attachment 347362 [details]
Patch

Bringing back this original patch... I was hoping to fix asan under flatpak here, but it's too much work for this bug.

[WebKit Commit Bot]

Comment on attachment 347362 [details]
Patch

Clearing flags on attachment: 347362

Committed r235010: <https://trac.webkit.org/changeset/235010>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/43461813>