Bug 6610

Summary: libunrar has undefined symbols (again)
Product: Fedora Reporter: FeRD (Frank Dana) <ferdnyc>
Component: unrarAssignee: Conrad Meyer <konrad>
Status: RESOLVED FIXED    
Severity: normal CC: andreas, hans, leigh123linux
Priority: P1    
Version: f37   
Hardware: x86_64   
OS: GNU/Linux   
namespace:
Attachments: patch to fix missing libunrar symbols

Description FeRD (Frank Dana) 2023-03-21 07:15:54 CET
Created attachment 2485 [details]
patch to fix missing libunrar symbols

Amusingly, not only was this reported WAAAAY BACK in 2010 (bug 1385), but it was an issue with the **exact** same symbols. Apparently, the fix both made it into the upstream unrar distribution, and then fell out again.

The current libunrar-6.2.5-1.fc37 /usr/lib64/libunrar.so has undefined symbols RecVolTest and RecVolRestore, making it non-loadable.

An easy way to test the library's symbol resolvability is to try importing it as a Python CDLL object:

$ python3 -c "import ctypes; \
l = ctypes.cdll.LoadLibrary('/usr/lib64/libunrar.so'); \
print(type(l))"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/lib64/python3.11/ctypes/__init__.py", line 454, in LoadLibrary
    return self._dlltype(name)
           ^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.11/ctypes/__init__.py", line 376, in __init__
    self._handle = _dlopen(self._name, mode)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^
OSError: /usr/lib64/libunrar.so: undefined symbol: _Z14RecVolumesTestP11CommandDataP7ArchivePKw

Attached is a makefile patch to add the recvol.o and rs.o to the library build, which fixes the missing symbols.

$ rfpkg --release f37 mockbuild
$ sudo dnf install ./results_unrar/6.2.5/2.fc37/libunrar-6.2.5-2.fc37.x86_64.rpm
$ python3 -c "import ctypes; \
l = ctypes.cdll.LoadLibrary('/usr/lib64/libunrar.so'); \
print(type(l))"
<class 'ctypes.CDLL'>
Comment 1 FeRD (Frank Dana) 2023-03-21 08:13:54 CET
Actually... on further reflection, this is our fault. It doesn't happen with the unmodified upstream makefile.

libunrar.so isn't **supposed** to contain any references to the symbols RecVolumesTest or RecVolumesRestore... all such references in extract.cpp
are wrapped inside preprocessor checks:

#if !defined(SFX_MODULE) && !defined(RARDLL)
   [...use RecVolumesTest/RecVolumesRestore...]
#endif

The problem is, we're building the `lib` target using the _same_ object files we previously built for the `unrar` target, even though those targets are built with different defines (set on the compile command line using `-DUNRAR` or `-DRARDLL`). And those defines affect what symbols are included in the build.

We may be patching out the invocations of the `clean` target from their makefile (for good reasons), but we should still be cleaning out the build dir between building unrar and building libunrar.so. They aren't supposed to share the same object files.

The correct fix, on further reflection, is to change the %build section of unrar.spec to:

%build
%set_build_flags
%make_build -f makefile unrar STRIP=:
rm -f *.o
%make_build -f makefile lib STRIP=:


(We could've caught this by building the targets in the other order — if `make unrar` is run _after_ `make lib`, in a dirty tree using the object files from the library build, then the existing object files break the unrar build:)

$ make -f makefile unrar STRIP=:                                           
c++ -o unrar  -pthread rar.o strlist.o strfn.o pathfn.o smallfn.o global.o file.o filefn.o filcreat.o archive.o arcread.o unicode.o system.o crypt.o crc.o rawread.o encname.o resource.o match.o timefn.o rdwrfn.o consio.o options.o errhnd.o rarvm.o secpassword.o rijndael.o getbits.o sha1.o sha256.o blake2s.o hash.o extinfo.o extract.o volume.o list.o find.o unpack.o headers.o threadpool.o rs16.o cmddata.o ui.o filestr.o recvol.o rs.o scantree.o qopen.o 	
options.hpp:95:7: warning: type ‘struct RAROptions’ violates the C++ One Definition Rule [-Wodr]
   95 | class RAROptions
      |       ^
options.hpp:95:7: note: a different type is defined in another translation unit
   95 | class RAROptions
      |       ^
options.hpp:219:11: note: the first difference of corresponding definitions is field ‘DllDestName’
  219 |     wchar DllDestName[NM];
      |           ^
options.hpp:95:7: note: a type with different number of fields is defined in another translation unit
   95 | class RAROptions
      |       ^
cmddata.hpp:11:7: warning: type ‘struct CommandData’ violates the C++ One Definition Rule [-Wodr]
   11 | class CommandData:public RAROptions
      |       ^
cmddata.hpp:11:7: note: a type with different bases is defined in another translation unit
   11 | class CommandData:public RAROptions
      |       ^
/usr/bin/ld: /usr/lib/gcc/x86_64-redhat-linux/12/../../../../lib64/crt1.o: in function `_start':
(.text+0x1b): undefined reference to `main'
collect2: error: ld returned 1 exit status
make: *** [makefile:146: unrar] Error 1
Comment 2 FeRD (Frank Dana) 2023-03-29 13:09:08 CEST
Comment on attachment 2485 [details]
patch to fix missing libunrar symbols

Withdraw patch, see comment 1.