povik / yosys-slang

SystemVerilog frontend for Yosys
ISC License
55 stars 7 forks source link

bad linkage of fmt #61

Closed markNZed closed 1 month ago

markNZed commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

markNZed commented 1 month ago

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

povik commented 1 month ago

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 commented 1 month ago
    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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

povik commented 1 month ago

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 commented 1 month ago

The cmake log is before I uninstalled

povik commented 1 month ago

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 commented 1 month ago

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

povik commented 1 month ago

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 commented 1 month ago

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

markNZed commented 1 month ago

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

hpretl commented 1 month ago

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

povik commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

hpretl commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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: https://github.com/povik/yosys-slang/pull/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 commented 1 month ago

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

hpretl commented 1 month ago

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 commented 1 month ago

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

hpretl commented 1 month ago

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 commented 1 month ago

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

povik commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

povik commented 1 month ago

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

hpretl commented 1 month ago

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