Segmentation fault with loudgain

[celynw]

First of all, really pleased I found this, looks like some great work.
Unfortunately, I'm not sure I have enough information to help the report.
I'm running Windows, but with WSL running Ubuntu 18.04.3.
Followed the instructions, very clear and familiar. No problems.

But running loudgain in any way on a file segfaults:

$ loudgain song.mp3
[✔] Scanning 'song.mp3' ...
[✔] Container: MP2/3 (MPEG audio layer 2/3) [mp3]
Segmentation fault (core dumped)

It's the same with any filetype:

...
[✔] Container: QuickTime / MOV [mov,mp4,m4a,3gp,3g2,mj2]
Segmentation fault (core dumped)
...
[✔] Container: Ogg [ogg]
Segmentation fault (core dumped)
...
[✔] Container: WAV / WAVE (Waveform Audio) [wav]
Segmentation fault (core dumped)

If running through rgbpm, it returns:

find: ‘loudgain’ terminated by signal 11:

For verbosity's sake, I'm including the build output:

$ cmake ..
-- The C compiler identification is GNU 7.4.0
-- The CXX compiler identification is GNU 7.4.0
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found PkgConfig: /usr/bin/pkg-config (found version "0.29.1")
-- Checking for module 'libavcodec'
--   Found libavcodec, version 58.66.100
-- Checking for module 'libavformat'
--   Found libavformat, version 58.35.104
-- Checking for module 'libswresample'
--   Found libswresample, version 3.6.100
-- Checking for module 'libavutil'
--   Found libavutil, version 56.38.100
-- Checking for module 'taglib'
--   Found taglib, version 1.11.1
-- Looking for include file pty.h
-- Looking for include file pty.h - found
-- Configuring done
-- Generating done
-- Build files have been written to: /mnt/c/Users/celyn/Applications/Utilities/loudgain/build

$ make -j
Scanning dependencies of target loudgain
[ 40%] Building CXX object CMakeFiles/loudgain.dir/src/tag.cc.o
[ 40%] Building C object CMakeFiles/loudgain.dir/src/loudgain.c.o
[ 60%] Building C object CMakeFiles/loudgain.dir/src/printf.c.o
[ 80%] Building C object CMakeFiles/loudgain.dir/src/scan.c.o
/mnt/c/Users/celyn/Applications/Utilities/loudgain/src/scan.c: In function ‘scan_init’:
/mnt/c/Users/celyn/Applications/Utilities/loudgain/src/scan.c:73:5: warning: ‘av_register_all’ is deprecated [-Wdeprecated-declarations]
     av_register_all();
     ^~~~~~~~~~~~~~~
In file included from /mnt/c/Users/celyn/Applications/Utilities/loudgain/src/scan.c:45:0:
/usr/local/include/libavformat/avformat.h:2056:6: note: declared here
 void av_register_all(void);
      ^~~~~~~~~~~~~~~
[100%] Linking CXX executable loudgain
[100%] Built target loudgain

$ sudo make install
[100%] Built target loudgain
Install the project...
-- Install configuration: ""
-- Installing: /usr/local/bin/loudgain
-- Installing: /usr/local/share/man/man1/loudgain.1
-- Installing: /usr/local/bin/rgbpm

Is there any way I can debug this?

[TylerVigario]

I'd like to add that FLAC & MP3 files work just fine for me. I noticed this issue with .m4a and stopped the whole scan. Any way we can debug further without trying to find memory errors within the source?

[Moonbase59]

Phew. Good question (and I’m happy to see someone uses it on a Windows machine!). Thanks for the build log, it looks okay but I wonder why it uses only version 56.38.100 of libavutil. I think (but I’m not sure) it should also have a version 58.xx?!

You are saying it only happens with m4a? Any possibility you could send me a (short) m4a file that produces the bug? My test suite contains m4a-aac, m4a-alac and m4a-nero files, and they all work out ok here.

If you do a loudgain -v, what does it show?

In the past we found that updating FFMpeg also helped in some cases (loudgain uses part of the FFMpeg libraries).

[deutrino]

I just noticed the m4a segfault on Debian 11, which offers Loudgain version "0.6.8+ds-1+b1" as a package.

$ loudgain -v
loudgain 0.6.8 - using:
  libebur128 1.2.5
  libavformat 58.45.100
  libswresample 3.7.100

These are downloaded files, I'll see if I can encode a random m4a file that reproduces the problem and upload it somewhere.

[complexlogic]

@TylerVigario @deutrino The reason is because the current code stores a reference to a local object from the taglib function TagLib::MP4::Tag::itemListMap(), but that object is destroyed when the function completes. The calling function later tries to access the destroyed object, which is UB and typically results in a segfault. A contributing factor here is the lack of a const qualifier on the function return type, which would have prevented this bug at compile time. That's why the taglib devs deprecated itemListMap in favor of itemMap.

This same issue also affects WMA/ASF.

To fix the bug, simply convert the reference to a local object. This effectively copies the object to the caller and prevents the issues seen above. See the below patch:

diff --git a/src/tag.cc b/src/tag.cc
index 1c149f3..bd39aeb 100644
--- a/src/tag.cc
+++ b/src/tag.cc
@@ -512,7 +512,7 @@ void tag_remove_mp4(TagLib::MP4::Tag *tag) {
   for(TagLib::MP4::ItemMap::Iterator item = items.begin();
       item != items.end(); ++item)
 #else
-  TagLib::MP4::ItemListMap &items = tag->itemListMap();
+  TagLib::MP4::ItemListMap items = tag->itemListMap();
 
   for(TagLib::MP4::ItemListMap::Iterator item = items.begin();
       item != items.end(); ++item)
@@ -591,7 +591,7 @@ bool tag_clear_mp4(scan_result *scan) {
 
 void tag_remove_asf(TagLib::ASF::Tag *tag) {
   TagLib::String desc;
-  TagLib::ASF::AttributeListMap &items = tag->attributeListMap();
+  TagLib::ASF::AttributeListMap items = tag->attributeListMap();
 
   for(TagLib::ASF::AttributeListMap::Iterator item = items.begin();
       item != items.end(); ++item)

@hughmcmaster I recommend applying the above patch to your Debian package to help @deutrino and others with this issue. This is a somewhat critical bug since it completely breaks M4A support with taglib <1.12 (current Stable) and WMA support for all versions. loudgain seems to have been abandoned, so it's unlikely this will be fixed upstream anytime soon, if ever.

[kt--]

@TylerVigario @deutrino The reason is because the current code stores a reference to a local object from the taglib function TagLib::MP4::Tag::itemListMap(), but that object is destroyed when the function completes.

@complexlogic's patch works for me