Skip to content

meson.build: disable the deprecated symbols test with LTO - #174

Merged
whot merged 1 commit into
linuxwacom:masterfrom
whot:wip/lto-disable-deprecated-tests
Nov 6, 2019
Merged

meson.build: disable the deprecated symbols test with LTO#174
whot merged 1 commit into
linuxwacom:masterfrom
whot:wip/lto-disable-deprecated-tests

Conversation

@whot

@whot whot commented Nov 4, 2019

Copy link
Copy Markdown
Member

If I understand this correctly:

  • our deprecated symbols are remapped into a specific library version
  • nothing in the test calls those symbols, so they're optimized away (they end
    up as local symbols)
  • our test uses asm to link to those functions but by then they're already
    gone

Rather than fixing this let's just disable the test, it doesn't provide us
with anything useful in the LTO condition anyway.

Fixes #152

If I understand this correctly:
- our deprecated symbols are remapped into a specific library version
- nothing in the test calls those symbols, so they're optimized away (they end
  up as local symbols)
- our test uses asm to link to those functions but by then they're already
  gone

Rather than fixing this let's just disable the test, it doesn't provide us
with anything useful in the LTO condition anyway.

Fixes linuxwacom#152

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
@whot
whot merged commit e5458ab into linuxwacom:master Nov 6, 2019
@kloczek

kloczek commented Nov 7, 2019

Copy link
Copy Markdown

This solution is kind of wrong. Why? Because assumes that someone will be using LTO by specify in meson options -D b_lto=enable.

In reality it is possible to use LTO with meson completely transparently by passing all necessary options in AR, NM, RANLIB, CFLAGS and LDFLAGS environment variables.

Other thing is that implementing LTO in this PR way assumes that someone will be using gcc. Build framework should not be assuming use any any type of the compiler. All what is necessary to do here is possible to pass over above env variables.

IMO it should be only meson option to disable those tests if there is no other way to fix that issue.

@whot

whot commented Nov 7, 2019

Copy link
Copy Markdown
Member Author

it's always possible to pass/modify/hack the build process with various scripts and environment variables. That's a moving target we cannot maintain though. Plus, if you control the environment to that level in your build process, you can also patch the code to change what's being built.

We already have an option to disable tests (-Dtests=false), adding another one just for one specific test to accommodate for a fairly specialised build method is not a good idea.

@kloczek

kloczek commented Nov 8, 2019

Copy link
Copy Markdown

Correct me if I'm wrong AFAIK -Dtests=false disables all tests or only those affected by LTO.

Still I'm not sure what is exact cause that that test usnit is failing with LTO. gcc/ld bug? Because if it is like this dicussing that issue here is a bit pointless.

@whot

whot commented Nov 10, 2019

Copy link
Copy Markdown
Member Author

Correct me if I'm wrong AFAIK -Dtests=false disables all tests or only those affected by LTO.

Correct, it disables everything.

Still I'm not sure what is exact cause that that test usnit is failing with LTO

Have a look at the source of the test. The test:

  • dlopen's libwacom.so
  • tries to dlsym the deprecated functions which should fail
  • then it calls the functions directly because we've got the asm statements for the right library version.

So it's a hacky test but it works. Whatever LTO does exactly, it seems to ignore the asm .symver statements, making those calls fail with linker errors. I don't know enough though to go further here though.

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.

1.0: libwacom is not LTO ready

2 participants