MSAN Errors Reported by Clang Memory Sanitizer

[jlsantiago0]

Describe the bug

test-msan-2021014-01.log

During global variable initialization before start of main, there are various global variables in different source files that cause other global object variables in other source files to be accessed before the constructor of the object has run. Many of these objects are STL objects and not in a valid state when this happens. A solution to this problem is to consolidate all of the global variables into one source file and list them in the order that their constructors need to run. C++ does not guarantee the order of global variable construction between source files.

For instance the attached log shows this global variable being initialized:

#12 0x48df44 in __cxx_global_var_init.22 /mnt/share/open/srt-testing/srt/apps/logsupport.cpp:102:12

The constructor eventually calls
/mnt/share/open/srt-testing/srt/apps/logsupport.cpp:95

Which uses another global variable: /mnt/share/open/srt-testing/srt/apps/logsupport.cpp:28

Whose constructor has not been called yet.

There are many cases like this.

To Reproduce

Run srt-live-transmit for instance:

./stage/bin/srt-live-transmit udp://239.66.133.30:4902 srt://:12345

Expected behavior

All global objects should have been properly constructed before used.

Screenshots
None.

Desktop (please provide the following information):

• OS: Linux
• SRT Current Master Branch.

Additional context
None.

test-msan-2021014-01.log

[maxsharabayko]

Related to #1547

[ethouris]

I don't know exactly how the results from Sanitizer should be interpreted, but my analysis of the code doesn't confirm your statements at all:

During global variable initialization before start of main, there are various global variables in different source files that cause other global object variables in other source files to be accessed before the constructor of the object has run. Many of these objects are STL objects and not in a valid state when this happens. A solution to this problem is to consolidate all of the global variables into one source file and list them in the order that their constructors need to run. C++ does not guarantee the order of global variable construction between source files.

True, but you have actually shown an example where all global variables are in the same source file. In this case, AFAIK, C++ guarantees that they are initialized in the order of declaration (between one another in this unit only).

For instance the attached log shows this global variable being initialized:

#12 0x48df44 in __cxx_global_var_init.22 /mnt/share/open/srt-testing/srt/apps/logsupport.cpp:102:12
The constructor eventually calls /mnt/share/open/srt-testing/srt/apps/logsupport.cpp:95
Which uses another global variable: /mnt/share/open/srt-testing/srt/apps/logsupport.cpp:28

Neither the constructor of LogFANames class (logsupport.cpp:95), nor the Install method (even though it's defined in another file it doesn't matter here) use the srt_level_names global variable (the one defined at logsupport.cpp:28). It's underlated to this variable at all. The only place where this variable is used is SrtParseLogLevel, which OTOH is also unrelated to srt_transmit_logfa_names (the variable defined at logsupport.cpp:102).

As I analyze the call stack you provided, it looks like this call passes down the value that is later accused of not having been initialized (symbols simplified):

    #9 0x5747ec in std::map<std::string, int>::operator[](std::string const&) /usr/lib64/gcc/x86_64-pc-linux-gnu/10.2.0/../../../../include/c++/10.2.0/bits/stl_map.h:497:17

It is unclear from the call stack which of two are being accused, but there are two possibilities:

• the id local variable (that's also what the announcement says)
• the map that supplies the node in the operator[] function

The initialization of both takes place:

• the id variable is of string type, which has a constructor, and as being a class it doesn't have distinct "default construction" and "default initialization". At worst it may be empty, but the next instruction fills it basing on the parameter passed.
• the map to which it is being inserted is initialized in the overall object constructor before the Install function is called.

Whose constructor has not been called yet.

And referring to this, you are speaking about a variable that is earlier defined as this one, and in the same translation unit, so according to the C++ initialization rules it's not possible that its constructor isn't called yet.

There are many cases like this.

I think in these cases the sanitizer is simply wrong, or at least I have no idea what criteria it has used to define this case as the creation of uninitialized value, but this at least isn't the case according to C++ standard.

Other cases I saw is referring to likely true_names and false_names variables. They are global variables also independent on the others, with explicitly given construction parameters and only used in the code after main(). No idea what exactly the sanitizer can see there uninitialized.

Not analyzed the whole report yet, but others look very similar.

[jlsantiago0]

HMM. You are right. The first messages seem to the complaining about using unitialized content of the stack variable string id; at srt/apps/logsupport.cpp:96 in the context of namemap[id] = value; at srt/apps/logsupport.cpp:98. This is either spurious or the result of an implementation detail of the c++ std library.

I guess we should consider this spurious and close the issue.

[ethouris]

The log, however, contains more information and this likely should be analyzed; I'll continue looking into the rest of them.

[ethouris]

Ok, per the amount of changes that went in in the meantime, I think the sanitizer analysis should be now repeated. This info is now likely outdated.

[mbakholdina]

Closing this issue. The work on the sanitizer issues will be tracked under #1813 further.