Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Underscore prefix detection broken if -g specified #5482

Closed
ePirat opened this issue Jun 12, 2019 · 18 comments · Fixed by #9989
Closed

Underscore prefix detection broken if -g specified #5482

ePirat opened this issue Jun 12, 2019 · 18 comments · Fixed by #9989

Comments

@ePirat
Copy link
Contributor

ePirat commented Jun 12, 2019

This is not an issue currently, due to #5481 hiding this bug but once that is solved, it will become an issue again.

If the cflags contain -g, the test for the underscore symbol would incorrectly return as soon as it finds the symbol with or without underscore, but when debug symbols are enabled using -g, this will yield the false result. This can easily be illustrated by a small testcase:

symbol_name = b'meson_uscore_prefix'
with open('out.obj', 'rb') as o:
    for line in o:
        # Check if the underscore form of the symbol is somewhere
        # in the output file.
        if b'_' + symbol_name in line:
            print('True')
        # Else, check if the non-underscored form is present
        elif symbol_name in line:
            print('False')

Would print:

False
True

as it first finds the symbol without the underscore, only later finds the symbol with underscore.

Of course this is not an issue when using the meson buildtype option, as meson will not use the -g argument from that for the test, but is an issue when -g is contained in externally provided flags by env variables or crossfile.

vlc-mirrorer pushed a commit to videolan/vlc that referenced this issue Jun 21, 2019
This fixes the meson underscore prefix test, which misbehaves
when -g is passed, as it would detect the debug string without
underscore first and incorrectly report that no underscore
prefix for symbols is used.

Fixes build issues with dav1d, which relies on the underscore
prefix check.

See: mesonbuild/meson#5482
Signed-off-by: Hugo Beauzée-Luyssen <hugo@beauzee.fr>
vlc-mirrorer pushed a commit to videolan/vlc that referenced this issue Jun 21, 2019
This fixes the meson underscore prefix test, which misbehaves
when -g is passed, as it would detect the debug string without
underscore first and incorrectly report that no underscore
prefix for symbols is used.

Fixes build issues with dav1d, which relies on the underscore
prefix check.

See: mesonbuild/meson#5482
Signed-off-by: Hugo Beauzée-Luyssen <hugo@beauzee.fr>
@ePirat
Copy link
Contributor Author

ePirat commented Jul 4, 2019

@jpakkane Possible solutions for this:

  1. Search the whole file and if both the underscore and non-underscore variant are found, assume symbols have underscore prefix. (This might still produce wrong results in some cases)

  2. Have the compiler output assembly and check the assembly. (This is probably the most reliable we can do without using external tools like nm)

  3. Use nm on the resulting binary and parse the output of nm. (This is probably the slowest of all approaches, and most complex one as we would need to check for nm or equivalent tool and parse the output)

Personally I am in favor of the second option.

@jpakkane
Copy link
Member

jpakkane commented Jul 5, 2019

Or possibly filter out -g from feature test arguments?

@ePirat
Copy link
Contributor Author

ePirat commented Jul 5, 2019

@jpakkane That might work, although not sure whats the easiest way to do that? Especially if any compiler would use multiple args for that.

Is there a way to filter part fo a list with another list in order, like:

['foo', 'bar', 'baz', 'xyz', 'abc'] filter with ['baz', 'xyz'] to get ['foo', 'bar', 'abc']

@jpakkane
Copy link
Member

filtered = [x for x in original_list if x not in things_to_take_out]

@ePirat
Copy link
Contributor Author

ePirat commented Jul 10, 2019

That won't respect order though, so could lead to wrong, confusing, results:

>>> original_list = ['-x', 'foo', '-x', 'debug']
>>> filter_list = ['-x', 'debug']
>>> [x for x in original_list if x not in filter_list]
['foo']

vlc-mirrorer pushed a commit to videolan/vlc-3.0 that referenced this issue Mar 26, 2020
This fixes the meson underscore prefix test, which misbehaves
when -g is passed, as it would detect the debug string without
underscore first and incorrectly report that no underscore
prefix for symbols is used.

Fixes build issues with dav1d, which relies on the underscore
prefix check.

See: mesonbuild/meson#5482
Signed-off-by: Hugo Beauzée-Luyssen <hugo@beauzee.fr>
(cherry picked from commit 3426d7bcf98fee15c239ea2b3d815c613df82efe)
Signed-off-by: Hugo Beauzée-Luyssen <hugo@beauzee.fr>
@ePirat
Copy link
Contributor Author

ePirat commented Apr 23, 2020

@jpakkane So the filtering approach is not really that feasible given that there are other flags than -g that can enable debug symbols. So probably it needs a more complex check that is more reliable to debug symbols presence.

lrusak added a commit to lrusak/xbmc that referenced this issue Sep 4, 2020
@lrusak
Copy link

lrusak commented Sep 5, 2020

I just ran into this when updating from meson 0.51.0 and after 976c303 I had to strip -g from our cross file flags. Any updates to solving this?

lrusak added a commit to lrusak/xbmc that referenced this issue Sep 5, 2020
lrusak added a commit to lrusak/xbmc that referenced this issue Sep 6, 2020
@eli-schwartz
Copy link
Member

-g0 negates -g and handles ordering just fine. Can we know ahead of time which potential arguments need to be removed and can they all be negated?

@lrusak
Copy link

lrusak commented Sep 7, 2020

I've also had to drop -gdwarf-2 as that broke the build also

theirix added a commit to theirix/conan-center-index that referenced this issue Jan 31, 2021
Reproduced on debug builds when Conan passes additional '-g' flag to CFLAGS.
Meson fails to drop duplicate '-g' flags and the dav1d check
'cc.symbols_have_underscore_prefix' fails which leads to undefined
symbols while linking.

Seems like bug introduced after 0.51.0, upstream ref mesonbuild/meson#5482
Correct behaviour can be checked by locating string '%define PREFIX 1' in
the generated config.asm.
conan-center-bot pushed a commit to conan-io/conan-center-index that referenced this issue Feb 1, 2021
* Add dav1d/0.8.1

* Use dl, pthread

* Add bin to PATH

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>

* Add pkg_config anme

* Enable assembly option by default

* Bump

* Downgrade to meson 0.51.0 due to prefix detection bug

Reproduced on debug builds when Conan passes additional '-g' flag to CFLAGS.
Meson fails to drop duplicate '-g' flags and the dav1d check
'cc.symbols_have_underscore_prefix' fails which leads to undefined
symbols while linking.

Seems like bug introduced after 0.51.0, upstream ref mesonbuild/meson#5482
Correct behaviour can be checked by locating string '%define PREFIX 1' in
the generated config.asm.

* Fix topics

* Drop pdb from lib/

* Disable assembly for debug msvc

* Fix typo

Co-authored-by: Chris Mc <prince.chrismc@gmail.com>

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
Co-authored-by: Chris Mc <prince.chrismc@gmail.com>
@xry111
Copy link
Contributor

xry111 commented Mar 16, 2021

flto in CFLAGS also causes the problem. And @ePirat 's option 2 won't fix it.

@ePirat
Copy link
Contributor Author

ePirat commented Mar 16, 2021

@xry111 Why would 2. not work here?

@xry111
Copy link
Contributor

xry111 commented Mar 16, 2021

@ePirat with -flto the outputed "assembly" contains strings like .gnu.lto_foo.0 so _foo will be found.

nkichukov pushed a commit to nkichukov/xbmc that referenced this issue Apr 5, 2021
havardgraff pushed a commit to pexip/gst-plugins-good that referenced this issue Aug 26, 2021
This is due to a bug in meson where it will not detect properly
the compiler if the symbols need an undercore.
mesonbuild/meson#5482

Fixes #821

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/845>
wantehchang added a commit to wantehchang/libavif that referenced this issue Sep 20, 2021
The cc.symbols_have_underscore_prefix() test in dav1d/meson.build
returns the incorrect value (true) if -fprofile-instr-generate is
specified in the CFLAGS environment variable. This is apparently a bug
in Meson's symbols_have_underscore_prefix() function. See
mesonbuild/meson#5482. Work around this bug by
adding a special case for Linux in dav1d/meson.build.

Part 1 of the fix for https://crbug.com/oss-fuzz/38512. Part 2 of the
fix is to change oss-fuzz/projects/libavif/build.sh to run
"bash dav1d_oss_fuzz.sh" instead of "bash dav1d.cmd".
wantehchang added a commit to AOMediaCodec/libavif that referenced this issue Sep 20, 2021
The cc.symbols_have_underscore_prefix() test in dav1d/meson.build
returns the incorrect value (true) if -fprofile-instr-generate is
specified in the CFLAGS environment variable. This is apparently a bug
in Meson's symbols_have_underscore_prefix() function. See
mesonbuild/meson#5482. Work around this bug by
adding a special case for Linux in dav1d/meson.build.

Part 1 of the fix for https://crbug.com/oss-fuzz/38512. Part 2 of the
fix is to change oss-fuzz/projects/libavif/build.sh to run
"bash dav1d_oss_fuzz.sh" instead of "bash dav1d.cmd".
@wantehchang
Copy link

Similar to the problem with GCC's -flto flag reported by @xry111, symbols_have_underscore_prefix() also returns True incorrectly if Clang's -fprofile-instr-generate flag is in CFLAGS. I think this is because the object file generated by the compiler contains strings like __profc_foo.

The following commands reproduce a linker error caused by this bug when building the dav1d library with Clang's -fprofile-instr-generate flag in CFLAGS on Linux:

$ git clone -b 0.9.2 --depth 1 https://code.videolan.org/videolan/dav1d.git
$ cd dav1d
$ mkdir build
$ cd build
$ export CC=clang
$ export CFLAGS=-fprofile-instr-generate
$ meson ..
$ ninja

You will see a lot of "undefined reference" linker errors. The problem is that the following code in dav1d/meson.build malfunctions and incorrectly adds an underscore prefix to symbols defined in assembly language files:

if cc.symbols_have_underscore_prefix()
    cdata.set10('PREFIX', true)
    cdata_asm.set10('PREFIX', true)
endif

@jannau
Copy link
Contributor

jannau commented Sep 21, 2021

upstream dav1d will move away from meson's cc.symbols_have_underscore_prefix(). See https://code.videolan.org/videolan/dav1d/-/merge_requests/1296
We need it only for external asm. The systems with underscore symbol prefix are well known and have to be supported explicitly in the form of asm code anyway. Adding an additional condition to the OS/architecture list when asm is added for a new system should not be a problem. I don't expect that there will be new OS/architecture combinations which use underscore symbol prefixes.

@nirbheek
Copy link
Member

As per discussion on IRC / Matrix with @ePirat, underscore detection will be moved to a hardcoded list based on toolchain and platform, and we will move the current broken compile check to a unit test that compares the compile check against the hardcoded list.

We could use things like nm or manually parsing object files, but that is actually harder to maintain than this system. Especially if new object formats / OSes / toolchains need to be added.

@nirbheek
Copy link
Member

nirbheek commented Jan 29, 2022

@ePirat do you have a draft branch / PR to implement the hard-coded list of underscore + toolchain + platform combination? We hit this in gstreamer again recently: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/978

@ePirat
Copy link
Contributor Author

ePirat commented Jan 29, 2022

@nirbheek Doing that is really tricky given the amount of platforms meson already supports, but I found a much better solution, will submit the PR for this probably tomorrow evening.

@ePirat
Copy link
Contributor Author

ePirat commented Feb 11, 2022

@nirbheek Sorry for the delay, was busy investigating other issues first. Here is the PR: #9989

ePirat added a commit to ePirat/meson that referenced this issue Feb 11, 2022
ePirat added a commit to ePirat/meson that referenced this issue Feb 11, 2022
ePirat added a commit to ePirat/meson that referenced this issue Feb 11, 2022
ePirat added a commit to ePirat/meson that referenced this issue Feb 11, 2022
ePirat added a commit to ePirat/meson that referenced this issue Mar 30, 2022
ePirat added a commit to ePirat/meson that referenced this issue Mar 31, 2022
BtbN added a commit to BtbN/FFmpeg-Builds that referenced this issue Oct 31, 2022
22.04 has a buggy meson that breaks with -flto
See:

mesonbuild/meson#9989
mesonbuild/meson#5482
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 a pull request may close this issue.

8 participants