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

bad linkage of fmt #61

Closed
markNZed opened this issue Oct 16, 2024 · 36 comments
Closed

bad linkage of fmt #61

markNZed opened this issue Oct 16, 2024 · 36 comments
Labels
bug Something isn't working

Comments

@markNZed
Copy link

Hi, I was trying to build yosys-slang against yosys version 0.46 and ran into this:

% read_slang ../data/picorv32/picorv32.v --top picorv32 -D DEBUG

1. Executing SLANG frontend.
yosys: symbol lookup error: yosys-slang/build/slang.so: undefined symbol: _ZN3fmt3v116detail10vformat_toIcEEvRNS1_6bufferIT_EENS0_17basic_string_viewIS4_EENS1_12vformat_argsIS4_E4typeENS1_10locale_refE

Could you please confirm which version of yosys runs with yosys-slang ?

@povik
Copy link
Owner

povik commented Oct 16, 2024

Hi,

this looks like an issue with linking the fmt dependency of slang (something to fix in the Makefile). I will have a second look at it later. The plugin should work with Yosys v0.44 and later.

@povik
Copy link
Owner

povik commented Oct 16, 2024

What OS/distribution are you running, and do you have libfmt installed? I will see about replicating it.

@markNZed
Copy link
Author

Ubuntu 22.04 and I installed libfmt version 8.1.1+ds1-2 (using apt)

@povik
Copy link
Owner

povik commented Oct 16, 2024

What does your cmake log say? I think I am wrong on instructing people to install libfmt because for proper linkage the libfmt in use should be vendored by slang, I try to force that with -DCMAKE_DISABLE_FIND_PACKAGE_fmt=ON. This is similar to #43 but for fmt instead of boost

@markNZed
Copy link
Author

    CXX build/abort_helpers.o
    CXX build/async_pattern.o
    CXX build/blackboxes.o
    CXX build/builder.o
    CXX build/diag.o
    CXX build/initial_eval.o
    CXX build/slang_frontend.o
In file included from /usr/local/share/yosys/include/kernel/bitpattern.h:23,
                 from src/slang_frontend.cc:19:
src/slang_frontend.cc: In member function ‘Yosys::RTLIL::SigSpec slang_frontend::SignalEvalContext::apply_conversion(const slang::ast::ConversionExpression&, Yosys::RTLIL::SigSpec)’:
src/slang_frontend.cc:1646:30: warning: comparison of integer expressions of different signedness: ‘int’ and ‘slang::uint64_t’ {aka ‘long unsigned int’} [-Wsign-compare]
 1646 |         log_assert(op.size() == from.getBitstreamWidth());
      |                    ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/share/yosys/include/kernel/log.h:226:78: note: in definition of macro ‘log_assert’
  226 | #  define log_assert(_assert_expr_) YOSYS_NAMESPACE_PREFIX log_assert_worker(_assert_expr_, #_assert_expr_, __FILE__, __LINE__)
      |                                                                              ^~~~~~~~~~~~~
src/builder.cc: In member function ‘Yosys::RTLIL::SigSpec slang_frontend::RTLILBuilder::Biop(Yosys::RTLIL::IdString, slang_frontend::RTLILBuilder::SigSpec, slang_frontend::RTLILBuilder::SigSpec, bool, bool, int)’:
src/builder.cc:306:46: warning: ‘bl’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  306 |                 int result = ThreeValued::XOR(carry, ThreeValued::XNOR(al, bl));
      |                              ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/builder.cc:306:46: warning: ‘al’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   LINK build/slang.so

@povik
Copy link
Owner

povik commented Oct 16, 2024

I was wondering about the output from cmake configuring the embedded slang build, you can see that if you run make configure-slang, or do a fresh build of the plugin.

As a workaround of the issue I suggest uninstalling libfmt, then fully rebuilding the plugin (having erased the content of build/ beforehand). That may help.

@povik povik changed the title Compatibility with Yosys bad linkage of fmt Oct 16, 2024
@povik
Copy link
Owner

povik commented Oct 16, 2024

For me the output of make configure-slang is the following:

$ make configure-slang
cmake -S .//third_party/slang -B build/slang \
	-DCMAKE_INSTALL_PREFIX=build/slang_install \
	-DSLANG_INCLUDE_TESTS=OFF \
	-DSLANG_INCLUDE_TOOLS=OFF \
	-DCMAKE_BUILD_TYPE=Release \
	-DSLANG_USE_MIMALLOC=OFF \
	-DCMAKE_CXX_FLAGS="-fPIC" \
	-DCMAKE_DISABLE_FIND_PACKAGE_Boost=ON \
	-DCMAKE_DISABLE_FIND_PACKAGE_fmt=ON
-- CMake version: 3.29.6
-- slang version: 6.0.170+1750f376
-- Build STATIC library as: svlang
-- {fmt} version: 11.0.2
-- Build type: Release
-- Using remote fmt library
-- Using vendored boost_unordered header
-- Configuring done (1.0s)
-- Generating done (0.0s)
-- Build files have been written to: /Users/pk/repos/yosys-slang/build/slang

@markNZed
Copy link
Author

cmake -S .//third_party/slang -B build/slang
-DCMAKE_INSTALL_PREFIX=build/slang_install
-DSLANG_INCLUDE_TESTS=OFF
-DSLANG_INCLUDE_TOOLS=OFF
-DCMAKE_BUILD_TYPE=Release
-DSLANG_USE_MIMALLOC=OFF
-DCMAKE_CXX_FLAGS="-fPIC"
-DCMAKE_DISABLE_FIND_PACKAGE_Boost=ON
-DCMAKE_DISABLE_FIND_PACKAGE_fmt=ON
-- CMake version: 3.22.1
-- slang version: 6.0.0+1750f376
-- Build STATIC library as: svlang
-- {fmt} version: 11.0.2
-- Build type: Release
-- Using remote fmt library
-- Using vendored boost_unordered header
-- Configuring done
-- Generating done
-- Build files have been written to: /workspace/llama-stack-client/backend/yosys-slang/build/slang

@markNZed
Copy link
Author

You are right that uninstalling fmt works for read_slang ../data/picorv32/picorv32.v --top picorv32 -D DEBUG Thanks!

@povik
Copy link
Owner

povik commented Oct 17, 2024

You are right that uninstalling fmt works for read_slang ../data/picorv32/picorv32.v --top picorv32 -D DEBUG Thanks!

Thanks for the report! I will keep the ticket around since this shoul've worked with fmt installed.

To double-check: the cmake log you posted is before or after you uninstalled fmt?

@markNZed
Copy link
Author

The cmake log is before I uninstalled

@povik
Copy link
Owner

povik commented Oct 17, 2024

So according to

-- Using remote fmt library

it did configure for using vendored fmt. I guess the linking step for the plugin must have found the system fmt anyway.

@markNZed
Copy link
Author

No, that log is BEFORE I uninstalled - that is why we see fmt still

@povik
Copy link
Owner

povik commented Oct 17, 2024

Yes, it's before you uninstalled, but when the log says "Using remote fmt library" it means it's rolling its own, not using the system version of the library.

@povik
Copy link
Owner

povik commented Oct 17, 2024

The log now, after you uninstalled, will also say "Using remote fmt library"

@markNZed
Copy link
Author

Ah, sorry, I thought you had misunderstood. Just let me know if you need more info.

@povik povik added the bug Something isn't working label Oct 17, 2024
@hpretl
Copy link

hpretl commented Oct 27, 2024

Same here, seeing (likely) this issue when building https://github.com/iic-jku/IIC-OSIC-TOOLS

@povik
Copy link
Owner

povik commented Oct 27, 2024

So far I haven't been able to reproduce this: I spun up a Ubuntu 22.04 container; installed libfmt8 and libfmt-dev; build yosys and yosys-slang, but didn't see the symbol lookup error on read_slang.

This was on aarch64, I will try on x86_64 next.

@hpretl
Copy link

hpretl commented Oct 27, 2024

I just checked, we are not installing libfmt8 or libfmt-dev during the container build as part of the base layer. Might this be an issue?

@povik
Copy link
Owner

povik commented Oct 27, 2024

It shouldn't be. So in your case these libraries are not available at runtime, are they also not available at build time?

@hpretl
Copy link

hpretl commented Oct 27, 2024

Whatever is in the base layer will be available during runtime. Whatever happens during build time (e.g., additional packages installed) will not be part of the final image, as we only selectively copy the stuff we want into the target. So if during install of yosys-slang libfmt is installed, it would not be in the final image.

@hpretl
Copy link

hpretl commented Oct 28, 2024

Not sure if this helps or is interesting, but there is no using of fmt in the binaries.

/foss/tools/yosys/share/yosys/plugins > ldd slang.so
	linux-vdso.so.1 (0x00007fffa2dc5000)
	libstdc++.so.6 => /lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007f018e108000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f018e021000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f018e001000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f018ddd8000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f018e99b000)
/foss/tools/yosys/bin > ldd yosys
	linux-vdso.so.1 (0x00007ffcea3c2000)
	libstdc++.so.6 => /lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fdb67c37000)
	libpython3.10.so.1.0 => /lib/x86_64-linux-gnu/libpython3.10.so.1.0 (0x00007fdb67660000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fdb67579000)
	libboost_python310.so.1.82.0 => /usr/local/lib/libboost_python310.so.1.82.0 (0x00007fdb67539000)
	libboost_filesystem.so.1.82.0 => /usr/local/lib/libboost_filesystem.so.1.82.0 (0x00007fdb67513000)
	libreadline.so.8 => /lib/x86_64-linux-gnu/libreadline.so.8 (0x00007fdb674bd000)
	libffi.so.8 => /lib/x86_64-linux-gnu/libffi.so.8 (0x00007fdb674b0000)
	libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007fdb67494000)
	libtcl8.6.so => /lib/x86_64-linux-gnu/libtcl8.6.so (0x00007fdb672e4000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fdb672c4000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fdb6709b000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fdb6980d000)
	libexpat.so.1 => /lib/x86_64-linux-gnu/libexpat.so.1 (0x00007fdb67068000)
	libtinfo.so.6 => /lib/x86_64-linux-gnu/libtinfo.so.6 (0x00007fdb67036000)

@hpretl
Copy link

hpretl commented Oct 28, 2024

I compiled yosys-slang again, and it definitely uses its own fmt, as I don't have it installed.

Question: It pulls yosys as a third-party dependency, but it is not using it during the build (I deleted it to make sure). Why is it having this dependency?

Further note, I am using yosys-0.46, and during build it throws the following warnings:

make[1]: Leaving directory '/tmp/yosys-slang'
    CXX build/abort_helpers.o
    CXX build/async_pattern.o
    CXX build/blackboxes.o
    CXX build/builder.o
    CXX build/diag.o
    CXX build/initial_eval.o
    CXX build/slang_frontend.o
In file included from /foss/tools/yosys/share/yosys/include/kernel/bitpattern.h:23,
                 from src/slang_frontend.cc:19:
src/slang_frontend.cc: In member function ‘Yosys::RTLIL::SigSpec slang_frontend::SignalEvalContext::apply_conversion(const slang::ast::ConversionExpression&, Yosys::RTLIL::SigSpec)’:
src/slang_frontend.cc:1646:30: warning: comparison of integer expressions of different signedness: ‘int’ and ‘slang::uint64_t’ {aka ‘long unsigned int’} [-Wsign-compare]
 1646 |         log_assert(op.size() == from.getBitstreamWidth());
      |                    ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
/foss/tools/yosys/share/yosys/include/kernel/log.h:226:78: note: in definition of macro ‘log_assert’
  226 | #  define log_assert(_assert_expr_) YOSYS_NAMESPACE_PREFIX log_assert_worker(_assert_expr_, #_assert_expr_, __FILE__, __LINE__)
      |                                                                              ^~~~~~~~~~~~~
src/builder.cc: In member function ‘Yosys::RTLIL::SigSpec slang_frontend::RTLILBuilder::Biop(Yosys::RTLIL::IdString, slang_frontend::RTLILBuilder::SigSpec, slang_frontend::RTLILBuilder::SigSpec, bool, bool, int)’:
src/builder.cc:306:46: warning: ‘bl’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  306 |                 int result = ThreeValued::XOR(carry, ThreeValued::XNOR(al, bl));
      |                              ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/builder.cc:306:46: warning: ‘al’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   LINK build/slang.so

@hpretl
Copy link

hpretl commented Oct 28, 2024

Good to have some time to kill at the airport, so I did further debug: when I compile yosys 0.45 and 0.46 and then slang-yosys, it works. However, when I build yosys with switch ENABLE_PYOSYS=1 the compile hangs... not sure how this will affect the container build. We are using ENABLE_PYOSYS=1 in the yosys container build, as OpenLane2 requires this.

@povik Could you please try to replicate the above if it leads to a fail?

@povik
Copy link
Owner

povik commented Oct 28, 2024

I compiled yosys-slang again, and it definitely uses its own fmt, as I don't have it installed.

Is it possible libfmt could have gotten installed as a dependency of something in the build container? It won't show up in ldd build/slang.so but might confuse the linking of the dynamic library nonetheless.

Question: It pulls yosys as a third-party dependency, but it is not using it during the build (I deleted it to make sure). Why is it having this dependency?

I take it you are referring to the tests/third_party/yosys/ submodule. That one is used for CI only. I made it a submodule so that I get automatic PRs testing on newer Yosys versions like this one: #65

@povik Could you please try to replicate the above if it leads to a fail?

I will test build against a pyosys-enabled Yosys v0.46. When you say the compile hangs, what does that look like exactly?

@hpretl
Copy link

hpretl commented Oct 28, 2024

A prompt pops up with /usr/bin/ld and then it keeps waiting. Never seen something like this.

@hpretl
Copy link

hpretl commented Oct 28, 2024

I compiled yosys-slang again, and it definitely uses its own fmt, as I don't have it installed.

Is it possible libfmt could have gotten installed as a dependency of something in the build container? It won't show up in ldd build/slang.so but might confuse the linking of the dynamic library nonetheless.

I don't think so... I carefully looked through the build logs, it really seems to link its own freshly built libfmt.

@povik
Copy link
Owner

povik commented Oct 28, 2024

Let me just note the libfmt linked should be a .a archive

@hpretl
Copy link

hpretl commented Oct 28, 2024

Yes, it is. Please also note the warning:

-- Configuring done
-- Generating done
CMake Warning:
  Manually-specified variables were not used by the project:

    CMAKE_DISABLE_FIND_PACKAGE_fmt


-- Build files have been written to: /tmp/yosys-slang/build/slang
make[2]: Leaving directory '/tmp/yosys-slang'
touch build/slang/.configured
make -C build/slang/
make[2]: Entering directory '/tmp/yosys-slang/build/slang'
make[3]: Entering directory '/tmp/yosys-slang/build/slang'
make[4]: Entering directory '/tmp/yosys-slang/build/slang'
make[4]: Leaving directory '/tmp/yosys-slang/build/slang'
make[4]: Entering directory '/tmp/yosys-slang/build/slang'
[  0%] Building CXX object _deps/fmt-build/CMakeFiles/fmt.dir/src/format.cc.o
[  1%] Building CXX object _deps/fmt-build/CMakeFiles/fmt.dir/src/os.cc.o
[  2%] Linking CXX static library ../../lib/libfmt.a
make[4]: Leaving directory '/tmp/yosys-slang/build/slang'

@povik
Copy link
Owner

povik commented Oct 28, 2024

That's interesting, I don't get that one.

@povik
Copy link
Owner

povik commented Oct 28, 2024

A prompt pops up with /usr/bin/ld and then it keeps waiting. Never seen something like this.

I see the same, let me investigate

@povik
Copy link
Owner

povik commented Oct 28, 2024

A prompt pops up with /usr/bin/ld and then it keeps waiting. Never seen something like this.

I see the same, let me investigate

It looks like this happens if you do make with ENABLE_PYOSYS=0 and another one with ENABLE_PYOSYS=1 without a make clean in between. I will raise this with the Yosys team but I am getting back to reproducing the yosys-slang issue.

@hpretl
Copy link

hpretl commented Oct 28, 2024

Ok, so I tested the build with the only change being that I removed the ENABLE_PYOSYS=1. In short, this causes the error because with ENABLE_PYOSYS=0 everything is OK.

@povik
Copy link
Owner

povik commented Oct 28, 2024

If you do make clean && make with ENABLE_PYOSYS=1, does it build fine? (It does on my end.)

@povik
Copy link
Owner

povik commented Oct 28, 2024

I have reproduced the original issue from this ticket and locally it's fixed by 6743611 (now in master)

@hpretl
Copy link

hpretl commented Oct 29, 2024

I can confirm the issue is fixed! I just tested a new image build on amd64 and aarch64. Thanks for the quick resolution!

@povik povik closed this as completed Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants