#strange UB with `std::vector<std::shared_ptr<...> >`

133 messages · Page 1 of 1 (latest)

tiny mica
#

I'm having a really weird UB issue when using a type called TranslatableString from a library:

class TranslatableString {
...
  std::vector<std::shared_ptr<const IArg> > args;
terminate called after throwing an instance of 'std::bad_array_new_length'
  what():  std::bad_array_new_length

I found where the issue comes from:

in __cxx_global_var_init.9(void) ()
at .../projectscene/internal/projectuiactions.cpp:51
51        TranslatableString("action", "New Project")),
(gdb)
grave copperBOT
#

When your question is answered use !solved or the button below to mark the question as resolved.

Remember to ask specific questions, provide necessary details, and reduce your question to its simplest form. For tips on how to ask a good question use !howto ask.

tiny mica
#

Seems a memory corruption issue...a SO or a use-after-free

visual locust
#

Need more info on TranslatableString and global variables that you have in your program

tiny mica
#

I'll rerun with ASAN to see if I can gather more information

thorny sentinel
#

sounds like you passed wrong value to operator new

visual locust
#

To me it feels like "static init order fiasco"

thorny sentinel
#

not a memory corruption issue

tiny mica
#

std::vector with size -1

thorny sentinel
tiny mica
#

that's memory corruption, right?

thorny sentinel
thorny sentinel
tiny mica
#

I'm excited, this is my first memory issue since I learnt C++!

thorny sentinel
#

it's probably not a memory issue

tiny mica
#

yeah?

thorny sentinel
#

yes

#

it could be, but i consider it less likely

#

this sounds like a logic error

#

most likely for some reason somewhere you're trying to create a std::vector of an invalid size

visual locust
#

Would you show TranslatableString's contructor?

tiny mica
thorny sentinel
#

that's probably where your -1 comes from

tiny mica
thorny sentinel
#

class members are implicitly inline btw the inline keyword probably does nothing there

tiny mica
#

that's what I thought, but I'm calling the function the same way MuseScore does and it works for them

#

what about static order fiasco you mentioned?

thorny sentinel
#

that would happen if you have global instances of these objects initialized before main

tiny mica
#

that's happening!

thorny sentinel
#

then you could run into the issue that the order of those initializations is not what you expect

#

well maybe that's part of the issue

#

is that repo the entire codebase?

#

like can i clone the code and check it out myself?

tiny mica
#
const ui::UiActionList ProjectUiActions::m_actions = {
        // File menu
        UiAction("project-new",
                UiCtxAny,
                muse::shortcuts::CTX_ANY,
                TranslatableString("action", "New Project")),
...
tiny mica
thorny sentinel
#

well is that the repo where you get the error?

tiny mica
thorny sentinel
#

so what exe should i build to see this error?

tiny mica
tiny mica
#

You also need Qt 6.10.1, if that isn't asking too much.

#

(there's a command to install it there using Python)

thorny sentinel
#

hmm i do have qt installed but it claims that it's not found 🤔

tiny mica
#

you need to set the CMAKE_PREFIX_PATH to where your Qt is

#

do you have Qt version 6.10.1? it requires exactly that version

thorny sentinel
#

yeah

tiny mica
#

and also set -DMUE_COMPILE_USE_SYSTEM_FREETYPE=ON and -GNinja -DCMAKE_C_COMPILER=/usr/bin/clang -DCMAKE_CXX_COMPILER=/usr/bin/clang++

thorny sentinel
#

holy fucking shit jesus christ how many dependencies do you have

tiny mica
#

skia doesn't provide prebuilt binaries, so you have to compile it from scratch...

#

with their own Google gn bullshit tool

#

use -j8 or it will take ages

thorny sentinel
#

currently im just chasing a shitload of missing dependencies and seeing if they're in fedora repos

#

i'd have preferred you use a package manager like vcpkg if you rely on so many dependencies

#

but it is what it is

#

in this case i'd suggest you try to make a minimal broken example

tiny mica
#

ooohhh...fedora. I'm currently focused on building the thing so documentation hasn't been a priority for me...

#

I can do that

thorny sentinel
#

again i suspect logic error not memory error

tiny mica
thorny sentinel
#

vcpkg doesn't build everything from scratch?

#

and okay you do you you can also use conan

tiny mica
#

@thorny sentinel Created a minimal repro repository here: https://gitlab.com/advanced-effects/Muse-Qml-App-Template
You should be able to:

git clone https://gitlab.com/advanced-effects/muse-qml-app-template.git
cd muse-qml-app-template
mkdir build
cd build
cmake .. -GNinja -DCMAKE_C_COMPILER=/usr/bin/clang -DCMAKE_CXX_COMPILER=/usr/bin/clang++ -DCMAKE_PREFIX_PATH="/home/your/path/to/qt" -DMUE_COMPILE_USE_SYSTEM_FREETYPE=ON -DMUSE_BUILD_WORKSPACE_MODULE=OFF
cmake --build . -j8
#

There are some linker errors I haven't been able to get rid of...

grave copperBOT
thorny sentinel
#

alr i'll check it out in a sec

#

eating lunch rn

tiny mica
#

oh! no problem

#

you don't need to keep helping...you already did too much

tiny mica
#

I think I know why the issue is happening

#

you know when CMake fails to include a script but doesn't tell you anything to make you lose 3 hours?

#

... but it still doesn't fix the linking issue

tiny mica
#

@thorny sentinelAfter 2 days debugging CMake, I finally got back to the original issue!

programar@B450-AORUS-ELITE-c426b102:~/Documents/Projects/Advanced-Effects/.build$ ./src/app/AdvancedEffects
QML debugging is enabled. Only use this in a safe environment.
terminate called after throwing an instance of 'std::bad_array_new_length'
  what():  std::bad_array_new_length
Avortat (s'ha bolcat la memòria)
thorny sentinel
#

😭

tiny mica
#

in Advanced Effects...

#

in the repro repository, I'm still figuring things out...

tiny mica
#

I still don't get how a list of static shared pointers could lead to a "bad length" of the array

mystic hamlet
#

Can you show the stack from the debugger?

tiny mica
#

Sure!

#

as I read it, the vector is being initialized with length -1. this is either because:

  1. memory corruption issues
  2. static initalization fiasco
  3. library set it to length -1
mystic hamlet
#

Okay, this happens during global variable initialization, before main() is entered

#

What size do you pass to the vector ctor?

#

I smell static init order fiasco

mystic hamlet
mystic hamlet
#

If you look at their source, what size do they pass?

tiny mica
#

well, let me clear that up. I construct a std::vector of actions, and in those actions, I pass in TranslatableStrings. It's in the TranslatableStrings that the error happens.

tiny mica
mystic hamlet
#

Also are you trying to use this library from constructors of your own global variables?

tiny mica
#

in translatablestring, line 144

#

my use is in lines 51, 55, ... Note the const keyword which was previously static but I changed to try to fix this error and accidentally pushed it.

supple talon
#

what is uiaction and where is it declared

tiny mica
supple talon
#

ended up finding something that looks like it in the muse repo

mystic hamlet
#

So looking at the stack again, someone constructs a global variable of type MnemonicString. But who thonk

supple talon
#

ProjectUiActions::m_actions is a vector of UiAction cause UiActionList is a type-alias defined by muse to a vector of UiAction
OP's code initializes that vector from a brace init list so I guess there's some initializer_list which I hope is optimized but whatever
OP's code has things along the lines of

UiAction("project-new",
                UiCtxAny,
                muse::shortcuts::CTX_ANY,
                TranslatableString("action", "New Project"))

for the individual UiAction, which aiui should invoke the constructor with this signature

    UiAction(const actions::ActionCode& code, UiContext ctx, std::string scCtx, const MnemonicString& title,
             Checkable ch = Checkable::No)
        : code(code), uiCtx(ctx), scCtx(scCtx), title(title), checkable(ch) {}

to note: actions::ActionCode is a type alias to std::string (https://gitlab.com/advanced-effects/framework/-/blob/main/src/actions/actiontypes.h?ref_type=heads#L33)
muse::shortcuts::CTX_ANY is a std::string (https://gitlab.com/advanced-effects/framework/-/blob/main/src/shortcuts/shortcutcontext.h?ref_type=heads#L35)
so the MnemonicString is being built from the prvalue TransalatableString("action", "New Project") or whatever the first time this pattern occurs

#

this should match specifically this constructor

    TranslatableString(const char* context, const char* str, const char* disambiguation = nullptr, int n = -1)
        : context(context), str(String::fromUtf8(str)), disambiguation(disambiguation), defaultN(n) {}

for the prvalue into this constructor for MnemonicString

    MnemonicString(const TranslatableString& raw)
        : m_raw(raw) {}
#

the vector of shared_ptr is in particular left out from the constructor of TranslatableString altogether, so it should be default initialized and have a size of 0

#

not -1

#

worst case scenario there's some oddity occuring with the initializer_list

#

which I guess would count as some form of static order initialization fiasco

#

or I guess another "fun" option would be a weird mixture of allocators, the codegen assuming that the memory is zero'd out cause that should be the case for initialization of static entities, but some instrumentation or whatever set the memory to some canary value resulting in size -1 :D

tiny mica
#

it was actually const all the time (I never changed it)

rain pilot
tiny mica
#

Maybe, but how can that lead to an error in TranslatableString?

supple talon
#

I'm gonna leave it there cause I was mainly trouble by what the call stack was, and I have my answer

#

from what I've somewhat seen it's sounds like a major pita to make a reproduction as well

tiny mica
#

pita?

supple talon
#

one thing I'd do in your shoes (but won't cause I'm not downloading your project), would be to try and setup a breakpoint for the initialization of m_actions

#

and step into it to see how the TranslatableString is created

tiny mica
#

will do!

supple talon
tiny mica
#

ohhh

rain pilot
supple talon
#

tbh I don't think those are likely to trigger static order initialization fiasco

#

UiCtxAny is a UiContext which is essentially a wrapper around a char const*
the std::string being in a weird state could lead to memory corruption though

but even then, those two globals are internal linkage, I think it's unlikely that'll lead to fiasco here

tiny mica
#

this started happening after I upgraded my muse framework version. Maybe something in Muse's API changed and I'm not doing it right anymore. So I think I'll go check for any differences between my code and upstream because that must be causing the issue

#

I can do this myself alone now. Thanks everyone for your help debugging this issue!

grave copperBOT
#

Thank you and let us know if you have any more questions!

This thread is now set to auto-hide after an hour of inactivity

tiny mica
#

and, by the way, is this a C++ only issue? or do this kind of things happen in other languages? (C, Go, Swift, C#...?)

supple talon
#

hard to say without knowing the exact cause

#

if this is a static initialization fiasco, then it shouldn't occur in c as c requires constant expressions, for other languages I'd have to look at how other languages deal with static initialization
if this is caused by a mixture of incompatible binaries, this will occur whenever you mix incompatible binaries, which I guess is more likely to happen if you're using a language that compiles to native, and more likely to happen the more incompatibilities you can drum up in your user code

#

which for other languages that don't compile to native usually translate to dependency hell

tiny mica
#

"incompatible binaries"?

#

as in binaries with different architectures / build types?

supple talon
#

not architectures, maybe build types depending on what you meant by that

#

in a broad sense, ABI compatibilities

thorny sentinel
#

though very slightly less so